[libvirt] [PATCH v4 0/5] Another version of active commit
Rebased to latest libvirt.git, and still waiting on Jeff's qemu patches to go into qemu.git before I can push any of these, https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg04060.html but reposting because of a feature Adam had been requesting: Patch 3/5 is new, and adds a new <activecommit/> marker to the 'virsh capabilities' output if qemu is detected as supporting active commit. I've resolved some of Peter's review comments in patches 1 and 2 (patch 1 underwent a bit of a rewrite to differentiate the qemu capability probe from normal use of the interface, and 2 dropped mention of RHEL 6.3 except in the commit message). At this point, I haven't altered 4 or 5, although I still need to do something about getting a persistent domain definition updated after a pivot changes the file in use by a running domain. I've forced an update to my git repo: http://repo.or.cz/w/libvirt/ericb.git/shortlog/refs/heads/gluster or git checkout git://repo.or.cz/libvirt/ericb.git gluster Eric Blake (5): blockjob: allow omitted arguments to QMP block-commit blockjob: turn on qemu capability bit for active commit blockjob: expose active commit capability blockcommit: track job type in xml blockcommit: turn on active commit docs/formatcaps.html.in | 46 +++++++++++++++- docs/formatdomain.html.in | 20 +++---- docs/schemas/capability.rng | 5 ++ docs/schemas/domaincommon.rng | 13 +++++ src/conf/capabilities.c | 3 +- src/conf/domain_conf.c | 33 +++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 13 +++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 53 ++++++++++++++---- src/qemu/qemu_monitor.c | 12 +++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 29 ++++++++-- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_process.c | 18 ++++--- tests/qemucapabilitiesdata/caps_1.3.1-1.replies | 62 ++++++++++++---------- tests/qemucapabilitiesdata/caps_1.4.2-1.replies | 62 ++++++++++++---------- tests/qemucapabilitiesdata/caps_1.5.3-1.replies | 62 ++++++++++++---------- tests/qemucapabilitiesdata/caps_1.6.0-1.replies | 62 ++++++++++++---------- tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 62 ++++++++++++---------- tests/qemumonitorjsontest.c | 47 ++++++++++++++++ .../qemuxml2argv-disk-active-commit.xml | 37 +++++++++++++ .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +- .../qemuxml2xmlout-disk-mirror-old.xml | 4 +- tests/qemuxml2xmltest.c | 1 + 25 files changed, 481 insertions(+), 173 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml -- 1.9.3
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 (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> --- src/qemu/qemu_monitor.c | 12 +++++++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 29 ++++++++++++++++++++++++--- src/qemu/qemu_monitor_json.h | 3 +-- tests/qemumonitorjsontest.c | 47 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 87 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 80d7b9d..2d584fc 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3261,6 +3261,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 31b3204..63e78d8 100644 --- 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 bedd959..75b33e8 100644 --- 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 absent; 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 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..d136576 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1992,6 +1992,52 @@ testQemuMonitorJSONqemuMonitorJSONSendKeyHoldtime(const void *data) } 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) { virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)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"); -- 1.9.3
On 06/24/14 01:30, Eric Blake wrote:
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 (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> --- src/qemu/qemu_monitor.c | 12 +++++++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 29 ++++++++++++++++++++++++--- src/qemu/qemu_monitor_json.h | 3 +-- tests/qemumonitorjsontest.c | 47 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 87 insertions(+), 5 deletions(-)
ACK, Peter
On 06/24/2014 01:10 AM, Peter Krempa wrote:
On 06/24/14 01:30, Eric Blake wrote:
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).
* 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.
ACK,
Now pushed, after amending the commit message to point to the qemu.git commit this was gated on. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Use the probing functionality added in the last patch to turn on a capability bit when active commit is present, and gate active commit on that capability. For my own reference: the difference between BLOCKJOB_SYNC and BLOCKJOB_ASYNC is whether qemu generated an event at the conclusion of blockpull; basically, RHEL 6.2 was the only release of qemu that has the sync semantics and lacks the event. RHEL 6.3 added blockcopy, but also picked up on the upstream style of qemu generating events. As no one is likely to backport active commit to RHEL 6.2, it's safe for blockcommit to always require async blockjob support. Modifying qemucapabilitiestest is painful; the .replies files would be so much easier if they had comments correlating which command generated the given reply. Maybe I'll fix that up later... * src/qemu/qemu_capabilities.h (QEMU_CAPS_ACTIVE_COMMIT): New capability. * src/qemu/qemu_driver.c (qemuDomainBlockCommit): Use the new bit * src/qemu/qemu_capabilities.c (virQEMUCaps): Name the new bit. (virQEMUCapsProbeQMPCommands): Set it. * tests/qemucapabilitiesdata/caps_1.3.1-1.replies: Update. * tests/qemucapabilitiesdata/caps_1.4.2-1.replies: Likewise. * tests/qemucapabilitiesdata/caps_1.5.3-1.replies: Likewise. * tests/qemucapabilitiesdata/caps_1.6.0-1.replies: Likewise. * tests/qemucapabilitiesdata/caps_1.6.50-1.replies: Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_capabilities.c | 7 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 10 ++-- tests/qemucapabilitiesdata/caps_1.3.1-1.replies | 62 +++++++++++++----------- tests/qemucapabilitiesdata/caps_1.4.2-1.replies | 62 +++++++++++++----------- tests/qemucapabilitiesdata/caps_1.5.3-1.replies | 62 +++++++++++++----------- tests/qemucapabilitiesdata/caps_1.6.0-1.replies | 62 +++++++++++++----------- tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 62 +++++++++++++----------- 8 files changed, 188 insertions(+), 140 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 245d6b5..d698db9 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -256,6 +256,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "usb-kbd", /* 165 */ "host-pci-multidomain", "msg-timestamp", + "active-commit", ); @@ -2176,6 +2177,12 @@ virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps, VIR_FORCE_CLOSE(fd); } + /* Probe for active commit of qemu 2.1 (for now, we are choosing + * to ignore the fact that qemu 2.0 can also do active commit) */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCK_COMMIT) && + qemuMonitorSupportsActiveCommit(mon)) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_ACTIVE_COMMIT); + return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d755caa..a18d298 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -206,6 +206,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_USB_KBD = 165, /* -device usb-kbd */ QEMU_CAPS_HOST_PCI_MULTIDOMAIN = 166, /* support domain > 0 in host pci address */ QEMU_CAPS_MSG_TIMESTAMP = 167, /* -msg timestamp */ + QEMU_CAPS_ACTIVE_COMMIT = 168, /* block-commit works without 'top' */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 22a8ca5..b57f77b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15509,7 +15509,10 @@ qemuDomainBlockCommit(virDomainPtr dom, "%s", _("domain is not running")); goto endjob; } - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCK_COMMIT)) { + /* Ensure that no one backports commit to RHEL 6.2, where cancel + * behaved differently */ + if (!(virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCK_COMMIT) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("online commit not supported with this QEMU binary")); goto endjob; @@ -15541,10 +15544,7 @@ qemuDomainBlockCommit(virDomainPtr dom, * process; qemu 2.1 is further improving active commit. We need * to start supporting it in libvirt. */ if (topSource == disk->src) { - /* We assume that no one will backport qemu 2.0 active commit - * to an earlier qemu without also backporting the block job - * ready event; but this makes sure of that fact */ - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) { + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_ACTIVE_COMMIT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("active commit not supported with this QEMU binary")); goto endjob; diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.replies b/tests/qemucapabilitiesdata/caps_1.3.1-1.replies index 63c18da..04d2141 100644 --- a/tests/qemucapabilitiesdata/caps_1.3.1-1.replies +++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.replies @@ -308,6 +308,14 @@ } { + "id": "libvirt-6", + "error": { + "class": "GenericError", + "desc": "Parameter 'top' is missing" + } +} + +{ "return": [ { "name": "SPICE_MIGRATE_COMPLETED" @@ -382,7 +390,7 @@ "name": "SHUTDOWN" } ], - "id": "libvirt-6" + "id": "libvirt-7" } { @@ -871,7 +879,7 @@ "name": "VGA" } ], - "id": "libvirt-7" + "id": "libvirt-8" } { @@ -973,7 +981,7 @@ "type": "hex32" } ], - "id": "libvirt-8" + "id": "libvirt-9" } { @@ -1111,11 +1119,11 @@ "type": "on/off" } ], - "id": "libvirt-9" + "id": "libvirt-10" } { - "id": "libvirt-10", + "id": "libvirt-11", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-blk-ccw' not found" @@ -1123,7 +1131,7 @@ } { - "id": "libvirt-11", + "id": "libvirt-12", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-net-ccw' not found" @@ -1131,7 +1139,7 @@ } { - "id": "libvirt-12", + "id": "libvirt-13", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-blk-s390' not found" @@ -1139,7 +1147,7 @@ } { - "id": "libvirt-13", + "id": "libvirt-14", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-net-s390' not found" @@ -1147,7 +1155,7 @@ } { - "id": "libvirt-14", + "id": "libvirt-15", "error": { "class": "DeviceNotFound", "desc": "Device 'pci-assign' not found" @@ -1197,7 +1205,7 @@ "type": "pci-host-devaddr" } ], - "id": "libvirt-15" + "id": "libvirt-16" } { @@ -1231,7 +1239,7 @@ "type": "pci-host-devaddr" } ], - "id": "libvirt-16" + "id": "libvirt-17" } { @@ -1305,7 +1313,7 @@ "type": "drive" } ], - "id": "libvirt-17" + "id": "libvirt-18" } { @@ -1359,7 +1367,7 @@ "type": "drive" } ], - "id": "libvirt-18" + "id": "libvirt-19" } { @@ -1401,11 +1409,11 @@ "type": "uint32" } ], - "id": "libvirt-19" + "id": "libvirt-20" } { - "id": "libvirt-20", + "id": "libvirt-21", "error": { "class": "DeviceNotFound", "desc": "Device 'usb-redir' not found" @@ -1455,7 +1463,7 @@ "type": "uint32" } ], - "id": "libvirt-21" + "id": "libvirt-22" } { @@ -1481,13 +1489,13 @@ "type": "drive" } ], - "id": "libvirt-22" + "id": "libvirt-23" } { "return": [ ], - "id": "libvirt-23" + "id": "libvirt-24" } { @@ -1497,7 +1505,7 @@ "type": "uint64" } ], - "id": "libvirt-24" + "id": "libvirt-25" } { @@ -1547,7 +1555,7 @@ "type": "drive" } ], - "id": "libvirt-25" + "id": "libvirt-26" } { @@ -1561,7 +1569,7 @@ "type": "hex32" } ], - "id": "libvirt-26" + "id": "libvirt-27" } { @@ -1615,7 +1623,7 @@ "name": "none" } ], - "id": "libvirt-26" + "id": "libvirt-28" } { @@ -1693,7 +1701,7 @@ "name": "Opteron_G5" } ], - "id": "libvirt-27" + "id": "libvirt-29" } { @@ -1701,11 +1709,11 @@ "enabled": false, "present": true }, - "id": "libvirt-28" + "id": "libvirt-30" } { - "id": "libvirt-29", + "id": "libvirt-31", "error": { "class": "CommandNotFound", "desc": "The command query-tpm-models has not been found" @@ -1713,7 +1721,7 @@ } { - "id": "libvirt-30", + "id": "libvirt-32", "error": { "class": "CommandNotFound", "desc": "The command query-tpm-types has not been found" @@ -1721,7 +1729,7 @@ } { - "id": "libvirt-31", + "id": "libvirt-33", "error": { "class": "CommandNotFound", "desc": "The command query-command-line-options has not been found" diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.replies b/tests/qemucapabilitiesdata/caps_1.4.2-1.replies index 4fb4061..aef359c 100644 --- a/tests/qemucapabilitiesdata/caps_1.4.2-1.replies +++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.replies @@ -320,6 +320,14 @@ } { + "id": "libvirt-6", + "error": { + "class": "GenericError", + "desc": "Parameter 'top' is missing" + } +} + +{ "return": [ { "name": "SPICE_MIGRATE_COMPLETED" @@ -394,7 +402,7 @@ "name": "SHUTDOWN" } ], - "id": "libvirt-6" + "id": "libvirt-7" } { @@ -910,7 +918,7 @@ "name": "VGA" } ], - "id": "libvirt-7" + "id": "libvirt-8" } { @@ -1012,7 +1020,7 @@ "type": "hex32" } ], - "id": "libvirt-8" + "id": "libvirt-9" } { @@ -1158,11 +1166,11 @@ "type": "on/off" } ], - "id": "libvirt-9" + "id": "libvirt-10" } { - "id": "libvirt-10", + "id": "libvirt-11", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-blk-ccw' not found" @@ -1170,7 +1178,7 @@ } { - "id": "libvirt-11", + "id": "libvirt-12", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-net-ccw' not found" @@ -1178,7 +1186,7 @@ } { - "id": "libvirt-12", + "id": "libvirt-13", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-blk-s390' not found" @@ -1186,7 +1194,7 @@ } { - "id": "libvirt-13", + "id": "libvirt-14", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-net-s390' not found" @@ -1194,7 +1202,7 @@ } { - "id": "libvirt-14", + "id": "libvirt-15", "error": { "class": "DeviceNotFound", "desc": "Device 'pci-assign' not found" @@ -1244,7 +1252,7 @@ "type": "pci-host-devaddr" } ], - "id": "libvirt-15" + "id": "libvirt-16" } { @@ -1278,7 +1286,7 @@ "type": "pci-host-devaddr" } ], - "id": "libvirt-16" + "id": "libvirt-17" } { @@ -1352,7 +1360,7 @@ "type": "drive" } ], - "id": "libvirt-17" + "id": "libvirt-18" } { @@ -1406,7 +1414,7 @@ "type": "drive" } ], - "id": "libvirt-18" + "id": "libvirt-19" } { @@ -1448,11 +1456,11 @@ "type": "uint32" } ], - "id": "libvirt-19" + "id": "libvirt-20" } { - "id": "libvirt-20", + "id": "libvirt-21", "error": { "class": "DeviceNotFound", "desc": "Device 'usb-redir' not found" @@ -1502,7 +1510,7 @@ "type": "uint32" } ], - "id": "libvirt-21" + "id": "libvirt-22" } { @@ -1528,13 +1536,13 @@ "type": "drive" } ], - "id": "libvirt-22" + "id": "libvirt-23" } { "return": [ ], - "id": "libvirt-23" + "id": "libvirt-24" } { @@ -1544,7 +1552,7 @@ "type": "uint64" } ], - "id": "libvirt-24" + "id": "libvirt-25" } { @@ -1594,7 +1602,7 @@ "type": "drive" } ], - "id": "libvirt-25" + "id": "libvirt-26" } { @@ -1608,7 +1616,7 @@ "type": "hex32" } ], - "id": "libvirt-26" + "id": "libvirt-27" } { @@ -1665,7 +1673,7 @@ "name": "none" } ], - "id": "libvirt-26" + "id": "libvirt-28" } { @@ -1743,7 +1751,7 @@ "name": "qemu64" } ], - "id": "libvirt-27" + "id": "libvirt-29" } { @@ -1751,11 +1759,11 @@ "enabled": false, "present": true }, - "id": "libvirt-28" + "id": "libvirt-30" } { - "id": "libvirt-29", + "id": "libvirt-31", "error": { "class": "CommandNotFound", "desc": "The command query-tpm-models has not been found" @@ -1763,7 +1771,7 @@ } { - "id": "libvirt-30", + "id": "libvirt-32", "error": { "class": "CommandNotFound", "desc": "The command query-tpm-types has not been found" @@ -1771,7 +1779,7 @@ } { - "id": "libvirt-31", + "id": "libvirt-33", "error": { "class": "CommandNotFound", "desc": "The command query-command-line-options has not been found" diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.replies b/tests/qemucapabilitiesdata/caps_1.5.3-1.replies index 686fa3e..1f84bb6 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.replies +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.replies @@ -335,6 +335,14 @@ } { + "id": "libvirt-6", + "error": { + "class": "GenericError", + "desc": "Parameter 'top' is missing" + } +} + +{ "return": [ { "name": "GUEST_PANICKED" @@ -415,7 +423,7 @@ "name": "SHUTDOWN" } ], - "id": "libvirt-6" + "id": "libvirt-7" } { @@ -976,7 +984,7 @@ "name": "VGA" } ], - "id": "libvirt-7" + "id": "libvirt-8" } { @@ -1078,7 +1086,7 @@ "type": "hex32" } ], - "id": "libvirt-8" + "id": "libvirt-9" } { @@ -1224,11 +1232,11 @@ "type": "on/off" } ], - "id": "libvirt-9" + "id": "libvirt-10" } { - "id": "libvirt-10", + "id": "libvirt-11", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-blk-ccw' not found" @@ -1236,7 +1244,7 @@ } { - "id": "libvirt-11", + "id": "libvirt-12", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-net-ccw' not found" @@ -1244,7 +1252,7 @@ } { - "id": "libvirt-12", + "id": "libvirt-13", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-blk-s390' not found" @@ -1252,7 +1260,7 @@ } { - "id": "libvirt-13", + "id": "libvirt-14", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-net-s390' not found" @@ -1260,7 +1268,7 @@ } { - "id": "libvirt-14", + "id": "libvirt-15", "error": { "class": "DeviceNotFound", "desc": "Device 'pci-assign' not found" @@ -1310,7 +1318,7 @@ "type": "pci-host-devaddr" } ], - "id": "libvirt-15" + "id": "libvirt-16" } { @@ -1352,7 +1360,7 @@ "type": "pci-host-devaddr" } ], - "id": "libvirt-16" + "id": "libvirt-17" } { @@ -1426,7 +1434,7 @@ "type": "drive" } ], - "id": "libvirt-17" + "id": "libvirt-18" } { @@ -1480,7 +1488,7 @@ "type": "drive" } ], - "id": "libvirt-18" + "id": "libvirt-19" } { @@ -1522,11 +1530,11 @@ "type": "uint32" } ], - "id": "libvirt-19" + "id": "libvirt-20" } { - "id": "libvirt-20", + "id": "libvirt-21", "error": { "class": "DeviceNotFound", "desc": "Device 'usb-redir' not found" @@ -1576,7 +1584,7 @@ "type": "uint32" } ], - "id": "libvirt-21" + "id": "libvirt-22" } { @@ -1602,13 +1610,13 @@ "type": "drive" } ], - "id": "libvirt-22" + "id": "libvirt-23" } { "return": [ ], - "id": "libvirt-23" + "id": "libvirt-24" } { @@ -1618,7 +1626,7 @@ "type": "uint64" } ], - "id": "libvirt-24" + "id": "libvirt-25" } { @@ -1668,7 +1676,7 @@ "type": "drive" } ], - "id": "libvirt-25" + "id": "libvirt-26" } { @@ -1682,7 +1690,7 @@ "type": "hex32" } ], - "id": "libvirt-26" + "id": "libvirt-27" } { @@ -1755,7 +1763,7 @@ "cpu-max": 1 } ], - "id": "libvirt-26" + "id": "libvirt-28" } { @@ -1833,7 +1841,7 @@ "name": "qemu64" } ], - "id": "libvirt-27" + "id": "libvirt-29" } { @@ -1841,19 +1849,19 @@ "enabled": false, "present": true }, - "id": "libvirt-28" + "id": "libvirt-30" } { "return": [ ], - "id": "libvirt-29" + "id": "libvirt-31" } { "return": [ ], - "id": "libvirt-30" + "id": "libvirt-32" } { @@ -2529,5 +2537,5 @@ "option": "drive" } ], - "id": "libvirt-31" + "id": "libvirt-33" } diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.replies b/tests/qemucapabilitiesdata/caps_1.6.0-1.replies index 95e0c37..c9dc29b 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.replies +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.replies @@ -341,6 +341,14 @@ } { + "id": "libvirt-6", + "error": { + "class": "GenericError", + "desc": "Parameter 'top' is missing" + } +} + +{ "return": [ { "name": "GUEST_PANICKED" @@ -424,7 +432,7 @@ "name": "SHUTDOWN" } ], - "id": "libvirt-6" + "id": "libvirt-7" } { @@ -1018,7 +1026,7 @@ "name": "VGA" } ], - "id": "libvirt-7" + "id": "libvirt-8" } { @@ -1120,7 +1128,7 @@ "type": "hex32" } ], - "id": "libvirt-8" + "id": "libvirt-9" } { @@ -1274,11 +1282,11 @@ "type": "on/off" } ], - "id": "libvirt-9" + "id": "libvirt-10" } { - "id": "libvirt-10", + "id": "libvirt-11", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-blk-ccw' not found" @@ -1286,7 +1294,7 @@ } { - "id": "libvirt-11", + "id": "libvirt-12", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-net-ccw' not found" @@ -1294,7 +1302,7 @@ } { - "id": "libvirt-12", + "id": "libvirt-13", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-blk-s390' not found" @@ -1302,7 +1310,7 @@ } { - "id": "libvirt-13", + "id": "libvirt-14", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-net-s390' not found" @@ -1310,7 +1318,7 @@ } { - "id": "libvirt-14", + "id": "libvirt-15", "error": { "class": "DeviceNotFound", "desc": "Device 'pci-assign' not found" @@ -1360,7 +1368,7 @@ "type": "pci-host-devaddr" } ], - "id": "libvirt-15" + "id": "libvirt-16" } { @@ -1402,7 +1410,7 @@ "type": "pci-host-devaddr" } ], - "id": "libvirt-16" + "id": "libvirt-17" } { @@ -1476,7 +1484,7 @@ "type": "drive" } ], - "id": "libvirt-17" + "id": "libvirt-18" } { @@ -1530,7 +1538,7 @@ "type": "drive" } ], - "id": "libvirt-18" + "id": "libvirt-19" } { @@ -1572,11 +1580,11 @@ "type": "uint32" } ], - "id": "libvirt-19" + "id": "libvirt-20" } { - "id": "libvirt-20", + "id": "libvirt-21", "error": { "class": "DeviceNotFound", "desc": "Device 'usb-redir' not found" @@ -1630,7 +1638,7 @@ "type": "uint32" } ], - "id": "libvirt-21" + "id": "libvirt-22" } { @@ -1656,7 +1664,7 @@ "type": "drive" } ], - "id": "libvirt-22" + "id": "libvirt-23" } { @@ -1666,7 +1674,7 @@ "type": "size" } ], - "id": "libvirt-23" + "id": "libvirt-24" } { @@ -1680,7 +1688,7 @@ "type": "uint64" } ], - "id": "libvirt-24" + "id": "libvirt-25" } { @@ -1730,7 +1738,7 @@ "type": "drive" } ], - "id": "libvirt-25" + "id": "libvirt-26" } { @@ -1744,7 +1752,7 @@ "type": "hex32" } ], - "id": "libvirt-26" + "id": "libvirt-27" } { @@ -1833,7 +1841,7 @@ "cpu-max": 1 } ], - "id": "libvirt-26" + "id": "libvirt-28" } { @@ -1911,7 +1919,7 @@ "name": "qemu64" } ], - "id": "libvirt-27" + "id": "libvirt-29" } { @@ -1919,19 +1927,19 @@ "enabled": false, "present": true }, - "id": "libvirt-28" + "id": "libvirt-30" } { "return": [ ], - "id": "libvirt-29" + "id": "libvirt-31" } { "return": [ ], - "id": "libvirt-30" + "id": "libvirt-32" } { @@ -2509,5 +2517,5 @@ "option": "drive" } ], - "id": "libvirt-31" + "id": "libvirt-33" } diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.replies b/tests/qemucapabilitiesdata/caps_1.6.50-1.replies index 3ecf185..a60542a 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.replies +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.replies @@ -347,6 +347,14 @@ } { + "id": "libvirt-6", + "error": { + "class": "GenericError", + "desc": "Parameter 'top' is missing" + } +} + +{ "return": [ { "name": "BLOCK_IMAGE_CORRUPTED" @@ -433,7 +441,7 @@ "name": "SHUTDOWN" } ], - "id": "libvirt-6" + "id": "libvirt-7" } { @@ -1024,7 +1032,7 @@ "name": "VGA" } ], - "id": "libvirt-7" + "id": "libvirt-8" } { @@ -1126,7 +1134,7 @@ "type": "hex32" } ], - "id": "libvirt-8" + "id": "libvirt-9" } { @@ -1280,11 +1288,11 @@ "type": "on/off" } ], - "id": "libvirt-9" + "id": "libvirt-10" } { - "id": "libvirt-10", + "id": "libvirt-11", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-blk-ccw' not found" @@ -1292,7 +1300,7 @@ } { - "id": "libvirt-11", + "id": "libvirt-12", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-net-ccw' not found" @@ -1300,7 +1308,7 @@ } { - "id": "libvirt-12", + "id": "libvirt-13", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-blk-s390' not found" @@ -1308,7 +1316,7 @@ } { - "id": "libvirt-13", + "id": "libvirt-14", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-net-s390' not found" @@ -1316,7 +1324,7 @@ } { - "id": "libvirt-14", + "id": "libvirt-15", "error": { "class": "DeviceNotFound", "desc": "Device 'pci-assign' not found" @@ -1366,7 +1374,7 @@ "type": "pci-host-devaddr" } ], - "id": "libvirt-15" + "id": "libvirt-16" } { @@ -1408,7 +1416,7 @@ "type": "pci-host-devaddr" } ], - "id": "libvirt-16" + "id": "libvirt-17" } { @@ -1482,7 +1490,7 @@ "type": "drive" } ], - "id": "libvirt-17" + "id": "libvirt-18" } { @@ -1536,7 +1544,7 @@ "type": "drive" } ], - "id": "libvirt-18" + "id": "libvirt-19" } { @@ -1578,11 +1586,11 @@ "type": "uint32" } ], - "id": "libvirt-19" + "id": "libvirt-20" } { - "id": "libvirt-20", + "id": "libvirt-21", "error": { "class": "DeviceNotFound", "desc": "Device 'usb-redir' not found" @@ -1590,7 +1598,7 @@ } { - "id": "libvirt-21", + "id": "libvirt-22", "error": { "class": "DeviceNotFound", "desc": "Device 'usb-host' not found" @@ -1620,7 +1628,7 @@ "type": "drive" } ], - "id": "libvirt-22" + "id": "libvirt-23" } { @@ -1630,7 +1638,7 @@ "type": "size" } ], - "id": "libvirt-23" + "id": "libvirt-24" } { @@ -1644,7 +1652,7 @@ "type": "uint64" } ], - "id": "libvirt-24" + "id": "libvirt-25" } { @@ -1694,7 +1702,7 @@ "type": "drive" } ], - "id": "libvirt-25" + "id": "libvirt-26" } { @@ -1708,7 +1716,7 @@ "type": "hex32" } ], - "id": "libvirt-26" + "id": "libvirt-27" } { @@ -1805,7 +1813,7 @@ "cpu-max": 1 } ], - "id": "libvirt-26" + "id": "libvirt-28" } { @@ -1883,7 +1891,7 @@ "name": "qemu64" } ], - "id": "libvirt-27" + "id": "libvirt-29" } { @@ -1891,19 +1899,19 @@ "enabled": false, "present": true }, - "id": "libvirt-28" + "id": "libvirt-30" } { "return": [ ], - "id": "libvirt-29" + "id": "libvirt-31" } { "return": [ ], - "id": "libvirt-30" + "id": "libvirt-32" } { @@ -2486,5 +2494,5 @@ "option": "drive" } ], - "id": "libvirt-31" + "id": "libvirt-33" } -- 1.9.3
On 06/24/14 01:30, Eric Blake wrote:
Use the probing functionality added in the last patch to turn on a capability bit when active commit is present, and gate active commit on that capability.
For my own reference: the difference between BLOCKJOB_SYNC and BLOCKJOB_ASYNC is whether qemu generated an event at the conclusion of blockpull; basically, RHEL 6.2 was the only release of qemu that has the sync semantics and lacks the event. RHEL 6.3 added blockcopy, but also picked up on the upstream style of qemu generating events. As no one is likely to backport active commit to RHEL 6.2, it's safe for blockcommit to always require async blockjob support.
Modifying qemucapabilitiestest is painful; the .replies files would be so much easier if they had comments correlating which command generated the given reply. Maybe I'll fix that up later...
* src/qemu/qemu_capabilities.h (QEMU_CAPS_ACTIVE_COMMIT): New capability. * src/qemu/qemu_driver.c (qemuDomainBlockCommit): Use the new bit * src/qemu/qemu_capabilities.c (virQEMUCaps): Name the new bit. (virQEMUCapsProbeQMPCommands): Set it. * tests/qemucapabilitiesdata/caps_1.3.1-1.replies: Update. * tests/qemucapabilitiesdata/caps_1.4.2-1.replies: Likewise. * tests/qemucapabilitiesdata/caps_1.5.3-1.replies: Likewise. * tests/qemucapabilitiesdata/caps_1.6.0-1.replies: Likewise. * tests/qemucapabilitiesdata/caps_1.6.50-1.replies: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_capabilities.c | 7 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 10 ++-- tests/qemucapabilitiesdata/caps_1.3.1-1.replies | 62 +++++++++++++----------- tests/qemucapabilitiesdata/caps_1.4.2-1.replies | 62 +++++++++++++----------- tests/qemucapabilitiesdata/caps_1.5.3-1.replies | 62 +++++++++++++----------- tests/qemucapabilitiesdata/caps_1.6.0-1.replies | 62 +++++++++++++----------- tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 62 +++++++++++++----------- 8 files changed, 188 insertions(+), 140 deletions(-)
ACK, Peter
On 06/24/2014 01:15 AM, Peter Krempa wrote:
On 06/24/14 01:30, Eric Blake wrote:
Use the probing functionality added in the last patch to turn on a capability bit when active commit is present, and gate active commit on that capability.
For my own reference: the difference between BLOCKJOB_SYNC and BLOCKJOB_ASYNC is whether qemu generated an event at the conclusion of blockpull; basically, RHEL 6.2 was the only release of qemu that has the sync semantics and lacks the event. RHEL 6.3 added blockcopy, but also picked up on the upstream style of qemu generating events. As no one is likely to backport active commit to RHEL 6.2, it's safe for blockcommit to always require async blockjob support.
Modifying qemucapabilitiestest is painful; the .replies files would be so much easier if they had comments correlating which command generated the given reply. Maybe I'll fix that up later...
ACK,
Pushed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Add an element to QEMU's capability XML, to show if the underlying qemu binary supports active commit. This allows the client to know ahead of time if they can rely on this method, or must fall back to older techniques such as blockpull. Without this information, the only way to check for active commit is to attempt one and check for errors. This attribute can be a simple binary (active commit is supported only if <activecommit/> is in the capabilities) rather than a full-blown toggle definition. (In contrast, the <disksnapshot> capability had to be a toggle because we forgot to add it at the same time as turning on the feature of external snapshots; and therefore, the absence of the attribute is not sufficient to conclude whether disk snapshots are supported.) Our documentation for features was rather sparse; this fleshes out more of the details for other existing capabilities (and cost me some time trawling git history). * docs/schemas/capability.rng (features): Add activecommit. * docs/formatcaps.html.in: Document it, and other features. * src/qemu/qemu_capabilities.c (virQEMUCapsInitGuestFromBinary): Set it. * src/conf/capabilities.c (virCapabilitiesFormatXML): Expose as simpler binary. Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/formatcaps.html.in | 46 +++++++++++++++++++++++++++++++++++++++++++- docs/schemas/capability.rng | 5 +++++ src/conf/capabilities.c | 3 ++- src/qemu/qemu_capabilities.c | 6 ++++++ 4 files changed, 58 insertions(+), 2 deletions(-) diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 137af25..6ac0f4a 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -95,7 +95,51 @@ <dt><code>features</code></dt> <dd>This optional element encases possible features that can be used - with a guest of described type.</dd> + with a guest of described type. Possible subelements are: + <dl> + <dt>pae</dt><dd>If present, 32-bit guests can use PAE + address space extensions, <span class="since">since + 0.4.1</span></dd> + <dt>nonpae</dt><dd>If present, 32-bit guests can be run + without requiring PAE, <span class="since">since + 0.4.1</span></dd> + <dt>ia64_be</dt><dd>If present, IA64 guests can be run in + big-endian mode, <span class="since">since 0.4.1</span></dd> + <dt>acpi</dt><dd>If this element is present, + the <code>default</code> attribute describes whether the + hypervisor exposes ACPI to the guest by default, and + the <code>toggle</code> attribute describes whether the + user can override this + default. <span class="since">Since 0.4.1</span></dd> + <dt>apic</dt><dd>If this element is present, + the <code>default</code> attribute describes whether the + hypervisor exposes APIC to the guest by default, and + the <code>toggle</code> attribute describes whether the + user can override this + default. <span class="since">Since 0.4.1</span></dd> + <dt>cpuselection</dt><dd>If this element is present, the + hypervisor supports the <code><cpu></code> element + within a domain definition for fine-grained control over + the CPU presented to the + guest. <span class="since">Since 0.7.5</span></dd> + <dt>deviceboot</dt><dd>If this element is present, + the <code><boot order='...'/></code> element can + be used inside devices, rather than the older boot + specification by category. <span class="since">Since + 0.8.8</span></dd> + <dt>disksnapshot</dt><dd>If this element is present, + the <code>default</code> attribute describes whether + external disk snapshots are supported. If absent, + external snapshots may still be supported, but it + requires attempting the API and checking for an error to + find out for sure. <span class="since">Since + 1.2.3</span></dd> + <dt>activecommit</dt><dd>Active commit (via the + virDomainBlockCommit API) is supported only if this + element is present. <span class="since">Since + 1.2.6</span></dd> + </dl> + </dd> </dl> <h3><a name="elementExamples">Examples</a></h3> diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index f954599..de32ee9 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -399,6 +399,11 @@ <empty/> </element> </optional> + <optional> + <element name='activecommit'> + <empty/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 19359a5..ebf6121 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -1005,7 +1005,8 @@ virCapabilitiesFormatXML(virCapsPtr caps) STREQ(caps->guests[i]->features[j]->name, "nonpae") || STREQ(caps->guests[i]->features[j]->name, "ia64_be") || STREQ(caps->guests[i]->features[j]->name, "cpuselection") || - STREQ(caps->guests[i]->features[j]->name, "deviceboot")) { + STREQ(caps->guests[i]->features[j]->name, "deviceboot") || + STREQ(caps->guests[i]->features[j]->name, "activecommit")) { virBufferAsprintf(&buf, "<%s/>\n", caps->guests[i]->features[j]->name); } else { diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d698db9..d1d33d5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -822,6 +822,12 @@ virQEMUCapsInitGuestFromBinary(virCapsPtr caps, if (!virCapabilitiesAddGuestFeature(guest, "disksnapshot", hasdisksnapshot, 0)) goto cleanup; + if (!virCapabilitiesAddGuestFeature(guest, "activecommit", + virQEMUCapsGet(qemubinCaps, + QEMU_CAPS_ACTIVE_COMMIT), + 0)) + goto cleanup; + if (virCapabilitiesAddGuestDomain(guest, "qemu", NULL, -- 1.9.3
On 06/24/14 01:30, Eric Blake wrote:
Add an element to QEMU's capability XML, to show if the underlying qemu binary supports active commit. This allows the client to know ahead of time if they can rely on this method, or must fall back to older techniques such as blockpull. Without this information, the only way to check for active commit is to attempt one and check for errors.
This attribute can be a simple binary (active commit is supported only if <activecommit/> is in the capabilities) rather than a full-blown toggle definition. (In contrast, the <disksnapshot> capability had to be a toggle because we forgot to add it at the same time as turning on the feature of external snapshots; and therefore, the absence of the attribute is not sufficient to conclude whether disk snapshots are supported.)
Our documentation for features was rather sparse; this fleshes out more of the details for other existing capabilities (and cost me some time trawling git history).
* docs/schemas/capability.rng (features): Add activecommit. * docs/formatcaps.html.in: Document it, and other features. * src/qemu/qemu_capabilities.c (virQEMUCapsInitGuestFromBinary): Set it. * src/conf/capabilities.c (virCapabilitiesFormatXML): Expose as simpler binary.
Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/formatcaps.html.in | 46 +++++++++++++++++++++++++++++++++++++++++++- docs/schemas/capability.rng | 5 +++++ src/conf/capabilities.c | 3 ++- src/qemu/qemu_capabilities.c | 6 ++++++ 4 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 137af25..6ac0f4a 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -95,7 +95,51 @@
<dt><code>features</code></dt> <dd>This optional element encases possible features that can be used - with a guest of described type.</dd> + with a guest of described type. Possible subelements are: + <dl> + <dt>pae</dt><dd>If present, 32-bit guests can use PAE + address space extensions, <span class="since">since + 0.4.1</span></dd> + <dt>nonpae</dt><dd>If present, 32-bit guests can be run + without requiring PAE, <span class="since">since + 0.4.1</span></dd> + <dt>ia64_be</dt><dd>If present, IA64 guests can be run in + big-endian mode, <span class="since">since 0.4.1</span></dd> + <dt>acpi</dt><dd>If this element is present, + the <code>default</code> attribute describes whether the + hypervisor exposes ACPI to the guest by default, and + the <code>toggle</code> attribute describes whether the + user can override this + default. <span class="since">Since 0.4.1</span></dd> + <dt>apic</dt><dd>If this element is present, + the <code>default</code> attribute describes whether the + hypervisor exposes APIC to the guest by default, and + the <code>toggle</code> attribute describes whether the + user can override this + default. <span class="since">Since 0.4.1</span></dd> + <dt>cpuselection</dt><dd>If this element is present, the + hypervisor supports the <code><cpu></code> element + within a domain definition for fine-grained control over + the CPU presented to the + guest. <span class="since">Since 0.7.5</span></dd> + <dt>deviceboot</dt><dd>If this element is present, + the <code><boot order='...'/></code> element can + be used inside devices, rather than the older boot + specification by category. <span class="since">Since + 0.8.8</span></dd> + <dt>disksnapshot</dt><dd>If this element is present, + the <code>default</code> attribute describes whether + external disk snapshots are supported. If absent, + external snapshots may still be supported, but it + requires attempting the API and checking for an error to + find out for sure. <span class="since">Since + 1.2.3</span></dd> + <dt>activecommit</dt><dd>Active commit (via the + virDomainBlockCommit API) is supported only if this + element is present. <span class="since">Since + 1.2.6</span></dd> + </dl> + </dd> </dl>
<h3><a name="elementExamples">Examples</a></h3>
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 19359a5..ebf6121 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -1005,7 +1005,8 @@ virCapabilitiesFormatXML(virCapsPtr caps) STREQ(caps->guests[i]->features[j]->name, "nonpae") || STREQ(caps->guests[i]->features[j]->name, "ia64_be") || STREQ(caps->guests[i]->features[j]->name, "cpuselection") || - STREQ(caps->guests[i]->features[j]->name, "deviceboot")) { + STREQ(caps->guests[i]->features[j]->name, "deviceboot") || + STREQ(caps->guests[i]->features[j]->name, "activecommit")) { virBufferAsprintf(&buf, "<%s/>\n", caps->guests[i]->features[j]->name); } else { diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d698db9..d1d33d5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -822,6 +822,12 @@ virQEMUCapsInitGuestFromBinary(virCapsPtr caps, if (!virCapabilitiesAddGuestFeature(guest, "disksnapshot", hasdisksnapshot, 0)) goto cleanup;
+ if (!virCapabilitiesAddGuestFeature(guest, "activecommit", + virQEMUCapsGet(qemubinCaps, + QEMU_CAPS_ACTIVE_COMMIT), + 0))
Yuck, integers for boolean values. Pre-existing though.
+ goto cleanup; + if (virCapabilitiesAddGuestDomain(guest, "qemu", NULL,
ACK, Peter
On 06/24/2014 02:06 AM, Peter Krempa wrote:
On 06/24/14 01:30, Eric Blake wrote:
Add an element to QEMU's capability XML, to show if the underlying qemu binary supports active commit. This allows the client to know ahead of time if they can rely on this method, or must fall back to older techniques such as blockpull. Without this information, the only way to check for active commit is to attempt one and check for errors.
This attribute can be a simple binary (active commit is supported only if <activecommit/> is in the capabilities) rather than a full-blown toggle definition. (In contrast, the <disksnapshot> capability had to be a toggle because we forgot to add it at the same time as turning on the feature of external snapshots; and therefore, the absence of the attribute is not sufficient to conclude whether disk snapshots are supported.)
Our documentation for features was rather sparse; this fleshes out more of the details for other existing capabilities (and cost me some time trawling git history).
I'm going to split this patch. The documentation for the pre-existing bits,
+ if (!virCapabilitiesAddGuestFeature(guest, "activecommit", + virQEMUCapsGet(qemubinCaps, + QEMU_CAPS_ACTIVE_COMMIT), + 0))
Yuck, integers for boolean values. Pre-existing though.
and a cleanup of int-to-bool, is worth having, but now that we have virConnectGetDomainCapabilities, I'd rather expose this new capability through that API than by overloading another feature into this code. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
A future patch is going to wire up qemu active block commit jobs; but as they have similar events and are canceled/pivoted in the same way as block copy jobs, it is easiest to track all bookkeeping for the commit job by reusing the <mirror> element. This patch adds domain XML to track which job was responsible for creating a mirroring situation, and adds a job='copy' attribute to all existing uses of <mirror>. Along the way, it also massages the qemu monitor backend to read the new field in order to generate the correct type of libvirt job (even though it requires a future patch to actually cause a qemu event that can be reported as an active commit). * docs/schemas/domaincommon.rng: Enhance schema. * docs/formatdomain.html.in: Document it. * src/conf/domain_conf.h (_virDomainDiskDef): Add a field. * src/conf/domain_conf.c (virDomainBlockJobType): String conversion. (virDomainDiskDefParseXML): Parse job type. (virDomainDiskDefFormat): Output job type. * src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Distinguish active from regular commit. * src/qemu/qemu_driver.c (qemuDomainBlockCopy): Set job type. (qemuDomainBlockPivot, qemuDomainBlockJobImpl): Clean up job type on completion. * tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml: Update tests. * tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Likewise. * tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml: New file. * tests/qemuxml2xmltest.c (mymain): Drive new test. Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/formatdomain.html.in | 20 ++++++------ docs/schemas/domaincommon.rng | 13 ++++++++ src/conf/domain_conf.c | 33 ++++++++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_driver.c | 3 ++ src/qemu/qemu_process.c | 18 +++++++---- .../qemuxml2argv-disk-active-commit.xml | 37 ++++++++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +-- .../qemuxml2xmlout-disk-mirror-old.xml | 4 +-- tests/qemuxml2xmltest.c | 1 + 10 files changed, 113 insertions(+), 21 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3075e16..16061b8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1878,16 +1878,18 @@ </dd> <dt><code>mirror</code></dt> <dd> - This element is present if the hypervisor has started a block - copy operation (via the <code>virDomainBlockRebase</code> - API), where the mirror location in the <code>source</code> - sub-element will eventually have the same contents as the - source, and with the file format in the + This element is present if the hypervisor has started a + long-running block job operation, where the mirror location in + the <code>source</code> sub-element will eventually have the + same contents as the source, and with the file format in the sub-element <code>format</code> (which might differ from the - format of the source). The details of the <code>source</code> - sub-element are determined by the <code>type</code> attribute - of the mirror, similar to what is done for the - overall <code>disk</code> device element. If + format of the source). The <code>job</code> attribute + mentions which API started the operation ("copy" for + the <code>virDomainBlockRebase</code> API, or "active-commit" + for the <code>virDomainBlockCommit</code> API). The details + of the <code>source</code> sub-element are determined by + the <code>type</code> attribute of the mirror, similar to what + is done for the overall <code>disk</code> device element. If attribute <code>ready</code> is present, then it is known the disk is ready to pivot; otherwise, the disk is probably still copying. For now, this element only valid in output; it is diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 33d0308..fe943f9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4186,6 +4186,13 @@ </attribute> </optional> <optional> + <attribute name='job'> + <choice> + <value>copy</value> + </choice> + </attribute> + </optional> + <optional> <interleave> <ref name='diskSourceFile'/> <optional> @@ -4195,6 +4202,12 @@ </optional> </group> <group> <!-- preferred format --> + <attribute name='job'> + <choice> + <value>copy</value> + <value>active-commit</value> + </choice> + </attribute> <interleave> <ref name="diskSource"/> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 02c394f..65a41c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -760,6 +760,12 @@ VIR_ENUM_IMPL(virDomainDiskDiscard, VIR_DOMAIN_DISK_DISCARD_LAST, "unmap", "ignore") +/* Internal mapping: subset of block job types that can be present in + * <mirror> XML (remaining types are not two-phase). */ +VIR_ENUM_DECL(virDomainBlockJob) +VIR_ENUM_IMPL(virDomainBlockJob, VIR_DOMAIN_BLOCK_JOB_TYPE_LAST, + "", "", "copy", "", "active-commit") + #define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE #define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE @@ -5377,10 +5383,26 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, xmlStrEqual(cur->name, BAD_CAST "mirror") && !(flags & VIR_DOMAIN_XML_INACTIVE)) { char *ready; + char *blockJob; if (VIR_ALLOC(def->mirror) < 0) goto error; + blockJob = virXMLPropString(cur, "job"); + if (blockJob) { + def->mirrorJob = virDomainBlockJobTypeFromString(blockJob); + if (def->mirrorJob <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown mirror job type '%s'"), + blockJob); + VIR_FREE(blockJob); + goto error; + } + VIR_FREE(blockJob); + } else { + def->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY; + } + mirrorType = virXMLPropString(cur, "type"); if (mirrorType) { def->mirror->type = virStorageTypeFromString(mirrorType); @@ -5403,6 +5425,12 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, _("mirror requires file name")); goto error; } + if (def->mirrorJob != VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("mirror without type only supported " + "by copy job")); + goto error; + } mirrorFormat = virXMLPropString(cur, "format"); } if (mirrorFormat) { @@ -15198,10 +15226,13 @@ virDomainDiskDefFormat(virBufferPtr buf, formatStr = virStorageFileFormatTypeToString(def->mirror->format); virBufferAsprintf(buf, "<mirror type='%s'", virStorageTypeToString(def->mirror->type)); - if (def->mirror->type == VIR_STORAGE_TYPE_FILE) { + if (def->mirror->type == VIR_STORAGE_TYPE_FILE && + def->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) { virBufferEscapeString(buf, " file='%s'", def->mirror->path); virBufferEscapeString(buf, " format='%s'", formatStr); } + virBufferEscapeString(buf, " job='%s'", + virDomainBlockJobTypeToString(def->mirrorJob)); if (def->mirroring) virBufferAddLit(buf, " ready='yes'"); virBufferAddLit(buf, ">\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6779a41..c03fe64 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -608,6 +608,7 @@ struct _virDomainDiskDef { virStorageSourcePtr mirror; bool mirroring; + int mirrorJob; /* virDomainBlockJobType */ struct { unsigned int cylinders; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b57f77b..79554e9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14993,6 +14993,7 @@ qemuDomainBlockPivot(virConnectPtr conn, virStorageSourceFree(disk->mirror); disk->mirror = NULL; disk->mirroring = false; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; cleanup: if (resume && virDomainObjIsActive(vm) && @@ -15120,6 +15121,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, virStorageSourceFree(disk->mirror); disk->mirror = NULL; disk->mirroring = false; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; } /* With synchronous block cancel, we must synthesize an event, and @@ -15397,6 +15399,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm, need_unlink = false; disk->mirror = mirror; mirror = NULL; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY; endjob: if (need_unlink && unlink(dest)) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0b8155b..0c5a3ac 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1023,28 +1023,32 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, if (disk) { /* Have to generate two variants of the event for old vs. new * client callbacks */ + if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT && + disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT) + type = disk->mirrorJob; path = virDomainDiskGetSource(disk); event = virDomainEventBlockJobNewFromObj(vm, path, type, status); event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type, status); /* XXX If we completed a block pull or commit, then recompute * the cached backing chain to match. Better would be storing - * the chain ourselves rather than reprobing, but this - * requires modifying domain_conf and our XML to fully track - * the chain across libvirtd restarts. For that matter, if - * qemu gains support for committing the active layer, we have - * to update disk->src. */ + * the chain ourselves rather than reprobing, but we haven't + * quite completed that conversion to use our XML tracking. */ if ((type == VIR_DOMAIN_BLOCK_JOB_TYPE_PULL || - type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT) && + type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT || + type == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT) && status == VIR_DOMAIN_BLOCK_JOB_COMPLETED) qemuDomainDetermineDiskChain(driver, vm, disk, true); - if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) { + if (disk->mirror && + (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY || + type == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)) { if (status == VIR_DOMAIN_BLOCK_JOB_READY) { disk->mirroring = true; } else if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) { virStorageSourceFree(disk->mirror); disk->mirror = NULL; disk->mirroring = false; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; } } } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml new file mode 100644 index 0000000..6ba5724 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml @@ -0,0 +1,37 @@ +<domain type='qemu' id='1'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/tmp/HostVG/QEMUGuest1-snap'/> + <backingStore type='block' index='1'> + <format type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <backingStore/> + </backingStore> + <mirror type='block' job='active-commit'> + <format type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + </mirror> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml index 72b03c9..abec180 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml @@ -17,7 +17,7 @@ <disk type='block' device='disk'> <source dev='/dev/HostVG/QEMUGuest1'/> <backingStore/> - <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy' ready='yes'> + <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy' job='copy' ready='yes'> <source file='/dev/HostVG/QEMUGuest1Copy'/> </mirror> <target dev='hda' bus='ide'/> @@ -33,7 +33,7 @@ <disk type='file' device='disk'> <source file='/tmp/data.img'/> <backingStore/> - <mirror type='file' file='/tmp/copy.img' format='qcow2'> + <mirror type='file' file='/tmp/copy.img' format='qcow2' job='copy'> <format type='qcow2'/> <source file='/tmp/copy.img'/> </mirror> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml index 72b03c9..abec180 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml @@ -17,7 +17,7 @@ <disk type='block' device='disk'> <source dev='/dev/HostVG/QEMUGuest1'/> <backingStore/> - <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy' ready='yes'> + <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy' job='copy' ready='yes'> <source file='/dev/HostVG/QEMUGuest1Copy'/> </mirror> <target dev='hda' bus='ide'/> @@ -33,7 +33,7 @@ <disk type='file' device='disk'> <source file='/tmp/data.img'/> <backingStore/> - <mirror type='file' file='/tmp/copy.img' format='qcow2'> + <mirror type='file' file='/tmp/copy.img' format='qcow2' job='copy'> <format type='qcow2'/> <source file='/tmp/copy.img'/> </mirror> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 200d50f..98b2c15 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -227,6 +227,7 @@ mymain(void) DO_TEST_DIFFERENT("disk-mirror-old"); DO_TEST_FULL("disk-mirror", false, WHEN_ACTIVE); DO_TEST_FULL("disk-mirror", true, WHEN_INACTIVE); + DO_TEST_FULL("disk-active-commit", false, WHEN_ACTIVE); DO_TEST("graphics-listen-network"); DO_TEST("graphics-vnc"); DO_TEST("graphics-vnc-websocket"); -- 1.9.3
On 06/24/14 01:30, Eric Blake wrote:
A future patch is going to wire up qemu active block commit jobs; but as they have similar events and are canceled/pivoted in the same way as block copy jobs, it is easiest to track all bookkeeping for the commit job by reusing the <mirror> element. This patch adds domain XML to track which job was responsible for creating a mirroring situation, and adds a job='copy' attribute to all existing uses of <mirror>. Along the way, it also massages the qemu monitor backend to read the new field in order to generate the correct type of libvirt job (even though it requires a future patch to actually cause a qemu event that can be reported as an active commit).
* docs/schemas/domaincommon.rng: Enhance schema. * docs/formatdomain.html.in: Document it. * src/conf/domain_conf.h (_virDomainDiskDef): Add a field. * src/conf/domain_conf.c (virDomainBlockJobType): String conversion. (virDomainDiskDefParseXML): Parse job type. (virDomainDiskDefFormat): Output job type. * src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Distinguish active from regular commit. * src/qemu/qemu_driver.c (qemuDomainBlockCopy): Set job type. (qemuDomainBlockPivot, qemuDomainBlockJobImpl): Clean up job type on completion. * tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml: Update tests. * tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Likewise. * tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml: New file. * tests/qemuxml2xmltest.c (mymain): Drive new test.
Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/formatdomain.html.in | 20 ++++++------ docs/schemas/domaincommon.rng | 13 ++++++++ src/conf/domain_conf.c | 33 ++++++++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_driver.c | 3 ++ src/qemu/qemu_process.c | 18 +++++++---- .../qemuxml2argv-disk-active-commit.xml | 37 ++++++++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +-- .../qemuxml2xmlout-disk-mirror-old.xml | 4 +-- tests/qemuxml2xmltest.c | 1 + 10 files changed, 113 insertions(+), 21 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
No change, ACK from the previous version stands. Peter
With this in place, I can (finally!) now do: virsh blockcommit $dom vda --shallow --verbose --pivot and watch qemu shorten the backing chain by one, followed by libvirt automatically updating the dumpxml output, effectively undoing the work of virsh snapshot-commit --no-metadata --disk-only. Commit is SOOOO much faster than blockpull, when I'm still fairly close in time to when the temporary qcow2 wrapper file was created via a snapshot operation! Still not done: the persistent XML is not updated; which means stopping and restarting a persistent guest will use the wrong file and likely fail to boot. * src/qemu/qemu_driver.c (qemuDomainBlockCommit): Implement live commit to regular files. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_driver.c | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 79554e9..ae81712 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15493,9 +15493,12 @@ qemuDomainBlockCommit(virDomainPtr dom, unsigned int baseIndex = 0; const char *top_parent = NULL; bool clean_access = false; + virStorageSourcePtr mirror = NULL; + - /* XXX Add support for COMMIT_ACTIVE, COMMIT_DELETE */ - virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1); + /* XXX Add support for COMMIT_DELETE */ + virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW | + VIR_DOMAIN_BLOCK_COMMIT_ACTIVE, -1); if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -15543,9 +15546,6 @@ qemuDomainBlockCommit(virDomainPtr dom, &top_parent))) goto endjob; - /* FIXME: qemu 2.0 supports active commit, but as a two-stage - * process; qemu 2.1 is further improving active commit. We need - * to start supporting it in libvirt. */ if (topSource == disk->src) { if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_ACTIVE_COMMIT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -15559,6 +15559,14 @@ qemuDomainBlockCommit(virDomainPtr dom, disk->dst); goto endjob; } + if (disk->mirror) { + virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, + _("disk '%s' already in active block job"), + disk->dst); + goto endjob; + } + if (VIR_ALLOC(mirror) < 0) + goto endjob; } else if (flags & VIR_DOMAIN_BLOCK_COMMIT_ACTIVE) { virReportError(VIR_ERR_INVALID_ARG, _("active commit requested but '%s' is not active"), @@ -15605,6 +15613,21 @@ qemuDomainBlockCommit(virDomainPtr dom, VIR_DISK_CHAIN_READ_WRITE) < 0)) goto endjob; + /* For an active commit, clone enough of the base to act as the mirror */ + if (mirror) { + /* XXX Support network commits */ + if (baseSource->type != VIR_STORAGE_TYPE_FILE && + baseSource->type != VIR_STORAGE_TYPE_BLOCK) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("active commit to non-file not supported yet")); + goto endjob; + } + mirror->type = baseSource->type; + if (VIR_STRDUP(mirror->path, baseSource->path) < 0) + goto endjob; + mirror->format = baseSource->format; + } + /* Start the commit operation. Pass the user's original spelling, * if any, through to qemu, since qemu may behave differently * depending on whether the input was specified as relative or @@ -15617,6 +15640,12 @@ qemuDomainBlockCommit(virDomainPtr dom, bandwidth); qemuDomainObjExitMonitor(driver, vm); + if (ret == 0 && mirror) { + disk->mirror = mirror; + mirror = NULL; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT; + } + endjob: if (ret < 0 && clean_access) { /* Revert access to read-only, if possible. */ @@ -15627,6 +15656,7 @@ qemuDomainBlockCommit(virDomainPtr dom, top_parent, VIR_DISK_CHAIN_READ_ONLY); } + virStorageSourceFree(mirror); if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; -- 1.9.3
On 06/24/14 01:30, Eric Blake wrote:
With this in place, I can (finally!) now do:
virsh blockcommit $dom vda --shallow --verbose --pivot
and watch qemu shorten the backing chain by one, followed by libvirt automatically updating the dumpxml output, effectively undoing the work of virsh snapshot-commit --no-metadata --disk-only. Commit is SOOOO much faster than blockpull, when I'm still fairly close in time to when the temporary qcow2 wrapper file was created via a snapshot operation!
Still not done: the persistent XML is not updated; which means stopping and restarting a persistent guest will use the wrong file and likely fail to boot.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Implement live commit to regular files.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_driver.c | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-)
ACK as previously. This will obviously need a followup to deal with networked disks too. Peter
On 06/23/2014 05:30 PM, Eric Blake wrote:
Rebased to latest libvirt.git, and still waiting on Jeff's qemu patches to go into qemu.git before I can push any of these, https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg04060.html but reposting because of a feature Adam had been requesting:
Patch 3/5 is new, and adds a new <activecommit/> marker to the 'virsh capabilities' output if qemu is detected as supporting active commit. I've resolved some of Peter's review comments in patches 1 and 2 (patch 1 underwent a bit of a rewrite to differentiate the qemu capability probe from normal use of the interface, and 2 dropped mention of RHEL 6.3 except in the commit message). At this point, I haven't altered 4 or 5, although I still need to do something about getting a persistent domain definition updated after a pivot changes the file in use by a running domain.
qemu.git (and therefore the upcoming qemu 2.1 release) now has everything we need: commit 7676e2c597000eff3a7233b40cca768b358f9bc9 Author: Jeff Cody <jcody@redhat.com> Date: Mon Jun 30 15:14:15 2014 +0200 block: make 'top' argument to block-commit optional Now that active layer block-commit is supported, the 'top' argument no longer needs to be mandatory. Change it to optional, with the default being the active layer in the device chain. [kwolf: Rebased and resolved conflict in tests/qemu-iotests/040] Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Benoit Canet <benoit@irqsave.net> Signed-off-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> but I'd rather wait until after the 1.2.6 release to push this series since I still don't have persistent XML manipulation working, and since we may want to rework how feature advertisement works. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Peter Krempa