From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35740) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WDmWX-0000S7-Dl for qemu-devel@nongnu.org; Wed, 12 Feb 2014 21:59:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WDmWR-00027G-Dp for qemu-devel@nongnu.org; Wed, 12 Feb 2014 21:59:37 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48783) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WDmWR-00027B-5f for qemu-devel@nongnu.org; Wed, 12 Feb 2014 21:59:31 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s1D2xUi8030326 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 12 Feb 2014 21:59:30 -0500 Date: Thu, 13 Feb 2014 10:59:36 +0800 From: Fam Zheng Message-ID: <20140213025936.GC28393@T430.nay.redhat.com> References: <1392226573-14901-1-git-send-email-imain@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1392226573-14901-1-git-send-email-imain@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2] RFC: Add blockdev-del QMP command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ian Main Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, armbru@redhat.com On Wed, 02/12 09:36, Ian Main wrote: > This is the sister command to blockdev-add. In Fam's example he uses > the drive_del HMP command to clean up but it would be much nicer to > have a way to do this via QMP. > > Signed-off-by: Ian Main Thank you for doing this! > --- > > v2: > - s/blockdev-delete/blockdev-del > - Fixed example. > > blockdev.c | 52 ++++++++++++++++++++++++++++++++++++++-------------- > qapi-schema.json | 11 +++++++++++ > qmp-commands.hx | 30 ++++++++++++++++++++++++++++++ > 3 files changed, 79 insertions(+), 14 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index 7372721..96d0da1 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -1733,21 +1733,9 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd, > } > } > > -int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) > +/* This is called by both do_drive_del() and qmp_blockdev_del */ > +static int drive_del_core(BlockDriverState *bs) > { > - const char *id = qdict_get_str(qdict, "id"); > - BlockDriverState *bs; > - > - bs = bdrv_find(id); > - if (!bs) { > - qerror_report(QERR_DEVICE_NOT_FOUND, id); > - return -1; > - } > - if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, NULL)) { > - qerror_report(QERR_DEVICE_IN_USE, id); > - return -1; > - } > - > /* quiesce block driver; prevent further io */ > bdrv_drain_all(); > bdrv_flush(bs); > @@ -1771,6 +1759,25 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) > return 0; > } > > +int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) > +{ > + const char *id = qdict_get_str(qdict, "id"); > + BlockDriverState *bs; > + > + bs = bdrv_find(id); > + if (!bs) { > + qerror_report(QERR_DEVICE_NOT_FOUND, id); > + return -1; > + } > + > + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, NULL)) { > + qerror_report(QERR_DEVICE_IN_USE, id); > + return -1; > + } > + > + return drive_del_core(bs); > +} > + > void qmp_block_resize(bool has_device, const char *device, > bool has_node_name, const char *node_name, > int64_t size, Error **errp) > @@ -2386,6 +2393,23 @@ fail: > qmp_output_visitor_cleanup(ov); > } > > +void qmp_blockdev_del(const char *device, Error **errp) > +{ > + BlockDriverState *bs; > + > + bs = bdrv_find(device); > + if (!bs) { > + error_setg(errp, "Block device not found"); > + return; > + } > + > + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, NULL)) { > + return; > + } > + > + drive_del_core(bs); > +} > + We could drop drive_del_core and move everything into qmp_blockdev_del(). In the original do_drive_del (maybe rename it to hmp_drive_del as a better name?), call qmp_blockdev_del with a local_err, and report error with qerror_report_err(). Thanks, Fam