From 8d813e074a1bbe7636c487487e661a5e1a713fd1 Mon Sep 17 00:00:00 2001 Message-Id: <8d813e074a1bbe7636c487487e661a5e1a713fd1@dist-git> From: Peter Krempa 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 Reviewed-by: Eric Blake (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 --- 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