[libvirt] [PATCH v6 0/2] turn on active commit
v5 was here, and got tentative ACKs: https://www.redhat.com/archives/libvir-list/2014-July/msg01414.html the only change is some reordering of logic in patch 2/2 (the former 4/4) to make sure that even for a super-fast commit where the event d occurs before the API call regains the lock that the event is correct. I'm still unsure whether to count this as a bug fix to a partial new feature and include it in 1.2.7, or defer it to 1.2.8. If we want it in this release, then getting it in before rc2 would be the nicest. Also at: 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 (2): blockcommit: track job type in xml blockcommit: turn on active commit docs/formatdomain.html.in | 15 ++++--- docs/schemas/domaincommon.rng | 13 ++++++ src/conf/domain_conf.c | 33 ++++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_driver.c | 48 +++++++++++++++++++--- src/qemu/qemu_process.c | 39 +++++++++++++++++- .../qemuxml2argv-disk-active-commit.xml | 37 +++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 6 +-- .../qemuxml2xmlout-disk-mirror-old.xml | 4 +- tests/qemuxml2xmltest.c | 1 + 10 files changed, 179 insertions(+), 18 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml -- 1.9.3
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). It also prepares to update persistent XML to match changes made to live XML when a copy completes. * 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 | 15 +++++---- docs/schemas/domaincommon.rng | 13 ++++++++ src/conf/domain_conf.c | 33 +++++++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_driver.c | 2 ++ src/qemu/qemu_process.c | 39 +++++++++++++++++++++- .../qemuxml2argv-disk-active-commit.xml | 37 ++++++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 6 ++-- .../qemuxml2xmlout-disk-mirror-old.xml | 4 +-- tests/qemuxml2xmltest.c | 1 + 10 files changed, 138 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 2f0be20..6719e1a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1912,16 +1912,19 @@ </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. The + overall <code>disk</code> device element. 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), <span class="since">since 1.2.7</span>. The attribute <code>ready</code>, if present, tracks progress of the job: <code>yes</code> if the disk is known to be ready to pivot, or, <span class="since">since diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7cb5a9e..11f0fd0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4290,6 +4290,13 @@ </attribute> </optional> <optional> + <attribute name='job'> + <choice> + <value>copy</value> + </choice> + </attribute> + </optional> + <optional> <interleave> <ref name='diskSourceFile'/> <optional> @@ -4299,6 +4306,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 2a8cdeb..3ce9cfc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -753,6 +753,12 @@ VIR_ENUM_IMPL(virDomainDiskMirrorState, VIR_DOMAIN_DISK_MIRROR_STATE_LAST, "abort", "pivot") +/* 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 @@ -5437,10 +5443,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); @@ -5463,6 +5485,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) { @@ -15401,10 +15429,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->mirrorState) { const char *mirror; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c77c85c..bffc0a5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -632,6 +632,7 @@ struct _virDomainDiskDef { virStorageSourcePtr mirror; int mirrorState; /* enum virDomainDiskMirrorState */ + int mirrorJob; /* virDomainBlockJobType */ struct { unsigned int cylinders; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 883f10d..9ee62ef 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14938,6 +14938,7 @@ qemuDomainBlockPivot(virConnectPtr conn, virStorageSourceFree(disk->mirror); disk->mirror = NULL; disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; } if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) ret = -1; @@ -15413,6 +15414,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm, need_unlink = false; disk->mirror = mirror; mirror = NULL; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY; if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) VIR_WARN("Unable to save status on vm %s after state change", diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d0132e3..407da5e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1017,6 +1017,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, const char *path; virDomainDiskDefPtr disk; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virDomainDiskDefPtr persistDisk = NULL; bool save = false; virObjectLock(vm); @@ -1025,6 +1026,9 @@ 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, @@ -1034,6 +1038,31 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, * to match. */ if (status == VIR_DOMAIN_BLOCK_JOB_COMPLETED) { if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT) { + if (vm->newDef) { + int indx = virDomainDiskIndexByName(vm->newDef, disk->dst, + false); + virStorageSourcePtr copy = NULL; + + if (indx >= 0) { + persistDisk = vm->newDef->disks[indx]; + copy = virStorageSourceCopy(disk->mirror, false); + if (virStorageSourceInitChainElement(copy, + persistDisk->src, + false) < 0) { + VIR_WARN("Unable to update persistent definition " + "on vm %s after block job", + vm->def->name); + virStorageSourceFree(copy); + copy = NULL; + persistDisk = NULL; + } + } + if (copy) { + virStorageSourceFree(persistDisk->src); + persistDisk->src = copy; + } + } + /* XXX We want to revoke security labels and disk * lease, as well as audit that revocation, before * dropping the original source. But it gets tricky @@ -1054,8 +1083,11 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, disk->mirror = NULL; save = disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_NONE; disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; qemuDomainDetermineDiskChain(driver, vm, disk, true); - } else if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) { + } else 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->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY; save = true; @@ -1063,6 +1095,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virStorageSourceFree(disk->mirror); disk->mirror = NULL; disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; save = true; } } @@ -1072,6 +1105,10 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) VIR_WARN("Unable to save status on vm %s after block job", vm->def->name); + if (persistDisk && virDomainSaveConfig(cfg->configDir, + vm->newDef) < 0) + VIR_WARN("Unable to update persistent definition on vm %s " + "after block job", vm->def->name); } virObjectUnlock(vm); virObjectUnref(cfg); 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 fa7af31..46f2a3e 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> @@ -42,7 +42,7 @@ <disk type='file' device='disk'> <source file='/tmp/logs.img'/> <backingStore/> - <mirror type='file' file='/tmp/logcopy.img' format='qcow2' ready='abort'> + <mirror type='file' file='/tmp/logcopy.img' format='qcow2' job='copy' ready='abort'> <format type='qcow2'/> <source file='/tmp/logcopy.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 8d46b40..451dedc 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -232,6 +232,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 07/30/14 06:05, 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). It also prepares to update persistent XML to match changes made to live XML when a copy completes.
* 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 | 15 +++++---- docs/schemas/domaincommon.rng | 13 ++++++++ src/conf/domain_conf.c | 33 +++++++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_driver.c | 2 ++ src/qemu/qemu_process.c | 39 +++++++++++++++++++++- .../qemuxml2argv-disk-active-commit.xml | 37 ++++++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 6 ++-- .../qemuxml2xmlout-disk-mirror-old.xml | 4 +-- tests/qemuxml2xmltest.c | 1 + 10 files changed, 138 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
ACK, same conditions as 1/2. 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! * src/qemu/qemu_driver.c (qemuDomainBlockCommit): Implement live commit. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_driver.c | 46 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9ee62ef..a3de784 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15512,9 +15512,11 @@ qemuDomainBlockCommit(virDomainPtr dom, char *topPath = NULL; char *basePath = NULL; char *backingPath = NULL; + virStorageSourcePtr mirror = NULL; - /* XXX Add support for COMMIT_ACTIVE, COMMIT_DELETE */ + /* XXX Add support for COMMIT_DELETE */ virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW | + VIR_DOMAIN_BLOCK_COMMIT_ACTIVE | VIR_DOMAIN_BLOCK_COMMIT_RELATIVE, -1); if (!(vm = qemuDomObjFromDomain(dom))) @@ -15563,9 +15565,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", @@ -15579,6 +15578,12 @@ 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; + } } else if (flags & VIR_DOMAIN_BLOCK_COMMIT_ACTIVE) { virReportError(VIR_ERR_INVALID_ARG, _("active commit requested but '%s' is not active"), @@ -15609,6 +15614,16 @@ qemuDomainBlockCommit(virDomainPtr dom, goto endjob; } + /* For an active commit, clone enough of the base to act as the mirror */ + if (topSource == disk->src) { + if (!(mirror = virStorageSourceCopy(baseSource, false))) + goto endjob; + if (virStorageSourceInitChainElement(mirror, + disk->src, + false) < 0) + goto endjob; + } + /* For the commit to succeed, we must allow qemu to open both the * 'base' image and the parent of 'top' as read/write; 'top' might * not have a parent, or might already be read-write. XXX It @@ -15653,13 +15668,33 @@ qemuDomainBlockCommit(virDomainPtr dom, * if any, through to qemu, since qemu may behave differently * depending on whether the input was specified as relative or * absolute (that is, our absolute top_canon may do the wrong - * thing if the user specified a relative name). */ + * thing if the user specified a relative name). Be prepared for + * a ready event to occur while locks are dropped. */ + if (mirror) { + disk->mirror = mirror; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT; + } qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorBlockCommit(priv->mon, device, topPath, basePath, backingPath, bandwidth); qemuDomainObjExitMonitor(driver, vm); + if (mirror) { + if (ret == 0) { + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + + mirror = NULL; + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + VIR_WARN("Unable to save status on vm %s after block job", + vm->def->name); + virObjectUnref(cfg); + } else { + disk->mirror = NULL; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; + } + } + endjob: if (ret < 0 && clean_access) { /* Revert access to read-only, if possible. */ @@ -15669,6 +15704,7 @@ qemuDomainBlockCommit(virDomainPtr dom, qemuDomainPrepareDiskChainElement(driver, vm, top_parent, VIR_DISK_CHAIN_READ_ONLY); } + virStorageSourceFree(mirror); if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; -- 1.9.3
On 07/30/14 06:05, 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!
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Implement live commit.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_driver.c | 46 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 5 deletions(-)
ACK and I don't object to push this before rc2 as we already stretched the feature freeze rules by pushing the NUMA series. Peter
On 07/30/2014 02:53 AM, Peter Krempa wrote:
On 07/30/14 06:05, 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!
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Implement live commit.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_driver.c | 46 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 5 deletions(-)
ACK and I don't object to push this before rc2 as we already stretched the feature freeze rules by pushing the NUMA series.
As I don't see an rc2 tag yet, and this patch is fairly localized (enabling a feature for qemu block commit, and shouldn't cause regressions on other code) I've gone ahead and pushed this series. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Jul 29, 2014 at 10:05:55PM -0600, Eric Blake wrote:
v5 was here, and got tentative ACKs: https://www.redhat.com/archives/libvir-list/2014-July/msg01414.html
the only change is some reordering of logic in patch 2/2 (the former 4/4) to make sure that even for a super-fast commit where the event d occurs before the API call regains the lock that the event is correct.
I'm still unsure whether to count this as a bug fix to a partial new feature and include it in 1.2.7, or defer it to 1.2.8. If we want it in this release, then getting it in before rc2 would be the nicest.
I just tested the same scenario[1] I tried in v3 w/ libvirt git+v6 and current QEMU git (d24e780427a08ea3b1b01e5058f1db37ec9a82ac). Works fine here. FWIW: Tested-by: Kashyap Chamarthy<kchamart@redhat.com> [1] http://www.redhat.com/archives/libvir-list/2014-June/msg01083.html
Also at: 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 (2): blockcommit: track job type in xml blockcommit: turn on active commit
docs/formatdomain.html.in | 15 ++++--- docs/schemas/domaincommon.rng | 13 ++++++ src/conf/domain_conf.c | 33 ++++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_driver.c | 48 +++++++++++++++++++--- src/qemu/qemu_process.c | 39 +++++++++++++++++- .../qemuxml2argv-disk-active-commit.xml | 37 +++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 6 +-- .../qemuxml2xmlout-disk-mirror-old.xml | 4 +- tests/qemuxml2xmltest.c | 1 + 10 files changed, 179 insertions(+), 18 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
-- 1.9.3
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- /kashyap
participants (3)
-
Eric Blake -
Kashyap Chamarthy -
Peter Krempa