Re: [Qemu-devel] [PATCH 44/55] spitz tosa: Simplify "drive is suitable for microdrive" test

[Markus Armbruster]

andrzej zaborowski <address@hidden> writes:

> On 3 August 2011 18:38, Markus Armbruster <address@hidden> wrote:
>> andrzej zaborowski <address@hidden> writes:
>>>                                                      2. if the
>>> underlaying storage can disappear for any other reason if that's
>>> possible to check.
>>
>> bdrv_is_removable() *isn't* such a check.
>
> Obviously I wasn't claiming it is, just that it might be useful, but
> not necessrily possible.  After all pretty much any storage can be
> "ejected" with enough force, depending on how far you want to go.
>
>>>> What's wrong with that again?  All sounds sensible to me.
>>>
>>> I'm not claiming otherwise, just double-checking this is what you want.
>
> So first you said you had a problem with _is_removable, and then you
> said nothing was wrong with the implementation you outlined, plase
> make up your mind.

I don't appreciate you quoting me out of context like that.

The sentence you quoted was in the middle of my attempt to get you to
explain what you're trying to accomplish there.  I started with
analyzing what your code does.  You asked "what's wrong with that", and
I replied that I'm not passing judgement on whether your code is right
or wrong, I just want to know what you're trying to do.

I'm getting tired of our misunderstandings, but let me try one more
time, and tiresomely verbose:

1. bdrv_is_removable() is ill-defined junk, and needs to go away.

   It is true when an ide-cd, scsi-cd or floppy qdev is attached, or
   when the BlockDriverState was created with -drive if={floppy,sd} or
   -drive if={ide,scsi,xen,none},media=cdrom ("created removable"),
   except when an ide-hd, scsi-hd, scsi-generic or virtio-blk qdev is
   attached.

   That's not semantics, that's an embarrassment.

2. I don't have an opinion on what should be done in
   tosa_microdrive_attach() and spitz_microdrive_attach().  Heck, I
   wouldn't touch it with a 9ft pole if I could help it.  But I have to,
   because it uses bdrv_is_removable().

3. My first try was to preserve the current behavior.  You don't like
   the result.  Fine.

4. For my second try, I tried to understand what you're trying to
   accomplish there.  Turns out you want to check whether "the
   underlaying storage can disappear for any other reason if that's
   possible to check."  Thanks for explaining that.

5. Sorry, that's not what the code there does.

6. The block layer does not offer an API to check that.  You're quite
   welcome to create it.

7. For what it's worth, IDE disks, SCSI disks and all the other
   non-removable decive models other than microdrives happily work
   without such a check.

What now?