Am 06.02.2013 21:02, schrieb Luiz Capitulino:
> This reverts commit 9ca111544c64b5abed2e79cf52e19a8f227b347b.
>
> That commit has added a weird side effect to QMP: on shutdown,
> QMP emits the DEVICE_TRAY_MOVED event for all empty drives
> that have closed trays.
>
> This happens because the tray state actually changes in
> the following code path:
>
> main loop exit
> bdrv_close_all()
> bdrv_close()
> bdrv_dev_change_media_cb()
>
> So, revert the commit as the real fix is to separate eject
> from bdrv_close() and also because it's not clear how important
> it is to have a 100% correct semantic for eject from HMP.
>
> Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
> ---
>
> ** IMPORTANT: This is an RFC because I'm not totally sure we should
> apply it for 1.4, for the following reasons:
>
> 1. The worst we get is a gratuitous state change that triggers
> an QMP event
>
> 2. Commit 9ca1115 is a bit old and was introduced before 1.4
> development
>
> 3. This is not the Right Thing either
>
> block.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/block.c b/block.c
> index 50dab8e..d58c6cc 100644
> --- a/block.c
> +++ b/block.c
> @@ -1178,9 +1178,9 @@ void bdrv_close(BlockDriverState *bs)
> bdrv_delete(bs->file);
> bs->file = NULL;
> }
> - }
>
> - bdrv_dev_change_media_cb(bs, false);
> + bdrv_dev_change_media_cb(bs, false);
> + }
>
> /*throttling disk I/O limits*/
> if (bs->io_limits_enabled) {
I think the right solution is to move the bdrv_dev_change_media_cb()
call to those callers of bdrv_close() that actually need it. I haven't
checked it yet in detail, but at the first sight it looks like
eject_device() and do_drive_del() are the only ones that need it.
Such a change I'd rather move to 1.5, though. Do you think it's
important to get rid of these (admittedly odd) artefacts for 1.4?
Kevin
