From 8d813e074a1bbe7636c487487e661a5e1a713fd1 Mon Sep 17 00:00:00 2001

Message-Id: <8d813e074a1bbe7636c487487e661a5e1a713fd1@dist-git>

From: Peter Krempa <pkrempa@redhat.com>

Date: Tue, 23 Jun 2020 12:24:04 +0200

Subject: [PATCH] qemu: Rewrite bitmap handling for block copy

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

Reuse qemuBlockGetBitmapMergeActions which allows the removal of the

ad-hoc implementation of bitmap merging for block copy.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

(cherry picked from commit 7bfff40fdfe5410c446c1cd8ec413e00530faf7d)

https://bugzilla.redhat.com/show_bug.cgi?id=1804593

Message-Id: <6a254120769a0071ef6867e4c15aca7d3760cf3c.1592906423.git.pkrempa@redhat.com>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

---

 src/qemu/qemu_block.c    | 113 ++-------------------------------------

 src/qemu/qemu_blockjob.c |  40 ++++++++++++++

 src/qemu/qemu_driver.c   |  13 ++---

 tests/qemublocktest.c    |   6 ++-

 4 files changed, 57 insertions(+), 115 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c

index ff9e000461..18c3861a2e 100644

--- a/src/qemu/qemu_block.c

+++ b/src/qemu/qemu_block.c

@@ -3053,38 +3053,6 @@ qemuBlockBitmapChainIsValid(virStorageSourcePtr src,

 }

-struct qemuBlockBitmapsHandleBlockcopyConcatData {

-    virHashTablePtr bitmaps_merge;

-    virJSONValuePtr actions;

-    const char *mirrornodeformat;

-    bool has_bitmaps;

-};

-

-

-static int

-qemuBlockBitmapsHandleBlockcopyConcatActions(void *payload,

-                                             const void *name,

-                                             void *opaque)

-{

-    struct qemuBlockBitmapsHandleBlockcopyConcatData *data = opaque;

-    virJSONValuePtr createactions = payload;

-    const char *bitmapname = name;

-    g_autoptr(virJSONValue) mergebitmaps = virHashSteal(data->bitmaps_merge, bitmapname);

-

-    data->has_bitmaps = true;

-

-    virJSONValueArrayConcat(data->actions, createactions);

-

-    if (qemuMonitorTransactionBitmapMerge(data->actions,

-                                          data->mirrornodeformat,

-                                          bitmapname,

-                                          &mergebitmaps) < 0)

-        return -1;

-

-    return 0;

-}

-

-

 /**

  * qemuBlockBitmapsHandleBlockcopy:

  * @src: disk source

@@ -3107,86 +3075,15 @@ qemuBlockBitmapsHandleBlockcopy(virStorageSourcePtr src,

                                 bool shallow,

                                 virJSONValuePtr *actions)

 {

-    g_autoptr(virHashTable) bitmaps = virHashNew(virJSONValueHashFree);

-    g_autoptr(virHashTable) bitmaps_merge = virHashNew(virJSONValueHashFree);

-    g_autoptr(virHashTable) bitmaps_skip = virHashNew(NULL);

-    g_autoptr(virJSONValue) tmpactions = virJSONValueNewArray();

-    qemuBlockNamedNodeDataPtr entry;

-    virStorageSourcePtr n;

-    size_t i;

-    struct qemuBlockBitmapsHandleBlockcopyConcatData data = { .bitmaps_merge = bitmaps_merge,

-                                                              .actions = tmpactions,

-                                                              .mirrornodeformat = mirror->nodeformat,

-                                                              .has_bitmaps = false, };

-

-    for (n = src; n; n = n->backingStore) {

-        if (!(entry = virHashLookup(blockNamedNodeData, n->nodeformat)))

-            continue;

-

-        for (i = 0; i < entry->nbitmaps; i++) {

-            qemuBlockNamedNodeDataBitmapPtr bitmap = entry->bitmaps[i];

-            virJSONValuePtr bitmap_merge;

-

-            if (virHashHasEntry(bitmaps_skip, bitmap->name))

-                continue;

-

-            if (!(bitmap_merge = virHashLookup(bitmaps_merge, bitmap->name))) {

-                g_autoptr(virJSONValue) tmp = NULL;

-                bool disabled = !bitmap->recording;

-

-                /* disable any non top-layer bitmaps */

-                if (n != src)

-                    disabled = true;

-

-                if (!bitmap->persistent ||

-                    !(qemuBlockBitmapChainIsValid(n, bitmap->name,

-                                                  blockNamedNodeData))) {

-                    ignore_value(virHashAddEntry(bitmaps_skip, bitmap->name, NULL));

-                    continue;

-                }

-

-                /* prepare the data for adding the bitmap to the mirror */

-                tmp = virJSONValueNewArray();

-

-                if (qemuMonitorTransactionBitmapAdd(tmp,

-                                                    mirror->nodeformat,

-                                                    bitmap->name,

-                                                    true,

-                                                    disabled,

-                                                    bitmap->granularity) < 0)

-                    return -1;

+    virStorageSourcePtr base = NULL;

-                if (virHashAddEntry(bitmaps, bitmap->name, tmp) < 0)

-                    return -1;

-

-                tmp = NULL;

-

-                /* prepare array for merging all the bitmaps from the original chain */

-                tmp = virJSONValueNewArray();

