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 <im...@redhat.com>
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