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_block.h | 3 +
src/qemu/qemu_conf.h | 2 +
src/qemu/qemu_domain.h | 15 +
src/qemu/qemu_block.c | 12 +
src/qemu/qemu_conf.c | 5 +
src/qemu/qemu_domain.c | 133 ++++++++
src/qemu/qemu_driver.c | 675 +++++++++++++++++++++++++++++++++++++++++
7 files changed, 845 insertions(+)
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index 934a1f125d..768a7cfc2e 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -159,3 +159,6 @@ qemuBlockSnapshotAddLegacy(virJSONValuePtr actions,
virDomainDiskDefPtr disk,
virStorageSourcePtr newsrc,
bool reuse);
+
+const char *
+qemuBlockNodeLookup(virDomainObjPtr vm, const char *disk);
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index e51514a344..da0735775c 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -28,6 +28,7 @@
#include "capabilities.h"
#include "network_conf.h"
#include "domain_conf.h"
+#include "checkpoint_conf.h"
#include "snapshot_conf.h"
#include "domain_event.h"
#include "virthread.h"
@@ -101,6 +102,7 @@ struct _virQEMUDriverConfig {
char *cacheDir;
char *saveDir;
char *snapshotDir;
+ char *checkpointDir;
char *channelTargetDir;
char *nvramDir;
char *swtpmStorageDir;
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 3eea8b0f96..b1ec4af82f 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -748,6 +748,21 @@ int qemuDomainMomentDiscardAll(void *payload,
int qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver,
virDomainObjPtr vm);
+int qemuDomainCheckpointWriteMetadata(virDomainObjPtr vm,
+ virDomainMomentObjPtr checkpoint,
+ virCapsPtr caps,
+ virDomainXMLOptionPtr xmlopt,
+ const char *checkpointDir);
+
+int qemuDomainCheckpointDiscard(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainMomentObjPtr chk,
+ bool update_current,
+ bool metadata_only);
+
+int qemuDomainCheckpointDiscardAllMetadata(virQEMUDriverPtr driver,
+ virDomainObjPtr vm);
+
void qemuDomainRemoveInactive(virQEMUDriverPtr driver,
virDomainObjPtr vm);
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 0a6522577d..5f5e330479 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1821,3 +1821,15 @@ qemuBlockStorageGetCopyOnReadProps(virDomainDiskDefPtr disk)
return ret;
}
+
+const char *
+qemuBlockNodeLookup(virDomainObjPtr vm, const char *disk)
+{
+ size_t i;
+
+ for (i = 0; i < vm->def->ndisks; i++) {
+ if (STREQ(vm->def->disks[i]->dst, disk))
+ return vm->def->disks[i]->src->nodeformat;
+ }
+ return NULL;
+}
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e0195dac29..7a02ea5b07 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -160,6 +160,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
goto error;
if (virAsprintf(&cfg->snapshotDir, "%s/snapshot", cfg->libDir) < 0)
goto error;
+ if (virAsprintf(&cfg->checkpointDir, "%s/checkpoint", cfg->libDir) < 0)
+ goto error;
if (virAsprintf(&cfg->autoDumpPath, "%s/dump", cfg->libDir) < 0)
goto error;
if (virAsprintf(&cfg->channelTargetDir,
@@ -223,6 +225,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
goto error;
if (virAsprintf(&cfg->snapshotDir, "%s/qemu/snapshot", cfg->configBaseDir) < 0)
goto error;
+ if (virAsprintf(&cfg->checkpointDir, "%s/qemu/checkpoint", cfg->configBaseDir) < 0)
+ goto error;
if (virAsprintf(&cfg->autoDumpPath, "%s/qemu/dump", cfg->configBaseDir) < 0)
goto error;
if (virAsprintf(&cfg->channelTargetDir,
@@ -335,6 +339,7 @@ static void virQEMUDriverConfigDispose(void *obj)
VIR_FREE(cfg->cacheDir);
VIR_FREE(cfg->saveDir);
VIR_FREE(cfg->snapshotDir);
+ VIR_FREE(cfg->checkpointDir);
VIR_FREE(cfg->channelTargetDir);
VIR_FREE(cfg->nvramDir);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6225ac23e2..03a143a228 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -55,6 +55,7 @@
#include "logging/log_manager.h"
#include "locking/domain_lock.h"
#include "virdomainsnapshotobjlist.h"
+#include "virdomaincheckpointobjlist.h"
#ifdef MAJOR_IN_MKDEV
# include <sys/mkdev.h>
@@ -8719,6 +8720,39 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm,
return ret;
}
+int
+qemuDomainCheckpointWriteMetadata(virDomainObjPtr vm,
+ virDomainMomentObjPtr checkpoint,
+ virCapsPtr caps,
+ virDomainXMLOptionPtr xmlopt,
+ const char *checkpointDir)
+{
+ unsigned int flags = VIR_DOMAIN_CHECKPOINT_FORMAT_SECURE;
+ virDomainCheckpointDefPtr def = virDomainCheckpointObjGetDef(checkpoint);
+ VIR_AUTOFREE(char *) newxml = NULL;
+ VIR_AUTOFREE(char *) chkDir = NULL;
+ VIR_AUTOFREE(char *) chkFile = NULL;
+
+ if (virDomainCheckpointGetCurrent(vm->checkpoints) == checkpoint)
+ flags |= VIR_DOMAIN_CHECKPOINT_FORMAT_CURRENT;
+ newxml = virDomainCheckpointDefFormat(def, caps, xmlopt, flags);
+ if (newxml == NULL)
+ return -1;
+
+ if (virAsprintf(&chkDir, "%s/%s", checkpointDir, vm->def->name) < 0)
+ return -1;
+ if (virFileMakePath(chkDir) < 0) {
+ virReportSystemError(errno, _("cannot create checkpoint directory '%s'"),
+ chkDir);
+ return -1;
+ }
+
+ if (virAsprintf(&chkFile, "%s/%s.xml", chkDir, def->parent.name) < 0)
+ return -1;
+
+ return virXMLSaveFile(chkFile, NULL, "checkpoint-edit", newxml);
+}
+
/* The domain is expected to be locked and inactive. Return -1 on normal
* failure, 1 if we skipped a disk due to try_all. */
static int
@@ -8915,12 +8949,100 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver,
}
+/* 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));
+ }
+ }
+
+ 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;
+}
+
+int
+qemuDomainCheckpointDiscardAllMetadata(virQEMUDriverPtr driver,
+ virDomainObjPtr vm)
+{
+ virQEMUMomentRemove rem = {
+ .driver = driver,
+ .vm = vm,
+ .metadata_only = true,
+ .momentDiscard = qemuDomainCheckpointDiscard,
+ };
+
+ virDomainCheckpointForEach(vm->checkpoints, qemuDomainMomentDiscardAll,
+ &rem);
+ virDomainCheckpointObjListRemoveAll(vm->checkpoints);
+
+ return rem.err;
+}
+
+
static void
qemuDomainRemoveInactiveCommon(virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
virQEMUDriverConfigPtr cfg;
VIR_AUTOFREE(char *) snapDir = NULL;
+ VIR_AUTOFREE(char *) chkDir = NULL;
cfg = virQEMUDriverGetConfig(driver);
@@ -8935,6 +9057,17 @@ qemuDomainRemoveInactiveCommon(virQEMUDriverPtr driver,
} else if (rmdir(snapDir) < 0 && errno != ENOENT) {
VIR_WARN("unable to remove snapshot directory %s", snapDir);
}
+ /* Remove any checkpoint metadata prior to removing the domain */
+ if (qemuDomainCheckpointDiscardAllMetadata(driver, vm) < 0) {
+ VIR_WARN("unable to remove all checkpoints for domain %s",
+ vm->def->name);
+ } else if (virAsprintf(&chkDir, "%s/%s", cfg->checkpointDir,
+ vm->def->name) < 0) {
+ VIR_WARN("unable to remove checkpoint directory %s/%s",
+ cfg->checkpointDir, vm->def->name);
+ } else if (rmdir(chkDir) < 0 && errno != ENOENT) {
+ VIR_WARN("unable to remove checkpoint directory %s", chkDir);
+ }
qemuExtDevicesCleanupHost(driver, vm->def);
virObjectUnref(cfg);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 97f3d7f786..702a715902 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -102,6 +102,7 @@
#include "virqemu.h"
#include "virdomainsnapshotobjlist.h"
#include "virenum.h"
+#include "virdomaincheckpointobjlist.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -221,6 +222,40 @@ qemuSnapObjFromSnapshot(virDomainObjPtr vm,
return qemuSnapObjFromName(vm, snapshot->name);
}
+/* Looks up the domain object from checkpoint and unlocks the
+ * driver. The returned domain object is locked and ref'd and the
+ * caller must call virDomainObjEndAPI() on it. */
+static virDomainObjPtr
+qemuDomObjFromCheckpoint(virDomainCheckpointPtr checkpoint)
+{
+ return qemuDomObjFromDomain(checkpoint->domain);
+}
+
+
+/* Looks up checkpoint object from VM and name */
+static virDomainMomentObjPtr
+qemuCheckObjFromName(virDomainObjPtr vm,
+ const char *name)
+{
+ virDomainMomentObjPtr chk = NULL;
+ chk = virDomainCheckpointFindByName(vm->checkpoints, name);
+ if (!chk)
+ virReportError(VIR_ERR_NO_DOMAIN_CHECKPOINT,
+ _("no domain checkpoint with matching name '%s'"),
+ name);
+
+ return chk;
+}
+
+
+/* Looks up checkpoint object from VM and checkpointPtr */
+static virDomainMomentObjPtr
+qemuCheckObjFromCheckpoint(virDomainObjPtr vm,
+ virDomainCheckpointPtr checkpoint)
+{
+ return qemuCheckObjFromName(vm, checkpoint->name);
+}
+
static int
qemuAutostartDomain(virDomainObjPtr vm,
void *opaque)
@@ -524,6 +559,123 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
}
+static int
+qemuDomainCheckpointLoad(virDomainObjPtr vm,
+ void *data)
+{
+ char *baseDir = (char *)data;
+ char *chkDir = NULL;
+ DIR *dir = NULL;
+ struct dirent *entry;
+ char *xmlStr;
+ char *fullpath;
+ virDomainCheckpointDefPtr def = NULL;
+ virDomainMomentObjPtr chk = NULL;
+ virDomainMomentObjPtr current = NULL;
+ bool cur;
+ unsigned int flags = VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE;
+ int ret = -1;
+ virCapsPtr caps = NULL;
+ int direrr;
+
+ virObjectLock(vm);
+ if (virAsprintf(&chkDir, "%s/%s", baseDir, vm->def->name) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to allocate memory for "
+ "checkpoint directory for domain %s"),
+ vm->def->name);
+ goto cleanup;
+ }
+
+ if (!(caps = virQEMUDriverGetCapabilities(qemu_driver, false)))
+ goto cleanup;
+
+ VIR_INFO("Scanning for checkpoints for domain %s in %s", vm->def->name,
+ chkDir);
+
+ if (virDirOpenIfExists(&dir, chkDir) <= 0)
+ goto cleanup;
+
+ while ((direrr = virDirRead(dir, &entry, NULL)) > 0) {
+ /* NB: ignoring errors, so one malformed config doesn't
+ kill the whole process */
+ VIR_INFO("Loading checkpoint file '%s'", entry->d_name);
+
+ if (virAsprintf(&fullpath, "%s/%s", chkDir, entry->d_name) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to allocate memory for path"));
+ continue;
+ }
+
+ if (virFileReadAll(fullpath, 1024*1024*1, &xmlStr) < 0) {
+ /* Nothing we can do here, skip this one */
+ virReportSystemError(errno,
+ _("Failed to read checkpoint file %s"),
+ fullpath);
+ VIR_FREE(fullpath);
+ continue;
+ }
+
+ def = virDomainCheckpointDefParseString(xmlStr, caps,
+ qemu_driver->xmlopt, &cur,
+ flags);
+ if (!def || virDomainCheckpointAlignDisks(def) < 0) {
+ /* Nothing we can do here, skip this one */
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to parse checkpoint XML from file '%s'"),
+ fullpath);
+ VIR_FREE(fullpath);
+ VIR_FREE(xmlStr);
+ continue;
+ }
+
+ chk = virDomainCheckpointAssignDef(vm->checkpoints, def);
+ if (chk == NULL) {
+ virObjectUnref(def);
+ } else if (cur) {
+ if (current)
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Too many checkpoints claiming to be current for domain %s"),
+ vm->def->name);
+ current = chk;
+ }
+
+ VIR_FREE(fullpath);
+ VIR_FREE(xmlStr);
+ }
+ if (direrr < 0)
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to fully read directory %s"),
+ chkDir);
+
+ virDomainCheckpointSetCurrent(vm->checkpoints, current);
+
+ if (virDomainCheckpointUpdateRelations(vm->checkpoints) < 0)
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Checkpoints have inconsistent relations for domain %s"),
+ vm->def->name);
+
+ /* FIXME: qemu keeps internal track of bitmaps, which form the
+ * basis for checkpoints; it would be nice if we could update our
+ * internal state to reflect that information automatically. But
+ * qemu 3.0 did not have access to this via qemu-img for offline
+ * images (you have to use QMP commands on a running guest), and
+ * it also does not track <parent> relations which we find
+ * important in our metadata.
+ */
+
+ virResetLastError();
+
+ ret = 0;
+ cleanup:
+ VIR_DIR_CLOSE(dir);
+ VIR_FREE(chkDir);
+ virObjectUnref(caps);
+ virObjectUnlock(vm);
+ return ret;
+}
+
+
static int
qemuDomainNetsRestart(virDomainObjPtr vm,
void *data ATTRIBUTE_UNUSED)
@@ -652,6 +804,11 @@ qemuStateInitialize(bool privileged,
cfg->snapshotDir);
goto error;
}
+ if (virFileMakePath(cfg->checkpointDir) < 0) {
+ virReportSystemError(errno, _("Failed to create checkpoint dir %s"),
+ cfg->checkpointDir);
+ goto error;
+ }
if (virFileMakePath(cfg->autoDumpPath) < 0) {
virReportSystemError(errno, _("Failed to create dump dir %s"),
cfg->autoDumpPath);
@@ -759,6 +916,13 @@ qemuStateInitialize(bool privileged,
(int)cfg->group);
goto error;
}
+ if (chown(cfg->checkpointDir, cfg->user, cfg->group) < 0) {
+ virReportSystemError(errno,
+ _("unable to set ownership of '%s' to %d:%d"),
+ cfg->checkpointDir, (int)cfg->user,
+ (int)cfg->group);
+ goto error;
+ }
if (chown(cfg->autoDumpPath, cfg->user, cfg->group) < 0) {
virReportSystemError(errno,
_("unable to set ownership of '%s' to %d:%d"),
@@ -892,6 +1056,10 @@ qemuStateInitialize(bool privileged,
qemuDomainSnapshotLoad,
cfg->snapshotDir);
+ virDomainObjListForEach(qemu_driver->domains,
+ qemuDomainCheckpointLoad,
+ cfg->checkpointDir);
+
virDomainObjListForEach(qemu_driver->domains,
qemuDomainManagedSaveLoad,
qemu_driver);
@@ -7710,6 +7878,7 @@ qemuDomainUndefineFlags(virDomainPtr dom,
if (qemuDomainSnapshotDiscardAllMetadata(driver, vm) < 0)
goto endjob;
}
+ /* TODO: Restrict deletion if checkpoints exist? */
name = qemuDomainManagedSavePath(driver, vm);
if (name == NULL)
@@ -15527,6 +15696,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
if (!(vm = qemuDomObjFromDomain(domain)))
goto cleanup;
+ if (virDomainListCheckpoints(vm->checkpoints, NULL, domain, NULL, 0) > 0) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("cannot create snapshot while checkpoint exists"));
+ goto cleanup;
+ }
+
cfg = virQEMUDriverGetConfig(driver);
if (virDomainSnapshotCreateXMLEnsureACL(domain->conn, vm->def, flags) < 0)
@@ -16728,6 +16903,492 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
return ret;
}
+
+/* Called prior to job lock */
+static virDomainCheckpointDefPtr
+qemuDomainCheckpointDefParseString(virQEMUDriverPtr driver, virCapsPtr caps,
+ const char *xmlDesc, bool *current,
+ unsigned int flags)
+{
+ virDomainCheckpointDefPtr ret = NULL;
+ unsigned int parse_flags = 0;
+ VIR_AUTOUNREF(virDomainCheckpointDefPtr) def = NULL;
+
+ if (flags & VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE)
+ parse_flags |= VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE;
+ if (!(def = virDomainCheckpointDefParseString(xmlDesc, caps, driver->xmlopt,
+ current, parse_flags)))
+ return NULL;
+
+ /* reject checkpoint names containing slashes or starting with dot as
+ * checkpoint definitions are saved in files named by the checkpoint name */
+ if (!(flags & VIR_DOMAIN_CHECKPOINT_CREATE_NO_METADATA)) {
+ if (strchr(def->parent.name, '/')) {
+ virReportError(VIR_ERR_XML_DETAIL,
+ _("invalid checkpoint name '%s': "
+ "name can't contain '/'"),
+ def->parent.name);
+ return NULL;
+ }
+
+ if (def->parent.name[0] == '.') {
+ virReportError(VIR_ERR_XML_DETAIL,
+ _("invalid checkpoint name '%s': "
+ "name can't start with '.'"),
+ def->parent.name);
+ return NULL;
+ }
+ }
+
+ VIR_STEAL_PTR(ret, def);
+ return ret;
+}
+
+
+/* Called inside job lock */
+static int
+qemuDomainCheckpointPrepare(virQEMUDriverPtr driver, virCapsPtr caps,
+ virDomainObjPtr vm,
+ virDomainCheckpointDefPtr def)
+{
+ int ret = -1;
+ size_t i;
+ char *xml = NULL;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ /* Easiest way to clone inactive portion of vm->def is via
+ * conversion in and back out of xml. */
+ if (!(xml = qemuDomainDefFormatLive(driver, vm->def, priv->origCPU,
+ true, true)) ||
+ !(def->parent.dom = virDomainDefParseString(xml, caps, driver->xmlopt, NULL,
+ VIR_DOMAIN_DEF_PARSE_INACTIVE |
+ VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
+ goto cleanup;
+
+ if (virDomainCheckpointAlignDisks(def) < 0 ||
+ qemuBlockNodeNamesDetect(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
+ goto cleanup;
+
+ for (i = 0; i < def->ndisks; i++) {
+ virDomainCheckpointDiskDefPtr disk = &def->disks[i];
+
+ if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+ continue;
+
+ if (vm->def->disks[i]->src->format > 0 &&
+ vm->def->disks[i]->src->format != VIR_STORAGE_FILE_QCOW2) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("checkpoint for disk %s unsupported "
+ "for storage type %s"),
+ disk->name,
+ virStorageFileFormatTypeToString(
+ vm->def->disks[i]->src->format));
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(xml);
+ return ret;
+}
+
+
+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;
+ virDomainMomentObjPtr current = NULL;
+ bool update_current = true;
+ bool *set_current = NULL;
+ bool redefine = flags & VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE;
+ virDomainMomentObjPtr other = NULL;
+ virQEMUDriverConfigPtr cfg = NULL;
+ virCapsPtr caps = NULL;
+ VIR_AUTOUNREF(virDomainCheckpointDefPtr) def = NULL;
+
+ virCheckFlags(VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE |
+ VIR_DOMAIN_CHECKPOINT_CREATE_NO_METADATA, NULL);
+ /* TODO: VIR_DOMAIN_CHECKPOINT_CREATE_QUIESCE */
+ /* TODO: Defer NO_METADATA */
+
+ if (redefine)
+ set_current = &update_current;
+ if (redefine || (flags & VIR_DOMAIN_CHECKPOINT_CREATE_NO_METADATA))
+ 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 = qemuDomainCheckpointDefParseString(driver, caps, xmlDesc,
+ set_current, flags)))
+ goto cleanup;
+
+ /* 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;
+ }
+
+ current = virDomainCheckpointGetCurrent(vm->checkpoints);
+ if (current) {
+ if (!redefine &&
+ VIR_STRDUP(chk->def->parent_name, current->def->name) < 0)
+ goto endjob;
+ if (update_current) {
+ virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
+ if (qemuDomainCheckpointWriteMetadata(vm, current,
+ 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 fail after this point, there's not a whole lot we can do;
+ * we've successfully created the checkpoint, so we have to go
+ * forward the best we can.
+ */
+ checkpoint = virGetDomainCheckpoint(domain, chk->def->name);
+
+ endjob:
+ if (checkpoint && !(flags & VIR_DOMAIN_CHECKPOINT_CREATE_NO_METADATA)) {
+ if (update_current)
+ virDomainCheckpointSetCurrent(vm->checkpoints, chk);
+ if (qemuDomainCheckpointWriteMetadata(vm, chk, driver->caps,
+ driver->xmlopt,
+ cfg->checkpointDir) < 0) {
+ /* if writing of metadata fails, error out rather than trying
+ * to silently carry on without completing the checkpoint */
+ virObjectUnref(checkpoint);
+ checkpoint = NULL;
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unable to save metadata for checkpoint %s"),
+ chk->def->name);
+ virDomainCheckpointObjListRemove(vm->checkpoints, chk);
+ } else {
+ other = virDomainCheckpointFindByName(vm->checkpoints,
+ chk->def->parent_name);
+ virDomainMomentSetParent(chk, other);
+ }
+ } else if (chk) {
+ virDomainCheckpointObjListRemove(vm->checkpoints, chk);
+ }
+
+ qemuDomainObjEndJob(driver, vm);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ VIR_FREE(xml);
+ virObjectUnref(caps);
+ virObjectUnref(cfg);
+ return checkpoint;
+}
+
+
+static int
+qemuDomainListAllCheckpoints(virDomainPtr domain,
+ virDomainCheckpointPtr **chks,
+ unsigned int flags)
+{
+ virDomainObjPtr vm = NULL;
+ int n = -1;
+
+ virCheckFlags(VIR_DOMAIN_CHECKPOINT_LIST_ROOTS |
+ VIR_DOMAIN_CHECKPOINT_LIST_TOPOLOGICAL |
+ VIR_DOMAIN_CHECKPOINT_FILTERS_ALL, -1);
+
+ if (!(vm = qemuDomObjFromDomain(domain)))
+ return -1;
+
+ if (virDomainListAllCheckpointsEnsureACL(domain->conn, vm->def) < 0)
+ goto cleanup;
+
+ n = virDomainListCheckpoints(vm->checkpoints, NULL, domain, chks, flags);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return n;
+}
+
+
+static int
+qemuDomainCheckpointListAllChildren(virDomainCheckpointPtr checkpoint,
+ virDomainCheckpointPtr **chks,
+ unsigned int flags)
+{
+ virDomainObjPtr vm = NULL;
+ virDomainMomentObjPtr chk = NULL;
+ int n = -1;
+
+ virCheckFlags(VIR_DOMAIN_CHECKPOINT_LIST_DESCENDANTS |
+ VIR_DOMAIN_CHECKPOINT_LIST_TOPOLOGICAL |
+ VIR_DOMAIN_CHECKPOINT_FILTERS_ALL, -1);
+
+ if (!(vm = qemuDomObjFromCheckpoint(checkpoint)))
+ return -1;
+
+ if (virDomainCheckpointListAllChildrenEnsureACL(checkpoint->domain->conn,
+ vm->def) < 0)
+ goto cleanup;
+
+ if (!(chk = qemuCheckObjFromCheckpoint(vm, checkpoint)))
+ goto cleanup;
+
+ n = virDomainListCheckpoints(vm->checkpoints, chk, checkpoint->domain,
+ chks, flags);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return n;
+}
+
+
+static virDomainCheckpointPtr
+qemuDomainCheckpointLookupByName(virDomainPtr domain,
+ const char *name,
+ unsigned int flags)
+{
+ virDomainObjPtr vm;
+ virDomainMomentObjPtr chk = NULL;
+ virDomainCheckpointPtr checkpoint = NULL;
+
+ virCheckFlags(0, NULL);
+
+ if (!(vm = qemuDomObjFromDomain(domain)))
+ return NULL;
+
+ if (virDomainCheckpointLookupByNameEnsureACL(domain->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (!(chk = qemuCheckObjFromName(vm, name)))
+ goto cleanup;
+
+ checkpoint = virGetDomainCheckpoint(domain, chk->def->name);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return checkpoint;
+}
+
+
+static virDomainCheckpointPtr
+qemuDomainCheckpointGetParent(virDomainCheckpointPtr checkpoint,
+ unsigned int flags)
+{
+ virDomainObjPtr vm;
+ virDomainMomentObjPtr chk = NULL;
+ virDomainCheckpointPtr parent = NULL;
+
+ virCheckFlags(0, NULL);
+
+ if (!(vm = qemuDomObjFromCheckpoint(checkpoint)))
+ return NULL;
+
+ if (virDomainCheckpointGetParentEnsureACL(checkpoint->domain->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (!(chk = qemuCheckObjFromCheckpoint(vm, checkpoint)))
+ goto cleanup;
+
+ if (!chk->def->parent_name) {
+ virReportError(VIR_ERR_NO_DOMAIN_CHECKPOINT,
+ _("checkpoint '%s' does not have a parent"),
+ chk->def->name);
+ goto cleanup;
+ }
+
+ parent = virGetDomainCheckpoint(checkpoint->domain, chk->def->parent_name);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return parent;
+}
+
+
+static char *
+qemuDomainCheckpointGetXMLDesc(virDomainCheckpointPtr checkpoint,
+ unsigned int flags)
+{
+ virQEMUDriverPtr driver = checkpoint->domain->conn->privateData;
+ virDomainObjPtr vm = NULL;
+ char *xml = NULL;
+ virDomainMomentObjPtr chk = NULL;
+ virDomainCheckpointDefPtr chkdef;
+ unsigned int format_flags;
+
+ virCheckFlags(VIR_DOMAIN_CHECKPOINT_XML_SECURE |
+ VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN, NULL);
+
+ if (!(vm = qemuDomObjFromCheckpoint(checkpoint)))
+ return NULL;
+
+ if (virDomainCheckpointGetXMLDescEnsureACL(checkpoint->domain->conn, vm->def, flags) < 0)
+ goto cleanup;
+
+ if (!(chk = qemuCheckObjFromCheckpoint(vm, checkpoint)))
+ goto cleanup;
+ chkdef = virDomainCheckpointObjGetDef(chk);
+
+ format_flags = virDomainCheckpointFormatConvertXMLFlags(flags);
+ if (chk == virDomainCheckpointGetCurrent(vm->checkpoints))
+ format_flags |= VIR_DOMAIN_CHECKPOINT_FORMAT_CURRENT;
+ xml = virDomainCheckpointDefFormat(chkdef, driver->caps, driver->xmlopt,
+ format_flags);
+
+ if (flags & VIR_DOMAIN_CHECKPOINT_XML_SIZE)
+ qemuDomainObjEndJob(driver, vm);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return xml;
+}
+
+
+static int
+qemuDomainCheckpointDelete(virDomainCheckpointPtr checkpoint,
+ unsigned int flags)
+{
+ virQEMUDriverPtr driver = checkpoint->domain->conn->privateData;
+ virDomainObjPtr vm = NULL;
+ int ret = -1;
+ virDomainMomentObjPtr chk = NULL;
+ virQEMUMomentRemove rem;
+ virQEMUMomentReparent rep;
+ bool metadata_only = !!(flags & VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY);
+ virQEMUDriverConfigPtr cfg = NULL;
+
+ virCheckFlags(VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN |
+ VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY |
+ VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY, -1);
+
+ if (!(vm = qemuDomObjFromCheckpoint(checkpoint)))
+ return -1;
+
+ cfg = virQEMUDriverGetConfig(driver);
+
+ if (virDomainCheckpointDeleteEnsureACL(checkpoint->domain->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ goto cleanup;
+
+ if (!(chk = qemuCheckObjFromCheckpoint(vm, checkpoint)))
+ goto endjob;
+
+ if (flags & (VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN |
+ VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY)) {
+ rem.driver = driver;
+ rem.vm = vm;
+ rem.metadata_only = metadata_only;
+ rem.err = 0;
+ rem.current = virDomainCheckpointGetCurrent(vm->checkpoints);
+ rem.found = false;
+ rem.momentDiscard = qemuDomainCheckpointDiscard;
+ virDomainMomentForEachDescendant(chk, qemuDomainMomentDiscardAll,
+ &rem);
+ if (rem.err < 0)
+ goto endjob;
+ if (rem.found) {
+ virDomainCheckpointSetCurrent(vm->checkpoints, chk);
+ if (flags & VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY) {
+ if (qemuDomainCheckpointWriteMetadata(vm, chk, driver->caps,
+ driver->xmlopt,
+ cfg->checkpointDir) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to set checkpoint '%s' as current"),
+ chk->def->name);
+ virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
+ goto endjob;
+ }
+ }
+ }
+ } else if (chk->nchildren) {
+ rep.dir = cfg->checkpointDir;
+ rep.parent = chk->parent;
+ rep.vm = vm;
+ rep.err = 0;
+ rep.caps = driver->caps;
+ rep.xmlopt = driver->xmlopt;
+ rep.writeMetadata = qemuDomainCheckpointWriteMetadata;
+ virDomainMomentForEachChild(chk, qemuDomainMomentReparentChildren,
+ &rep);
+ if (rep.err < 0)
+ goto endjob;
+ virDomainMomentMoveChildren(chk, chk->parent);
+ }
+
+ if (flags & VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY) {
+ virDomainMomentDropChildren(chk);
+ ret = 0;
+ } else {
+ ret = qemuDomainCheckpointDiscard(driver, vm, chk, true, metadata_only);
+ }
+
+ endjob:
+ qemuDomainObjEndJob(driver, vm);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ virObjectUnref(cfg);
+ return ret;
+}
+
static int qemuDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd,
char **result, unsigned int flags)
{
@@ -21595,6 +22256,12 @@ static int qemuDomainRename(virDomainPtr dom,
goto endjob;
}
+ if (virDomainListCheckpoints(vm->checkpoints, NULL, dom, NULL, flags) > 0) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("cannot rename domain with checkpoints"));
+ goto endjob;
+ }
+
if (virDomainObjListRename(driver->domains, vm, new_name, flags,
qemuDomainRenameCallback, driver) < 0)
goto endjob;
@@ -22415,6 +23082,14 @@ static virHypervisorDriver qemuHypervisorDriver = {
.connectBaselineHypervisorCPU = qemuConnectBaselineHypervisorCPU, /* 4.4.0 */
.nodeGetSEVInfo = qemuNodeGetSEVInfo, /* 4.5.0 */
.domainGetLaunchSecurityInfo = qemuDomainGetLaunchSecurityInfo, /* 4.5.0 */
+ .domainCheckpointCreateXML = qemuDomainCheckpointCreateXML, /* 5.6.0 */
+ .domainCheckpointGetXMLDesc = qemuDomainCheckpointGetXMLDesc, /* 5.6.0 */
+
+ .domainListAllCheckpoints = qemuDomainListAllCheckpoints, /* 5.6.0 */
+ .domainCheckpointListAllChildren = qemuDomainCheckpointListAllChildren, /* 5.6.0 */
+ .domainCheckpointLookupByName = qemuDomainCheckpointLookupByName, /* 5.6.0 */
+ .domainCheckpointGetParent = qemuDomainCheckpointGetParent, /* 5.6.0 */
+ .domainCheckpointDelete = qemuDomainCheckpointDelete, /* 5.6.0 */
};
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Sat, Jul 06, 2019 at 22:56:11 -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_block.h | 3 +
> src/qemu/qemu_conf.h | 2 +
> src/qemu/qemu_domain.h | 15 +
> src/qemu/qemu_block.c | 12 +
> src/qemu/qemu_conf.c | 5 +
> src/qemu/qemu_domain.c | 133 ++++++++
> src/qemu/qemu_driver.c | 675 +++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 845 insertions(+)
[...]
> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> index 0a6522577d..5f5e330479 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -1821,3 +1821,15 @@ qemuBlockStorageGetCopyOnReadProps(virDomainDiskDefPtr disk)
>
> return ret;
> }
> +
> +const char *
> +qemuBlockNodeLookup(virDomainObjPtr vm, const char *disk)
This looks like it belongs more into qemu_domain.c since it primarily
looks up the disk and not the node name.
> +{
> + size_t i;
> +
> + for (i = 0; i < vm->def->ndisks; i++) {
> + if (STREQ(vm->def->disks[i]->dst, disk))
> + return vm->def->disks[i]->src->nodeformat;
> + }
> + return NULL;
> +}
[...]
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 97f3d7f786..702a715902 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
[...]
> @@ -221,6 +222,40 @@ qemuSnapObjFromSnapshot(virDomainObjPtr vm,
> return qemuSnapObjFromName(vm, snapshot->name);
> }
>
> +/* Looks up the domain object from checkpoint and unlocks the
> + * driver. The returned domain object is locked and ref'd and the
> + * caller must call virDomainObjEndAPI() on it. */
> +static virDomainObjPtr
> +qemuDomObjFromCheckpoint(virDomainCheckpointPtr checkpoint)
> +{
> + return qemuDomObjFromDomain(checkpoint->domain);
> +}
> +
> +
> +/* Looks up checkpoint object from VM and name */
> +static virDomainMomentObjPtr
> +qemuCheckObjFromName(virDomainObjPtr vm,
> + const char *name)
> +{
> + virDomainMomentObjPtr chk = NULL;
> + chk = virDomainCheckpointFindByName(vm->checkpoints, name);
> + if (!chk)
> + virReportError(VIR_ERR_NO_DOMAIN_CHECKPOINT,
> + _("no domain checkpoint with matching name '%s'"),
> + name);
> +
> + return chk;
> +}
> +
> +
> +/* Looks up checkpoint object from VM and checkpointPtr */
> +static virDomainMomentObjPtr
> +qemuCheckObjFromCheckpoint(virDomainObjPtr vm,
> + virDomainCheckpointPtr checkpoint)
Both of the function above contain 'Check' which looks more like a verb
than a noun. Could you please expand it?
> +{
> + return qemuCheckObjFromName(vm, checkpoint->name);
> +}
> +
> static int
> qemuAutostartDomain(virDomainObjPtr vm,
> void *opaque)
> @@ -524,6 +559,123 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
> }
>
>
> +static int
> +qemuDomainCheckpointLoad(virDomainObjPtr vm,
> + void *data)
> +{
> + char *baseDir = (char *)data;
> + char *chkDir = NULL;
> + DIR *dir = NULL;
> + struct dirent *entry;
> + char *xmlStr;
> + char *fullpath;
> + virDomainCheckpointDefPtr def = NULL;
> + virDomainMomentObjPtr chk = NULL;
> + virDomainMomentObjPtr current = NULL;
> + bool cur;
> + unsigned int flags = VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE;
> + int ret = -1;
> + virCapsPtr caps = NULL;
> + int direrr;
> +
> + virObjectLock(vm);
> + if (virAsprintf(&chkDir, "%s/%s", baseDir, vm->def->name) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to allocate memory for "
> + "checkpoint directory for domain %s"),
> + vm->def->name);
> + goto cleanup;
> + }
> +
> + if (!(caps = virQEMUDriverGetCapabilities(qemu_driver, false)))
> + goto cleanup;
> +
> + VIR_INFO("Scanning for checkpoints for domain %s in %s", vm->def->name,
> + chkDir);
> +
> + if (virDirOpenIfExists(&dir, chkDir) <= 0)
> + goto cleanup;
> +
> + while ((direrr = virDirRead(dir, &entry, NULL)) > 0) {
> + /* NB: ignoring errors, so one malformed config doesn't
> + kill the whole process */
> + VIR_INFO("Loading checkpoint file '%s'", entry->d_name);
> +
> + if (virAsprintf(&fullpath, "%s/%s", chkDir, entry->d_name) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Failed to allocate memory for path"));
> + continue;
> + }
> +
> + if (virFileReadAll(fullpath, 1024*1024*1, &xmlStr) < 0) {
> + /* Nothing we can do here, skip this one */
> + virReportSystemError(errno,
> + _("Failed to read checkpoint file %s"),
> + fullpath);
> + VIR_FREE(fullpath);
> + continue;
> + }
> +
> + def = virDomainCheckpointDefParseString(xmlStr, caps,
> + qemu_driver->xmlopt, &cur,
> + flags);
> + if (!def || virDomainCheckpointAlignDisks(def) < 0) {
> + /* Nothing we can do here, skip this one */
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to parse checkpoint XML from file '%s'"),
> + fullpath);
> + VIR_FREE(fullpath);
> + VIR_FREE(xmlStr);
leaks 'def'
> + continue;
> + }
> +
> + chk = virDomainCheckpointAssignDef(vm->checkpoints, def);
> + if (chk == NULL) {
> + virObjectUnref(def);
> + } else if (cur) {
> + if (current)
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Too many checkpoints claiming to be current for domain %s"),
> + vm->def->name);
> + current = chk;
> + }
> +
> + VIR_FREE(fullpath);
> + VIR_FREE(xmlStr);
> + }
> + if (direrr < 0)
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to fully read directory %s"),
> + chkDir);
> +
> + virDomainCheckpointSetCurrent(vm->checkpoints, current);
> +
> + if (virDomainCheckpointUpdateRelations(vm->checkpoints) < 0)
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Checkpoints have inconsistent relations for domain %s"),
> + vm->def->name);
> +
> + /* FIXME: qemu keeps internal track of bitmaps, which form the
> + * basis for checkpoints; it would be nice if we could update our
> + * internal state to reflect that information automatically. But
> + * qemu 3.0 did not have access to this via qemu-img for offline
> + * images (you have to use QMP commands on a running guest), and
> + * it also does not track <parent> relations which we find
> + * important in our metadata.
> + */
That would be pointless to do if qemu is not running because users can
modify the files behind our back anyways and since we don't have inotify
on all the possible files (which would be impossible) we'd have to redo
all of this anyways when starting the vm. Thus probably drop this
comment.
> +
> + virResetLastError();
> +
> + ret = 0;
> + cleanup:
> + VIR_DIR_CLOSE(dir);
> + VIR_FREE(chkDir);
> + virObjectUnref(caps);
> + virObjectUnlock(vm);
> + return ret;
> +}
> +
> +
> static int
> qemuDomainNetsRestart(virDomainObjPtr vm,
> void *data ATTRIBUTE_UNUSED)
[...]
> @@ -7710,6 +7878,7 @@ qemuDomainUndefineFlags(virDomainPtr dom,
> if (qemuDomainSnapshotDiscardAllMetadata(driver, vm) < 0)
> goto endjob;
> }
> + /* TODO: Restrict deletion if checkpoints exist? */
This should be fairly easy since you've done a similar thing in
qemuDomainRename.
> name = qemuDomainManagedSavePath(driver, vm);
> if (name == NULL)
[...]
> @@ -16728,6 +16903,492 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
> return ret;
> }
>
> +
> +/* Called prior to job lock */
> +static virDomainCheckpointDefPtr
> +qemuDomainCheckpointDefParseString(virQEMUDriverPtr driver, virCapsPtr caps,
> + const char *xmlDesc, bool *current,
> + unsigned int flags)
> +{
> + virDomainCheckpointDefPtr ret = NULL;
> + unsigned int parse_flags = 0;
> + VIR_AUTOUNREF(virDomainCheckpointDefPtr) def = NULL;
> +
> + if (flags & VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE)
> + parse_flags |= VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE;
> + if (!(def = virDomainCheckpointDefParseString(xmlDesc, caps, driver->xmlopt,
> + current, parse_flags)))
> + return NULL;
> +
> + /* reject checkpoint names containing slashes or starting with dot as
> + * checkpoint definitions are saved in files named by the checkpoint name */
> + if (!(flags & VIR_DOMAIN_CHECKPOINT_CREATE_NO_METADATA)) {
> + if (strchr(def->parent.name, '/')) {
> + virReportError(VIR_ERR_XML_DETAIL,
> + _("invalid checkpoint name '%s': "
> + "name can't contain '/'"),
> + def->parent.name);
That's the job for the validator.
> + return NULL;
> + }
> +
> + if (def->parent.name[0] == '.') {
> + virReportError(VIR_ERR_XML_DETAIL,
> + _("invalid checkpoint name '%s': "
> + "name can't start with '.'"),
> + def->parent.name);
> + return NULL;
> + }
> + }
> +
> + VIR_STEAL_PTR(ret, def);
> + return ret;
> +}
[...]
> +static int
> +qemuDomainCheckpointPrepare(virQEMUDriverPtr driver, virCapsPtr caps,
> + virDomainObjPtr vm,
> + virDomainCheckpointDefPtr def)
> +{
> + int ret = -1;
> + size_t i;
> + char *xml = NULL;
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> +
> + /* Easiest way to clone inactive portion of vm->def is via
> + * conversion in and back out of xml. */
> + if (!(xml = qemuDomainDefFormatLive(driver, vm->def, priv->origCPU,
> + true, true)) ||
> + !(def->parent.dom = virDomainDefParseString(xml, caps, driver->xmlopt, NULL,
> + VIR_DOMAIN_DEF_PARSE_INACTIVE |
> + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
> + goto cleanup;
> +
> + if (virDomainCheckpointAlignDisks(def) < 0 ||
> + qemuBlockNodeNamesDetect(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
As I've pointed out multiple times already, calling qemuBlockNodeNamesDetect
should not be necessary with -drive and MUST not be done with
QEMU_CAPS_BLOCKDEV. This will mess up the libvirt metadata about the
backing chain!!!
> + goto cleanup;
> +
> + for (i = 0; i < def->ndisks; i++) {
> + virDomainCheckpointDiskDefPtr disk = &def->disks[i];
> +
> + if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
> + continue;
> +
> + if (vm->def->disks[i]->src->format > 0 &&
> + vm->def->disks[i]->src->format != VIR_STORAGE_FILE_QCOW2) {
I'd ignore the possibility of format AUTO in this case. It should not
even be possible.
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("checkpoint for disk %s unsupported "
> + "for storage type %s"),
> + disk->name,
> + virStorageFileFormatTypeToString(
> + vm->def->disks[i]->src->format));
The rest looks good to me, but I want to see a fixed version of this
which does not call qemuBlockNodeNamesDetect needlessly before giving my
final ACK.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 7/8/19 11:33 AM, Peter Krempa wrote:
> On Sat, Jul 06, 2019 at 22:56:11 -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>
>> ---
>> +/* Looks up checkpoint object from VM and checkpointPtr */
>> +static virDomainMomentObjPtr
>> +qemuCheckObjFromCheckpoint(virDomainObjPtr vm,
>> + virDomainCheckpointPtr checkpoint)
>
> Both of the function above contain 'Check' which looks more like a verb
> than a noun. Could you please expand it?
Consider it done.
>> + if (!def || virDomainCheckpointAlignDisks(def) < 0) {
>> + /* Nothing we can do here, skip this one */
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("Failed to parse checkpoint XML from file '%s'"),
>> + fullpath);
>> + VIR_FREE(fullpath);
>> + VIR_FREE(xmlStr);
>
> leaks 'def'
>
>> + continue;
>> + }
Indeed; fixed for v10.
>> +
>> + /* reject checkpoint names containing slashes or starting with dot as
>> + * checkpoint definitions are saved in files named by the checkpoint name */
>> + if (!(flags & VIR_DOMAIN_CHECKPOINT_CREATE_NO_METADATA)) {
>> + if (strchr(def->parent.name, '/')) {
>> + virReportError(VIR_ERR_XML_DETAIL,
>> + _("invalid checkpoint name '%s': "
>> + "name can't contain '/'"),
>> + def->parent.name);
>
> That's the job for the validator.
And thanks to forced RNG validation, this whole function goes away.
>> + /* Easiest way to clone inactive portion of vm->def is via
>> + * conversion in and back out of xml. */
>> + if (!(xml = qemuDomainDefFormatLive(driver, vm->def, priv->origCPU,
>> + true, true)) ||
>> + !(def->parent.dom = virDomainDefParseString(xml, caps, driver->xmlopt, NULL,
>> + VIR_DOMAIN_DEF_PARSE_INACTIVE |
>> + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
>> + goto cleanup;
>> +
>> + if (virDomainCheckpointAlignDisks(def) < 0 ||
>> + qemuBlockNodeNamesDetect(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
>
> As I've pointed out multiple times already, calling qemuBlockNodeNamesDetect
> should not be necessary with -drive and MUST not be done with
> QEMU_CAPS_BLOCKDEV. This will mess up the libvirt metadata about the
> backing chain!!!
Well, it _was_ necessary with -drive if you had just done 'virsh start
$dom', but not necessary for libvirtd restarted with an already running
domain. I have a patch for that in v10. I suspect we may also have
problems with hot-plug or media changes (floppies and cdroms), which may
need to update node names when not CAPS_BLOCKDEV, but I didn't spend
time chasing those down so far.
>
>> + goto cleanup;
>> +
>> + for (i = 0; i < def->ndisks; i++) {
>> + virDomainCheckpointDiskDefPtr disk = &def->disks[i];
>> +
>> + if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
>> + continue;
>> +
>> + if (vm->def->disks[i]->src->format > 0 &&
>> + vm->def->disks[i]->src->format != VIR_STORAGE_FILE_QCOW2) {
>
> I'd ignore the possibility of format AUTO in this case. It should not
> even be possible.
Okay.
>
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("checkpoint for disk %s unsupported "
>> + "for storage type %s"),
>> + disk->name,
>> + virStorageFileFormatTypeToString(
>> + vm->def->disks[i]->src->format));
>
> The rest looks good to me, but I want to see a fixed version of this
> which does not call qemuBlockNodeNamesDetect needlessly before giving my
> final ACK.
v10 should be landing on the list shortly.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.