[libvirt] [PATCH v10 18/19] backup: Wire up qemu checkpoint commands over QMP

[Daniel P. Berrangé]

On Wed, Jul 24, 2019 at 12:56:08AM -0500, Eric Blake wrote:
> Time to actually issue the QMP transactions that create and delete
> persistent checkpoints.  For create, we only need one transaction:
> inside, we visit all disks affected by the checkpoint, and create a
> new enabled bitmap, as well as disabling the bitmap of the first
> ancestor checkpoint (if any) that also had a bitmap.  For deletion, we
> need multiple calls: for each disk, if there is an ancestor checkpoint
> with a bitmap, then the bitmap must be merged (including activating
> the ancestor bitmap), all before deleting the bitmap from the
> checkpoint being removed.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/qemu/qemu_domain.c | 122 ++++++++++++++++++++++++++++-------------
>  src/qemu/qemu_driver.c |  98 ++++++++++++++++++++++++++++++++-
>  2 files changed, 180 insertions(+), 40 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index d71b7d1984..062a08ed97 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -9255,48 +9255,97 @@ qemuDomainCheckpointDiscard(virQEMUDriverPtr driver,
>                              bool update_parent,
>                              bool metadata_only)
>  {
> -    char *chkFile = NULL;
> -    int ret = -1;
> -    virDomainMomentObjPtr parentchk = NULL;
> -    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    virDomainMomentObjPtr parent = NULL;
> +    virDomainMomentObjPtr moment;
> +    virDomainCheckpointDefPtr parentdef = NULL;
> +    size_t i, j;
> +    VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
> +    VIR_AUTOFREE(char *) chkFile = NULL;
> 
> -    if (!metadata_only) {
> -        if (!virDomainObjIsActive(vm)) {
> -            virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> -                           _("cannot remove checkpoint from inactive domain"));
> -            goto cleanup;
> -        } else {
> -            /* TODO: Implement QMP sequence to merge bitmaps */
> -            // qemuDomainObjPrivatePtr priv;
> -            // priv = vm->privateData;
> -            // qemuDomainObjEnterMonitor(driver, vm);
> -            // /* we continue on even in the face of error */
> -            // qemuMonitorDeleteCheckpoint(priv->mon, chk->def->name);
> -            // ignore_value(qemuDomainObjExitMonitor(driver, vm));
> -        }
> +    if (!metadata_only && !virDomainObjIsActive(vm)) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("cannot remove checkpoint from inactive domain"));
> +        return -1;
>      }

This semi-answerss my previous question on earlier patch. I guess we
just shouldn't add the commented out lines at all in teh first place,
given you are deleting them entirely here.


> @@ -17096,7 +17166,15 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain,
>           * makes sense, such as checking that qemu-img recognizes the
>           * checkpoint bitmap name in at least one of the domain's disks?  */
>      } else {
> -        /* TODO: issue QMP transaction command */
> +        if (!(actions = virJSONValueNewArray()))
> +            goto endjob;
> +        if (qemuDomainCheckpointAddActions(vm, actions, other,
> +                                           virDomainCheckpointObjGetDef(chk)) < 0)
> +            goto endjob;
> +        qemuDomainObjEnterMonitor(driver, vm);
> +        ret = qemuMonitorTransaction(priv->mon, &actions);
> +        if (qemuDomainObjExitMonitor(driver, vm) < 0 || ret < 0)
> +            goto endjob;
>      }

Ah, and this now answers my other question.


Makes me wonder if there's really a benefit to splitting the code across
two patches, as opposed to just one. As long as it git bisect's with a
clean built though its not a show stopper.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|