shaba/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-22 17:34:18 +03:00
Code Releases Activity

qemu: checkpoint: Don't chain bitmaps for checkpoints

Browse Source 
Chaining bitmaps for checkpoints (disabling the active one and creating
a new) severely overcomplicated all operations in regards to bitmaps.

Specifically it requires us re-matching the on-disk state to the
internal metadata and in case of merging during block jobs it makes it
almost impossible to cover all corner cases.

Since the checkpoints and incremental backups were not yet enabled,
let's change the design to keep one bitmap per checkpoint. In case of
layered snapshots this will be filled in by using dirty-bitmap-populate.

Finally the main reason for this unnecessary complexity was the fear
that qemu's performance could degrade. In the end I think that
addressing the performance issue will be better done in qemu (e.g by
keeping an internal bitmap updated with changes and merging it
periodically back to the real bitmaps. QEMU writes out changes to disk
at shutdown so consistency is not a problem).

Removing the relationships between bitmaps frees us from complex
handling and also makes all the surrounding code more robust as one
broken bitmap doesn't necessarily invalidate whole chains of backups.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Peter Krempa 2020-04-17 09:18:49 +02:00
parent 69cacbe506
commit 1f92aa454a
1 changed files with 1 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/qemu/qemu_checkpoint.c
View File

@ -456,7 +456,6 @@ qemuCheckpointPrepare(virQEMUDriverPtr driver,
static int
qemuCheckpointAddActions(virDomainObjPtr vm,
virJSONValuePtr actions,
virDomainMomentObjPtr old_current,
virDomainCheckpointDefPtr def)
{
size_t i;
@ -464,7 +463,6 @@ qemuCheckpointAddActions(virDomainObjPtr vm,
for (i = 0; i < def->ndisks; i++) {
virDomainCheckpointDiskDef *chkdisk = &def->disks[i];
virDomainDiskDefPtr domdisk = virDomainDiskByTarget(vm->def, chkdisk->name);
virDomainCheckpointDiskDef *parentchkdisk = NULL;
/* checkpoint definition validator mandates that the corresponding
* domdisk should exist */
@ -475,23 +473,6 @@ qemuCheckpointAddActions(virDomainObjPtr vm,
if (qemuMonitorTransactionBitmapAdd(actions, domdisk->src->nodeformat,
chkdisk->bitmap, true, false, 0) < 0)
return -1;
/* We only want one active bitmap for a disk along the
* checkpoint chain, then later differential backups will
* merge the bitmaps (only one active) between the bounding
* checkpoint and the leaf checkpoint. If the same disks are
* involved in each checkpoint, this search terminates in one
* iteration; but it is also possible to have to search
* further than the immediate parent to find another
* checkpoint with a bitmap on the same disk. */
if ((parentchkdisk = qemuCheckpointFindActiveDiskInParent(vm, old_current,
chkdisk->name))) {
if (qemuMonitorTransactionBitmapDisable(actions,
domdisk->src->nodeformat,
parentchkdisk->bitmap) < 0)
return -1;
}
}
return 0;
}
@ -540,7 +521,7 @@ qemuCheckpointCreateCommon(virQEMUDriverPtr driver,
tmpactions = virJSONValueNewArray();
if (qemuCheckpointAddActions(vm, tmpactions, parent, *def) < 0)
if (qemuCheckpointAddActions(vm, tmpactions, *def) < 0)
return -1;
if (!(*chk = virDomainCheckpointAssignDef(vm->checkpoints, *def)))