[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] RFC: Add blockdev-del QMP command
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH v2] RFC: Add blockdev-del QMP command |
Date: |
Thu, 13 Feb 2014 10:59:36 +0800 |
User-agent: |
Mutt/1.5.22 (2013-10-16) |
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 <address@hidden>
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