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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
