On Wed, Mar 04, 2020 at 06:26:31PM +0100, Peter Krempa wrote:
> Starting a commit job will require disabling bitmaps in the base image
> so that they are not dirtied by the commit job. We need to store a list
> of the bitmaps so that we can later re-enable them.
>
> Add a field and status XML handling code as well as a test.
>
> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> ---
> src/qemu/qemu_blockjob.h | 2 ++
> src/qemu/qemu_domain.c | 26 +++++++++++++++++++
> .../blockjob-blockdev-in.xml | 4 +++
> 3 files changed, 32 insertions(+)
>
> diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h
> index 72c7fa053e..e2e28ca4d3 100644
> --- a/src/qemu/qemu_blockjob.h
> +++ b/src/qemu/qemu_blockjob.h
> @@ -88,6 +88,8 @@ struct _qemuBlockJobCommitData {
> virStorageSourcePtr top;
> virStorageSourcePtr base;
> bool deleteCommittedImages;
> + char **disabledBitmapsBase; /* a NULL-terminated list of bitmap names which
> + were disabled in @base for the commit job */
> };
Is it guaranteed that the new member 'disabledBitmapsBase' is properly
initialised? How about something along the lines of
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 2e53821d43..f9fc83430e 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -312,6 +312,7 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
job->data.commit.top = top;
job->data.commit.base = base;
job->data.commit.deleteCommittedImages = delete_imgs;
+ job->data.commit.disabledBitmapsBase = NULL;
job->jobflags = jobflags;
if (qemuBlockJobRegister(job, vm, disk, true) < 0)
Even if explicit initialisation is somehow not needed here, I'd consider that
fact worth a line of explanation in the commit message.
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 0a478d2080..c5f4b0ae7f 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2534,6 +2534,9 @@ static void
> qemuDomainPrivateBlockJobFormatCommit(qemuBlockJobDataPtr job,
> virBufferPtr buf)
> {
> + g_auto(virBuffer) disabledBitmapsBuf = VIR_BUFFER_INIT_CHILD(buf);
> + char **bitmaps = job->data.commit.disabledBitmapsBase;
> +
> if (job->data.commit.base)
> virBufferAsprintf(buf, "<base node='%s'/>\n", job->data.commit.base->nodeformat);
>
> @@ -2545,6 +2548,11 @@ qemuDomainPrivateBlockJobFormatCommit(qemuBlockJobDataPtr job,
>
> if (job->data.commit.deleteCommittedImages)
> virBufferAddLit(buf, "<deleteCommittedImages/>\n");
> +
> + while (bitmaps && *bitmaps)
> + virBufferEscapeString(&disabledBitmapsBuf, "<bitmap name='%s'/>\n", *(bitmaps++));
> +
> + virXMLFormatElement(buf, "disabledBaseBitmaps", NULL, &disabledBitmapsBuf);
> }
>
>
> @@ -3157,6 +3165,9 @@ static int
> qemuDomainObjPrivateXMLParseBlockjobDataCommit(qemuBlockJobDataPtr job,
> xmlXPathContextPtr ctxt)
> {
> + g_autofree xmlNodePtr *nodes = NULL;
> + ssize_t nnodes;
> +
> if (job->type == QEMU_BLOCKJOB_TYPE_COMMIT) {
> qemuDomainObjPrivateXMLParseBlockjobNodename(job,
> "string(./topparent/@node)",
> @@ -3183,6 +3194,21 @@ qemuDomainObjPrivateXMLParseBlockjobDataCommit(qemuBlockJobDataPtr job,
> !job->data.commit.base)
> return -1;
>
> + if ((nnodes = virXPathNodeSet("./disabledBaseBitmaps/bitmap", ctxt, &nodes)) > 0) {
> + size_t i;
> +
> + job->data.commit.disabledBitmapsBase = g_new0(char *, nnodes + 1);
> +
> + for (i = 0; i < nnodes; i++) {
> + char *tmp;
> +
> + if (!(tmp = virXMLPropString(nodes[i], "name")))
> + return -1;
> +
> + job->data.commit.disabledBitmapsBase[i] = g_steal_pointer(&tmp);
> + }
> + }
> +
> return 0;
> }
>
> diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
> index ca6d110179..cc17a17ff4 100644
> --- a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
> +++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
> @@ -243,6 +243,10 @@
> <base node='libvirt-19-format'/>
> <top node='libvirt-17-format'/>
> <deleteCommittedImages/>
> + <disabledBaseBitmaps>
> + <bitmap name='test'/>
> + <bitmap name='test1'/>
> + </disabledBaseBitmaps>
> </blockjob>
> <blockjob name='create-libvirt-1337-storage' type='create' state='running'>
> <create mode='storage'/>
> --
> 2.24.1
>
