[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