From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:57638) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QRJox-0008Px-4D for qemu-devel@nongnu.org; Tue, 31 May 2011 03:57:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QRJow-0001Ji-1c for qemu-devel@nongnu.org; Tue, 31 May 2011 03:56:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:15785) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QRJov-0001Ja-MH for qemu-devel@nongnu.org; Tue, 31 May 2011 03:56:58 -0400 Message-ID: <4DE49FF2.2080902@redhat.com> Date: Tue, 31 May 2011 09:59:46 +0200 From: Kevin Wolf MIME-Version: 1.0 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> In-Reply-To: <20110530112123.39ca4dc6@doriath> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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: amit.shah@redhat.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, armbru@redhat.com Am 30.05.2011 16:21, schrieb Luiz Capitulino: > 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. 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