On 07/29/14 06:20, 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 | 37 +++++++++++++++++++++- .../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, 136 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e770a77..4ea5493 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;
Hmm, looks like this would be better of in the next patch but one of the tests probably wouldn't work without this.
path = virDomainDiskGetSource(disk); event = virDomainEventBlockJobNewFromObj(vm, path, type, status); event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type, @@ -1036,6 +1040,31 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, bool has_mirror = !!disk->mirror;
if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_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; + } + }
Won't this allow us to enable block copy on persistent domains?
+ /* 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 @@ -1061,7 +1090,8 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, }
if (disk->mirror && - type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) { + (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_READY; save = true; @@ -1069,6 +1099,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virStorageSourceFree(disk->mirror); disk->mirror = NULL; disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_DEFAULT; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; save = true; } } @@ -1078,6 +1109,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);
ACK, although this patch an the next one are a bit borderline now that libvirt is frozen for the next release. Peter