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@redhat.com> --- 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 :|