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
