[libvirt] [PATCH 39/26] snapshot: reject unimplemented disk snapshot features

[Eric Blake]

On 08/22/2011 02:41 PM, Eric Blake wrote:
> My RFC for snapshot support [1] proposes several rules for when it is
> safe to delete or revert to an external snapshot, predicated on
> the existence of new API flags.  These will be incrementally added
> in future patches, but until then, blindly mishandling a disk
> snapshot risks corrupting internal state, so it is better to
> outright reject the attempts until the other pieces are in place,
> thus incrementally relaxing the restrictions added in this patch.
>
> [1] https://www.redhat.com/archives/libvir-list/2011-August/msg00361.html
>
> * src/qemu/qemu_driver.c (qemuDomainSnapshotCountExternal): New
> function.
> (qemuDomainSnapshotDelete): Use it to add safety valve.
> (qemuDomainRevertToSnapshot): Add safety valve.

> @@ -9293,6 +9314,16 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
>           goto cleanup;
>       }
>
> +    if (snap->def->state == VIR_DOMAIN_DISK_SNAPSHOT ||
> +        (virDomainSnapshotForEachDescendant(&vm->snapshots, snap,
> +                                            qemuDomainSnapshotCountExternal,
> +&external)&&  external)) {
> +        qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                        _("deletion of external disk snapshots not supported "
> +                          "yet"));
> +        goto cleanup;
> +    }

This was a little too strict - I couldn't even get rid of my metadata, 
which should always be a safe operation.  Conversely, undefine and 
destroy with the options to delete *_SNAPSHOT_FULL should also be 
complaining.  Squash this in:

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index bf3c137..9f3102b 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -1777,6 +1777,19 @@ qemuDomainSnapshotDiscardAll(void *payload,
          curr->err = err;
  }

+/* Count how many snapshots in a set have external disk snapshots.  */
+static void
+qemuDomainSnapshotCountExternal(void *payload,
+                                const void *name ATTRIBUTE_UNUSED,
+                                void *data)
+{
+    virDomainSnapshotObjPtr snap = payload;
+    int *count = data;
+
+    if (snap->def->state == VIR_DOMAIN_DISK_SNAPSHOT)
+        (*count)++;
+}
+
  static int
  qemuDomainDestroyFlags(virDomainPtr dom,
                         unsigned int flags)
@@ -1787,6 +1800,7 @@ qemuDomainDestroyFlags(virDomainPtr dom,
      virDomainEventPtr event = NULL;
      qemuDomainObjPrivatePtr priv;
      int nsnapshots;
+    int external = 0;

      virCheckFlags(VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA |
                    VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL, -1);
@@ -1814,6 +1828,16 @@ qemuDomainDestroyFlags(virDomainPtr dom,
              goto cleanup;
          }

+        if ((flags & VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL) &&
+            virHashForEach(vm->snapshots.objs, 
qemuDomainSnapshotCountExternal,
+                           &external) &&
+            external) {
+            qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                            _("deletion of %d external disk snapshots not "
+                              "supported yet"), external);
+            goto cleanup;
+        }
+
          rem.driver = driver;
          rem.vm = vm;
          rem.metadata_only = !(flags & VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL);
@@ -4943,6 +4967,7 @@ qemuDomainUndefineFlags(virDomainPtr dom,
      char *name = NULL;
      int ret = -1;
      int nsnapshots;
+    int external = 0;

      virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE |
                    VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA |
@@ -4972,6 +4997,16 @@ qemuDomainUndefineFlags(virDomainPtr dom,
              goto cleanup;
          }

+        if ((flags & VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL) &&
+            virHashForEach(vm->snapshots.objs, 
qemuDomainSnapshotCountExternal,
+                           &external) &&
+            external) {
+            qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                            _("deletion of %d external disk snapshots not "
+                              "supported yet"), external);
+            goto cleanup;
+        }
+
          rem.driver = driver;
          rem.vm = vm;
          rem.metadata_only = !(flags & VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL);
@@ -9502,19 +9537,6 @@ qemuDomainSnapshotReparentChildren(void *payload,
                                                 rep->driver->snapshotDir);
  }

-/* Count how many snapshots in a set have external disk snapshots.  */
-static void
-qemuDomainSnapshotCountExternal(void *payload,
-                                const void *name ATTRIBUTE_UNUSED,
-                                void *data)
-{
-    virDomainSnapshotObjPtr snap = payload;
-    int *count = data;
-
-    if (snap->def->state == VIR_DOMAIN_DISK_SNAPSHOT)
-        (*count)++;
-}
-
  static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
                                      unsigned int flags)
  {
@@ -9549,10 +9571,11 @@ static int 
qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
          goto cleanup;
      }

-    if (snap->def->state == VIR_DOMAIN_DISK_SNAPSHOT ||
-        (virDomainSnapshotForEachDescendant(&vm->snapshots, snap,
- 
qemuDomainSnapshotCountExternal,
-                                            &external) && external)) {
+    if (!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY) &&
+        (snap->def->state == VIR_DOMAIN_DISK_SNAPSHOT ||
+         (virDomainSnapshotForEachDescendant(&vm->snapshots, snap,
+ 
qemuDomainSnapshotCountExternal,
+                                             &external) && external))) {
          qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                          _("deletion of external disk snapshots not 
supported "
                            "yet"));


-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org