Re: [PATCH 18/19] qemu: migration: Migrate block dirty bitmaps corresponding to checkpoints

18 Feb 2021

On Thu, Feb 11, 2021 at 16:37:57 +0100, Peter Krempa wrote:
...
Preserve block dirty bitmaps after migration with
QEMU_MONITOR_MIGRATE_NON_SHARED_(DISK|INC).
This patch implements functions which offer the bitmaps to the
destination, check for eligibility on destination and then configure
source for the migration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_migration.c | 333 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 331 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 36424f8493..16bfad0390 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
...
@@ -2528,6 +2619,92 @@ qemuMigrationDstPrepare(virDomainObjPtr vm,
                                      migrateFrom, fd, NULL);
 }
+
+/**
+ * qemuMigrationDstPrepareAnyBlockDirtyBitmaps:
+ * @vm: domain object
+ * @mig: migration cookie
+ * @migParams: migration parameters
+ * @flags: migration flags
+ *
+ * Checks whether block dirty bitmaps offered by the migration source are
+ * to be migrated (e.g. they don't exist, the destination is compatible etc)
+ * and sets up destination qemu for migrating the bitmaps as well as updates the
+ * list of eligible bitmaps in the migration cookie to be sent back to the
+ * source.
+ */
+static int
+qemuMigrationDstPrepareAnyBlockDirtyBitmaps(virDomainObjPtr vm,
+                                            qemuMigrationCookiePtr mig,
+                                            qemuMigrationParamsPtr migParams,
+                                            unsigned int flags)
+{
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    g_autoptr(virJSONValue) mapping = NULL;
+    g_autoptr(GHashTable) blockNamedNodeData = NULL;
+    GSList *nextdisk;
+
+    if (!mig->nbd ||
+        !mig->blockDirtyBitmaps ||
+        !(flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)) ||
+        !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING))
+        return 0;
Shouldn't we report an error in case the source sent bitmaps, but local
QEMU does not support QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING?
...
+
+    if (qemuMigrationCookieBlockDirtyBitmapsMatchDisks(vm->def, mig->blockDirtyBitmaps) < 0)
+        return -1;
+
+    if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, QEMU_ASYNC_JOB_MIGRATION_IN)))
+        return -1;
+
+    for (nextdisk = mig->blockDirtyBitmaps; nextdisk; nextdisk = nextdisk->next) {
+        qemuMigrationBlockDirtyBitmapsDiskPtr disk = nextdisk->data;
+        qemuBlockNamedNodeDataPtr nodedata;
+        GSList *nextbitmap;
+
+        if (!(nodedata = virHashLookup(blockNamedNodeData, disk->nodename))) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("failed to find data for block node '%s'"),
+                           disk->nodename);
+            return -1;
+        }
+
+        /* don't migrate bitmaps into non-qcow2v3+ images */
How about "Bitmaps can only be migrated to qcow2 v3+"?
...
+        if (disk->disk->src->format != VIR_STORAGE_FILE_QCOW2 ||
+            nodedata->qcow2v2) {
+            disk->skip = true;
Is skipping the disk the right thing to do? Should we report an error
and abort migration instead? Just checking, maybe we can't do so for
backward compatibility...
...
+            continue;
+        }
+
+        for (nextbitmap = disk->bitmaps; nextbitmap; nextbitmap = nextbitmap->next) {
+            qemuMigrationBlockDirtyBitmapsDiskBitmapPtr bitmap = nextbitmap->data;
+            size_t k;
+
+            /* don't migrate into existing bitmaps */
+            for (k = 0; k < nodedata->nbitmaps; k++) {
+                if (STREQ(bitmap->bitmapname, nodedata->bitmaps[k]->name)) {
+                    bitmap->skip = true;
And similar questions for bitmaps here.
...
+                    break;
+                }
+            }
+
+            if (bitmap->skip)
+                continue;
+        }
+    }
+
+    if (qemuMigrationCookieBlockDirtyBitmapsToParams(mig->blockDirtyBitmaps,
+                                                     &mapping) < 0)
+        return -1;
+
+    if (!mapping)
+        return 0;
+
+    qemuMigrationParamsSetBlockDirtyBitmapMapping(migParams, &mapping);
+    mig->flags |= QEMU_MIGRATION_COOKIE_BLOCK_DIRTY_BITMAPS;
+    return 0;
+}
+
+
 static int
 qemuMigrationDstPrepareAny(virQEMUDriverPtr driver,
                            virConnectPtr dconn,
...
+static int
+qemuMigrationSrcRunPrepareBlockDirtyBitmaps(virDomainObjPtr vm,
+                                            qemuMigrationCookiePtr mig,
+                                            qemuMigrationParamsPtr migParams,
+                                            unsigned int flags)
+
+{
+    g_autoptr(virJSONValue) mapping = NULL;
+
+    if (!mig->blockDirtyBitmaps)
+        return 0;
+
+    if (qemuMigrationCookieBlockDirtyBitmapsMatchDisks(vm->def, mig->blockDirtyBitmaps) < 0)
+        return -1;
+
+    /* For QEMU_MONITOR_MIGRATE_NON_SHARED_INC we can migrate the bitmaps
+     * directly, otherwise we must create merged bitmaps from the whole
+     * chain */
+
+    if (!(flags & QEMU_MONITOR_MIGRATE_NON_SHARED_INC) &&
+        qemuMigrationSrcRunPrepareBlockDirtyBitmapsMerge(vm, mig))
"< 0" is missing in the check.
...
+        return -1;
+
+    if (qemuMigrationCookieBlockDirtyBitmapsToParams(mig->blockDirtyBitmaps,
+                                                     &mapping) < 0)
+        return -1;
+
+    qemuMigrationParamsSetBlockDirtyBitmapMapping(migParams, &mapping);
+    return 0;
+}
+
+
 static int
 qemuMigrationSrcRun(virQEMUDriverPtr driver,
                     virDomainObjPtr vm,
Jirka

    

Re: [PATCH 18/19] qemu: migration: Migrate block dirty bitmaps corresponding to checkpoints

Jiri Denemark