snapshot: prevent stranding snapshot data on domain destruction

Just as leaving managed save metadata behind can cause problems
when creating a new domain that happens to collide with the name
of the just-deleted domain, the same is true of leaving any
snapshot metadata behind.  For safety sake, extend the semantic
change of commit b26a9fa9 to also cover snapshot metadata as a
reason to reject undefining an inactive domain.  A future patch
will make sure that shutdown of a transient domain automatically
deletes snapshot metadata (whether by destroy, shutdown, or
guest-initiated action).  Management apps of transient domains
should take care to capture xml of snapshots, if it is necessary
to recreate the snapshot metadata on a later transient domain
with the same name and uuid.

This also documents a new flag that hypervisors can choose to
support as a shortcut for taking care of the metadata as part of
the undefine process; however, nontrivial driver support for these
flags will be deferred to future patches.

Note that ESX and VBox can never be transient; therefore, they
do not have to worry about automatic cleanup after shutdown
(the persistent domain still remains); likewise they never
store snapshot metadata, so the undefine flag is trivial.
The nontrivial work remaining is thus in the qemu driver.

* include/libvirt/libvirt.h.in
(VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA): New flag.
* src/libvirt.c (virDomainUndefine, virDomainUndefineFlags):
Document new limitations and flag.
* src/esx/esx_driver.c (esxDomainUndefineFlags): Trivial
implementation.
* src/vbox/vbox_tmpl.c (vboxDomainUndefineFlags): Likewise.
* src/qemu/qemu_driver.c (qemuDomainUndefineFlags): Enforce
the limitations.

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 59c9e63dacfb97332fb07a3de3a086765e904e06..88648659d653b844826e97d00923c207359dc1e8 100644 (file)
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -923,10 +923,12 @@ virConnectPtr           virDomainGetConnect     (virDomainPtr domain);
  * Domain creation and destruction
  */
 
+
 /*
  * typedef enum {
  * } virDomainDestroyFlagsValues;
  */
+
 virDomainPtr            virDomainCreateXML      (virConnectPtr conn,
                                                  const char *xmlDesc,
                                                  unsigned int flags);
