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 <ebl...@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
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
