Re: [Qemu-devel] [PATCH 2/3] QMP: Add BLOCK_MEDIA_EJECT event documentation

[Kevin Wolf]

Am 30.05.2011 16:21, schrieb Luiz Capitulino:
> On Mon, 30 May 2011 10:46:07 +0200
> Kevin Wolf <address@hidden> wrote:
> 
>> Am 27.05.2011 21:31, schrieb Luiz Capitulino:
>>> Signed-off-by: Luiz Capitulino <address@hidden>
>>> ---
>>>  QMP/qmp-events.txt |   18 ++++++++++++++++++
>>>  1 files changed, 18 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
>>> index 0ce5d4e..d53c129 100644
>>> --- a/QMP/qmp-events.txt
>>> +++ b/QMP/qmp-events.txt
>>> @@ -1,6 +1,24 @@
>>>                     QEMU Monitor Protocol Events
>>>                     ============================
>>>  
>>> +BLOCK_MEDIA_EJECT
>>> +-----------------
>>> +
>>> +Emitted when a removable disk media (such as a CDROM or floppy) is ejected.
>>> +
>>> +Data:
>>> +
>>> +- "device": device name (json-string)
>>> +
>>> +Example:
>>> +
>>> +{ "event": "BLOCK_MEDIA_EJECT",
>>> +    "data": { "device": "ide1-cd0" },
>>> +    "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
>>> +
>>> +NOTE: A disk media can be ejected by the guest or by monitor commands (such
>>> +as "eject" and "change")
>>
>> The monitor command 'eject' already caused a lot of confusion, please
>> don't make the same mistake in this event name. Even though I know more
>> or less what eject can mean in qemu, I'm not sure what "eject" means for
>> you in the context of this event.
> 
> I'll change it to report the tray status instead, as suggested by Markus.
> 
>> The 'eject' monitor command means that the image is closed and the
>> BlockDriverState doesn't point to any image file any more. And then
>> there's bdrv_eject(), which is what the guest can do, and it's about the
>> virtual tray status.
>>
>> Having a single event for both doesn't make sense because they are
>> fundamentally different. Something like BLOCKDEV_CLOSE would be the
>> right name for the 'eject' monitor command and maybe something like
>> BLOCKDEV_TRAY_STATUS for the other one.
> 
> Well, there are two problems here. First, we shouldn't report something
> like BLOCKDEV_CLOSE because closing a BlockDriverState is something
> internal to qemu that clients/users shouldn't know about.

Of course they know about it. After all, it was them who created the
BlockDriverState using -drive or drive_add (or eventually blockdev_add).

Anyway, I'm not saying that there's a good use case for BLOCKDEV_CLOSE,
it might be completely useless. I'm just saying that we must not mix it
with the tray status event.

> The second
> problem is that, unfortunately, clients do use "eject" to eject a
> removable media. Actually it's _the_ interface available for that, so
> not emitting the event there will probably confuse clients as much as
> not having the event at all.
> 
> Maybe, a better solution is to fix eject to really eject the media
> instead of closing its BlockDriverState and drop the event from the change
> command.

Hm, it would surely change the behaviour which means that we have to be
careful with it. But the change makes some sense to me. If we do this
change, I think we should also add a command for closing the tray, so
that you get access to the image again.

Kevin