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

[andrzej zaborowski]

On 4 August 2011 10:02, Kevin Wolf <address@hidden> wrote:
> Am 03.08.2011 22:20, schrieb andrzej zaborowski:
>> On 3 August 2011 20:24, Markus Armbruster <address@hidden> wrote:
>>> 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.
>>
>> I got quite annoyed when you started putting words in my mouth by
>> saying I said anything about CD-ROM.. the code in spitz/tosa is not
>> concerned with CD-ROMs even if downstream it boils down to that, it is
>> concerned with whether the device is removable or not, and that's what
>> the check does.  It doesn't help readability or anything by inlining
>> that check.  If you're trying to check for A then don't spell it out
>> as B, be explicit.  It's not a big deal but I just don't see the
>> point, sorry.
>>
>>>
>>> The sentence you quoted was in the middle of my attempt to get you to
>>> explain what you're trying to accomplish there.
>>
>> I already said about 3 times what it's trying to acomplish.  You also
>> have used the word "removable" so I'm sure you know what it means and
>> don't need further explanation.  But let's define it this way: if a
>> GUI is going to display an "eject" button next to a drive in the qemu
>> device tree, that's a removable device.  CD-ROM is an example of that.
>>  An IDE HDD is an example of something that's not going to have that
>> button (I assume).
>
> But this is a property of the device, not of the backend. This means
> that it belongs in the device emulation and not in block.c.

By device do you mean hw/ide/microdrive.c?  I'm not saying it belongs
in block.c, but logically it belongs in the same place as
bdrv_is_inserted, bdrv_is_locked, bdrv_eject etc. no?  So it is a
property of whatever "media" is property of.

>
> If you want to have a function spitz_microdrive_is_removable() or
> similar in the device model I don't really mind (even though I don't see
> the point), but the block layer is the wrong place for it.

Cheers
(I'm slow to reply because I'm on travel)