[libvirt] [PATCH v2 11/11] qemu: Implement bulk snapshot operations

Eric Blake eblake at redhat.com
Sat Feb 23 21:24:45 UTC 2019


Implement the new flags for bulk snapshot dump and redefine. This
borrows from ideas in the test driver, but is further complicated
by the fact that qemu must write snapshot XML to disk, and thus must
do additional validation after the initial parse to ensure the user
didn't attempt to rename a snapshot with "../" or similar.

Of note: all prior callers of qemuDomainSnapshotDiscardAllMetadata()
were at points where it did not matter if vm->current_snapshot and
the metaroot in vm->snapshots were left pointing to stale memory,
because we were about to delete the entire vm object; but now it is
important to reset things properly so that the domain still shows
as having no snapshots on failure.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 src/qemu/qemu_domain.h |  2 +-
 src/qemu/qemu_domain.c | 35 +++++++++++++++++------
 src/qemu/qemu_driver.c | 64 ++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 89 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 7c6b50184c..37c9813ec5 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -683,7 +683,7 @@ int qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm,
                                     virDomainSnapshotObjPtr snapshot,
                                     virCapsPtr caps,
                                     virDomainXMLOptionPtr xmlopt,
-                                    char *snapshotDir);
+                                    const char *snapshotDir);

 int qemuDomainSnapshotForEachQcow2(virQEMUDriverPtr driver,
                                    virDomainObjPtr vm,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index cb1665c8f9..cf8b6e8eaf 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7704,6 +7704,7 @@ qemuDomainDefCopy(virQEMUDriverPtr driver,

 static int
 qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
+                               virDomainObjPtr vm,
                                virDomainDefPtr def,
                                virCPUDefPtr origCPU,
                                unsigned int flags,
@@ -7713,8 +7714,10 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
     virDomainDefPtr copy = NULL;
     virCapsPtr caps = NULL;
     virQEMUCapsPtr qemuCaps = NULL;
+    bool snapshots = flags & VIR_DOMAIN_XML_SNAPSHOTS;

-    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS | VIR_DOMAIN_XML_UPDATE_CPU, -1);
+    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS | VIR_DOMAIN_XML_UPDATE_CPU |
+                  VIR_DOMAIN_XML_SNAPSHOTS, -1);

     if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
         goto cleanup;
@@ -7881,7 +7884,14 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
     }

  format:
-    ret = virDomainDefFormatInternal(def, caps, NULL, NULL,
+    if (snapshots && !vm) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("snapshots XML requested but not provided"));
+        goto cleanup;
+    }
+    ret = virDomainDefFormatInternal(def, caps,
+                                     snapshots ? vm->snapshots : NULL,
+                                     snapshots ? vm->current_snapshot : NULL,
                                      virDomainDefFormatConvertXMLFlags(flags),
                                      buf, driver->xmlopt);

@@ -7899,19 +7909,21 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver,
                        unsigned int flags,
                        virBufferPtr buf)
 {
-    return qemuDomainDefFormatBufInternal(driver, def, NULL, flags, buf);
+    return qemuDomainDefFormatBufInternal(driver, NULL, def, NULL, flags, buf);
 }


 static char *
 qemuDomainDefFormatXMLInternal(virQEMUDriverPtr driver,
+                               virDomainObjPtr vm,
                                virDomainDefPtr def,
                                virCPUDefPtr origCPU,
                                unsigned int flags)
 {
     virBuffer buf = VIR_BUFFER_INITIALIZER;

-    if (qemuDomainDefFormatBufInternal(driver, def, origCPU, flags, &buf) < 0)
+    if (qemuDomainDefFormatBufInternal(driver, vm, def, origCPU, flags,
+                                       &buf) < 0)
         return NULL;

     return virBufferContentAndReset(&buf);
@@ -7923,7 +7935,7 @@ qemuDomainDefFormatXML(virQEMUDriverPtr driver,
                        virDomainDefPtr def,
                        unsigned int flags)
 {
-    return qemuDomainDefFormatXMLInternal(driver, def, NULL, flags);
+    return qemuDomainDefFormatXMLInternal(driver, NULL, def, NULL, flags);
 }


@@ -7942,7 +7954,7 @@ char *qemuDomainFormatXML(virQEMUDriverPtr driver,
         origCPU = priv->origCPU;
     }

-    return qemuDomainDefFormatXMLInternal(driver, def, origCPU, flags);
+    return qemuDomainDefFormatXMLInternal(driver, vm, def, origCPU, flags);
 }

 char *
@@ -7959,7 +7971,7 @@ qemuDomainDefFormatLive(virQEMUDriverPtr driver,
     if (compatible)
         flags |= VIR_DOMAIN_XML_MIGRATABLE;

-    return qemuDomainDefFormatXMLInternal(driver, def, origCPU, flags);
+    return qemuDomainDefFormatXMLInternal(driver, NULL, def, origCPU, flags);
 }


