qemu: checkpoint: Don't merge checkpoints during deletion

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>
This commit is contained in:
Peter Krempa 2020-04-21 08:17:58 +02:00
parent b3845f0e3d
commit 4c33c5568c
3 changed files with 25 additions and 136 deletions

View File

@ -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;
for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
qemuBlockNamedNodeDataBitmapPtr bitmapdata;
if ((tmp = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
n, delbitmap))) {
break;
}
if (!(bitmapdata = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
n, delbitmap)))
continue;
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;
}
arr = virJSONValueNewArray();
if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,
n->nodeformat,
srcbitmap->name) < 0)
return -1;
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,

View File

@ -76,7 +76,6 @@ int
qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
virHashTablePtr blockNamedNodeData,
const char *delbitmap,
const char *parentbitmap,
virJSONValuePtr actions,
const char *diskdst,
GSList **reopenimages);

View File

@ -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 { \