[libvirt] [PATCH 3/5] snapshot: Support deleting external disk snapshots when deleting

[Povilas Kanapickas]

Signed-off-by: Povilas Kanapickas <povilas at radix.lt>
---
 src/qemu/qemu_domain.c | 45 ++++++++++++++++++++++++++++++++++++++----
 src/qemu/qemu_driver.c |  7 ++++---
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ba3fff607a..73c41788f8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8244,6 +8244,38 @@ qemuDomainSnapshotForEachQcow2(virQEMUDriverPtr driver,
                                              op, try_all, def->ndisks);
 }
 
+static int
+qemuDomainSnapshotDiscardExternal(virDomainSnapshotObjPtr snap)
+{
+    int i;
+    virDomainSnapshotDiskDefPtr snap_disk;
+
+    // FIXME: libvirt stores the VM definition and the list of disks that
+    // snapshot has been taken of since 0.9.5. Before that we must assume that
+    // all disks from within the VM definition have been snapshotted and that
+    // they all were internal disks. Did libvirt support external snapshots
+    // before 0.9.5? If so, if we ever end up in this code path we may incur
+    // data loss as we'll delete whole QEMU file thinking it's an external
+    // snapshot.
+    if (!snap->def->dom) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       _("Snapshots created using libvirt 9.5 and containing "
+                         "external disk snapshots cannot be safely "
+                         "discarded."));
+        return -1;
+    }
+
+    for (i = 0; i < snap->def->ndisks; ++i) {
+        snap_disk = &(snap->def->disks[i]);
+        if (snap_disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+            continue;
+        if (unlink(snap_disk->src->path) < 0)
+            VIR_WARN("Failed to unlink snapshot disk '%s'",
+                     snap_disk->src->path);
+    }
+    return 0;
+}
+
 /* Discard one snapshot (or its metadata), without reparenting any children.  */
 int
 qemuDomainSnapshotDiscard(virQEMUDriverPtr driver,
@@ -8260,10 +8292,15 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver,
 
     if (!metadata_only) {
         if (!virDomainObjIsActive(vm)) {
-            /* Ignore any skipped disks */
-            if (qemuDomainSnapshotForEachQcow2(driver, vm, snap, "-d",
-                                               true) < 0)
-                goto cleanup;
+            if (virDomainSnapshotIsExternal(snap)) {
+                if (qemuDomainSnapshotDiscardExternal(snap) < 0)
+                    goto cleanup;
+            } else {
+                /* Ignore any skipped disks */
+                if (qemuDomainSnapshotForEachQcow2(driver, vm, snap, "-d",
+                                                   true) < 0)
+                    goto cleanup;
+            }
         } else {
             priv = vm->privateData;
             qemuDomainObjEnterMonitor(driver, vm);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index eb104d306b..7cd44d6957 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16662,7 +16662,7 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
     if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))
         goto endjob;
 
-    if (!metadata_only) {
+    if (!metadata_only && virDomainObjIsActive(vm)) {
         if (!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) &&
             virDomainSnapshotIsExternal(snap))
             external++;
@@ -16673,8 +16673,9 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
                                                &external);
         if (external) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           _("deletion of %d external disk snapshots not "
-                             "supported yet"), external);
+                           _("deletion of %d external disk snapshots while the "
+                             "domain is active is not supported yet"),
+                           external);
             goto endjob;
         }
     }
-- 
2.17.1