From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:46015) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QRK14-0002zi-L5 for qemu-devel@nongnu.org; Tue, 31 May 2011 04:09:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QRK13-0003MS-MG for qemu-devel@nongnu.org; Tue, 31 May 2011 04:09:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52814) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QRK13-0003ML-E4 for qemu-devel@nongnu.org; Tue, 31 May 2011 04:09:29 -0400 Message-ID: <4DE4A2E1.5060103@redhat.com> Date: Tue, 31 May 2011 10:12:17 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1306524712-13050-1-git-send-email-lcapitulino@redhat.com> <1306524712-13050-4-git-send-email-lcapitulino@redhat.com> <20110530110955.0c88675d@doriath> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/3] QMP: Introduce the BLOCK_MEDIA_EJECT event List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: amit.shah@redhat.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, Luiz Capitulino Am 30.05.2011 16:49, schrieb Markus Armbruster: > Luiz Capitulino writes: > >> On Sat, 28 May 2011 09:58:24 +0200 >> Markus Armbruster wrote: >> >>> Luiz Capitulino writes: >>>> diff --git a/block.h b/block.h >>>> index 1f58eab..e4053dd 100644 >>>> --- a/block.h >>>> +++ b/block.h >>>> @@ -50,6 +50,7 @@ typedef enum { >>>> BDRV_ACTION_REPORT, BDRV_ACTION_IGNORE, BDRV_ACTION_STOP >>>> } BlockMonEventAction; >>>> >>>> +void bdrv_eject_mon_event(const BlockDriverState *bdrv); >>>> void bdrv_error_mon_event(const BlockDriverState *bdrv, >>>> BlockMonEventAction action, int is_read); >>>> void bdrv_info_print(Monitor *mon, const QObject *data); >>>> diff --git a/blockdev.c b/blockdev.c >>>> index 6e0eb83..5fd0043 100644 >>>> --- a/blockdev.c >>>> +++ b/blockdev.c >>>> @@ -661,6 +661,11 @@ static int eject_device(Monitor *mon, BlockDriverState *bs, int force) >>>> return -1; >>>> } >>>> } >>>> + >>>> + if (bdrv_is_removable(bs) && bdrv_is_inserted(bs)) { >>>> + bdrv_eject_mon_event(bs); >>>> + } >>>> + >>>> bdrv_close(bs); >>>> return 0; >>>> } >>> >>> This covers monitor-initiated eject (commands eject and change). >>> >>> The event is not suppressed when the tray is already open (previous >>> guest-initiated eject), is it?. Contradicts spec. >> >> That's a bug. >> >>> The event is suppressed when the tray is empty. >>> >>> "eject -f" on a non-removable drive does not trigger an event. Why >>> treat it specially? I'm not saying you shouldn't, just wondering. >> >> Ejecting a non-removable drive is a qemu bug. > > It's clearly intentional, so it's a (mis-)feature, not a bug. Is there really a use case for it? The closest thing to a specification that we have is the help text and it says: .help = "eject a removable medium (use -f to force it)", QMP describes it like this: Eject a removable medium. So I start tending to agree that this whole trouble around the 'eject' monitor command is in fact a long standing bug rather than overloaded semantics. Nowhere is stated that it disconnects a BlockDriverState from the image, and I can't imagine a use case for this semantics either. Do we break anything if we make eject really eject the medium (we have a virtual tray status now) instead of just closing the image? I think the most visible change is that we'll eject the host medium when using pass-through. I consider this an additional bugfix. Kevin