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.

Signed-off-by: Marc-André Lureau <[email protected]>
---
 qapi/block.json                             | 34 +++++++++++++
 block/monitor/block-hmp-cmds.c              | 60 ++--------------------
 block/monitor/qmp-cmds.c                    | 79 +++++++++++++++++++++++++++++
 block/monitor/meson.build                   |  1 +
 tests/qemu-iotests/tests/image-fleecing     |  4 +-
 tests/qemu-iotests/tests/image-fleecing.out | 52 +++++++++----------
 6 files changed, 145 insertions(+), 85 deletions(-)

diff --git a/qapi/block.json b/qapi/block.json
index 46955bbb3e3..0a588714ea9 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.1
+#
+# .. 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 }
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index c7113b8ea5e..254a79ce855 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)
 {
@@ -551,67 +550,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/qmp-cmds.c b/block/monitor/qmp-cmds.c
new file mode 100644
index 00000000000..e5759d824f1
--- /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/block/monitor/meson.build b/block/monitor/meson.build
index 1022516e93c..74faced9e17 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/tests/qemu-iotests/tests/image-fleecing 
b/tests/qemu-iotests/tests/image-fleecing
index 5e3b2c7e46a..62ef15ff046 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -202,12 +202,12 @@ def do_test(vm, use_cbw, use_snapshot_access_filter, 
base_img_path,
     for p in overwrite:
         cmd = 'write -P%s %s %s' % p
         log(cmd)
-        log(vm.hmp_qemu_io(qom_path, cmd, qdev=True))
+        log(vm.qmp('x-qemu-io', qdev='sda', command=cmd))
 
     if push_backup:
         # Check that previous operations were done during backup, not after
         # If backup is already finished, it's possible that it was finished
-        # even before hmp qemu_io write, and we didn't actually test
+        # even before qemu_io write, and we didn't actually test
         # copy-before-write operation. This should not happen, as we use
         # speed=1. But worth checking.
         result = vm.qmp('query-block-jobs')
diff --git a/tests/qemu-iotests/tests/image-fleecing.out 
b/tests/qemu-iotests/tests/image-fleecing.out
index acfc89ff0e9..0a819539130 100644
--- a/tests/qemu-iotests/tests/image-fleecing.out
+++ b/tests/qemu-iotests/tests/image-fleecing.out
@@ -31,13 +31,13 @@ read -P0 0x3fe0000 64k
 --- Testing COW ---
 
 write -P0xab 0 64k
-{"return": ""}
+{"return": {}}
 write -P0xad 0x00f8000 64k
-{"return": ""}
+{"return": {}}
 write -P0x1d 0x2008000 64k
-{"return": ""}
+{"return": {}}
 write -P0xea 0x3fe0000 64k
-{"return": ""}
+{"return": {}}
 
 --- Verifying Data ---
 
@@ -101,13 +101,13 @@ read -P0 0x3fe0000 64k
 --- Testing COW ---
 
 write -P0xab 0 64k
-{"return": ""}
+{"return": {}}
 write -P0xad 0x00f8000 64k
-{"return": ""}
+{"return": {}}
 write -P0x1d 0x2008000 64k
-{"return": ""}
+{"return": {}}
 write -P0xea 0x3fe0000 64k
-{"return": ""}
+{"return": {}}
 
 --- Verifying Data ---
 
@@ -172,13 +172,13 @@ read -P0 0x3fe0000 64k
 --- Testing COW ---
 
 write -P0xab 0 64k
-{"return": ""}
+{"return": {}}
 write -P0xad 0x00f8000 64k
-{"return": ""}
+{"return": {}}
 write -P0x1d 0x2008000 64k
-{"return": ""}
+{"return": {}}
 write -P0xea 0x3fe0000 64k
-{"return": ""}
+{"return": {}}
 
 --- Verifying Data ---
 
@@ -238,25 +238,25 @@ read -P0xd5 1M 64k
 read -P0xdc 32M 64k
 read -P0xcd 0x3ff0000 64k
 read -P0 0x00f8000 32k
-read failed: Invalid argument
+qemu-io: read failed: Invalid argument
 
 read -P0 0x2010000 32k
-read failed: Invalid argument
+qemu-io: read failed: Invalid argument
 
 read -P0 0x3fe0000 64k
-read failed: Invalid argument
+qemu-io: read failed: Invalid argument
 
 
 --- Testing COW ---
 
 write -P0xab 0 64k
-{"return": ""}
+{"return": {}}
 write -P0xad 0x00f8000 64k
-{"return": ""}
+{"return": {}}
 write -P0x1d 0x2008000 64k
-{"return": ""}
+{"return": {}}
 write -P0xea 0x3fe0000 64k
-{"return": ""}
+{"return": {}}
 
 --- Verifying Data ---
 
@@ -265,13 +265,13 @@ read -P0xd5 1M 64k
 read -P0xdc 32M 64k
 read -P0xcd 0x3ff0000 64k
 read -P0 0x00f8000 32k
-read failed: Invalid argument
+qemu-io: read failed: Invalid argument
 
 read -P0 0x2010000 32k
-read failed: Invalid argument
+qemu-io: read failed: Invalid argument
 
 read -P0 0x3fe0000 64k
-read failed: Invalid argument
+qemu-io: read failed: Invalid argument
 
 
 --- Cleanup ---
@@ -318,13 +318,13 @@ Done
 --- Testing COW ---
 
 write -P0xab 0 64k
-{"return": ""}
+{"return": {}}
 write -P0xad 0x00f8000 64k
-{"return": ""}
+{"return": {}}
 write -P0x1d 0x2008000 64k
-{"return": ""}
+{"return": {}}
 write -P0xea 0x3fe0000 64k
-{"return": ""}
+{"return": {}}
 {"data": {"device": "push-backup", "len": 67108864, "offset": 67108864, 
"speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": 
{"microseconds": "USECS", "seconds": "SECS"}}
 {"return": {}}
 

-- 
2.54.0


Reply via email to