[libvirt] [PATCH v3 0/5] Next round of active commit support
Browseable at: http://repo.or.cz/w/libvirt/ericb.git/shortlog/refs/heads/gluster or git checkout git://repo.or.cz/libvirt/ericb.git gluster Resolves some of the review comments, and in particular FINALLY turns on capability probing. However, I can't push this to libvirt until Jeff's patch actually makes it into qemu.git: https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg04060.html and still lacking cleanup of the persistent XML after a pivot Eric Blake (5): blockjob: allow omitted arguments to QMP block-commit blockjob: turn on qemu capability bit for active commit virsh: expose new active commit controls blockcommit: track job type in xml blockcommit: turn on active commit 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_capabilities.c | 7 +++ src/qemu/qemu_capabilities.h | 3 +- src/qemu/qemu_driver.c | 59 +++++++++++++++----- 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 +- 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 | 49 +++++++++++++++++ .../qemuxml2argv-disk-active-commit.xml | 37 +++++++++++++ .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +- .../qemuxml2xmlout-disk-mirror-old.xml | 4 +- tests/qemuxml2xmltest.c | 1 + tools/virsh-domain.c | 56 ++++++++++++++++--- tools/virsh.pod | 32 +++++++---- 24 files changed, 484 insertions(+), 191 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 (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@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
On 06/19/14 01:22, 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 (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@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_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;
I think those return values should be documented in the comment for qemuMonitorBlockCommit so that we avoid possible confusion.
+ } + goto cleanup; + } ret = qemuMonitorJSONCheckError(cmd, reply);
cleanup:
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;
If these ever fail it will be hard to diagnose but not really worth doing anything about it.
+ + ret = 0; + cleanup: + qemuMonitorTestFree(test); + return ret; +} + +static int testQemuMonitorJSONqemuMonitorJSONGetDumpGuestMemoryCapability(const void *data) { virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
ACK when you document the possible return values. Peter
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. While at it, leave a few more bread-crumbs about what blockjob sync/async means, and enforce that sync cancel is only ever encountered with blockpull (at this point, RHEL 6.2 is likely to be the only platform to have sync cancel). 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. (QEMU_CAPS_BLOCKJOB_SYNC): Enhance comment. * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Likewise. (qemuDomainBlockCopy): Likewise. (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 | 3 +- src/qemu/qemu_driver.c | 16 +++--- 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, 193 insertions(+), 143 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 08c3d04..b96fec1 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", ); @@ -2149,6 +2150,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) && + qemuMonitorBlockCommit(mon, "bogus", NULL, NULL, 0) == -2) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_ACTIVE_COMMIT); + return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d755caa..617986d 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -127,7 +127,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_SCSI_BLOCK = 88, /* -device scsi-block */ QEMU_CAPS_TRANSACTION = 89, /* transaction monitor command */ QEMU_CAPS_BLOCKJOB_SYNC = 90, /* RHEL 6.2 block_job_cancel */ - QEMU_CAPS_BLOCKJOB_ASYNC = 91, /* qemu 1.1 block-job-cancel */ + QEMU_CAPS_BLOCKJOB_ASYNC = 91, /* qemu 1.1/RHEL 6.3 block-job-cancel */ QEMU_CAPS_SCSI_CD = 92, /* -device scsi-cd */ QEMU_CAPS_IDE_CD = 93, /* -device ide-cd */ QEMU_CAPS_NO_USER_CONFIG = 94, /* -no-user-config */ @@ -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 ca58d6b..004b63d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15130,8 +15130,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, * to prevent newly scheduled block jobs from confusing us. */ if (mode == BLOCK_JOB_ABORT) { if (!async) { - /* Older qemu that lacked async reporting also lacked - * active commit, so we can hardcode the event to pull. + /* Older qemu (RHEL 6.2) that lacked async reporting also + * lacked copy and commit, so we can hardcode type_pull. * We have to generate two variants of the event. */ int type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL; int status = VIR_DOMAIN_BLOCK_JOB_CANCELED; @@ -15291,6 +15291,8 @@ qemuDomainBlockCopy(virDomainObjPtr vm, goto endjob; } + /* Ensure that no one backports copy to RHEL 6.2, where cancel + * behaved differently */ if (!(virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_MIRROR) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -15509,7 +15511,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 +15546,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/19/14 01:22, 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.
While at it, leave a few more bread-crumbs about what blockjob sync/async means, and enforce that sync cancel is only ever encountered with blockpull (at this point, RHEL 6.2 is likely to be the only platform to have sync cancel).
Hmm, I don't like mentioning old rhel versions in upstream given that this version won't work properly there. Also I have to cite one of your previous mails:
On 06/14/14 14:50, Eric Blake wrote:
"While at it" is often an indication that a patch does too much at once.
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. (QEMU_CAPS_BLOCKJOB_SYNC): Enhance comment. * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Likewise. (qemuDomainBlockCopy): Likewise. (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.
It would be cool if we'd tweak the capabilities test to do automatic response numbering so we'd don't need to rewrite a whole lot of the stuff if we add something like this.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_capabilities.c | 7 +++ src/qemu/qemu_capabilities.h | 3 +- src/qemu/qemu_driver.c | 16 +++--- 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, 193 insertions(+), 143 deletions(-)
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d755caa..617986d 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -127,7 +127,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_SCSI_BLOCK = 88, /* -device scsi-block */ QEMU_CAPS_TRANSACTION = 89, /* transaction monitor command */ QEMU_CAPS_BLOCKJOB_SYNC = 90, /* RHEL 6.2 block_job_cancel */ - QEMU_CAPS_BLOCKJOB_ASYNC = 91, /* qemu 1.1 block-job-cancel */ + QEMU_CAPS_BLOCKJOB_ASYNC = 91, /* qemu 1.1/RHEL 6.3 block-job-cancel */
This hunk seems unrelated. I don't think upstream cares about RHEL 6.3 now that we have 6.5 and also this version wouldn't really work there.
QEMU_CAPS_SCSI_CD = 92, /* -device scsi-cd */ QEMU_CAPS_IDE_CD = 93, /* -device ide-cd */ QEMU_CAPS_NO_USER_CONFIG = 94, /* -no-user-config */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ca58d6b..004b63d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15130,8 +15130,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, * to prevent newly scheduled block jobs from confusing us. */ if (mode == BLOCK_JOB_ABORT) { if (!async) { - /* Older qemu that lacked async reporting also lacked - * active commit, so we can hardcode the event to pull. + /* Older qemu (RHEL 6.2) that lacked async reporting also + * lacked copy and commit, so we can hardcode type_pull. * We have to generate two variants of the event. */ int type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL; int status = VIR_DOMAIN_BLOCK_JOB_CANCELED;
Unrelated again ...
@@ -15291,6 +15291,8 @@ qemuDomainBlockCopy(virDomainObjPtr vm, goto endjob; }
+ /* Ensure that no one backports copy to RHEL 6.2, where cancel + * behaved differently */ if (!(virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_MIRROR) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
Capability adding parts look okay, but I'm not okay with mentioning the obsolete RHEL stuff in upstream. And if somebody else thinks it's okay, it at least shouldn't be part of this patch. Peter
Add knobs to virsh to manage a 2-phase active commit of the top layer, similar to knobs already present on blockcopy. While this code will fail until later patches actually implement the new knobs in the qemu driver, doing it now proves that the API is usable and also makes it easier for testing the qemu changes as they are made. * tools/virsh-domain.c (cmdBlockCommit): Add --active, --pivot, and --finish options, modeled after blockcopy. (blockJobImpl): Support --active flag. * tools/virsh.pod (blockcommit): Document new flags. (blockjob): Mention 2-phase commit interaction. Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virsh-domain.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++------ tools/virsh.pod | 32 +++++++++++++++++++++--------- 2 files changed, 73 insertions(+), 15 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index d136862..0ae1538 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1492,6 +1492,10 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, flags |= VIR_DOMAIN_BLOCK_COMMIT_SHALLOW; if (vshCommandOptBool(cmd, "delete")) flags |= VIR_DOMAIN_BLOCK_COMMIT_DELETE; + if (vshCommandOptBool(cmd, "active") || + vshCommandOptBool(cmd, "pivot") || + vshCommandOptBool(cmd, "keep-overlay")) + flags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE; ret = virDomainBlockCommit(dom, path, base, top, bandwidth, flags); break; case VSH_CMD_BLOCK_JOB_COPY: @@ -1592,13 +1596,18 @@ static const vshCmdOptDef opts_block_commit[] = { .type = VSH_OT_DATA, .help = N_("path of top file to commit from (default top of chain)") }, + {.name = "active", + .type = VSH_OT_BOOL, + .help = N_("trigger two-stage active commit of top file") + }, {.name = "delete", .type = VSH_OT_BOOL, .help = N_("delete files that were successfully committed") }, {.name = "wait", .type = VSH_OT_BOOL, - .help = N_("wait for job to complete") + .help = N_("wait for job to complete " + "(with --active, wait for job to sync)") }, {.name = "verbose", .type = VSH_OT_BOOL, @@ -1606,7 +1615,15 @@ static const vshCmdOptDef opts_block_commit[] = { }, {.name = "timeout", .type = VSH_OT_INT, - .help = N_("with --wait, abort if copy exceeds timeout (in seconds)") + .help = N_("implies --wait, abort if copy exceeds timeout (in seconds)") + }, + {.name = "pivot", + .type = VSH_OT_BOOL, + .help = N_("implies --active --wait, pivot when commit is synced") + }, + {.name = "keep-overlay", + .type = VSH_OT_BOOL, + .help = N_("implies --active --wait, quit when commit is synced") }, {.name = "async", .type = VSH_OT_BOOL, @@ -1620,8 +1637,11 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; bool ret = false; - bool blocking = vshCommandOptBool(cmd, "wait"); bool verbose = vshCommandOptBool(cmd, "verbose"); + bool pivot = vshCommandOptBool(cmd, "pivot"); + bool finish = vshCommandOptBool(cmd, "keep-overlay"); + bool active = vshCommandOptBool(cmd, "active") || pivot || finish; + bool blocking = vshCommandOptBool(cmd, "wait"); int timeout = 0; struct sigaction sig_action; struct sigaction old_sig_action; @@ -1632,7 +1652,12 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) bool quit = false; int abort_flags = 0; + blocking |= vshCommandOptBool(cmd, "timeout") || pivot || finish; if (blocking) { + if (pivot && finish) { + vshError(ctl, "%s", _("cannot mix --pivot and --keep-overlay")); + return false; + } if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0) return false; if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) @@ -1650,8 +1675,7 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) sigaction(SIGINT, &sig_action, &old_sig_action); GETTIMEOFDAY(&start); - } else if (verbose || vshCommandOptBool(cmd, "timeout") || - vshCommandOptBool(cmd, "async")) { + } else if (verbose || vshCommandOptBool(cmd, "async")) { vshError(ctl, "%s", _("missing --wait option")); return false; } @@ -1683,6 +1707,8 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) if (verbose) vshPrintJobProgress(_("Block Commit"), info.end - info.cur, info.end); + if (active && info.cur == info.end) + break; GETTIMEOFDAY(&curr); if (intCaught || (timeout && @@ -1709,7 +1735,25 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) /* printf [100 %] */ vshPrintJobProgress(_("Block Commit"), 0, 1); } - vshPrint(ctl, "\n%s", quit ? _("Commit aborted") : _("Commit complete")); + if (!quit && pivot) { + abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT; + if (virDomainBlockJobAbort(dom, path, abort_flags) < 0) { + vshError(ctl, _("failed to pivot job for disk %s"), path); + goto cleanup; + } + } else if (finish && !quit && + virDomainBlockJobAbort(dom, path, abort_flags) < 0) { + vshError(ctl, _("failed to finish job for disk %s"), path); + goto cleanup; + } + if (quit) + vshPrint(ctl, "\n%s", _("Commit aborted")); + else if (pivot) + vshPrint(ctl, "\n%s", _("Successfully pivoted")); + else if (!finish) + vshPrint(ctl, "\n%s", _("Now in synchronized phase")); + else + vshPrint(ctl, "\n%s", _("Commit complete")); ret = true; cleanup: diff --git a/tools/virsh.pod b/tools/virsh.pod index ceec1a0..b41473e 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -774,8 +774,9 @@ address of virtual interface (such as I<detach-interface> or I<domif-setlink>) will accept the MAC address printed by this command. =item B<blockcommit> I<domain> I<path> [I<bandwidth>] -{[I<base>] | [I<--shallow>]} [I<top>] [I<--delete>] -[I<--wait> [I<--verbose>] [I<--timeout> B<seconds>] [I<--async>]] +[I<base>] [I<--shallow>] [I<top>] [I<--delete>] +[I<--wait> [I<--async>] [I<--verbose>]] [I<--timeout> B<seconds>] +[I<--active>] [{I<--pivot> | I<--keep-overlay>}] Reduce the length of a backing image chain, by committing changes at the top of the chain (snapshot or delta files) into backing images. By @@ -785,19 +786,32 @@ operation is constrained to committing just that portion of the chain; I<--shallow> can be used instead of I<base> to specify the immediate backing file of the resulting top image to be committed. The files being committed are rendered invalid, possibly as soon as the operation -starts; using the I<--delete> flag will remove these files at the successful -completion of the commit operation. +starts; using the I<--delete> flag will attempt to remove these invalidated +files at the successful completion of the commit operation. + +When I<top> is omitted or specified as the active image, it is also +possible to specify I<--active> to trigger a two-phase active commit. In +the first phase, I<top> is copied into I<base> and the job can only be +canceled, with top still containing data not yet in base. In the second +phase, I<top> and I<base> remain identical until a call to B<blockjob> +with the I<--abort> flag (keeping top as the active image that tracks +changes from that point in time) or the I<--pivot> flag (making base +the new active image and invalidating top). By default, this command returns as soon as possible, and data for the entire disk is committed in the background; the progress of the operation can be checked with B<blockjob>. However, if I<--wait> is -specified, then this command will block until the operation completes, -or cancel the operation if the optional I<timeout> in seconds elapses +specified, then this command will block until the operation completes +(or for I<--active>, enters the second phase), or until the operation +is canceled because the optional I<timeout> in seconds elapses or SIGINT is sent (usually with C<Ctrl-C>). Using I<--verbose> along with I<--wait> will produce periodic status updates. If job cancellation is triggered, I<--async> will return control to the user as fast as possible, otherwise the command may continue to block a little while -longer until the job is done cleaning up. +longer until the job is done cleaning up. Using I<--pivot> is shorthand +for combining I<--active> I<--wait> with an automatic B<blockjob> +I<--pivot>; and using I<--keep-overlay> is shorthand for combining +I<--active> I<--wait> with an automatic B<blockjob> I<--abort>. I<path> specifies fully-qualified path of the disk; it corresponds to a unique target name (<target dev='name'/>) or source file (<source @@ -924,8 +938,8 @@ also B<domblklist> for listing these names). If I<--abort> is specified, the active job on the specified disk will be aborted. If I<--async> is also specified, this command will return immediately, rather than waiting for the cancellation to complete. If -I<--pivot> is specified, this requests that an active copy job -be pivoted over to the new copy. +I<--pivot> is specified, this requests that an active copy or active +commit job be pivoted over to the new image. If I<--info> is specified, the active job information on the specified disk will be printed. I<bandwidth> can be used to set bandwidth limit for the active job. -- 1.9.3
On 06/19/14 01:22, Eric Blake wrote:
Add knobs to virsh to manage a 2-phase active commit of the top layer, similar to knobs already present on blockcopy. While this code will fail until later patches actually implement the new knobs in the qemu driver, doing it now proves that the API is usable and also makes it easier for testing the qemu changes as they are made.
* tools/virsh-domain.c (cmdBlockCommit): Add --active, --pivot, and --finish options, modeled after blockcopy. (blockJobImpl): Support --active flag. * tools/virsh.pod (blockcommit): Document new flags. (blockjob): Mention 2-phase commit interaction.
Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virsh-domain.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++------ tools/virsh.pod | 32 +++++++++++++++++++++--------- 2 files changed, 73 insertions(+), 15 deletions(-)
This doesn't depend on anything qemu-specific and thus is safe to push async. ACK. Peter
On 06/19/2014 03:28 AM, Peter Krempa wrote:
On 06/19/14 01:22, Eric Blake wrote:
Add knobs to virsh to manage a 2-phase active commit of the top layer, similar to knobs already present on blockcopy. While this code will fail until later patches actually implement the new knobs in the qemu driver, doing it now proves that the API is usable and also makes it easier for testing the qemu changes as they are made.
* tools/virsh-domain.c (cmdBlockCommit): Add --active, --pivot, and --finish options, modeled after blockcopy. (blockJobImpl): Support --active flag. * tools/virsh.pod (blockcommit): Document new flags. (blockjob): Mention 2-phase commit interaction.
Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virsh-domain.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++------ tools/virsh.pod | 32 +++++++++++++++++++++--------- 2 files changed, 73 insertions(+), 15 deletions(-)
This doesn't depend on anything qemu-specific and thus is safe to push async.
ACK.
Pushed this one. I'm holding off on the others for a bit more (to see how Jeff's qemu patches fare, and possibly rebase my patches on top of your work for copying virStorageSource) -- 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 79b85d5..f8a5238 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 4114289..94152ff 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 a6ac95a..27e6118 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 004b63d..d19ca18 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 @@ -15399,6 +15401,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 f1c0041..c30a5c9 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/19/14 01:22, 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
ACK, 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 d19ca18..089668d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15495,9 +15495,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; @@ -15545,9 +15548,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", @@ -15561,6 +15561,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"), @@ -15607,6 +15615,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 @@ -15619,6 +15642,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. */ @@ -15629,6 +15658,7 @@ qemuDomainBlockCommit(virDomainPtr dom, top_parent, VIR_DISK_CHAIN_READ_ONLY); } + virStorageSourceFree(mirror); if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; -- 1.9.3
On 06/19/14 01:22, 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(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d19ca18..089668d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c
@@ -15607,6 +15615,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 */
This will be really easy with my deep-copy of the source struct.
+ 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
ACK, Peter
I just tried to test this series built from Eric's libvirt git repo, and QEMU built from its git. On a live (or offline guest) I see the below: $ virsh blockcommit f20vm1 vda --shallow --wait \ --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit I also tried: $ virsh blockcommit --domain f20vm1 vda \ --base /home/kashyap/vmimages/snap3-f20vm1.qcow2 \ --top /home/kashyap/vmimages/snap4-f20vm1.qcow2 \ --wait --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit Is that expected? Test notes below on QEMU and Libvirt commit details. Details ======= QEMU info --------- Build a local QEMU with this git commit. $ git log | head -6 commit 0360fbd076e8bdbb9498598b0c559464346babe4 Merge: af44da8 d3606f0 Author: Peter Maydell <peter.maydell@linaro.org> Date: Tue Jun 17 16:08:06 2014 +0100 Merge remote-tracking branch 'remotes/riku/linux-user-for-upstream' into staging libvirt info ------------ How I built the libvirt: $ git clone git://repo.or.cz/libvirt/ericb.git && cd ericb $ git log | head -5 commit c23db4dbc9b4ed91b37ba5d8ee1eff6bb8b32b0e Author: Eric Blake <eblake@redhat.com> Date: Thu Jun 5 13:26:56 2014 -0600 blockcommit: turn on active commit $ ./autgen.sh && make -j4 && make rpm $ cd ~/rpmbuild/RPMS/x86_64 $ rpm -Uvh *.rpm $ virsh --version 1.2.6 Test ---- 0. Edit the libvirt XML of the guest to point to the newly build QEMU from above. $ virsh dumpxml f20vm1 | grep qemu-system-x86_64 <emulator>/home/kashyap/build/qemu/x86_64-softmmu/qemu-system-x86_64</emulator> 1. Start the guest and list it: $ virsh list Id Name State ---------------------------------------------------- 5 f20vm1 running (Note: All the images, including base are QCOW2 disk images.) 2. Create 4 live, disk-only external snapshots. $ virsh snapshot-create-as f20vm1 snap1 snap1-desc \ --diskspec vda,file=/home/kashyap/vmimages/snap1-f20vm1.qcow2 --disk-only (Repeat the above 4 times. Before taking each snapshot, add a file (/root/file1 or some such) to distinguish content from each snapshot image.) 3. List the snapshots $ virsh snapshot-list f20vm1 Name Creation Time State ------------------------------------------------------------ snap1 2014-06-21 02:28:26 +0530 disk-snapshot snap2 2014-06-21 02:30:27 +0530 disk-snapshot snap3 2014-06-21 02:31:38 +0530 disk-snapshot snap4 2014-06-21 02:33:52 +0530 disk-snapshot So, the current chain is: base <-- snap1 <-- snap2 <-- snap3 <-- snap4 Desired chain: base <-- snap4 4. Invoke blockcommit test: $ virsh blockcommit f20vm1 vda --shallow --wait \ --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit What am I missing? -- /kashyap
On Sat, Jun 21, 2014 at 04:04:45PM +0530, Kashyap Chamarthy wrote:
I just tried to test this series built from Eric's libvirt git repo, and QEMU built from its git.
On a live (or offline guest) I see the below:
$ virsh blockcommit f20vm1 vda --shallow --wait \ --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit
Okay, I had a chat with Peter Krempa on #virt (OFTC). Couple of things: - I didn't restart libvirtd after I updated RPMs built from git. - After a restart, I tried active commit again via libvirt, this time it failed saying it's not supported on the QEMU binary I built from commit 0360fbd (17JUN). - I guess for your testing you must have tested by applying Jef's patches "block: make 'top' argument to bloc"[1] manually. Eric just pinged on #virt (OFTC) to clarify the above as I was writing this email. Thanks. [1] https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg04060.html
I also tried:
$ virsh blockcommit --domain f20vm1 vda \ --base /home/kashyap/vmimages/snap3-f20vm1.qcow2 \ --top /home/kashyap/vmimages/snap4-f20vm1.qcow2 \ --wait --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit
Is that expected?
Test notes below on QEMU and Libvirt commit details.
Details =======
QEMU info ---------
Build a local QEMU with this git commit.
$ git log | head -6 commit 0360fbd076e8bdbb9498598b0c559464346babe4 Merge: af44da8 d3606f0 Author: Peter Maydell <peter.maydell@linaro.org> Date: Tue Jun 17 16:08:06 2014 +0100
Merge remote-tracking branch 'remotes/riku/linux-user-for-upstream' into staging
libvirt info ------------
How I built the libvirt:
$ git clone git://repo.or.cz/libvirt/ericb.git && cd ericb
$ git log | head -5 commit c23db4dbc9b4ed91b37ba5d8ee1eff6bb8b32b0e Author: Eric Blake <eblake@redhat.com> Date: Thu Jun 5 13:26:56 2014 -0600
blockcommit: turn on active commit
$ ./autgen.sh && make -j4 && make rpm
$ cd ~/rpmbuild/RPMS/x86_64
$ rpm -Uvh *.rpm
$ virsh --version 1.2.6
Test ----
0. Edit the libvirt XML of the guest to point to the newly build QEMU from above.
$ virsh dumpxml f20vm1 | grep qemu-system-x86_64 <emulator>/home/kashyap/build/qemu/x86_64-softmmu/qemu-system-x86_64</emulator>
1. Start the guest and list it:
$ virsh list Id Name State ---------------------------------------------------- 5 f20vm1 running
(Note: All the images, including base are QCOW2 disk images.)
2. Create 4 live, disk-only external snapshots.
$ virsh snapshot-create-as f20vm1 snap1 snap1-desc \ --diskspec vda,file=/home/kashyap/vmimages/snap1-f20vm1.qcow2 --disk-only
(Repeat the above 4 times. Before taking each snapshot, add a file (/root/file1 or some such) to distinguish content from each snapshot image.)
3. List the snapshots
$ virsh snapshot-list f20vm1 Name Creation Time State ------------------------------------------------------------ snap1 2014-06-21 02:28:26 +0530 disk-snapshot snap2 2014-06-21 02:30:27 +0530 disk-snapshot snap3 2014-06-21 02:31:38 +0530 disk-snapshot snap4 2014-06-21 02:33:52 +0530 disk-snapshot
So, the current chain is:
base <-- snap1 <-- snap2 <-- snap3 <-- snap4
Desired chain:
base <-- snap4
4. Invoke blockcommit test:
$ virsh blockcommit f20vm1 vda --shallow --wait \ --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit
What am I missing?
-- /kashyap
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- /kashyap
On 06/21/2014 04:34 AM, Kashyap Chamarthy wrote:
I just tried to test this series built from Eric's libvirt git repo, and QEMU built from its git.
On a live (or offline guest) I see the below:
$ virsh blockcommit f20vm1 vda --shallow --wait \ --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit
I also tried:
$ virsh blockcommit --domain f20vm1 vda \ --base /home/kashyap/vmimages/snap3-f20vm1.qcow2 \ --top /home/kashyap/vmimages/snap4-f20vm1.qcow2 \ --wait --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit
Is that expected?
Unfortunately, yes, unless you have also applied Jeff's pending patches for qemu that actually enable optional 'top': https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg04060.html
What am I missing?
You're very bleeding edge; I won't push my patches to libvirt.git until qemu.git also has matching patches, so in the meantime, you have to carry patches on both repos. Peter has been doing some rpm builds of the bits and pieces for integration testing, but I'm not sure if he is planning on publishing them for public consumption yet. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Jun 23, 2014 at 09:39:08AM -0600, Eric Blake wrote:
On 06/21/2014 04:34 AM, Kashyap Chamarthy wrote:
I just tried to test this series built from Eric's libvirt git repo, and QEMU built from its git.
On a live (or offline guest) I see the below:
$ virsh blockcommit f20vm1 vda --shallow --wait \ --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit
I also tried:
$ virsh blockcommit --domain f20vm1 vda \ --base /home/kashyap/vmimages/snap3-f20vm1.qcow2 \ --top /home/kashyap/vmimages/snap4-f20vm1.qcow2 \ --wait --verbose --pivot --active error: unsupported flags (0x4) in function qemuDomainBlockCommit
Is that expected?
Unfortunately, yes, unless you have also applied Jeff's pending patches for qemu that actually enable optional 'top':
https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg04060.html
Yep, I just applied the above patch to today's QEMU git and tested with same libvirt (from your repos.or.cz instance) commit ID that I pointed to in my earlier email in this thread, and active blockcommit w/ 'virsh' works for me. Following are details of a simple test I did. Before active blockcommit ------------------------- - I had a chain like that: [base] <-- [snap1] <-- [snap2] <-- [snap3] <-- [snap4] (current-active) - Current active block device in use: $ virsh domblklist f20vm1 Target Source ------------------------------------------------ vda /home/kashyap/vmimages/snap4.qcow2 Perform active blockcommit -------------------------- The below commmand will commit contents of [snap4] into [snap3] and makes [snap3] as the current active image invalidating [snap4]: $ virsh blockcommit f20vm1 vda --shallow --wait \ --verbose --pivot --active Block Commit: [100 %] Successfully pivoted After active blockcommit ------------------------ The chain has reduced to below (shortened by one image): [base] <-- [snap1] <-- [snap2] <-- [snap3] (current-active) Again, check what's the current active block device via 'virsh' (it should reflect 'snap3'): $ virsh domblklist f20vm1 Target Source ------------------------------------------------ vda /home/kashyap/vmimages/snap3.qcow2 Also, it updated the 'virsh dumpxml' output just fine. Lastly, I ran active blockcommit again (w/ --shallow), so the chain is now reduced to [base] <-- [snap1] <-- [snap2] (current-active) And, here's its associated 'virsh dumpxml' output showing the uppdated backing store (chain reduced by two): $ virsh dumpxml f20vm1 | grep "snap2.qcow2" -A10 -B2 <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/home/kashyap/vmimages/snap2.qcow2'/> <backingStore type='file' index='1'> <format type='qcow2'/> <source file='/home/kashyap/vmimages/snap1.qcow2'/> <backingStore type='file' index='2'> <format type='qcow2'/> <source file='/home/kashyap/vmimages/f20vm1.qcow2'/> <backingStore/> </backingStore> </backingStore> <target dev='vda' bus='virtio'/> So, works as described in the patch titled "blockcommit: turn on active commit" (on libvir-list). Will test more once patches are committed to git.
You're very bleeding edge; I won't push my patches to libvirt.git until qemu.git also has matching patches, so in the meantime, you have to carry patches on both repos. Peter has been doing some rpm builds of the bits and pieces for integration testing, but I'm not sure if he is planning on publishing them for public consumption yet.
No worries, I can test manually apply patches ('git am -3') from the list to test. -- /kashyap
participants (3)
-
Eric Blake -
Kashyap Chamarthy -
Peter Krempa