Add an x-qemu-io QMP command that runs qemu-io commands on block devices. The command accepts a device name (block backend name, node-name, or qdev ID) and a qemu-io command string.
Refactor hmp_qemu_io() to be a thin wrapper around the new QMP command, following the standard HMP-over-QMP pattern used by other block commands. This change is also required for the qtest qemu-io command in the following patch. Reviewed-by: Daniel P. Berrangé <[email protected]> Signed-off-by: Marc-André Lureau <[email protected]> --- block/monitor/block-hmp-cmds.c | 60 ++------------------------------ block/monitor/meson.build | 1 + block/monitor/qmp-cmds.c | 79 ++++++++++++++++++++++++++++++++++++++++++ qapi/block.json | 34 ++++++++++++++++++ 4 files changed, 117 insertions(+), 57 deletions(-) diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c index 9930620a4a16..22fa03089ea3 100644 --- a/block/monitor/block-hmp-cmds.c +++ b/block/monitor/block-hmp-cmds.c @@ -56,7 +56,6 @@ #include "block/qapi.h" #include "block/block_int.h" #include "block/block-hmp-cmds.h" -#include "qemu-io.h" static void hmp_drive_add_node(Monitor *mon, const char *optstr) { @@ -541,67 +540,14 @@ void hmp_eject(Monitor *mon, const QDict *qdict) void hmp_qemu_io(Monitor *mon, const QDict *qdict) { - BlockBackend *blk = NULL; - BlockDriverState *bs = NULL; - BlockBackend *local_blk = NULL; bool qdev = qdict_get_try_bool(qdict, "qdev", false); const char *device = qdict_get_str(qdict, "device"); const char *command = qdict_get_str(qdict, "command"); Error *err = NULL; - int ret; - if (qdev) { - blk = blk_by_qdev_id(device, &err); - if (!blk) { - goto fail; - } - } else { - blk = blk_by_name(device); - if (!blk) { - bs = bdrv_lookup_bs(NULL, device, &err); - if (!bs) { - goto fail; - } - } - } - - if (bs) { - blk = local_blk = blk_new(bdrv_get_aio_context(bs), 0, BLK_PERM_ALL); - ret = blk_insert_bs(blk, bs, &err); - if (ret < 0) { - goto fail; - } - } - - /* - * Notably absent: Proper permission management. This is sad, but it seems - * almost impossible to achieve without changing the semantics and thereby - * limiting the use cases of the qemu-io HMP command. - * - * In an ideal world we would unconditionally create a new BlockBackend for - * qemuio_command(), but we have commands like 'reopen' and want them to - * take effect on the exact BlockBackend whose name the user passed instead - * of just on a temporary copy of it. - * - * Another problem is that deleting the temporary BlockBackend involves - * draining all requests on it first, but some qemu-iotests cases want to - * issue multiple aio_read/write requests and expect them to complete in - * the background while the monitor has already returned. - * - * This is also what prevents us from saving the original permissions and - * restoring them later: We can't revoke permissions until all requests - * have completed, and we don't know when that is nor can we really let - * anything else run before we have revoken them to avoid race conditions. - * - * What happens now is that command() in qemu-io-cmds.c can extend the - * permissions if necessary for the qemu-io command. And they simply stay - * extended, possibly resulting in a read-only guest device keeping write - * permissions. Ugly, but it appears to be the lesser evil. - */ - qemuio_command(blk, command, &err); - -fail: - blk_unref(local_blk); + qmp_x_qemu_io(qdev ? NULL : device, + qdev ? device : NULL, + command, &err); hmp_handle_error(mon, err); } diff --git a/block/monitor/meson.build b/block/monitor/meson.build index 1022516e93c5..74faced9e17f 100644 --- a/block/monitor/meson.build +++ b/block/monitor/meson.build @@ -1,2 +1,3 @@ system_ss.add(files('block-hmp-cmds.c')) block_ss.add(files('bitmap-qmp-cmds.c')) +system_ss.add(files('qmp-cmds.c')) diff --git a/block/monitor/qmp-cmds.c b/block/monitor/qmp-cmds.c new file mode 100644 index 000000000000..e5759d824f13 --- /dev/null +++ b/block/monitor/qmp-cmds.c @@ -0,0 +1,79 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +#include "qemu/osdep.h" + +#include "system/block-backend.h" +#include "block/block_int.h" +#include "qapi/qapi-commands-block.h" +#include "qapi/error.h" +#include "qemu-io.h" + +void qmp_x_qemu_io(const char *device, const char *qdev, + const char *command, Error **errp) +{ + BlockBackend *blk = NULL; + BlockBackend *local_blk = NULL; + BlockDriverState *bs = NULL; + int ret; + + if (!device && !qdev) { + error_setg(errp, "Must specify either device or qdev"); + return; + } + if (qdev && device) { + error_setg(errp, "Cannot specify both qdev and device"); + return; + } + + if (qdev) { + blk = blk_by_qdev_id(qdev, errp); + if (!blk) { + return; + } + } else { + blk = blk_by_name(device); + if (!blk) { + bs = bdrv_lookup_bs(NULL, device, errp); + if (!bs) { + return; + } + } + } + + if (bs) { + blk = local_blk = blk_new(bdrv_get_aio_context(bs), 0, BLK_PERM_ALL); + ret = blk_insert_bs(blk, bs, errp); + if (ret < 0) { + goto fail; + } + } + + /* + * Notably absent: Proper permission management. This is sad, but it seems + * almost impossible to achieve without changing the semantics and thereby + * limiting the use cases of the qemu-io command. + * + * In an ideal world we would unconditionally create a new BlockBackend for + * qemuio_command(), but we have commands like 'reopen' and want them to + * take effect on the exact BlockBackend whose name the user passed instead + * of just on a temporary copy of it. + * + * Another problem is that deleting the temporary BlockBackend involves + * draining all requests on it first, but some qemu-iotests cases want to + * issue multiple aio_read/write requests and expect them to complete in + * the background while the monitor has already returned. + * + * This is also what prevents us from saving the original permissions and + * restoring them later: We can't revoke permissions until all requests + * have completed, and we don't know when that is nor can we really let + * anything else run before we have revoken them to avoid race conditions. + * + * What happens now is that command() in qemu-io-cmds.c can extend the + * permissions if necessary for the qemu-io command. And they simply stay + * extended, possibly resulting in a read-only guest device keeping write + * permissions. Ugly, but it appears to be the lesser evil. + */ + qemuio_command(blk, command, errp); + +fail: + blk_unref(local_blk); +} diff --git a/qapi/block.json b/qapi/block.json index 46955bbb3e34..012a9d73333b 100644 --- a/qapi/block.json +++ b/qapi/block.json @@ -603,3 +603,37 @@ '*boundaries-zap': ['uint64'], '*boundaries-flush': ['uint64'] }, 'allow-preconfig': true } + +## +# @x-qemu-io: +# +# Run a qemu-io command on a block device. Take either a block +# backend name or a qdev ID to identify the device. +# +# @device: the block backend name, node-name to run the +# command on. +# +# @qdev: the qdev ID of the block device to run the +# command on. +# +# @command: the qemu-io command string to execute. +# +# Features: +# +# @unstable: This command is for testing only. +# +# Since: 11.2 +# +# .. qmp-example:: +# +# -> { "execute": "x-qemu-io", +# "arguments": { "device": "virtio0", +# "command": "read 0 512" } } +# <- { "return": {} } +## +{ 'command': 'x-qemu-io', + 'data': { '*device': 'str', + '*qdev': 'str', + 'command': 'str' }, + 'features': [ 'unstable' ], + 'allow-preconfig': true } -- 2.55.0.543.g5ebe2ebe4ea8
