Re: [PATCH v10 16/19] backup: qemu: Implement metadata tracking for checkpoint APIs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 24, 2019 at 12:56:06AM -0500, Eric Blake wrote:
> A lot of this work heavily copies from the existing snapshot
> APIs.  The interaction with qemu during create/delete still
> needs to be implemented, but this takes care of all the libvirt
> metadata (saving and restoring XML, and tracking the relations
> between multiple checkpoints).
> 
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
>  src/qemu/qemu_domain.h |  15 +
>  src/qemu/qemu_domain.c | 131 +++++++++
>  src/qemu/qemu_driver.c | 606 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 752 insertions(+)


> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index e7f28aa2b8..44ac7eb73e 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c


> +/* Discard one checkpoint (or its metadata), without reparenting any children.  */
> +int
> +qemuDomainCheckpointDiscard(virQEMUDriverPtr driver,
> +                            virDomainObjPtr vm,
> +                            virDomainMomentObjPtr chk,
> +                            bool update_parent,
> +                            bool metadata_only)
> +{
> +    char *chkFile = NULL;
> +    int ret = -1;
> +    virDomainMomentObjPtr parentchk = NULL;
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +
> +    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));

Shouldn't we have a virReportError + goto cleamnup here until
this is actually implemented for real ?

> +        }
> +    }
> +
> +    if (virAsprintf(&chkFile, "%s/%s/%s.xml", cfg->checkpointDir,
> +                    vm->def->name, chk->def->name) < 0)
> +        goto cleanup;
> +
> +    if (chk == virDomainCheckpointGetCurrent(vm->checkpoints)) {
> +        virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
> +        if (update_parent && chk->def->parent_name) {
> +            parentchk = virDomainCheckpointFindByName(vm->checkpoints,
> +                                                      chk->def->parent_name);
> +            if (!parentchk) {
> +                VIR_WARN("missing parent checkpoint matching name '%s'",
> +                         chk->def->parent_name);
> +            } else {
> +                virDomainCheckpointSetCurrent(vm->checkpoints, parentchk);
> +                if (qemuDomainCheckpointWriteMetadata(vm, parentchk, driver->caps,
> +                                                      driver->xmlopt,
> +                                                      cfg->checkpointDir) < 0) {
> +                    VIR_WARN("failed to set parent checkpoint '%s' as current",
> +                             chk->def->parent_name);
> +                    virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
> +                }
> +            }
> +        }
> +    }
> +
> +    if (unlink(chkFile) < 0)
> +        VIR_WARN("Failed to unlink %s", chkFile);
> +    if (update_parent)
> +        virDomainMomentDropParent(chk);
> +    virDomainCheckpointObjListRemove(vm->checkpoints, chk);
> +
> +    ret = 0;
> +
> + cleanup:
> +    VIR_FREE(chkFile);
> +    virObjectUnref(cfg);
> +    return ret;
> +}

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index fae2bd3b08..a75a16492b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c




> +static virDomainCheckpointPtr
> +qemuDomainCheckpointCreateXML(virDomainPtr domain,
> +                              const char *xmlDesc,
> +                              unsigned int flags)
> +{
> +    virQEMUDriverPtr driver = domain->conn->privateData;
> +    virDomainObjPtr vm = NULL;
> +    char *xml = NULL;
> +    virDomainMomentObjPtr chk = NULL;
> +    virDomainCheckpointPtr checkpoint = NULL;
> +    bool update_current = true;
> +    bool redefine = flags & VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE;
> +    unsigned int parse_flags = 0;
> +    virDomainMomentObjPtr other = NULL;
> +    virQEMUDriverConfigPtr cfg = NULL;
> +    virCapsPtr caps = NULL;
> +    VIR_AUTOUNREF(virDomainCheckpointDefPtr) def = NULL;
> +
> +    virCheckFlags(VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE, NULL);
> +    /* TODO: VIR_DOMAIN_CHECKPOINT_CREATE_QUIESCE */
> +
> +    if (redefine) {
> +        parse_flags |= VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE;
> +        update_current = false;
> +    }
> +
> +    if (!(vm = qemuDomObjFromDomain(domain)))
> +        goto cleanup;
> +
> +    if (virDomainSnapshotObjListNum(vm->snapshots, NULL, 0) > 0) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("cannot create checkpoint while snapshot exists"));
> +        goto cleanup;
> +    }
> +
> +    cfg = virQEMUDriverGetConfig(driver);
> +
> +    if (virDomainCheckpointCreateXMLEnsureACL(domain->conn, vm->def, flags) < 0)
> +        goto cleanup;
> +
> +    if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> +        goto cleanup;
> +
> +    if (qemuProcessAutoDestroyActive(driver, vm)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("domain is marked for auto destroy"));
> +        goto cleanup;
> +    }
> +
> +    if (!virDomainObjIsActive(vm)) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("cannot create checkpoint for inactive domain"));
> +        goto cleanup;
> +    }
> +
> +    if (!(def = virDomainCheckpointDefParseString(xmlDesc, caps, driver->xmlopt,
> +                                                  parse_flags)))
> +        goto cleanup;
> +    /* Unlike snapshots, the RNG schema already ensured a sane filename. */
> +
> +    /* We are going to modify the domain below. */
> +    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> +        goto cleanup;
> +
> +    if (redefine) {
> +        if (virDomainCheckpointRedefinePrep(domain, vm, &def, &chk,
> +                                            driver->xmlopt,
> +                                            &update_current) < 0)
> +            goto endjob;
> +    } else if (qemuDomainCheckpointPrepare(driver, caps, vm, def) < 0) {
> +        goto endjob;
> +    }
> +
> +    if (!chk) {
> +        if (!(chk = virDomainCheckpointAssignDef(vm->checkpoints, def)))
> +            goto endjob;
> +
> +        def = NULL;
> +    }
> +
> +    other = virDomainCheckpointGetCurrent(vm->checkpoints);
> +    if (other) {
> +        if (!redefine &&
> +            VIR_STRDUP(chk->def->parent_name, other->def->name) < 0)
> +            goto endjob;
> +        if (update_current) {
> +            virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
> +            if (qemuDomainCheckpointWriteMetadata(vm, other,
> +                                                  driver->caps, driver->xmlopt,
> +                                                  cfg->checkpointDir) < 0)
> +                goto endjob;
> +        }
> +    }
> +
> +    /* actually do the checkpoint */
> +    if (redefine) {
> +        /* XXX Should we validate that the redefined checkpoint even
> +         * 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 we want to merge this now, is there something we should be doing in
these XXX/TODO branches ?  Is the code doing anything useful with both
of those if/else branches being no-ops ?


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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux