[PATCH 12/19] qemu: migration_cookie: Add XML handling for setting up bitmap migration

[Jiri Denemark]

On Thu, Feb 11, 2021 at 16:37:51 +0100, Peter Krempa wrote:
> In cases where we are copying the storage we need to ensure that also
> bitmaps are copied properly. This patch adds migration cookie XML
> infrastructure which will allow the migration sides reach consensus on
> which bitmaps to migrate.
> 
> Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> ---
>  src/qemu/qemu_migration_cookie.c | 134 +++++++++++++++++++++++++++++++
>  src/qemu/qemu_migration_cookie.h |  34 ++++++++
>  2 files changed, 168 insertions(+)
> 
> diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c
> index 6f2b1b2f57..94ba9c83d0 100644
> --- a/src/qemu/qemu_migration_cookie.c
> +++ b/src/qemu/qemu_migration_cookie.c
...
> @@ -758,6 +795,48 @@ qemuMigrationCookieNBDXMLFormat(qemuMigrationCookieNBDPtr nbd,
>  }
> 
> 
> +static void
> +qemuMigrationCookieBlockDirtyBitmapsFormat(virBufferPtr buf,
> +                                           GSList *bitmaps)
> +{
> +    g_auto(virBuffer) disksBuf = VIR_BUFFER_INIT_CHILD(buf);
> +    GSList *nextdisk;
> +
> +    for (nextdisk = bitmaps; nextdisk; nextdisk = nextdisk->next) {
> +        qemuMigrationBlockDirtyBitmapsDiskPtr disk = nextdisk->data;
> +        g_auto(virBuffer) diskAttrBuf = VIR_BUFFER_INITIALIZER;
> +        g_auto(virBuffer) diskChildBuf = VIR_BUFFER_INIT_CHILD(&disksBuf);
> +        bool hasBitmaps = false;
> +        GSList *nextbitmap;
> +
> +        if (disk->skip || !disk->bitmaps)
> +            continue;
> +
> +        for (nextbitmap = disk->bitmaps; nextbitmap; nextbitmap = nextbitmap->next) {
> +            qemuMigrationBlockDirtyBitmapsDiskBitmapPtr bitmap = nextbitmap->data;
> +
> +            if (bitmap->skip)
> +                continue;
> +
> +            virBufferAsprintf(&diskChildBuf,
> +                              "<bitmap name='%s' alias='%s'/>\n",
> +                              bitmap->bitmapname, bitmap->alias);
> +
> +            hasBitmaps = true;
> +        }
> +
> +        if (!hasBitmaps)
> +            continue;

You could drop hasBitmaps and call virBufferUse instead, but not a big
deal.

> +
> +        virBufferAsprintf(&diskAttrBuf, " target='%s'", disk->target);
> +        virXMLFormatElement(&disksBuf, "disk", &diskAttrBuf, &diskChildBuf);
> +    }
> +
> +
> +    virXMLFormatElement(buf, "blockDirtyBitmaps", NULL, &disksBuf);
> +}
> +
> +
>  int
>  qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver,
>                               virQEMUCapsPtr qemuCaps,
> @@ -829,6 +908,9 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver,
>      if (mig->flags & QEMU_MIGRATION_COOKIE_CAPS)
>          qemuMigrationCookieCapsXMLFormat(buf, mig->caps);
> 
> +    if (mig->flags & QEMU_MIGRATION_COOKIE_BLOCK_DIRTY_BITMAPS)
> +        qemuMigrationCookieBlockDirtyBitmapsFormat(buf, mig->blockDirtyBitmaps);
> +
>      virBufferAdjustIndent(buf, -2);
>      virBufferAddLit(buf, "</qemu-migration>\n");
>      return 0;
> @@ -1132,6 +1214,53 @@ qemuMigrationCookieXMLParseMandatoryFeatures(xmlXPathContextPtr ctxt,
>  }
> 
> 
> +static int
> +qemuMigrationCookieBlockDirtyBitmapsParse(xmlXPathContextPtr ctxt,
> +                                          qemuMigrationCookiePtr mig)
> +{
> +    g_autoslist(qemuMigrationBlockDirtyBitmapsDisk) disks = NULL;
> +    g_autofree xmlNodePtr *disknodes = NULL;
> +    int ndisknodes;
> +    size_t i;
> +    VIR_XPATH_NODE_AUTORESTORE(ctxt)
> +
> +    if ((ndisknodes = virXPathNodeSet("./blockDirtyBitmaps/disk", ctxt, &disknodes)) < 0)
> +        return -1;
> +
> +    for (i = 0; i < ndisknodes; i++) {
> +        GSList *bitmaps = NULL;
> +        qemuMigrationBlockDirtyBitmapsDiskPtr disk;
> +        g_autofree xmlNodePtr *bitmapnodes = NULL;
> +        int nbitmapnodes;
> +        size_t j;
> +
> +        ctxt->node = disknodes[i];
> +
> +        if ((nbitmapnodes = virXPathNodeSet("./bitmap", ctxt, &bitmapnodes)) < 0)
> +            return -1;
> +
> +        for (j = 0; j < nbitmapnodes; j++) {
> +            qemuMigrationBlockDirtyBitmapsDiskBitmapPtr bitmap;
> +
> +            bitmap = g_new0(qemuMigrationBlockDirtyBitmapsDiskBitmap, 1);
> +            bitmap->bitmapname = virXMLPropString(bitmapnodes[j], "name");
> +            bitmap->alias = virXMLPropString(bitmapnodes[j], "alias");

So what if the attributes do not exist? And virXMLPropString does not
abort on OOM. You should check for the result being non-NULL here.

> +            bitmaps = g_slist_prepend(bitmaps, bitmap);
> +        }
> +
> +        disk = g_new0(qemuMigrationBlockDirtyBitmapsDisk, 1);
> +        disk->target = virXMLPropString(disknodes[i], "target");

And here as well.

> +        disk->bitmaps = g_slist_reverse(bitmaps);
> +
> +        disks = g_slist_prepend(disks, disk);
> +    }
> +
> +    mig->blockDirtyBitmaps = g_slist_reverse(g_steal_pointer(&disks));
> +
> +    return 0;
> +}
> +
> +
>  static int
>  qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
>                              virQEMUDriverPtr driver,
...

With the checks for virXMLPropString result added

Reviewed-by: Jiri Denemark <jdenemar at redhat.com>