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

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