Rather than one file per snapshot, store all qemu snapshots in a
single file, using the recently added bulk snapshot list
operations. For now, this doesn't change how often libvirt writes a
snapshot file, but it does open the door for the next patch to update
the signature to qemuDomainSnapshotWriteMetadata() and call it less
frequently. One of the main benefits for doing a bulk write is that
you only have to do a single file system write at the end of an
operation, rather than potentially 3 during
virDomainSnapshotCreateXML(REDEFINE|CURRENT) (delete the old snapshot
definition being redefined, rewrite the previous current snapshot to
longer be current, and store the new snapshot definition) or even more
during virDomainSnapshotDelete(DESCENDENTS) (a file system hit per
snapshot being deleted). It also makes it perhaps a bit more feasible
to roll back to earlier state if something fails horribly midway
through an operation (until you write the new file, the old file is
still a reliable record of what state to roll back to), compared to
the current code which has to track lots of things locally; although I
did not attempt to play with any patches along those lines.
Another benefit of the bulk write - it's less code to maintain, and
will make it easier for me to model qemu's checkpoint storage in the
same way (and for checkpoints, I don't even have to worry about legacy
parsing).
This is a one-way upgrade - if you have snapshots created by an older
libvirt, the new libvirt will correctly load those snapshots and
convert to the new format. But as the new libvirt will no longer
output the old format, reverting back to the old libvirt will make it
appear that all snapshots have disappeared (merely hidden until you
upgrade libvirt again). But then again, we've never promised that
downgrading libvirt after an upgrade was supposed to work flawlessly.
There is a slight chance for confusion if a user named two separate
domains 'foo' and 'foo.xml'; the new scheme expects 'foo.xml' to be a
regular file for the former domain, while the old scheme expected
'foo.xml' to be a directory for the latter domain; if we are worried
about that, we could tweak the code to instead output new state in a
file named 'foo' instead of the more typical 'foo.xml' and just key
off of whether the file is regular or a directory when deciding if
this is the first libvirt run after an upgrade. But I felt that the
chances of a user abusing domain names like that is not worth the
effort.
The bulk snapshot file can be huge (over RPC, we allow <domain> to be
up to 4M, and we allow up to 16k snapshots; since each each snapshot
includes a <domain>, a worst-case domain would result in gigabytes of
bulk data); it is no worse on overall filesystem usage than before,
but now in a single file vs. a series of files it requires more memory
to read in at once. I don't know if we have to worry about that in
practice, but my patch does cap things to read in no more than an
arbitrarily-picked 128M, which we may have to raise in the future.
Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---
src/qemu/qemu_domain.c | 59 ++++++++-------------------
src/qemu/qemu_driver.c | 93 +++++++++++++++++++++++++++++++++---------
2 files changed, 91 insertions(+), 61 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ea7b31dab3..424f839a00 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8448,45 +8448,28 @@ qemuFindQemuImgBinary(virQEMUDriverPtr driver)
int
qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm,
- virDomainMomentObjPtr snapshot,
+ virDomainMomentObjPtr snapshot ATTRIBUTE_UNUSED,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
const char *snapshotDir)
{
- char *newxml = NULL;
- int ret = -1;
- char *snapDir = NULL;
- char *snapFile = NULL;
char uuidstr[VIR_UUID_STRING_BUFLEN];
- unsigned int flags = VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE |
- VIR_DOMAIN_SNAPSHOT_FORMAT_INTERNAL;
- virDomainSnapshotDefPtr def = virDomainSnapshotObjGetDef(snapshot);
+ unsigned int flags = VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE;
+ VIR_AUTOFREE(char *) newxml = NULL;
+ VIR_AUTOFREE(char *) snapDir = NULL;
+ VIR_AUTOFREE(char *) snapFile = NULL;
+ VIR_AUTOCLEAN(virBuffer) buf = VIR_BUFFER_INITIALIZER;
- if (virDomainSnapshotGetCurrent(vm->snapshots) == snapshot)
- flags |= VIR_DOMAIN_SNAPSHOT_FORMAT_CURRENT;
virUUIDFormat(vm->def->uuid, uuidstr);
- newxml = virDomainSnapshotDefFormat(uuidstr, def, caps, xmlopt, flags);
- if (newxml == NULL)
+ if (virDomainSnapshotObjListFormat(&buf, uuidstr, vm->snapshots, caps,
+ xmlopt, flags) < 0)
return -1;
- if (virAsprintf(&snapDir, "%s/%s", snapshotDir, vm->def->name) < 0)
- goto cleanup;
- if (virFileMakePath(snapDir) < 0) {
- virReportSystemError(errno, _("cannot create snapshot directory '%s'"),
- snapDir);
- goto cleanup;
- }
-
- if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, def->common.name) < 0)
- goto cleanup;
-
- ret = virXMLSaveFile(snapFile, NULL, "snapshot-edit", newxml);
+ if (virAsprintf(&snapFile, "%s/%s.xml", snapshotDir, vm->def->name) < 0)
+ return -1;
- cleanup:
- VIR_FREE(snapFile);
- VIR_FREE(snapDir);
- VIR_FREE(newxml);
- return ret;
+ newxml = virBufferContentAndReset(&buf);
+ return virXMLSaveFile(snapFile, NULL, "snapshot-edit", newxml);
}
/* The domain is expected to be locked and inactive. Return -1 on normal
@@ -8589,7 +8572,6 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver,
bool update_parent,
bool metadata_only)
{
- char *snapFile = NULL;
int ret = -1;
qemuDomainObjPrivatePtr priv;
virDomainMomentObjPtr parentsnap = NULL;
@@ -8610,10 +8592,6 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver,
}
}
- if (virAsprintf(&snapFile, "%s/%s/%s.xml", cfg->snapshotDir,
- vm->def->name, snap->def->name) < 0)
- goto cleanup;
-
if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) {
virDomainSnapshotSetCurrent(vm->snapshots, NULL);
if (update_parent && snap->def->parent) {
@@ -8635,8 +8613,6 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver,
}
}
- if (unlink(snapFile) < 0)
- VIR_WARN("Failed to unlink %s", snapFile);
if (update_parent)
virDomainMomentDropParent(snap);
virDomainSnapshotObjListRemove(vm->snapshots, snap);
@@ -8644,7 +8620,6 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver,
ret = 0;
cleanup:
- VIR_FREE(snapFile);
virObjectUnref(cfg);
return ret;
}
@@ -8691,7 +8666,7 @@ qemuDomainRemoveInactiveCommon(virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
virQEMUDriverConfigPtr cfg;
- VIR_AUTOFREE(char *) snapDir = NULL;
+ VIR_AUTOFREE(char *) snapFile = NULL;
cfg = virQEMUDriverGetConfig(driver);
@@ -8699,12 +8674,12 @@ qemuDomainRemoveInactiveCommon(virQEMUDriverPtr driver,
if (qemuDomainSnapshotDiscardAllMetadata(driver, vm) < 0) {
VIR_WARN("unable to remove all snapshots for domain %s",
vm->def->name);
- } else if (virAsprintf(&snapDir, "%s/%s", cfg->snapshotDir,
+ } else if (virAsprintf(&snapFile, "%s/%s.xml", cfg->snapshotDir,
vm->def->name) < 0) {
- VIR_WARN("unable to remove snapshot directory %s/%s",
+ VIR_WARN("unable to remove snapshots storage %s/%s.xml",
cfg->snapshotDir, vm->def->name);
- } else if (rmdir(snapDir) < 0 && errno != ENOENT) {
- VIR_WARN("unable to remove snapshot directory %s", snapDir);
+ } else if (unlink(snapFile) < 0 && errno != ENOENT) {
+ VIR_WARN("unable to remove snapshots storage %s", snapFile);
}
qemuExtDevicesCleanupHost(driver, vm->def);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9c2245b095..018d1cdc87 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -406,12 +406,14 @@ qemuSecurityInit(virQEMUDriverPtr driver)
}
+/* Older qemu used a series of $dir/snapshot/domainname/snap.xml
+ * files, instead of the modern $dir/snapshot/domainname.xml bulk
+ * file. Called while vm is locked. */
static int
-qemuDomainSnapshotLoad(virDomainObjPtr vm,
- void *data)
+qemuDomainSnapshotLoadLegacy(virDomainObjPtr vm,
+ char *snapDir,
+ virCapsPtr caps)
{
- char *baseDir = (char *)data;
- char *snapDir = NULL;
DIR *dir = NULL;
struct dirent *entry;
char *xmlStr;
@@ -424,21 +426,8 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL);
int ret = -1;
- virCapsPtr caps = NULL;
int direrr;
- virObjectLock(vm);
- if (virAsprintf(&snapDir, "%s/%s", baseDir, vm->def->name) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to allocate memory for "
- "snapshot directory for domain %s"),
- vm->def->name);
- goto cleanup;
- }
-
- if (!(caps = virQEMUDriverGetCapabilities(qemu_driver, false)))
- goto cleanup;
-
VIR_INFO("Scanning for snapshots for domain %s in %s", vm->def->name,
snapDir);
@@ -503,6 +492,74 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
_("Snapshots have inconsistent relations for domain %s"),
vm->def->name);
+ virResetLastError();
+
+ ret = 0;
+ cleanup:
+ VIR_DIR_CLOSE(dir);
+ return ret;
+}
+
+
+/* Load all snapshots associated with domain */
+static int
+qemuDomainSnapshotLoad(virDomainObjPtr vm,
+ void *data)
+{
+ char *baseDir = (char *)data;
+ unsigned int flags = (VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE |
+ VIR_DOMAIN_SNAPSHOT_PARSE_DISKS);
+ int ret = -1;
+ virCapsPtr caps = NULL;
+ VIR_AUTOFREE(char *) snapFile = NULL;
+ VIR_AUTOFREE(char *) snapDir = NULL;
+ VIR_AUTOFREE(char *) xmlStr = NULL;
+
+ virObjectLock(vm);
+ VIR_INFO("Scanning for snapshots for domain %s in %s", vm->def->name,
+ baseDir);
+ if (virAsprintf(&snapFile, "%s/%s.xml", baseDir, vm->def->name) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to allocate memory for "
+ "snapshots storage for domain %s"),
+ vm->def->name);
+ goto cleanup;
+ }
+
+ if (!(caps = virQEMUDriverGetCapabilities(qemu_driver, false)))
+ goto cleanup;
+
+ if (virFileExists(snapFile)) {
+ /* State last saved by modern libvirt in single file. As each
+ * snapshot contains a <domain>, it can be quite large. */
+ if (virFileReadAll(snapFile, 128*1024*1024*1, &xmlStr) < 0) {
+ /* Nothing we can do here */
+ virReportSystemError(errno,
+ _("Failed to read snapshot file %s"),
+ snapFile);
+ goto cleanup;
+ }
+
+ ret = virDomainSnapshotObjListParse(xmlStr, vm->def->uuid,
+ vm->snapshots, caps,
+ qemu_driver->xmlopt, flags);
+ if (ret < 0)
+ goto cleanup;
+ VIR_INFO("Read in %d snapshots for domain %s", ret, vm->def->name);
+ } else if (virAsprintf(&snapDir, "%s/%s", baseDir, vm->def->name) >= 0 &&
+ virFileExists(snapDir)) {
+ /* State may have been saved by earlier libvirt; if so, try to
+ * read it in, convert to modern style, and remove the old
+ * directory if successful. */
+ if (qemuDomainSnapshotLoadLegacy(vm, snapDir, caps) < 0)
+ goto cleanup;
+ if (qemuDomainSnapshotWriteMetadata(vm, NULL, caps,
+ qemu_driver->xmlopt, baseDir) < 0)
+ goto cleanup;
+ if (virFileDeleteTree(snapDir) < 0)
+ VIR_WARN("Unable to remove legacy snapshot directory %s", snapDir);
+ }
+
/* FIXME: qemu keeps internal track of snapshots. We can get access
* to this info via the "info snapshots" monitor command for running
* domains, or via "qemu-img snapshot -l" for shutoff domains. It would
@@ -516,8 +573,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
ret = 0;
cleanup:
- VIR_DIR_CLOSE(dir);
- VIR_FREE(snapDir);
virObjectUnref(caps);
virObjectUnlock(vm);
return ret;
--
2.20.1
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
