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@redhat.com> --- 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