313 lines
13 KiB
Diff
313 lines
13 KiB
Diff
|
From e7b1604c5848861db0896b3c7869c0ad1d971084 Mon Sep 17 00:00:00 2001
|
||
|
Message-Id: <e7b1604c5848861db0896b3c7869c0ad1d971084@dist-git>
|
||
|
From: Peter Krempa <pkrempa@redhat.com>
|
||
|
Date: Tue, 23 Jun 2020 12:23:53 +0200
|
||
|
Subject: [PATCH] qemu: checkpoint: Don't merge checkpoints during deletion
|
||
|
MIME-Version: 1.0
|
||
|
Content-Type: text/plain; charset=UTF-8
|
||
|
Content-Transfer-Encoding: 8bit
|
||
|
|
||
|
Now that we've switched to the simple handling, the first thing that can
|
||
|
be massively simplified is checkpoint deletion. We now need to only go
|
||
|
through the backing chain and find the appropriately named bitmaps and
|
||
|
delete them, no complex lookups or merging.
|
||
|
|
||
|
Note that compared to other functions this deletes the bitmap in all
|
||
|
layers compared to others where we expect only exactly 1 bitmap of a
|
||
|
name in the backing chain to prevent potential problems.
|
||
|
|
||
|
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
|
||
|
Reviewed-by: Eric Blake <eblake@redhat.com>
|
||
|
(cherry picked from commit 4c33c5568c3ee3c5952e5f3b217d78df95683b71)
|
||
|
https://bugzilla.redhat.com/show_bug.cgi?id=1804593
|
||
|
Message-Id: <aa99845392d5d5141672bf8d50655034e6783794.1592906423.git.pkrempa@redhat.com>
|
||
|
Reviewed-by: Ján Tomko <jtomko@redhat.com>
|
||
|
---
|
||
|
src/qemu/qemu_checkpoint.c | 153 ++++++-------------------------------
|
||
|
src/qemu/qemu_checkpoint.h | 1 -
|
||
|
tests/qemublocktest.c | 7 +-
|
||
|
3 files changed, 25 insertions(+), 136 deletions(-)
|
||
|
|
||
|
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
|
||
|
index 9762a3eb9d..b44ff05c32 100644
|
||
|
--- a/src/qemu/qemu_checkpoint.c
|
||
|
+++ b/src/qemu/qemu_checkpoint.c
|
||
|
@@ -105,140 +105,41 @@ qemuCheckpointWriteMetadata(virDomainObjPtr vm,
|
||
|
}
|
||
|
|
||
|
|
||
|
-/**
|
||
|
- * qemuCheckpointFindActiveDiskInParent:
|
||
|
- * @vm: domain object
|
||
|
- * @from: starting moment object
|
||
|
- * @diskname: name (target) of the disk to find
|
||
|
- *
|
||
|
- * Find the first checkpoint starting from @from continuing through parents
|
||
|
- * of the checkpoint which describes disk @diskname. Return the pointer to the
|
||
|
- * definition of the disk.
|
||
|
- */
|
||
|
-static virDomainCheckpointDiskDef *
|
||
|
-qemuCheckpointFindActiveDiskInParent(virDomainObjPtr vm,
|
||
|
- virDomainMomentObjPtr from,
|
||
|
- const char *diskname)
|
||
|
-{
|
||
|
- virDomainMomentObjPtr parent = from;
|
||
|
- virDomainCheckpointDefPtr parentdef = NULL;
|
||
|
- size_t i;
|
||
|
-
|
||
|
- while (parent) {
|
||
|
- parentdef = virDomainCheckpointObjGetDef(parent);
|
||
|
-
|
||
|
- for (i = 0; i < parentdef->ndisks; i++) {
|
||
|
- virDomainCheckpointDiskDef *chkdisk = &parentdef->disks[i];
|
||
|
-
|
||
|
- if (STRNEQ(chkdisk->name, diskname))
|
||
|
- continue;
|
||
|
-
|
||
|
- /* currently inspected checkpoint doesn't describe the disk,
|
||
|
- * continue into parent checkpoint */
|
||
|
- if (chkdisk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
|
||
|
- break;
|
||
|
-
|
||
|
- return chkdisk;
|
||
|
- }
|
||
|
-
|
||
|
- parent = virDomainCheckpointFindByName(vm->checkpoints,
|
||
|
- parentdef->parent.parent_name);
|
||
|
- }
|
||
|
-
|
||
|
- return NULL;
|
||
|
-}
|
||
|
-
|
||
|
-
|
||
|
int
|
||
|
qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
|
||
|
virHashTablePtr blockNamedNodeData,
|
||
|
const char *delbitmap,
|
||
|
- const char *parentbitmap,
|
||
|
virJSONValuePtr actions,
|
||
|
const char *diskdst,
|
||
|
GSList **reopenimages)
|
||
|
{
|
||
|
- virStorageSourcePtr n = src;
|
||
|
+ virStorageSourcePtr n;
|
||
|
+ bool found = false;
|
||
|
|
||
|
/* find the backing chain entry with bitmap named '@delbitmap' */
|
||
|
- while (n) {
|
||
|
- qemuBlockNamedNodeDataBitmapPtr tmp;
|
||
|
-
|
||
|
- if ((tmp = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
|
||
|
- n, delbitmap))) {
|
||
|
- break;
|
||
|
- }
|
||
|
-
|
||
|
- n = n->backingStore;
|
||
|
- }
|
||
|
-
|
||
|
- if (!n) {
|
||
|
- virReportError(VIR_ERR_INTERNAL_ERROR,
|
||
|
- _("bitmap '%s' not found in backing chain of '%s'"),
|
||
|
- delbitmap, diskdst);
|
||
|
- return -1;
|
||
|
- }
|
||
|
-
|
||
|
- while (n) {
|
||
|
- qemuBlockNamedNodeDataBitmapPtr srcbitmap;
|
||
|
-
|
||
|
- if (!(srcbitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
|
||
|
- n, delbitmap)))
|
||
|
- break;
|
||
|
-
|
||
|
- /* For the actual checkpoint deletion we will merge any bitmap into the
|
||
|
- * bitmap of the parent checkpoint (@parentbitmap) or for any image
|
||
|
- * where the parent checkpoint bitmap is not present we must rename
|
||
|
- * the bitmap of the deleted checkpoint into the bitmap of the parent
|
||
|
- * checkpoint as qemu can't currently take the allocation map and turn
|
||
|
- * it into a bitmap and thus we wouldn't be able to do a backup. */
|
||
|
- if (parentbitmap) {
|
||
|
- qemuBlockNamedNodeDataBitmapPtr dstbitmap;
|
||
|
- g_autoptr(virJSONValue) arr = NULL;
|
||
|
-
|
||
|
- dstbitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
|
||
|
- n, parentbitmap);
|
||
|
-
|
||
|
- if (dstbitmap) {
|
||
|
- if (srcbitmap->recording && !dstbitmap->recording) {
|
||
|
- if (qemuMonitorTransactionBitmapEnable(actions,
|
||
|
- n->nodeformat,
|
||
|
- dstbitmap->name) < 0)
|
||
|
- return -1;
|
||
|
- }
|
||
|
-
|
||
|
- } else {
|
||
|
- if (qemuMonitorTransactionBitmapAdd(actions,
|
||
|
- n->nodeformat,
|
||
|
- parentbitmap,
|
||
|
- true,
|
||
|
- !srcbitmap->recording,
|
||
|
- srcbitmap->granularity) < 0)
|
||
|
- return -1;
|
||
|
- }
|
||
|
+ for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
|
||
|
+ qemuBlockNamedNodeDataBitmapPtr bitmapdata;
|
||
|
|
||
|
- arr = virJSONValueNewArray();
|
||
|
-
|
||
|
- if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,
|
||
|
- n->nodeformat,
|
||
|
- srcbitmap->name) < 0)
|
||
|
- return -1;
|
||
|
+ if (!(bitmapdata = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
|
||
|
+ n, delbitmap)))
|
||
|
+ continue;
|
||
|
|
||
|
- if (qemuMonitorTransactionBitmapMerge(actions,
|
||
|
- n->nodeformat,
|
||
|
- parentbitmap, &arr) < 0)
|
||
|
- return -1;
|
||
|
- }
|
||
|
+ found = true;
|
||
|
|
||
|
if (qemuMonitorTransactionBitmapRemove(actions,
|
||
|
n->nodeformat,
|
||
|
- srcbitmap->name) < 0)
|
||
|
+ bitmapdata->name) < 0)
|
||
|
return -1;
|
||
|
|
||
|
if (n != src)
|
||
|
*reopenimages = g_slist_prepend(*reopenimages, n);
|
||
|
+ }
|
||
|
|
||
|
- n = n->backingStore;
|
||
|
+ if (!found) {
|
||
|
+ virReportError(VIR_ERR_INTERNAL_ERROR,
|
||
|
+ _("bitmap '%s' not found in backing chain of '%s'"),
|
||
|
+ delbitmap, diskdst);
|
||
|
+ return -1;
|
||
|
}
|
||
|
|
||
|
return 0;
|
||
|
@@ -247,8 +148,7 @@ qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
|
||
|
|
||
|
static int
|
||
|
qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
|
||
|
- virDomainCheckpointDefPtr chkdef,
|
||
|
- virDomainMomentObjPtr parent)
|
||
|
+ virDomainCheckpointDefPtr chkdef)
|
||
|
{
|
||
|
qemuDomainObjPrivatePtr priv = vm->privateData;
|
||
|
virQEMUDriverPtr driver = priv->driver;
|
||
|
@@ -270,8 +170,6 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
|
||
|
for (i = 0; i < chkdef->ndisks; i++) {
|
||
|
virDomainCheckpointDiskDef *chkdisk = &chkdef->disks[i];
|
||
|
virDomainDiskDefPtr domdisk = virDomainDiskByTarget(vm->def, chkdisk->name);
|
||
|
- virDomainCheckpointDiskDef *parentchkdisk = NULL;
|
||
|
- const char *parentbitmap = NULL;
|
||
|
|
||
|
/* domdisk can be missing e.g. when it was unplugged */
|
||
|
if (!domdisk)
|
||
|
@@ -280,15 +178,8 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
|
||
|
if (chkdisk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
|
||
|
continue;
|
||
|
|
||
|
- /* If any ancestor checkpoint has a bitmap for the same
|
||
|
- * disk, then this bitmap must be merged to the
|
||
|
- * ancestor. */
|
||
|
- if ((parentchkdisk = qemuCheckpointFindActiveDiskInParent(vm, parent,
|
||
|
- chkdisk->name)))
|
||
|
- parentbitmap = parentchkdisk->bitmap;
|
||
|
-
|
||
|
if (qemuCheckpointDiscardDiskBitmaps(domdisk->src, blockNamedNodeData,
|
||
|
- chkdisk->bitmap, parentbitmap,
|
||
|
+ chkdisk->bitmap,
|
||
|
actions, domdisk->dst,
|
||
|
&reopenimages) < 0)
|
||
|
return -1;
|
||
|
@@ -336,7 +227,6 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver,
|
||
|
bool update_parent,
|
||
|
bool metadata_only)
|
||
|
{
|
||
|
- virDomainMomentObjPtr parent = NULL;
|
||
|
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
|
||
|
g_autofree char *chkFile = NULL;
|
||
|
bool chkcurrent = chk == virDomainCheckpointGetCurrent(vm->checkpoints);
|
||
|
@@ -352,14 +242,17 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver,
|
||
|
|
||
|
if (!metadata_only) {
|
||
|
virDomainCheckpointDefPtr chkdef = virDomainCheckpointObjGetDef(chk);
|
||
|
- parent = virDomainCheckpointFindByName(vm->checkpoints,
|
||
|
- chk->def->parent_name);
|
||
|
- if (qemuCheckpointDiscardBitmaps(vm, chkdef, parent) < 0)
|
||
|
+ if (qemuCheckpointDiscardBitmaps(vm, chkdef) < 0)
|
||
|
return -1;
|
||
|
}
|
||
|
|
||
|
if (chkcurrent) {
|
||
|
+ virDomainMomentObjPtr parent = NULL;
|
||
|
+
|
||
|
virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
|
||
|
+ parent = virDomainCheckpointFindByName(vm->checkpoints,
|
||
|
+ chk->def->parent_name);
|
||
|
+
|
||
|
if (update_parent && parent) {
|
||
|
virDomainCheckpointSetCurrent(vm->checkpoints, parent);
|
||
|
if (qemuCheckpointWriteMetadata(vm, parent,
|
||
|
diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h
|
||
|
index cf1e9e46cb..0d267a188c 100644
|
||
|
--- a/src/qemu/qemu_checkpoint.h
|
||
|
+++ b/src/qemu/qemu_checkpoint.h
|
||
|
@@ -76,7 +76,6 @@ int
|
||
|
qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
|
||
|
virHashTablePtr blockNamedNodeData,
|
||
|
const char *delbitmap,
|
||
|
- const char *parentbitmap,
|
||
|
virJSONValuePtr actions,
|
||
|
const char *diskdst,
|
||
|
GSList **reopenimages);
|
||
|
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
|
||
|
index 2f675d7e03..201e5df6b4 100644
|
||
|
--- a/tests/qemublocktest.c
|
||
|
+++ b/tests/qemublocktest.c
|
||
|
@@ -830,7 +830,6 @@ struct testQemuCheckpointDeleteMergeData {
|
||
|
const char *name;
|
||
|
virStorageSourcePtr chain;
|
||
|
const char *deletebitmap;
|
||
|
- const char *parentbitmap;
|
||
|
const char *nodedatafile;
|
||
|
};
|
||
|
|
||
|
@@ -864,7 +863,6 @@ testQemuCheckpointDeleteMerge(const void *opaque)
|
||
|
if (qemuCheckpointDiscardDiskBitmaps(data->chain,
|
||
|
nodedata,
|
||
|
data->deletebitmap,
|
||
|
- data->parentbitmap,
|
||
|
actions,
|
||
|
"testdisk",
|
||
|
&reopenimages) >= 0) {
|
||
|
@@ -1318,19 +1316,18 @@ mymain(void)
|
||
|
|
||
|
TEST_BACKUP_BITMAP_CALCULATE("empty", bitmapSourceChain, "a", "empty");
|
||
|
|
||
|
-#define TEST_CHECKPOINT_DELETE_MERGE(testname, delbmp, parbmp, named) \
|
||
|
+#define TEST_CHECKPOINT_DELETE_MERGE(testname, delbmp, named) \
|
||
|
do { \
|
||
|
checkpointdeletedata.name = testname; \
|
||
|
checkpointdeletedata.chain = bitmapSourceChain; \
|
||
|
checkpointdeletedata.deletebitmap = delbmp; \
|
||
|
- checkpointdeletedata.parentbitmap = parbmp; \
|
||
|
checkpointdeletedata.nodedatafile = named; \
|
||
|
if (virTestRun("checkpoint delete " testname, \
|
||
|
testQemuCheckpointDeleteMerge, &checkpointdeletedata) < 0) \
|
||
|
ret = -1; \
|
||
|
} while (0)
|
||
|
|
||
|
- TEST_CHECKPOINT_DELETE_MERGE("empty", "a", NULL, "empty");
|
||
|
+ TEST_CHECKPOINT_DELETE_MERGE("empty", "a", "empty");
|
||
|
|
||
|
#define TEST_BITMAP_VALIDATE(testname, bitmap, rc) \
|
||
|
do { \
|
||
|
--
|
||
|
2.27.0
|
||
|
|