@@ -8386,7 +8398,7 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm,
                                 virDomainSnapshotObjPtr snapshot,
                                 virCapsPtr caps,
                                 virDomainXMLOptionPtr xmlopt,
-                                char *snapshotDir)
+                                const char *snapshotDir)
 {
     char *newxml = NULL;
     int ret = -1;
@@ -8605,6 +8617,7 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver,
                                      virDomainObjPtr vm)
 {
     virQEMUSnapRemove rem;
+    virDomainSnapshotObjPtr snap;

     rem.driver = driver;
     rem.vm = vm;
@@ -8612,6 +8625,12 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver,
     rem.err = 0;
     virDomainSnapshotForEach(vm->snapshots, qemuDomainSnapshotDiscardAll,
                              &rem);
+    if (rem.current)
+        vm->current_snapshot = NULL;
+    /* Update the metaroot to match that all children were dropped */
+    snap = virDomainSnapshotFindByName(vm->snapshots, NULL);
+    snap->nchildren = 0;
+    snap->first_child = NULL;

     return rem.err;
 }
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b3f4dc6f5c..4df9e18e4c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7337,8 +7337,8 @@ static char
     virDomainObjPtr vm;
     char *ret = NULL;

-    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS | VIR_DOMAIN_XML_UPDATE_CPU,
-                  NULL);
+    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS | VIR_DOMAIN_XML_UPDATE_CPU |
+                  VIR_DOMAIN_XML_SNAPSHOTS, NULL);

     if (!(vm = qemuDomObjFromDomain(dom)))
         goto cleanup;
@@ -15733,6 +15733,31 @@ qemuDomainSnapshotValidate(virDomainSnapshotDefPtr def, int state,
     return 0;
 }

+/* Struct and hash-iterator callback used when bulk redefining snapshots */
+struct qemuDomainSnapshotBulk {
+    virDomainObjPtr vm;
+    virQEMUDriverPtr driver;
+    const char *snapshotDir;
+    unsigned int flags;
+};
+
+static int
+qemuDomainSnapshotBulkRedefine(void *payload, const void *name ATTRIBUTE_UNUSED,
+                               void *opaque)
+{
+    virDomainSnapshotObjPtr snap = payload;
+    struct qemuDomainSnapshotBulk *data = opaque;
+
+    if (qemuDomainSnapshotValidate(snap->def, snap->def->state,
+                                   data->flags) < 0)
+        return -1;
+    if (qemuDomainSnapshotWriteMetadata(data->vm, snap, data->driver->caps,
+                                        data->driver->xmlopt,
+                                        data->snapshotDir) < 0)
+        return -1;
+    return 0;
+}
+
 static virDomainSnapshotPtr
 qemuDomainSnapshotCreateXML(virDomainPtr domain,
                             const char *xmlDesc,
@@ -15762,7 +15787,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
                   VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT |
                   VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE |
                   VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC |
-                  VIR_DOMAIN_SNAPSHOT_CREATE_LIVE, NULL);
+                  VIR_DOMAIN_SNAPSHOT_CREATE_LIVE |
+                  VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE_LIST, NULL);

     VIR_REQUIRE_FLAG_RET(VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE,
                          VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY,
@@ -15794,6 +15820,38 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
         goto cleanup;
     }

+    if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE_LIST) {
+        struct qemuDomainSnapshotBulk bulk = {
+            .vm = vm,
+            .driver = driver,
+            .snapshotDir = cfg->snapshotDir,
+            .flags = flags,
+        };
+
+        if (virDomainSnapshotDefParseList(xmlDesc, vm->def->uuid,
+                                          vm->snapshots, &vm->current_snapshot,
+                                          caps, driver->xmlopt,
+                                          parse_flags) < 0)
+            goto cleanup;
+        /* Validate and save the snapshots to disk. Since we don't get
+         * here unless there were no snapshots beforehand, just delete
+         * everything if anything failed, ignoring further errors. */
+        if (virDomainSnapshotForEach(vm->snapshots,
+                                     qemuDomainSnapshotBulkRedefine,
+                                     &bulk) < 0) {
+            virErrorPtr orig_err = virSaveLastError();
+
+            qemuDomainSnapshotDiscardAllMetadata(driver, vm);
+            virSetError(orig_err);
+            virFreeError(orig_err);
+            goto cleanup;
+        }
+        /* Return is arbitrary, so use the first root */
+        snap = virDomainSnapshotFindByName(vm->snapshots, NULL);
+        snapshot = virGetDomainSnapshot(domain, snap->first_child->def->name);
+        goto cleanup;
+    }
+
     if (!vm->persistent && (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("cannot halt after transient domain snapshot"));
-- 
2.20.1




More information about the libvir-list mailing list