@@ -1247,7 +1249,11 @@ virDomainPtr            virDomainDefineXML      (virConnectPtr conn,
 int                     virDomainUndefine       (virDomainPtr domain);
 
 typedef enum {
-    VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = (1 << 0),
+    VIR_DOMAIN_UNDEFINE_MANAGED_SAVE       = (1 << 0), /* Also remove any
+                                                          managed save */
+    VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA = (1 << 1), /* If last use of domain,
+                                                          then also remove any
+                                                          snapshot metadata */
 
     /* Future undefine control flags should come here. */
 } virDomainUndefineFlagsValues;

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 4bbb3153f46d354bb4d1cd54ee6ad84c03be0617..2bdb5d0cf2a8047caa2670e2ed1e23d9164dd452 100644 (file)
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3309,7 +3309,10 @@ esxDomainUndefineFlags(virDomainPtr domain,
     esxVI_String *propertyNameList = NULL;
     esxVI_VirtualMachinePowerState powerState;
 
-    virCheckFlags(0, -1);
+    /* No managed save, so we explicitly reject
+     * VIR_DOMAIN_UNDEFINE_MANAGED_SAVE.  No snapshot metadata for
+     * ESX, so we can trivially ignore that flag.  */
+    virCheckFlags(VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1);
 
     if (priv->vCenter != NULL) {
         ctx = priv->vCenter;

diff --git a/src/libvirt.c b/src/libvirt.c
index c1b154e59020769827247d845bb00cda0fa2d478..0b8f1d328cf41016665488cec1c2ea47a8e888af 100644 (file)
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -2058,6 +2058,10 @@ error:
  * does not free the associated virDomainPtr object.
  * This function may require privileged access
  *
+ * If the domain is transient and has any snapshot metadata (see
+ * virDomainSnapshotNum()), then that metadata will automatically
+ * be deleted when the domain quits.
+ *
  * Returns 0 in case of success and -1 in case of failure.
  */
 int
@@ -2107,7 +2111,9 @@ error:
  * This function may require privileged access.
  *
  * Calling this function with no @flags set (equal to zero)
- * is equivalent to calling virDomainDestroy.
+ * is equivalent to calling virDomainDestroy.  Using virDomainShutdown()
+ * may produce cleaner results for the guest's disks, but depends on guest
+ * support.
  *
  * Returns 0 in case of success and -1 in case of failure.
  */
@@ -2912,12 +2918,17 @@ error:
  * virDomainShutdown:
  * @domain: a domain object
  *
- * Shutdown a domain, the domain object is still usable there after but
+ * Shutdown a domain, the domain object is still usable thereafter but
  * the domain OS is being stopped. Note that the guest OS may ignore the
- * request.
+ * request.  For guests that react to a shutdown request, the differences
+ * from virDomainDestroy() are that the guests disk storage will be in a
+ * stable state rather than having the (virtual) power cord pulled, and
+ * this command returns as soon as the shutdown request is issued rather
+ * than blocking until the guest is no longer running.
  *
- * TODO: should we add an option for reboot, knowing it may not be doable
- *       in the general case ?
+ * If the domain is transient and has any snapshot metadata (see
+ * virDomainSnapshotNum()), then that metadata will automatically
+ * be deleted when the domain quits.
  *
  * Returns 0 in case of success and -1 in case of failure.
  */
@@ -6879,8 +6890,9 @@ error:
  * the domain configuration is removed.
  *
  * If the domain has a managed save image (see
- * virDomainHasManagedSaveImage()), then the undefine will fail. See
- * virDomainUndefineFlags() for more control.
+ * virDomainHasManagedSaveImage()), or if it is inactive and has any
+ * snapshot metadata (see virDomainSnapshotNum()), then the undefine will
+ * fail. See virDomainUndefineFlags() for more control.
  *
  * Returns 0 in case of success, -1 in case of error
  */
@@ -6931,6 +6943,15 @@ error:
  * then including VIR_DOMAIN_UNDEFINE_MANAGED_SAVE in @flags will also remove
  * that file, and omitting the flag will cause the undefine process to fail.
  *
+ * If the domain is inactive and has any snapshot metadata (see
+ * virDomainSnapshotNum()), then including
+ * VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA in @flags will also remove
+ * that metadata.  Omitting the flag will cause the undefine of an
+ * inactive domain to fail.  Active snapshots will retain snapshot
+ * metadata until the (now-transient) domain halts, regardless of
+ * whether this flag is present.  On hypervisors where snapshots do
+ * not use libvirt metadata, this flag has no effect.
+ *
  * Returns 0 in case of success, -1 in case of error
  */
 int

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b002b1d61b5450cb20adf11d03b16bba8025f22f..db1a86afcdfe5ade81ca975f78cacf17807c112f 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4838,6 +4838,7 @@ qemuDomainUndefineFlags(virDomainPtr dom,
     virDomainEventPtr event = NULL;
     char *name = NULL;
     int ret = -1;
+    int nsnapshots;
 
     virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, -1);
 
@@ -4852,6 +4853,14 @@ qemuDomainUndefineFlags(virDomainPtr dom,
         goto cleanup;
     }
 
+    if (!virDomainObjIsActive(vm) &&
+        (nsnapshots = virDomainSnapshotObjListNum(&vm->snapshots, 0))) {
+        qemuReportError(VIR_ERR_OPERATION_INVALID,
+                        _("cannot delete inactive domain with %d snapshots"),
+                        nsnapshots);
+        goto cleanup;
+    }
+
     if (!vm->persistent) {
         qemuReportError(VIR_ERR_OPERATION_INVALID,
                         "%s", _("cannot undefine transient domain"));

diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 636f5f2cf5cc79760269e27ee22d692284860afe..ebed4d9071214eaaa52ccc30e5da9ac350a37304 100644 (file)
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -4982,7 +4982,10 @@ vboxDomainUndefineFlags(virDomainPtr dom, unsigned int flags)
 #if VBOX_API_VERSION >= 4000
     vboxArray media = VBOX_ARRAY_INITIALIZER;
 #endif
-    virCheckFlags(0, -1);
+    /* No managed save, so we explicitly reject
+     * VIR_DOMAIN_UNDEFINE_MANAGED_SAVE.  No snapshot metadata for
+     * VBox, so we can trivially ignore that flag.  */
+    virCheckFlags(VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1);
 
     vboxIIDFromUUID(&iid, dom->uuid);