From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:47102) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QR3rP-00057J-5R for qemu-devel@nongnu.org; Mon, 30 May 2011 10:54:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QR3rO-0006Vd-6F for qemu-devel@nongnu.org; Mon, 30 May 2011 10:54:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:31137) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QR3rN-0006VZ-VM for qemu-devel@nongnu.org; Mon, 30 May 2011 10:54:26 -0400 From: Markus Armbruster References: <1306524712-13050-1-git-send-email-lcapitulino@redhat.com> <1306524712-13050-3-git-send-email-lcapitulino@redhat.com> <4DE3594F.20406@redhat.com> <20110530112123.39ca4dc6@doriath> Date: Mon, 30 May 2011 16:54:22 +0200 In-Reply-To: <20110530112123.39ca4dc6@doriath> (Luiz Capitulino's message of "Mon, 30 May 2011 11:21:23 -0300") Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH 2/3] QMP: Add BLOCK_MEDIA_EJECT event documentation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Kevin Wolf , amit.shah@redhat.com, aliguori@us.ibm.com, qemu-devel@nongnu.org Luiz Capitulino writes: > On Mon, 30 May 2011 10:46:07 +0200 > Kevin Wolf wrote: > >> Am 27.05.2011 21:31, schrieb Luiz Capitulino: >> > Signed-off-by: Luiz Capitulino >> > --- >> > 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. Monitor command "eject" conflates three actions: open tray, remove media (if any), close tray. Monitor command "change" conflates four actions: open tray, remove media (if any), insert media, close tray. Except they don't really move the tray in a guest-visible manner. They teleport the media. I figure that should be changed.