Re: [RFC PATCH 02/16] qemu: checkpoint: split out checkpoint deletion bitmaps
On Fri, Jan 24, 2020 at 11:13:01 -0600, Eric Blake wrote:
On 1/9/20 12:21 PM, Peter Krempa wrote:
> qemuCheckpointDiscard is a massive function that can be separated into
> smaller bits. Extract the part that actually modifies the disk from the
> metadata handling.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> src/qemu/qemu_checkpoint.c | 137 ++++++++++++++++++++-----------------
> 1 file changed, 76 insertions(+), 61 deletions(-)
>
> diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
> index d13d4c2a37..9ff3129570 100644
> --- a/src/qemu/qemu_checkpoint.c
> +++ b/src/qemu/qemu_checkpoint.c
> @@ -104,6 +104,81 @@ qemuCheckpointWriteMetadata(virDomainObjPtr vm,
> }
>
>
> +static int
> +qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
> + virDomainCheckpointDefPtr chkdef,
> + bool chkcurrent,
> + virDomainMomentObjPtr parent)
> +{
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + virQEMUDriverPtr driver = priv->driver;
> + virDomainMomentObjPtr moment;
> + virDomainCheckpointDefPtr parentdef = NULL;
> + bool search_parents;
> + int rc;
> + g_autoptr(virJSONValue) actions = NULL;
> + size_t i;
> + size_t j;
> +
> + if (!(actions = virJSONValueNewArray()))
> + return -1;
Can virJSONValueNewArray() still return failure now that we have shifted to
using g_new()?
It can't fail any more, but it internally uses VIR_ALLOC which is
declared with ATTRIBUTE_RETURN_CHECK thus we have to check the return
value. Also even if we did not, e.g. coverity heuristically assumes that
if a big number of callers check the return value and moans if some
don't.
That said. I can add a ignore_value().
But changing it (if needed) should be independent of the code motion shown
here.
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org