On Mon, Jul 29, 2019 at 10:40:14AM +0200, Peter Krempa wrote:
On Fri, Jul 26, 2019 at 14:12:57 +0200, Ján Tomko wrote:On Wed, Jul 24, 2019 at 11:07:35PM +0200, Peter Krempa wrote: > Introduce the handler for finalizing a block pull job which will allow > to use it with blockdev. > > This patch also contains some additional machinery which is required to > store all the relevant job data in the status XML which will also be > reused with other block job types. > > Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx> > --- > src/qemu/qemu_blockjob.c | 191 +++++++++++++++++- > src/qemu/qemu_blockjob.h | 18 ++ > src/qemu/qemu_domain.c | 77 +++++++ > src/qemu/qemu_driver.c | 33 ++- > .../blockjob-blockdev-in.xml | 4 + > 5 files changed, 313 insertions(+), 10 deletions(-) > > diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c > index 0c0ae89f10..a29af7ec48 100644 > --- a/src/qemu/qemu_blockjob.c > +++ b/src/qemu/qemu_blockjob.c > +/** > + * qemuBlockJobProcessEventCompletedPull: > + * @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 pull job > + * (block-stream in qemu terminology. The pull job copies all the data from the > + * images in the backing chain up to the 'base' image. The 'base' image becomes > + * the backing store of the active top level image. If 'base' was not used > + * everything is pulled into the top level image and the top level image will > + * cease to have backing store. All intermediate images between the active image > + * and base image are no longer required and can be unplugged. > + */ > +static void > +qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr driver, > + virDomainObjPtr vm, > + qemuBlockJobDataPtr job, > + qemuDomainAsyncJob asyncJob) > +{ > + virStorageSourcePtr baseparent = NULL; > + virDomainDiskDefPtr cfgdisk = NULL; > + virStorageSourcePtr cfgbase = NULL; > + virStorageSourcePtr cfgbaseparent = NULL; > + virStorageSourcePtr n; > + virStorageSourcePtr tmp; > + > + VIR_DEBUG("pull 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.pull.base))) > + cfgbase = cfgdisk->src->backingStore; > + Consider: cfgdisk = ...(); if (cfgdisk) cfgbase = ...; else ...();There is no 'else' so this format does not make much sense.> + if (!cfgdisk)
This is the 'else'
> + qemuBlockJobClearConfigChain(vm, job->disk); > + > + /* when pulling if 'base' is right below the top image we don't have to modify it */ > + if (job->disk->src->backingStore == job->data.pull.base) > + return; > + > + if (job->data.pull.base) { > + for (n = job->disk->src->backingStore; n && n != job->data.pull.base; n = n->backingStore) { > + /* find the image on top of 'base' */ > + > + if (cfgbase) { > + cfgbaseparent = cfgbase; > + cfgbase = cfgbase->backingStore; > + } > + > + baseparent = n; > + } > + } > + > + tmp = job->disk->src->backingStore; > + job->disk->src->backingStore = job->data.pull.base; > + if (baseparent) > + baseparent->backingStore = NULL; > + qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, tmp); > + virObjectUnref(tmp); > + > + if (cfgdisk) { > + tmp = cfgdisk->src->backingStore; You can unref tmp directly hereNo I can't. 'cfgbaseparent' is in the chain of images below now 'tmp' and thus the undef would unref everything ...> + cfgdisk->src->backingStore = cfgbase; > + if (cfgbaseparent) > + cfgbaseparent->backingStore = NULL;... this inhibits deletion of the part of the chain we still want.> + virObjectUnref(tmp); > + } > +}[...]> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index c508f55287..ec1dda4870 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -2390,6 +2390,21 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload, > return -1; > } > > + switch ((qemuBlockJobType) job->type) { > + case QEMU_BLOCKJOB_TYPE_PULL: > + if (job->data.pull.base) > + virBufferAsprintf(&childBuf, "<base node='%s'/>\n", job->data.pull.base->nodeformat); > + break; > + > + case QEMU_BLOCKJOB_TYPE_COMMIT: > + case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT: > + case QEMU_BLOCKJOB_TYPE_COPY: > + case QEMU_BLOCKJOB_TYPE_NONE: > + case QEMU_BLOCKJOB_TYPE_INTERNAL: > + case QEMU_BLOCKJOB_TYPE_LAST: > + break; > + } > + > return virXMLFormatElement(data->buf, "blockjob", &attrBuf, &childBuf); > } > > @@ -2793,6 +2808,64 @@ qemuDomainObjPrivateXMLParseBlockjobChain(xmlNodePtr node, > } > > > +static void > +qemuDomainObjPrivateXMLParseBlockjobNodename(qemuBlockJobDataPtr job, > + const char *xpath, > + virStorageSourcePtr *src, > + xmlXPathContextPtr ctxt) > +{ > + VIR_AUTOFREE(char *) nodename = NULL; > + > + *src = NULL; > + > + if (!(nodename = virXPathString(xpath, ctxt))) > + return; > + > + if (job->disk && > + (*src = virStorageSourceFindByNodeName(job->disk->src, nodename, NULL))) > + return; > + > + if (job->chain && > + (*src = virStorageSourceFindByNodeName(job->chain, nodename, NULL))) > + return; > + > + if (job->mirrorChain && > + (*src = virStorageSourceFindByNodeName(job->mirrorChain, nodename, NULL))) > + return; > + > + /* the node was in the XML but was not found in the job definitions */ > + VIR_DEBUG("marking block job '%s' as invalid: node name '%s' missing", > + job->name, nodename); > + job->invalidData = true; > +} > + > + > +static void > +qemuDomainObjPrivateXMLParseBlockjobDataSpecific(qemuBlockJobDataPtr job, > + xmlXPathContextPtr ctxt) > +{ > + switch ((qemuBlockJobType) job->type) { > + case QEMU_BLOCKJOB_TYPE_PULL: > + qemuDomainObjPrivateXMLParseBlockjobNodename(job, > + "string(./base/@node)", > + &job->data.pull.base, > + ctxt); > + /* base is not present if pulling everything */ > + break; > + > + case QEMU_BLOCKJOB_TYPE_COMMIT: > + case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT: > + case QEMU_BLOCKJOB_TYPE_COPY: > + case QEMU_BLOCKJOB_TYPE_NONE: > + case QEMU_BLOCKJOB_TYPE_INTERNAL: > + case QEMU_BLOCKJOB_TYPE_LAST: virReportEnumRangeErrorIn a void function?> + break; > + } > + > + return; > +} > + > + > static int > qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm, > xmlNodePtr node, > @@ -2863,10 +2936,14 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm, > job->errmsg = virXPathString("string(./errmsg)", ctxt); > job->invalidData = invalidData; > job->disk = disk; > + if (invalidData) > + VIR_DEBUG("marking block job '%s' as invalid: basic data broken", job->name); > This hunk belongs to a separate patch. > if (mirror) > qemuBlockJobDiskRegisterMirror(job); > > + qemuDomainObjPrivateXMLParseBlockjobDataSpecific(job, ctxt); > + Possibly this one too.Well I'd have to introduce a function which doesn't do anything here and then populate it later? Should I do it?
Nah. Jano
> if (qemuBlockJobRegister(job, vm, disk, false) < 0) > return -1; >
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
