98 lines
4.4 KiB
Diff
98 lines
4.4 KiB
Diff
From 6c139a45537d021f6fd4287bd3c74cba06e6a264 Mon Sep 17 00:00:00 2001
|
|
Message-Id: <6c139a45537d021f6fd4287bd3c74cba06e6a264@dist-git>
|
|
From: Peter Krempa <pkrempa@redhat.com>
|
|
Date: Tue, 23 Jun 2020 12:23:47 +0200
|
|
Subject: [PATCH] qemu: checkpoint: Don't chain bitmaps for checkpoints
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
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>
|
|
(cherry picked from commit 1f92aa454aa3b04ece9f7479d517bf08ba547f0a)
|
|
https://bugzilla.redhat.com/show_bug.cgi?id=1804593
|
|
Message-Id: <a863b2015f72690ebd90fd518e8163e194909214.1592906423.git.pkrempa@redhat.com>
|
|
Reviewed-by: Ján Tomko <jtomko@redhat.com>
|
|
---
|
|
src/qemu/qemu_checkpoint.c | 21 +--------------------
|
|
1 file changed, 1 insertion(+), 20 deletions(-)
|
|
|
|
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
|
|
index 34cf122eb3..9762a3eb9d 100644
|
|
--- a/src/qemu/qemu_checkpoint.c
|
|
+++ b/src/qemu/qemu_checkpoint.c
|
|
@@ -460,7 +460,6 @@ qemuCheckpointPrepare(virQEMUDriverPtr driver,
|
|
static int
|
|
qemuCheckpointAddActions(virDomainObjPtr vm,
|
|
virJSONValuePtr actions,
|
|
- virDomainMomentObjPtr old_current,
|
|
virDomainCheckpointDefPtr def)
|
|
{
|
|
size_t i;
|
|
@@ -468,7 +467,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 */
|
|
@@ -479,23 +477,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;
|
|
}
|
|
@@ -544,7 +525,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)))
|
|
--
|
|
2.27.0
|
|
|