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

[Kevin Wolf]

Am 09.08.2011 06:32, schrieb 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 mean just anything in hw/. I'm not familiar with how these devices
work internally, so if hw/ide/microdrive.c is the right place, fine.

> 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.

Depends. As long as you're talking about purely virtual device state, I
would say yes, they belong there, too. However, we have things like
CD-ROM passthrough where bdrv_is_inserted etc. are supposed to return
the status of the physical host device. These are host state and need to
be in the block layer.

Kevin