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@xxxxxxxxxx>
---
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 9b4f3ad396..c24d97443c 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;
@@ -268,8 +168,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)
@@ -278,15 +176,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;
@@ -334,7 +225,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);
@@ -350,14 +240,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 63112eea24..2dc06fd420 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) {
@@ -1319,19 +1317,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.26.2
