Introduce the handler for finalizing a block commit and active bloc commit job which will allow to use it with blockdev. Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx> --- src/qemu/qemu_blockjob.c | 212 ++++++++++++++++++ src/qemu/qemu_blockjob.h | 18 ++ src/qemu/qemu_domain.c | 32 +++ src/qemu/qemu_driver.c | 50 +++-- .../blockjob-blockdev-in.xml | 15 ++ 5 files changed, 311 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index a29af7ec48..4cbdc34b66 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -237,6 +237,43 @@ qemuBlockJobDiskNewPull(virDomainObjPtr vm, } +qemuBlockJobDataPtr +qemuBlockJobDiskNewCommit(virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virStorageSourcePtr topparent, + virStorageSourcePtr top, + virStorageSourcePtr base) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + VIR_AUTOUNREF(qemuBlockJobDataPtr) job = NULL; + VIR_AUTOFREE(char *) jobname = NULL; + qemuBlockJobType jobtype = QEMU_BLOCKJOB_TYPE_COMMIT; + + if (topparent == NULL) + jobtype = QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT; + + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { + if (virAsprintf(&jobname, "commit-%s-%s", disk->dst, top->nodeformat) < 0) + return NULL; + } else { + if (!(jobname = qemuAliasDiskDriveFromDisk(disk))) + return NULL; + } + + if (!(job = qemuBlockJobDataNew(jobtype, jobname))) + return NULL; + + job->data.commit.topparent = topparent; + job->data.commit.top = top; + job->data.commit.base = base; + + if (qemuBlockJobRegister(job, vm, disk, true) < 0) + return NULL; + + VIR_RETURN_PTR(job); +} + + /** * qemuBlockJobDiskRegisterMirror: * @job: block job to register 'mirror' chain on @@ -816,6 +853,167 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr driver, } +/** + * qemuBlockJobProcessEventCompletedCommit: + * @driver: qemu driver object + * @vm: domain object + * @job: job data + * @asyncJob: qemu asynchronous job type (for monitor interaction) + * + * This function executes the finalizing steps after a successful block commit + * job. The commit job moves the blocks from backing chain images starting from + * 'top' into the 'base' image. The overlay of the 'top' image ('topparent') + * then directly references the 'base' image. All intermediate images can be + * removed/deleted. + */ +static void +qemuBlockJobProcessEventCompletedCommit(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuBlockJobDataPtr job, + qemuDomainAsyncJob asyncJob) +{ + virStorageSourcePtr baseparent = NULL; + virDomainDiskDefPtr cfgdisk = NULL; + virStorageSourcePtr cfgnext = NULL; + virStorageSourcePtr cfgtopparent = NULL; + virStorageSourcePtr cfgtop = NULL; + virStorageSourcePtr cfgbase = NULL; + virStorageSourcePtr cfgbaseparent = NULL; + virStorageSourcePtr n; + + VIR_DEBUG("commit job '%s' on VM '%s' completed", job->name, vm->def->name); + + /* if the job isn't associated with a disk there's nothing to do */ + if (!job->disk) + return; + + if ((cfgdisk = qemuBlockJobGetConfigDisk(vm, job->disk, job->data.commit.base))) + cfgnext = cfgdisk->src; + + if (!cfgdisk) + qemuBlockJobClearConfigChain(vm, job->disk); + + for (n = job->disk->src; n && n != job->data.commit.base; n = n->backingStore) { + if (cfgnext) { + if (n == job->data.commit.topparent) + cfgtopparent = cfgnext; + + if (n == job->data.commit.top) + cfgtop = cfgnext; + + cfgbaseparent = cfgnext; + cfgnext = cfgnext->backingStore; + } + baseparent = n; + } + + if (!n) + return; + + /* revert access to images */ + qemuDomainStorageSourceAccessAllow(driver, vm, job->data.commit.base, true, false); + if (job->data.commit.topparent != job->disk->src) + qemuDomainStorageSourceAccessAllow(driver, vm, job->data.commit.topparent, true, false); + + baseparent->backingStore = NULL; + job->data.commit.topparent->backingStore = job->data.commit.base; + + qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, job->data.commit.top); + virObjectUnref(job->data.commit.top); + job->data.commit.top = NULL; + + if (cfgbaseparent) { + cfgbase = cfgbaseparent->backingStore; + cfgbaseparent->backingStore = NULL; + + if (cfgtopparent) + cfgtopparent->backingStore = cfgbase; + else + cfgdisk->src = cfgbase; + + virObjectUnref(cfgtop); + } +} + + +/** + * qemuBlockJobProcessEventCompletedActiveCommit: + * @driver: qemu driver object + * @vm: domain object + * @job: job data + * @asyncJob: qemu asynchronous job type (for monitor interaction) + * + * This function executes the finalizing steps after a successful active layer + * block commit job. The commit job moves the blocks from backing chain images + * starting from the active disk source image into the 'base' image. The disk + * source then changes to the 'base' image. All intermediate images can be + * removed/deleted. + */ +static void +qemuBlockJobProcessEventCompletedActiveCommit(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuBlockJobDataPtr job, + qemuDomainAsyncJob asyncJob) +{ + virStorageSourcePtr baseparent = NULL; + virDomainDiskDefPtr cfgdisk = NULL; + virStorageSourcePtr cfgnext = NULL; + virStorageSourcePtr cfgtop = NULL; + virStorageSourcePtr cfgbase = NULL; + virStorageSourcePtr cfgbaseparent = NULL; + virStorageSourcePtr n; + + VIR_DEBUG("active commit job '%s' on VM '%s' completed", job->name, vm->def->name); + + /* if the job isn't associated with a disk there's nothing to do */ + if (!job->disk) + return; + + if ((cfgdisk = qemuBlockJobGetConfigDisk(vm, job->disk, job->data.commit.base))) + cfgnext = cfgdisk->src; + + for (n = job->disk->src; n && n != job->data.commit.base; n = n->backingStore) { + if (cfgnext) { + if (n == job->data.commit.top) + cfgtop = cfgnext; + + cfgbaseparent = cfgnext; + cfgnext = cfgnext->backingStore; + } + baseparent = n; + } + + if (!n) + return; + + if (!cfgdisk) { + /* in case when the config disk chain didn't match but the disk top seems + * to be identical we need to modify the disk source since the active + * commit makes the top level image invalid. + */ + qemuBlockJobRewriteConfigDiskSource(vm, job->disk, job->data.commit.base); + } else { + cfgbase = cfgbaseparent->backingStore; + cfgbaseparent->backingStore = NULL; + cfgdisk->src = cfgbase; + virObjectUnref(cfgtop); + } + + /* Move security driver metadata */ + if (qemuSecurityMoveImageMetadata(driver, vm, job->disk->src, job->data.commit.base) < 0) + VIR_WARN("Unable to move disk metadata on vm %s", vm->def->name); + + baseparent->backingStore = NULL; + job->disk->src = job->data.commit.base; + + qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, job->data.commit.top); + virObjectUnref(job->data.commit.top); + job->data.commit.top = NULL; + /* the mirror element does not serve functional purpose for the commit job */ + virObjectUnref(job->disk->mirror); + job->disk->mirror = NULL; +} + static void qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job, virQEMUDriverPtr driver, @@ -830,7 +1028,13 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job, break; case QEMU_BLOCKJOB_TYPE_COMMIT: + qemuBlockJobProcessEventCompletedCommit(driver, vm, job, asyncJob); + break; + case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT: + qemuBlockJobProcessEventCompletedActiveCommit(driver, vm, job, asyncJob); + break; + case QEMU_BLOCKJOB_TYPE_COPY: case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: @@ -845,7 +1049,15 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job, switch ((qemuBlockJobType) job->type) { case QEMU_BLOCKJOB_TYPE_PULL: case QEMU_BLOCKJOB_TYPE_COMMIT: + break; + case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT: + if (job->disk) { + virObjectUnref(job->disk->mirror); + job->disk->mirror = NULL; + } + break; + case QEMU_BLOCKJOB_TYPE_COPY: case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index d5848fb72c..8139a1a324 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -77,6 +77,16 @@ struct _qemuBlockJobPullData { }; +typedef struct _qemuBlockJobCommitData qemuBlockJobCommitData; +typedef qemuBlockJobCommitData *qemuBlockJobDataCommitPtr; + +struct _qemuBlockJobCommitData { + virStorageSourcePtr topparent; + virStorageSourcePtr top; + virStorageSourcePtr base; +}; + + typedef struct _qemuBlockJobData qemuBlockJobData; typedef qemuBlockJobData *qemuBlockJobDataPtr; @@ -91,6 +101,7 @@ struct _qemuBlockJobData { union { qemuBlockJobPullData pull; + qemuBlockJobCommitData commit; } data; int type; /* qemuBlockJobType */ @@ -132,6 +143,13 @@ qemuBlockJobDiskNewPull(virDomainObjPtr vm, virDomainDiskDefPtr disk, virStorageSourcePtr base); +qemuBlockJobDataPtr +qemuBlockJobDiskNewCommit(virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virStorageSourcePtr topparent, + virStorageSourcePtr top, + virStorageSourcePtr base); + qemuBlockJobDataPtr qemuBlockJobDiskGetJob(virDomainDiskDefPtr disk) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ec1dda4870..26a951b6d3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2398,6 +2398,12 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload, case QEMU_BLOCKJOB_TYPE_COMMIT: case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT: + if (job->data.commit.base) + virBufferAsprintf(&childBuf, "<base node='%s'/>\n", job->data.commit.base->nodeformat); + if (job->data.commit.top) + virBufferAsprintf(&childBuf, "<top node='%s'/>\n", job->data.commit.top->nodeformat); + if (job->data.commit.topparent) + virBufferAsprintf(&childBuf, "<topparent node='%s'/>\n", job->data.commit.topparent->nodeformat); case QEMU_BLOCKJOB_TYPE_COPY: case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: @@ -2854,7 +2860,29 @@ qemuDomainObjPrivateXMLParseBlockjobDataSpecific(qemuBlockJobDataPtr job, break; case QEMU_BLOCKJOB_TYPE_COMMIT: + qemuDomainObjPrivateXMLParseBlockjobNodename(job, + "string(./topparent/@node)", + &job->data.commit.topparent, + ctxt); + + if (!job->data.commit.topparent) + goto broken; + + ATTRIBUTE_FALLTHROUGH; case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT: + qemuDomainObjPrivateXMLParseBlockjobNodename(job, + "string(./top/@node)", + &job->data.commit.top, + ctxt); + qemuDomainObjPrivateXMLParseBlockjobNodename(job, + "string(./base/@node)", + &job->data.commit.base, + ctxt); + if (!job->data.commit.top || + !job->data.commit.base) + goto broken; + break; + case QEMU_BLOCKJOB_TYPE_COPY: case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: @@ -2863,6 +2891,10 @@ qemuDomainObjPrivateXMLParseBlockjobDataSpecific(qemuBlockJobDataPtr job, } return; + + broken: + VIR_DEBUG("marking block job '%s' as invalid: malformed job data", job->name); + job->invalidData = true; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 705c1a06c0..79d13674c6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17939,7 +17939,8 @@ qemuDomainBlockCommit(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; qemuDomainObjPrivatePtr priv; virDomainObjPtr vm = NULL; - VIR_AUTOFREE(char *) device = NULL; + const char *device = NULL; + const char *jobname = NULL; int ret = -1; virDomainDiskDefPtr disk = NULL; virStorageSourcePtr topSource; @@ -17953,8 +17954,10 @@ qemuDomainBlockCommit(virDomainPtr dom, VIR_AUTOFREE(char *) backingPath = NULL; unsigned long long speed = bandwidth; qemuBlockJobDataPtr job = NULL; - qemuBlockJobType jobtype = QEMU_BLOCKJOB_TYPE_COMMIT; VIR_AUTOUNREF(virStorageSourcePtr) mirror = NULL; + const char *nodetop = NULL; + const char *nodebase = NULL; + bool persistjob = false; /* XXX Add support for COMMIT_DELETE */ virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW | @@ -17989,9 +17992,6 @@ qemuDomainBlockCommit(virDomainPtr dom, if (!(disk = qemuDomainDiskByName(vm->def, path))) goto endjob; - if (!(device = qemuAliasDiskDriveFromDisk(disk))) - goto endjob; - if (virStorageSourceIsEmpty(disk->src)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk %s has no source file to be committed"), @@ -18023,8 +18023,6 @@ qemuDomainBlockCommit(virDomainPtr dom, disk->dst); goto endjob; } - - jobtype = QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT; } else if (flags & VIR_DOMAIN_BLOCK_COMMIT_ACTIVE) { virReportError(VIR_ERR_INVALID_ARG, _("active commit requested but '%s' is not active"), @@ -18097,7 +18095,8 @@ qemuDomainBlockCommit(virDomainPtr dom, qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, false, false) < 0)) goto endjob; - if (!(job = qemuBlockJobDiskNew(vm, disk, jobtype, device))) + if (!(job = qemuBlockJobDiskNewCommit(vm, disk, top_parent, topSource, + baseSource))) goto endjob; disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; @@ -18107,15 +18106,34 @@ qemuDomainBlockCommit(virDomainPtr dom, * 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). */ + + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { + persistjob = true; + jobname = job->name; + nodetop = topSource->nodeformat; + nodebase = baseSource->nodeformat; + device = disk->src->nodeformat; + if (!backingPath && top_parent && + !(backingPath = qemuBlockGetBackingStoreString(baseSource))) + goto endjob; + } else { + device = job->name; + } + qemuDomainObjEnterMonitor(driver, vm); - basePath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src, - baseSource); - topPath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src, - topSource); - if (basePath && topPath) - ret = qemuMonitorBlockCommit(priv->mon, device, NULL, false, - topPath, NULL, basePath, NULL, backingPath, - speed); + + if (!jobname) { + basePath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src, + baseSource); + topPath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src, + topSource); + } + + if ((basePath && topPath) || jobname) + ret = qemuMonitorBlockCommit(priv->mon, device, jobname, persistjob, + topPath, nodetop, basePath, nodebase, + backingPath, speed); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || ret < 0) { ret = -1; goto endjob; diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml index e962b837ac..d06bbb7059 100644 --- a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml +++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml @@ -238,6 +238,17 @@ <disk dst='vdb'/> <base node='libvirt-14-format'/> </blockjob> + <blockjob name='pull-vdd-libvirt-17-format' type='active-commit' state='ready'> + <disk dst='vdd'/> + <base node='libvirt-19-format'/> + <top node='libvirt-17-format'/> + </blockjob> + <blockjob name='pull-vdc-libvirt-9-format' type='commit' state='running'> + <disk dst='vdc'/> + <base node='libvirt-11-format'/> + <top node='libvirt-9-format'/> + <topparent node='libvirt-2-format'/> + </blockjob> <blockjob name='drive-virtio-disk0' type='copy' state='ready'> <disk dst='vda' mirror='yes'/> </blockjob> @@ -581,6 +592,10 @@ </backingStore> </backingStore> </backingStore> + <mirror type='file' job='active-commit' ready='yes'> + <format type='qcow2'/> + <source file='/tmp/activecommit2.qcow2'/> + </mirror> <target dev='vdd' bus='virtio'/> <alias name='virtio-disk3'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> -- 2.21.0 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list