On 6/15/20 12:09 PM, Peter Krempa wrote:
If a disk is not captured by one of the intermediate checkpoints the
code would fail, but we can easily calculate the bitmaps to merge
correctly by skipping over checkpoints which don't describe the disk.
If I'm understanding the setup, you can hit this by having two disks, do
a checkpoint on both A+B, then another checkpoint on just B, then
another checkpoint on both disks again; requesting a differential backup
from the first checkpoint now has to realize that no bitmap was created
for A during the second checkpoint, but that's not fatal
Visually:
time-> ----C1-------------C2-------------C3-------now
A :bitmap1 :bitmap3
B :bitmap1 :bitmap2 :bitmap3
the differential from C1-now merges bitmap1+3 for disk A, and
bitmap1+2+3 for disk B.
Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
---
src/qemu/qemu_backup.c | 24 ++++++++++++++++++++++++
tests/qemublocktest.c | 6 ++++++
2 files changed, 30 insertions(+)
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 69df6c14c6..686ed57e82 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -240,6 +240,30 @@ qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental,
for (incridx = 0; incremental[incridx]; incridx++) {
g_autoptr(virJSONValue) tmp = virJSONValueNewArray();
virStorageSourcePtr tmpsrc = NULL;
+ virDomainCheckpointDefPtr chkdef = (virDomainCheckpointDefPtr) incremental[incridx];
+ bool checkpoint_has_disk = false;
+ size_t i;
+
+ for (i = 0; i < chkdef->ndisks; i++) {
+ if (STRNEQ_NULLABLE(diskdst, chkdef->disks[i].name))
+ continue;
+
+ if (chkdef->disks[i].type == VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+ checkpoint_has_disk = true;
+
+ break;
+ }
+
+ if (!checkpoint_has_disk) {
+ if (!incremental[incridx + 1]) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("disk '%s' not found in checkpoint '%s'"),
+ diskdst, incremental[incridx]->name);
+ return NULL;
+ }
+
+ continue;
+ }
if (qemuBackupGetBitmapMergeRange(n, incremental[incridx]->name,
&tmp, &tmpsrc, diskdst,
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 0cdedb9ad4..f00d2ff129 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -727,6 +727,12 @@ testQemuBackupGetIncrementalMoment(const char *name)
if (!(checkpoint = virDomainCheckpointDefNew()))
abort();
+ checkpoint->disks = g_new0(virDomainCheckpointDiskDef, 1);
+ checkpoint->ndisks = 1;
+
+ checkpoint->disks[0].name = g_strdup("testdisk");
+ checkpoint->disks[0].type = VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP;
+
I'm not quite sure how this test change matches the code change above,
but the code change looked reasonable.
checkpoint->parent.name = g_strdup(name);
return (virDomainMomentDefPtr) checkpoint;
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
