[PATCH 02/32] qemu: backup: Fix backup of disk skipped in an intermediate checkpoint

Eric Blake eblake at redhat.com
Wed Jun 17 18:21:04 UTC 2020


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 at 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




More information about the libvir-list mailing list