[libvirt] [PATCH v3 1/5] blockjob: allow omitted arguments to QMP block-commit
Eric Blake
eblake at redhat.com
Wed Jun 18 23:22:16 UTC 2014
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).
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 (qemuMonitorBlockCommit): Allow NULL for
top and base, for probing purposes.
* src/qemu/qemu_monitor.c (qemuMonitorBlockCommit): Likewise.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockCommit):
Likewise.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockCommit):
Likewise, with special return values based on probe.
* tests/qemumonitorjsontest.c (mymain): Enable...
(testQemuMonitorJSONqemuMonitorJSONBlockCommit2): ...a new test.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
src/qemu/qemu_monitor.c | 2 +-
src/qemu/qemu_monitor.h | 3 +--
src/qemu/qemu_monitor_json.c | 20 ++++++++++++++++--
src/qemu/qemu_monitor_json.h | 3 +--
tests/qemumonitorjsontest.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 70 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 80d7b9d..013a6ad 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3240,7 +3240,7 @@ qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *device,
unsigned long long speed;
VIR_DEBUG("mon=%p, device=%s, top=%s, base=%s, bandwidth=%ld",
- mon, device, top, base, bandwidth);
+ mon, device, NULLSTR(top), NULLSTR(base), bandwidth);
/* Convert bandwidth MiB to bytes - unfortunately the JSON QMP protocol is
* limited to LLONG_MAX also for unsigned values */
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 31b3204..1c7857e 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -663,8 +663,7 @@ int qemuMonitorBlockCommit(qemuMonitorPtr mon,
const char *top,
const char *base,
unsigned long bandwidth)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
- ATTRIBUTE_NONNULL(4);
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int qemuMonitorArbitraryCommand(qemuMonitorPtr mon,
const char *cmd,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index bedd959..0e4262d 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3467,14 +3467,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 e29158e..89e668c 100644
--- 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 2099dc8..faa968f 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1992,6 +1992,54 @@ testQemuMonitorJSONqemuMonitorJSONSendKeyHoldtime(const void *data)
}
static int
+testQemuMonitorJSONqemuMonitorJSONBlockCommit2(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 (qemuMonitorBlockCommit(qemuMonitorTestGetMonitor(test),
+ "bogus", NULL, NULL, 0) != -2)
+ goto cleanup;
+
+ if (qemuMonitorTestAddItemParams(test, "block-commit", error2,
+ "device", "\"bogus\"",
+ NULL, NULL) < 0)
+ goto cleanup;
+
+ if (qemuMonitorBlockCommit(qemuMonitorTestGetMonitor(test),
+ "bogus", NULL, NULL, 0) != -3)
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ qemuMonitorTestFree(test);
+ return ret;
+}
+
+static int
testQemuMonitorJSONqemuMonitorJSONGetDumpGuestMemoryCapability(const void *data)
{
virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
@@ -2263,6 +2311,7 @@ mymain(void)
DO_TEST(qemuMonitorJSONSendKey);
DO_TEST(qemuMonitorJSONGetDumpGuestMemoryCapability);
DO_TEST(qemuMonitorJSONSendKeyHoldtime);
+ DO_TEST(qemuMonitorJSONBlockCommit2);
DO_TEST_CPU_DATA("host");
DO_TEST_CPU_DATA("full");
--
1.9.3
More information about the libvir-list
mailing list