[libvirt] [PATCH 2/6] snapshot: prevent stranding snapshot data on domain destruction

Eric Blake eblake at redhat.com
Fri Aug 12 18:49:01 UTC 2011


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 losing the last reference to a domain (undefine
on an inactive, or shutdown/destroy on a transient).  The caller
must first take care of snapshots, possible via the existing
virDomainSnapshotDelete.

This also documents some new flags that hypervisors can choose to
support to shortcuts taking care of the metadata as part of the
shutdown 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 affect shutdown/destroy (the persistent domain
still remains); likewise they never store snapshot metadata,
so one of the two flags is trivial.  The bulk of the nontrivial
work remaining is thus in the qemu driver.

* include/libvirt/libvirt.h.in (VIR_DOMAIN_UNDEFINE_SNAPSHOTS)
(VIR_DOMAIN_DESTROY_SNAPSHOTS): New flags.
* src/libvirt.c (virDomainUndefine, virDomainUndefineFlags)
(virDomainDestroy, virDomainDestroyFlags, virDomainShutdown):
Document new limitations and flags.
* src/esx/esx_driver.c (esxDomainUndefineFlags): Enforce the
limitations.
* src/vbox/vbox_tmpl.c (vboxDomainUndefineFlags): Likewise.
* src/qemu/qemu_driver.c (qemuDomainUndefineFlags)
(qemuDomainShutdown, qemuDomainDestroyFlags): Likewise.
---
 include/libvirt/libvirt.h.in |   25 ++++++++++++++++++---
 src/esx/esx_driver.c         |   11 ++++++++-
 src/libvirt.c                |   47 +++++++++++++++++++++++++++++++++++------
 src/qemu/qemu_driver.c       |   26 +++++++++++++++++++++++
 src/vbox/vbox_tmpl.c         |   11 ++++++++-
 5 files changed, 105 insertions(+), 15 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index c672145..e6509ec 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -919,10 +919,20 @@ virConnectPtr           virDomainGetConnect     (virDomainPtr domain);
  * Domain creation and destruction
  */

-/*
- * typedef enum {
- * } virDomainDestroyFlagsValues;
+
+/* Counterparts to virDomainUndefineFlagsValues, but note that running
+ * domains have no managed save data, so no flag is provided for that.
  */
+typedef enum {
+    /* VIR_DOMAIN_DESTROY_MANAGED_SAVE = (1 << 0), */ /* Reserved */
+    VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA = (1 << 1), /* If last use of domain,
+                                                         then also remove any
+                                                         snapshot metadata */
+    VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL     = (1 << 2), /* If last use of domain,
+                                                         then also remove any
+                                                         snapshot data */
+} virDomainDestroyFlagsValues;
+
 virDomainPtr            virDomainCreateXML      (virConnectPtr conn,
                                                  const char *xmlDesc,
                                                  unsigned int flags);
