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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3226
> Virtualization: qemu.org | libvirt.org