-

-                if (virHashAddEntry(bitmaps_merge, bitmap->name, tmp) < 0)

-                    return -1;

-

-                bitmap_merge = g_steal_pointer(&tmp);

-            }

+    if (shallow)

+        base = src->backingStore;

-            if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(bitmap_merge,

-                                                                 n->nodeformat,

-                                                                 bitmap->name) < 0)

-                return -1;

-        }

-

-        if (shallow)

-            break;

-    }

-

-    if (virHashForEach(bitmaps, qemuBlockBitmapsHandleBlockcopyConcatActions,

-                       &data) < 0)

+    if (qemuBlockGetBitmapMergeActions(src, base, mirror, NULL, NULL, mirror, actions,

+                                       blockNamedNodeData) < 0)

         return -1;

-    if (data.has_bitmaps)

-        *actions = g_steal_pointer(&tmpactions);

-

     return 0;

 }

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c

index d106165175..6e33f8666c 100644

--- a/src/qemu/qemu_blockjob.c

+++ b/src/qemu/qemu_blockjob.c

@@ -1281,6 +1281,43 @@ qemuBlockJobProcessEventCompletedActiveCommit(virQEMUDriverPtr driver,

 }

+static int

+qemuBlockJobProcessEventCompletedCopyBitmaps(virDomainObjPtr vm,

+                                             qemuBlockJobDataPtr job,

+                                             qemuDomainAsyncJob asyncJob)

+{

+    qemuDomainObjPrivatePtr priv = vm->privateData;

+    g_autoptr(virHashTable) blockNamedNodeData = NULL;

+    g_autoptr(virJSONValue) actions = NULL;

+    bool shallow = job->jobflags & VIR_DOMAIN_BLOCK_COPY_SHALLOW;

+

+    if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV_REOPEN))

+        return 0;

+

+    if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, asyncJob)))

+        return -1;

+

+    if (qemuBlockBitmapsHandleBlockcopy(job->disk->src,

+                                        job->disk->mirror,

+                                        blockNamedNodeData,

+                                        shallow,

+                                        &actions) < 0)

+        return 0;

+

+    if (!actions)

+        return 0;

+

+    if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0)

+        return -1;

+

+    qemuMonitorTransaction(priv->mon, &actions);

+

+    if (qemuDomainObjExitMonitor(priv->driver, vm) < 0)

+        return -1;

+

+    return 0;

+}

+

 static void

 qemuBlockJobProcessEventConcludedCopyPivot(virQEMUDriverPtr driver,

                                            virDomainObjPtr vm,

@@ -1295,6 +1332,8 @@ qemuBlockJobProcessEventConcludedCopyPivot(virQEMUDriverPtr driver,

         !job->disk->mirror)

         return;

+    qemuBlockJobProcessEventCompletedCopyBitmaps(vm, job, asyncJob);

+

     /* for shallow copy without reusing external image the user can either not

      * specify the backing chain in which case libvirt will open and use the

      * chain the user provided or not specify a chain in which case we'll

@@ -1328,6 +1367,7 @@ qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriverPtr driver,

         !job->disk->mirror)

         return;

+    /* activeWrite bitmap is removed automatically here */

     qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, job->disk->mirror);

     virObjectUnref(job->disk->mirror);

     job->disk->mirror = NULL;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c

index ed5498d2bc..9e3a455814 100644

--- a/src/qemu/qemu_driver.c

+++ b/src/qemu/qemu_driver.c

@@ -17596,14 +17596,15 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,

         if (blockdev && !job->jobflagsmissing) {

             bool shallow = job->jobflags & VIR_DOMAIN_BLOCK_COPY_SHALLOW;

             bool reuse = job->jobflags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT;

-            g_autoptr(virHashTable) blockNamedNodeData = NULL;

-            if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, QEMU_ASYNC_JOB_NONE)))

-                return -1;

+            actions = virJSONValueNewArray();

-            if (qemuBlockBitmapsHandleBlockcopy(disk->src, disk->mirror,

-                                                blockNamedNodeData,

-                                                shallow, &actions) < 0)

+            if (qemuMonitorTransactionBitmapAdd(actions,

+                                                disk->mirror->nodeformat,

+                                                "libvirt-tmp-activewrite",

+                                                false,

+                                                false,

+                                                0) < 0)

                 return -1;

             /* Open and install the backing chain of 'mirror' late if we can use

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c

index e1468f13fa..193b4aaed0 100644

--- a/tests/qemublocktest.c

+++ b/tests/qemublocktest.c

@@ -881,6 +881,7 @@ testQemuBlockBitmapBlockcopy(const void *opaque)

     g_autoptr(virJSONValue) nodedatajson = NULL;

     g_autoptr(virHashTable) nodedata = NULL;

     g_autoptr(virStorageSource) fakemirror = virStorageSourceNew();

+    g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;

     if (!fakemirror)

         return -1;

@@ -903,10 +904,13 @@ testQemuBlockBitmapBlockcopy(const void *opaque)

                                         data->shallow, &actions) < 0)

         return -1;

+

     if (actions &&

-        !(actual = virJSONValueToString(actions, true)))

+        virJSONValueToBuffer(actions, &buf, true) < 0)

         return -1;

+    actual = virBufferContentAndReset(&buf;;

+

     return virTestCompareToFile(actual, expectpath);

 }

-- 

2.27.0