blockjob: allow omitted arguments to QMP block-commit

We are about to turn on support for active block commit.  Although
qemu 2.0 was the first version to mostly support it, that version
mis-handles 0-length files, and doesn't have anything available for
easy probing.  But qemu 2.1 fixed bugs, and made life simpler by
letting the 'top' argument be optional.  Unless someone begs for
active commit with qemu 2.0, for now we are just going to enable
it only by probing for qemu 2.1 behavior (anyone backporting active
commit can also backport the optional argument behavior).  This
requires qemu.git commit 7676e2c597000eff3a7233b40cca768b358f9bc9.

Although all our actual uses of block-commit supply arguments for
both base and top, we can omit both arguments and use a bogus
device string to trigger an interesting behavior in qemu.  All QMP
commands first do argument validation, failing with GenericError
if a mandatory argument is missing.  Once that passes, the code
in the specific command gets to do further checking, and the qemu
developers made sure that if device is the only supplied argument,
then the block-commit code will look up the device first, with a
failure of DeviceNotFound, before attempting any further argument
validation (most other validations fail with GenericError).  Thus,
the category of error class can reliably be used to decipher
whether the top argument was optional, which in turn implies a
working active commit.  Since we expect our bogus device string to
trigger an error either way, the code is written to return a
distinct return value without spamming the logs.

* src/qemu/qemu_monitor.h (qemuMonitorSupportsActiveCommit): New
prototype.
* src/qemu/qemu_monitor.c (qemuMonitorSupportsActiveCommit):
Implement it.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockCommit):
Allow NULL for top and base, for probing purposes.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockCommit):
Likewise, implementing the probe.
* tests/qemumonitorjsontest.c (mymain): Enable...
(testQemuMonitorJSONqemuMonitorSupportsActiveCommit): ...a new test.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 739fe6130338de9600b86f5261da7deb678dfedb..457cecdccb0d967663097eb752fd273a0a478187 100644 (file)
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3258,6 +3258,18 @@ qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *device,
     return ret;
 }
 
+
+/* Probe whether active commits are supported by a given qemu binary. */
+bool
+qemuMonitorSupportsActiveCommit(qemuMonitorPtr mon)
+{
+    if (!mon->json)
+        return false;
+
+    return qemuMonitorJSONBlockCommit(mon, "bogus", NULL, NULL, 0) == -2;
+}
+
+
 /* Use the block-job-complete monitor command to pivot a block copy
  * job.  */
 int

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 31b320498cf3adbdd1f31ce79188a588738369df..63e78d8e4426bbc0436c2cba39ba59279f96f020 100644 (file)
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -665,6 +665,7 @@ int qemuMonitorBlockCommit(qemuMonitorPtr mon,
                            unsigned long bandwidth)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
     ATTRIBUTE_NONNULL(4);
+bool qemuMonitorSupportsActiveCommit(qemuMonitorPtr mon);
 
 int qemuMonitorArbitraryCommand(qemuMonitorPtr mon,
                                 const char *cmd,

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 85168eb2ea4e1b5a68adfa3b06439cea5c0d804f..b5e8a20ef50b2e50dd4210e7ad478c14c8510018 100644 (file)
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3454,7 +3454,14 @@ qemuMonitorJSONTransaction(qemuMonitorPtr mon, virJSONValuePtr actions)
     return ret;
 }
 
-/* speed is in bytes/sec */
+/* speed is in bytes/sec. Returns 0 on success, -1 with error message
+ * emitted on failure.
+ *
+ * Additionally, can be used to probe if active commit is supported:
+ * pass in a bogus device and NULL top and base.  The probe return is
+ * -2 if active commit is detected, -3 if inconclusive; with no error
+ * message issued.
+ */
 int
 qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device,
                            const char *top, const char *base,
@@ -3467,14 +3474,30 @@ qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device,
     cmd = qemuMonitorJSONMakeCommand("block-commit",
                                      "s:device", device,
                                      "U:speed", speed,
-                                     "s:top", top,
-                                     "s:base", base,
+                                     "S:top", top,
+                                     "S:base", base,
                                      NULL);
     if (!cmd)
         return -1;
 
     if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
         goto cleanup;
+    if (!top && !base) {
+        /* Normally we always specify top and base; but omitting them
+         * allows for probing whether qemu is new enough to support
+         * live commit.  */
+        if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
+            VIR_DEBUG("block-commit supports active commit");
+            ret = -2;
+        } else {
+            /* This is a false negative for qemu 2.0; but probably not
+             * worth the additional complexity to worry about it */
+            VIR_DEBUG("block-commit requires 'top' parameter, "
+                      "assuming it lacks active commit");
+            ret = -3;
+        }
+        goto cleanup;
+    }
     ret = qemuMonitorJSONCheckError(cmd, reply);
 
  cleanup:

diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index e29158e94718078dddf46c7de270277f4f8decf0..89e668cd7b3cefc78b77528fd3437cd643cd14dd 100644 (file)
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -262,8 +262,7 @@ int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon,
                                const char *top,
                                const char *base,
                                unsigned long long bandwidth)
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
-    ATTRIBUTE_NONNULL(4);
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
 int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon,
                                     const char *cmd_str,

diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 2099dc874cc377e24b2f8527a20d80a0599e9b50..d136576ffc023df8886c5d78c49fdf482c361229 100644 (file)
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1991,6 +1991,52 @@ testQemuMonitorJSONqemuMonitorJSONSendKeyHoldtime(const void *data)
     return ret;
 }
 
+static int
+testQemuMonitorJSONqemuMonitorSupportsActiveCommit(const void *data)
+{
+    virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+    qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+    int ret = -1;
+    const char *error1 =
+        "{"
+        "  \"error\": {"
+        "    \"class\": \"DeviceNotFound\","
+        "    \"desc\": \"Device 'bogus' not found\""
+        "  }"
+        "}";
+    const char *error2 =
+        "{"
+        "  \"error\": {"
+        "    \"class\": \"GenericError\","
+        "    \"desc\": \"Parameter 'top' is missing\""
+        "  }"
+        "}";
+
+    if (!test)
+        return -1;
+
+    if (qemuMonitorTestAddItemParams(test, "block-commit", error1,
+                                     "device", "\"bogus\"",
+                                     NULL, NULL) < 0)
+        goto cleanup;
+
+    if (!qemuMonitorSupportsActiveCommit(qemuMonitorTestGetMonitor(test)))
+        goto cleanup;
+
+    if (qemuMonitorTestAddItemParams(test, "block-commit", error2,
+                                     "device", "\"bogus\"",
+                                     NULL, NULL) < 0)
+        goto cleanup;
+
+    if (qemuMonitorSupportsActiveCommit(qemuMonitorTestGetMonitor(test)))
+        goto cleanup;
+
+    ret = 0;
+ cleanup:
+    qemuMonitorTestFree(test);
+    return ret;
+}
+
 static int
 testQemuMonitorJSONqemuMonitorJSONGetDumpGuestMemoryCapability(const void *data)
 {
@@ -2263,6 +2309,7 @@ mymain(void)
     DO_TEST(qemuMonitorJSONSendKey);
     DO_TEST(qemuMonitorJSONGetDumpGuestMemoryCapability);
     DO_TEST(qemuMonitorJSONSendKeyHoldtime);
+    DO_TEST(qemuMonitorSupportsActiveCommit);
 
     DO_TEST_CPU_DATA("host");
     DO_TEST_CPU_DATA("full");