Re: [Qemu-devel] [PATCH v2 2/4] blockdev: Fix 'change' for slot devices

[Max Reitz <mreitz@redhat.com>]

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

On 22.01.2016 10:58, Alberto Garcia wrote:
> On Wed 20 Jan 2016 07:29:19 PM CET, Max Reitz wrote:
>> @@ -2424,6 +2442,15 @@ static void qmp_blockdev_insert_anon_medium(const char *device,
>>  
>>      blk_insert_bs(blk, bs);
>>  
>> +    if (!blk_dev_has_tray(blk)) {
>> +        /* For tray-less devices, blockdev-close-tray is a no-op (or may not be
>> +         * called at all); therefore, the medium needs to be pushed into the
>> +         * slot here.
>> +         * Do it after blk_insert_bs() so blk_is_inserted(blk) returns the @load
>> +         * value passed here (i.e. true). */
>> +        blk_dev_change_media_cb(blk, true);
>> +    }
>> +
>>      QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list);
>>  }
> 
> Any reason why you do this before updating bdrv_states ?

The reason is that I just moved it after blk_insert_bs(). But that's
wrong, QTAILQ_INSERT_TAIL() should be paired with blk_insert_bs()
without anything in between, thanks!

(I don't think it changes anything in practice, but it still is wrong.)

> If the device has a tray this would happen afterwards, in
> qmp_blockdev_close_tray().

Well, tray devices are no longer really a good comparison, because in
the opposite case (ejecting a medium), for them we open the tray and
then eject the medium; however, for trayless devices we now eject the
medium and only then "open the tray" (invoke blk_dev_change_media_cb()).

Anyway, will fix, thanks.

Max


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