@@ -1240,7 +1250,14 @@ 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 */
+    VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL     = (1 << 2), /* If last use of domain,
+                                                          then also remove any
+                                                          snapshot data */

     /* Future undefine control flags should come here. */
 } virDomainUndefineFlagsValues;
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 837b774..c71b1b3 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -1950,7 +1950,9 @@ esxDomainDestroyFlags(virDomainPtr domain,
     esxVI_TaskInfoState taskInfoState;
     char *taskInfoErrorMessage = NULL;

-    virCheckFlags(0, -1);
+    /* No transient domains, so these flags are trivially ignored.  */
+    virCheckFlags(VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA |
+                  VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL, -1);

     if (priv->vCenter != NULL) {
         ctx = priv->vCenter;
@@ -3309,7 +3311,9 @@ 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.  */
+    virCheckFlags(VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1);

     if (priv->vCenter != NULL) {
         ctx = priv->vCenter;
@@ -3337,6 +3341,9 @@ esxDomainUndefineFlags(virDomainPtr domain,
         goto cleanup;
     }

+    /* ESX snapshots maintain no metadata, so we can trivially ignore
+     * that flag.  XXX Wire up VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL.  */
+
     if (esxVI_UnregisterVM(ctx, virtualMachine->obj) < 0) {
         goto cleanup;
     }
diff --git a/src/libvirt.c b/src/libvirt.c
index 3e031b6..deb95b2 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -2039,6 +2039,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 the destroy will fail. See
+ * virDomainDestroyFlags() for more control.
+ *
  * Returns 0 in case of success and -1 in case of failure.
  */
 int
@@ -2088,7 +2092,20 @@ 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.
+ *
+ * If the domain is transient and has any snapshot metadata (see
+ * virDomainSnapshotNum()), then including
+ * VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA in @flags will also remove
+ * that metadata, including VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL will
+ * remove the metadata and snapshot data contents, and omitting both
+ * of these flags will cause the destroy process to fail.
+ * VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA and
+ * VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL have no effect if the domain is
+ * persistent, and VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA has no effect
+ * if libvirt does not maintain snapshot metadata.
  *
  * Returns 0 in case of success and -1 in case of failure.
  */
@@ -2856,12 +2873,16 @@ 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 the shutdown will fail.
  *
  * Returns 0 in case of success and -1 in case of failure.
  */
@@ -6808,8 +6829,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 the domain is active 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
  */
@@ -6860,6 +6882,17 @@ 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, including VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL will
+ * remove the metadata and snapshot data contents, and omitting both
+ * of these flags will cause the undefine process to fail.
+ * VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA and
+ * VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL have no effect if the domain is
+ * active, and VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA has no effect if
+ * libvirt does not maintain snapshot metadata.
+ *
  * 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 ffea714..678ac1b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1466,6 +1466,7 @@ static int qemuDomainShutdown(virDomainPtr dom) {
     virDomainObjPtr vm;
     int ret = -1;
     qemuDomainObjPrivatePtr priv;
+    int nsnapshots;

     qemuDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
@@ -1479,6 +1480,14 @@ static int qemuDomainShutdown(virDomainPtr dom) {
         goto cleanup;
     }

+    if (!vm->persistent &&
+        (nsnapshots = virDomainSnapshotObjListNum(&vm->snapshots))) {
+        qemuReportError(VIR_ERR_OPERATION_INVALID,
+                        _("cannot delete transient domain with %d snapshots"),
+                        nsnapshots);
+        goto cleanup;
+    }
+
     if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
         goto cleanup;

@@ -1574,6 +1583,7 @@ qemuDomainDestroyFlags(virDomainPtr dom,
     int ret = -1;
     virDomainEventPtr event = NULL;
     qemuDomainObjPrivatePtr priv;
+    int nsnapshots;

     virCheckFlags(0, -1);

@@ -1587,6 +1597,14 @@ qemuDomainDestroyFlags(virDomainPtr dom,
         goto cleanup;
     }

+    if (!vm->persistent &&
+        (nsnapshots = virDomainSnapshotObjListNum(&vm->snapshots))) {
+        qemuReportError(VIR_ERR_OPERATION_INVALID,
+                        _("cannot delete transient domain with %d snapshots"),
+                        nsnapshots);
+        goto cleanup;
+    }
+
     priv = vm->privateData;
     priv->fakeReboot = false;

@@ -4706,6 +4724,7 @@ qemuDomainUndefineFlags(virDomainPtr dom,
     virDomainEventPtr event = NULL;
     char *name = NULL;
     int ret = -1;
+    int nsnapshots;

     virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, -1);

@@ -4720,10 +4739,17 @@ qemuDomainUndefineFlags(virDomainPtr dom,
         goto cleanup;
     }

+    /* XXX We should allow undefine to convert running persistent into
+     * transient domain, rather than reject things here.  */
     if (virDomainObjIsActive(vm)) {
         qemuReportError(VIR_ERR_OPERATION_INVALID,
                         "%s", _("cannot delete active domain"));
         goto cleanup;
+    } else if ((nsnapshots = virDomainSnapshotObjListNum(&vm->snapshots))) {
+        qemuReportError(VIR_ERR_OPERATION_INVALID,
+                        _("cannot delete inactive domain with %d snapshots"),
+                        nsnapshots);
+        goto cleanup;
     }

     if (!vm->persistent) {
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index fa19aaa..91a5423 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -1701,7 +1701,9 @@ vboxDomainDestroyFlags(virDomainPtr dom,
     PRBool isAccessible  = PR_FALSE;
     nsresult rc;

-    virCheckFlags(0, -1);
+    /* No transient domains, so these flags are trivially ignored.  */
+    virCheckFlags(VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA |
+                  VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL, -1);

     vboxIIDFromUUID(&iid, dom->uuid);
     rc = VBOX_OBJECT_GET_MACHINE(iid.value, &machine);
@@ -4982,10 +4984,15 @@ 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.  */
+    virCheckFlags(VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1);

     vboxIIDFromUUID(&iid, dom->uuid);

+    /* VBox snapshots maintain no metadata, so we can trivially ignore
+     * that flag.  XXX Wire up VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL.  */
+
 #if VBOX_API_VERSION < 4000
     /* Block for checking if HDD's are attched to VM.
      * considering just IDE bus for now. Also skipped
-- 
1.7.4.4




More information about the libvir-list mailing list