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@redhat.com>
---
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 */
};
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
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@redhat.com>
> ---
> 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
>
On Thu, Mar 05, 2020 at 10:43:15 +0100, Pavel Mores wrote:
> 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@redhat.com>
> > ---
> > 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.
g_new0 and VIR_ALLOC (which uses calloc) always zero-out the memory
which is allocated. This means that setting integers to 0, pointers to
NULL and booleans to false is always assumed when allocating a new
object and we rarely re-initialize it explicitly if ever.
© 2016 - 2026 Red Hat, Inc.