Re: [Qemu-devel] [PATCH 1/2] hw/sd: Add medium insertion status

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1770 bytes --]

On 07.01.2016 22:45, Peter Maydell wrote:
> On 7 January 2016 at 21:03, Max Reitz <mreitz@redhat.com> wrote:
>> Right now, the change_media_cb (sd_cardchange()) completely ignores its
>> @load parameter. This means that issuing a blockdev-open-tray command
>> will actually not have any effect.
>>
>> Fix this by keeping track of the medium insertion status in the SDState
>> and updating it in sd_init() and sd_cardchange().
>>
>> Cc: qemu-stable <qemu-stable@nongnu.org>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  hw/sd/sd.c | 16 ++++++++++------
>>  1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 1a9935c..0751ba2 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -114,6 +114,7 @@ struct SDState {
>>      uint8_t *buf;
>>
>>      bool enable;
>> +    bool medium_inserted;
> 
> When would this be different from blk_is_inserted(sd->blk) ?

First, whenever you use a host CD-ROM drive for an SD card. But I highly
doubt this worked before, because the device model is actually never
notified if the medium in the host drive is exchanged.

For any other medium, blk_is_inserted() should generally be always true.

Second, whenever the change_media_cb is called with @load set to false.
This means that the medium is to be unloaded, which was completely
ignored by sd.c so far (see commit message). It just interpreted a call
to sd_cardchange() as a notification that the medium might have changed,
but this is wrong.

> I don't see why we need a separate flag here rather
> than querying the block layer.

Because of the second case.

> If we do need a flag, why doesn't it need to be migrated?

Oh, yes, you're right, it probably should.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]