Re: [PATCH 03/13] qemu: blockjob: Store list of bitmaps disabled prior to commit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> 





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux