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