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