[PATCH 01/32] qemu: backup: Split up code traversing checkpoint list looking for bitmaps

Eric Blake eblake at redhat.com
Wed Jun 17 18:15:14 UTC 2020


On 6/15/20 12:09 PM, Peter Krempa wrote:
> The algorithm is getting quite complex. Split out the lookup of range of
> backing chain storage sources and bitmaps contained in them which
> correspond to one checkpoint.
> 
> Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> ---
>   src/qemu/qemu_backup.c | 111 ++++++++++++++++++++++-------------------
>   1 file changed, 61 insertions(+), 50 deletions(-)
> 
> diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
> index b317b841cd..69df6c14c6 100644
> --- a/src/qemu/qemu_backup.c
> +++ b/src/qemu/qemu_backup.c
> @@ -173,72 +173,83 @@ qemuBackupDiskDataCleanup(virDomainObjPtr vm,
>   }
> 
> 
> +static int
> +qemuBackupGetBitmapMergeRange(virStorageSourcePtr from,
> +                              const char *bitmapname,
> +                              virJSONValuePtr *actions,
> +                              virStorageSourcePtr *to,
> +                              const char *diskdst,
> +                              virHashTablePtr blockNamedNodeData)
> +{
> +    g_autoptr(virJSONValue) ret = virJSONValueNewArray();

Naming a pointer 'ret' when you will be returning an int seems awkward.

> +    virStorageSourcePtr tmpsrc = NULL;
> +    virStorageSourcePtr n;
> +    bool foundbitmap = false;
> +
> +    for (n = from; virStorageSourceIsBacking(n); n = n->backingStore) {
> +        qemuBlockNamedNodeDataBitmapPtr bitmap = NULL;
> +
> +        if (!(bitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
> +                                                             n,
> +                                                             bitmapname)))
> +            break;
> +
> +        foundbitmap = true;
> +
> +        if (bitmap->inconsistent) {
> +            virReportError(VIR_ERR_INVALID_ARG,
> +                           _("bitmap '%s' for image '%s%u' is inconsistent"),
> +                           bitmap->name, diskdst, n->id);
> +            return -1;
> +        }
> +
> +        if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(ret,
> +                                                             n->nodeformat,
> +                                                             bitmapname) < 0)
> +            return -1;
> +
> +        tmpsrc = n;
> +    }
> +
> +    if (!foundbitmap) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("failed to find bitmap '%s' in image '%s%u'"),
> +                       bitmapname, diskdst, from->id);
> +        return -1;
> +    }
> +
> +    *actions = g_steal_pointer(&ret);

Maybe s/ret/act/.

Otherwise, this looks like a sane refactoring.
Reviewed-by: Eric Blake <eblake at redhat.com>

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