From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39454) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RrtI4-00038E-Ru for qemu-devel@nongnu.org; Mon, 30 Jan 2012 10:37:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RrtI0-0005gb-15 for qemu-devel@nongnu.org; Mon, 30 Jan 2012 10:37:08 -0500 Received: from mx1.redhat.com ([209.132.183.28]:5889) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RrtHz-0005gO-IT for qemu-devel@nongnu.org; Mon, 30 Jan 2012 10:37:03 -0500 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q0UFb2Fu026011 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 30 Jan 2012 10:37:02 -0500 Date: Mon, 30 Jan 2012 13:36:56 -0200 From: Luiz Capitulino Message-ID: <20120130133656.2c164bd9@doriath> In-Reply-To: <4F22943F.5080704@redhat.com> References: <20120124161628.4bf2592c@doriath.home> <4F22943F.5080704@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qmp: add BLOCK_MEDIUM_EJECT event List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Kevin Wolf , Eric Blake , qemu-devel , Markus Armbruster On Fri, 27 Jan 2012 13:10:39 +0100 Paolo Bonzini wrote: > On 01/24/2012 07:16 PM, Luiz Capitulino wrote: > > @@ -237,6 +238,17 @@ static bool do_check_io_limits(BlockIOLimit *io_limits) > > return true; > > } > > > > +static void on_medium_eject(BlockDriverState *bs, int is_ejected) > > +{ > > + QObject *data; > > + > > + data = qobject_from_jsonf("{ 'device': %s, 'ejected': %i }", > > + bdrv_get_device_name(bs), is_ejected); > > + monitor_protocol_event(QEVENT_BLOCK_MEDIUM_EJECT, data); > > + > > + qobject_decref(data); > > +} > > + > > DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) > > { > > const char *buf; > > @@ -503,6 +515,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) > > QTAILQ_INSERT_TAIL(&drives, dinfo, next); > > > > bdrv_set_on_error(dinfo->bdrv, on_read_error, on_write_error); > > + bdrv_dev_set_medium_eject_notify(dinfo->bdrv, on_medium_eject); > > > > /* disk I/O throttling */ > > bdrv_set_io_limits(dinfo->bdrv, &io_limits); > > block.c/blockdev.c separation is nice, but do we really need a function > pointer? Also, we're already emitting the BLOCK_IO_ERROR event in > block.c; is that another place to cleanup, or is this overkill and we > can just put bdrv_dev_medium_eject_notify in block.c? This patch has changed after this whole discussion. My current version (not posted yet) adds a bdrv_emit_qmp_error_event() function to block.c and call it from bdrv_eject(). But that accounts only for the guest initiated ejects... > > diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c > > index 0adb27b..4b4bc61 100644 > > --- a/hw/ide/atapi.c > > +++ b/hw/ide/atapi.c > > @@ -885,6 +885,7 @@ static void cmd_start_stop_unit(IDEState*s, uint8_t* buf) > > } > > bdrv_eject(s->bs, !start); > > s->tray_open = !start; > > + bdrv_dev_medium_eject_notify(s->bs, !start); > > } > > > > ide_atapi_cmd_ok(s); > > diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > > index 5d8bf53..35e55f4 100644 > > --- a/hw/scsi-disk.c > > +++ b/hw/scsi-disk.c > > @@ -1061,6 +1061,7 @@ static int scsi_disk_emulate_start_stop(SCSIDiskReq *r) > > } > > bdrv_eject(s->qdev.conf.bs, !start); > > s->tray_open = !start; > > + bdrv_dev_medium_eject_notify(s->qdev.conf.bs, !start); > > } > > return 0; > > } > > Can you do the call directly in bdrv_eject (but I would skip > bdrv_close)? The only other place where bdrv_eject is called is from > block/raw.c's raw_eject, but I think you should only emit the event if > bs->device_name[0] (otherwise the event is quite useless) and bs->file > will fail the test. Good point.