[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 6/6] snapshot: teach virsh about new undefine flags



Similar to 'undefine --managed-save' (commit 83e849c1), we must
assume that the old API is unsafe, and emulate it ourselves.
Additionally, we have the wrinkle that while virDomainUndefineFlags
and managed save cleanup were introduced in 0.9.4, it wasn't until
0.9.5 that snapshots block undefine of a domain.  Simulate as much
as possible with older servers, but the --snapshots-metadata
support requires a server >= 0.9.5.  Same story for
virDomainDestroyFlags, and virDomainShutdownFlags doesn't exist yet.
Oh well.

* tools/virsh.c (cmdUndefine, cmdDestroy, cmdShutdown): Add
--snapshots-full and --snapshots-metadata flags.
(vshRemoveAllSnapshots): New helper method.
* tools/virsh.pod (undefine, destroy, shutdown): Document them.
---
 tools/virsh.c   |  400 ++++++++++++++++++++++++++++++++++++++++++++++++-------
 tools/virsh.pod |   36 +++++-
 2 files changed, 386 insertions(+), 50 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index c094911..7275e4c 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1410,6 +1410,59 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd)
     return ret;
 }

+/* Helper for undefine, shutdown, and destroy */
+static int
+vshRemoveAllSnapshots(virDomainPtr dom, int nsnapshots, bool full)
+{
+    int ret = -1;
+    char **names;
+    int actual;
+    int i;
+    virDomainSnapshotPtr snapshot = NULL;
+    int flags = 0;
+
+    /* It's likely that if virDomainUndefineFlags was unsupported to
+     * get us here in the first place, then
+     * VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY will also be
+     * unsupported.  But better to hear that from the driver.
+     */
+    if (!full)
+        flags |= VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY;
+
+    if (VIR_ALLOC_N(names, nsnapshots) < 0)
+        goto cleanup;
+
+    actual = virDomainSnapshotListNames(dom, names, nsnapshots, 0);
+    if (actual < 0)
+        goto cleanup;
+
+    /* Sadly, this is an O(n) loop over a function call that is also a
+     * minimum of O(n), for a complexity of O(n^2), whereas newer
+     * servers that support the delete in the undefine action are
+     * O(n). Oh well.  */
+    for (i = 0; i < actual; i++) {
+        if (snapshot)
+            virDomainSnapshotFree(snapshot);
+
+        snapshot = virDomainSnapshotLookupByName(dom, names[i], 0);
+        if (snapshot == NULL)
+            continue;
+
+        if (virDomainSnapshotDelete(snapshot, flags) < 0)
+            goto cleanup;
+    }
+
+    ret = 0;
+
+cleanup:
+    if (snapshot)
+        virDomainSnapshotFree(snapshot);
+    for (i = 0; i < actual; i++)
+        VIR_FREE(names[i]);
+    VIR_FREE(names);
+    return ret;
+}
+
 /*
  * "undefine" command
  */
@@ -1422,6 +1475,10 @@ static const vshCmdInfo info_undefine[] = {
 static const vshCmdOptDef opts_undefine[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name or uuid")},
     {"managed-save", VSH_OT_BOOL, 0, N_("remove domain managed state file")},
+    {"snapshots-metadata", VSH_OT_BOOL, 0,
+     N_("remove all domain snapshot metadata, if inactive")},
+    {"snapshots-full", VSH_OT_BOOL, 0,
+     N_("remove all domain snapshot contents, if inactive")},
     {NULL, 0, 0, NULL}
 };

@@ -1429,18 +1486,36 @@ static bool
 cmdUndefine(vshControl *ctl, const vshCmd *cmd)
 {
     virDomainPtr dom;
-    bool ret = true;
+    bool ret = false;
     const char *name = NULL;
+    /* Flags to attempt.  */
     int flags = 0;
-    int managed_save = vshCommandOptBool(cmd, "managed-save");
+    /* User-requested actions.  */
+    bool managed_save = vshCommandOptBool(cmd, "managed-save");
+    bool snapshots_metadata = vshCommandOptBool(cmd, "snapshots-metadata");
+    bool snapshots_full = vshCommandOptBool(cmd, "snapshots-full");
+    /* Positive if these items exist.  */
     int has_managed_save = 0;
+    int has_snapshots_metadata = 0;
+    int has_snapshots = 0;
+    /* True if undefine will not strand data, even on older servers.  */
+    bool managed_save_safe = false;
+    bool snapshots_safe = false;
     int rc = -1;
+    int running;

-    if (managed_save)
+    if (managed_save) {
         flags |= VIR_DOMAIN_UNDEFINE_MANAGED_SAVE;
-
-    if (!managed_save)
-        flags = -1;
+        managed_save_safe = true;
+    }
+    if (snapshots_metadata) {
+        flags |= VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA;
+        snapshots_safe = true;
+    }
+    if (snapshots_full) {
+        flags |= VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL;
+        snapshots_safe = true;
+    }

     if (!vshConnectionUsability(ctl, ctl->conn))
         return false;
@@ -1448,61 +1523,124 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
     if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
         return false;

-    has_managed_save = virDomainHasManagedSaveImage(dom, 0);
-    if (has_managed_save < 0) {
-        if (last_error->code != VIR_ERR_NO_SUPPORT) {
-            virshReportError(ctl);
-            virDomainFree(dom);
-            return false;
-        } else {
+    /* Do some flag manipulation.  The goal here is to disable bits
+     * from flags to reduce the likelihood of a server rejecting
+     * unknown flag bits, as well as to track conditions which are
+     * safe by default for the given hypervisor and server version.  */
+    running = virDomainIsActive(dom);
+    if (running < 0) {
+        virshReportError(ctl);
+        goto cleanup;
+    }
+    if (!running) {
+        /* Undefine with snapshots only fails for inactive domains,
+         * and managed save only exists on inactive domains; if
+         * running, then we don't want to remove anything.  */
+        has_managed_save = virDomainHasManagedSaveImage(dom, 0);
+        if (has_managed_save < 0) {
+            if (last_error->code != VIR_ERR_NO_SUPPORT) {
+                virshReportError(ctl);
+                goto cleanup;
+            }
             virFreeError(last_error);
             last_error = NULL;
-        }
-    }
-
-    if (flags == -1) {
-        if (has_managed_save == 1) {
-            vshError(ctl,
-                     _("Refusing to undefine while domain managed save "
-                       "image exists"));
-            virDomainFree(dom);
-            return false;
+            has_managed_save = 0;
         }

-        rc = virDomainUndefine(dom);
-    } else {
-        rc = virDomainUndefineFlags(dom, flags);
-
-        /* It might fail when virDomainUndefineFlags is not
-         * supported on older libvirt, try to undefine the
-         * domain with combo virDomainManagedSaveRemove and
-         * virDomainUndefine.
-         */
-        if (rc < 0) {
+        has_snapshots = virDomainSnapshotNum(dom, 0);
+        if (has_snapshots < 0) {
             if (last_error->code != VIR_ERR_NO_SUPPORT) {
                 virshReportError(ctl);
-                goto end;
-            } else {
+                goto cleanup;
+            }
+            virFreeError(last_error);
+            last_error = NULL;
+            has_snapshots = 0;
+        }
+        if (has_snapshots) {
+            has_snapshots_metadata
+                = virDomainSnapshotNum(dom, VIR_DOMAIN_SNAPSHOT_NUM_METADATA);
+            if (has_snapshots_metadata < 0) {
+                /* The server did not know the new flag, assume that all
+                   snapshots have metadata.  */
                 virFreeError(last_error);
                 last_error = NULL;
+                has_snapshots_metadata = has_snapshots;
+            } else {
+                /* The server knew the new flag, all aspects of
+                 * undefineFlags are safe.  */
+                managed_save_safe = snapshots_safe = true;
             }
+        }
+    }
+    if (!has_managed_save) {
+        flags &= ~VIR_DOMAIN_UNDEFINE_MANAGED_SAVE;
+        managed_save_safe = true;
+    }
+    if (has_snapshots == 0) {
+        flags &= ~VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL;
+        snapshots_safe = true;
+    }
+    if (has_snapshots_metadata == 0) {
+        flags &= ~VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA;
+        snapshots_safe = true;
+    }

-            if ((has_managed_save == 1) &&
-                virDomainManagedSaveRemove(dom, 0) < 0)
-                goto end;
+    /* Generally we want to try the new API first.  However, while
+     * virDomainUndefineFlags was introduced at the same time as
+     * VIR_DOMAIN_UNDEFINE_MANAGED_SAVE in 0.9.4, the
+     * VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA flag was not present
+     * until 0.9.5; skip to piecewise emulation if we couldn't prove
+     * above that the new API is safe.  */
+    if (managed_save_safe && snapshots_safe) {
+        rc = virDomainUndefineFlags(dom, flags);
+        if (rc == 0 || (last_error->code != VIR_ERR_NO_SUPPORT &&
+                        last_error->code != VIR_ERR_INVALID_ARG))
+            goto out;
+        virFreeError(last_error);
+        last_error = NULL;
+    }
+
+    /* The new API is unsupported or unsafe; fall back to doing things
+     * piecewise.  */
+    if (has_managed_save) {
+        if (!managed_save) {
+            vshError(ctl, "%s",
+                     _("Refusing to undefine while domain managed save "
+                       "image exists"));
+            goto cleanup;
+        }
+        if (virDomainManagedSaveRemove(dom, 0) < 0) {
+            virshReportError(ctl);
+            goto cleanup;
+        }
+    }

-            rc = virDomainUndefine(dom);
+    if (has_snapshots) {
+        if (has_snapshots_metadata &&
+            !(snapshots_metadata || snapshots_full)) {
+            vshError(ctl,
+                     _("Refusing to undefine while %d snapshots exist"),
+                has_snapshots);
+            goto cleanup;
         }
+
+        if ((snapshots_metadata || snapshots_full) &&
+            vshRemoveAllSnapshots(dom, has_snapshots, snapshots_full) < 0)
+            goto cleanup;
     }

-end:
+    rc = virDomainUndefine(dom);
+
+out:
     if (rc == 0) {
         vshPrint(ctl, _("Domain %s has been undefined\n"), name);
+        ret = true;
     } else {
         vshError(ctl, _("Failed to undefine domain %s"), name);
-        ret = false;
     }

+cleanup:
     virDomainFree(dom);
     return ret;
 }
@@ -2488,6 +2626,10 @@ static const vshCmdInfo info_shutdown[] = {

 static const vshCmdOptDef opts_shutdown[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
+    {"snapshots-metadata", VSH_OT_BOOL, 0,
+     N_("remove all domain snapshot metadata, if transient")},
+    {"snapshots-full", VSH_OT_BOOL, 0,
+     N_("remove all domain snapshot contents, if transient")},
     {NULL, 0, 0, NULL}
 };

@@ -2495,8 +2637,15 @@ static bool
 cmdShutdown(vshControl *ctl, const vshCmd *cmd)
 {
     virDomainPtr dom;
-    bool ret = true;
+    bool ret = false;
     const char *name;
+    /* User-requested actions.  */
+    bool snapshots_metadata = vshCommandOptBool(cmd, "snapshots-metadata");
+    bool snapshots_full = vshCommandOptBool(cmd, "snapshots-full");
+    /* Positive if these items exist.  */
+    int has_snapshots_metadata = 0;
+    int has_snapshots = 0;
+    int persistent;

     if (!vshConnectionUsability(ctl, ctl->conn))
         return false;
@@ -2504,13 +2653,67 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd)
     if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
         return false;

+    /* Do some flag manipulation.  The goal here is to track
+     * conditions which are safe by default for the given hyperviser
+     * and server version.  */
+    persistent = virDomainIsPersistent(dom);
+    if (persistent < 0) {
+        virshReportError(ctl);
+        goto cleanup;
+    }
+    if (!persistent) {
+        /* Snapshot safety is only needed for transient domains.  */
+        has_snapshots = virDomainSnapshotNum(dom, 0);
+        if (has_snapshots < 0) {
+            if (last_error->code != VIR_ERR_NO_SUPPORT) {
+                virshReportError(ctl);
+                goto cleanup;
+            }
+            virFreeError(last_error);
+            last_error = NULL;
+            has_snapshots = 0;
+        }
+        if (has_snapshots) {
+            has_snapshots_metadata
+                = virDomainSnapshotNum(dom, VIR_DOMAIN_SNAPSHOT_NUM_METADATA);
+            if (has_snapshots_metadata < 0) {
+                /* The server did not know the new flag, assume that all
+                   snapshots have metadata.  */
+                virFreeError(last_error);
+                last_error = NULL;
+                has_snapshots_metadata = has_snapshots;
+            }
+        }
+    }
+
+    /* XXX Once virDomainShutdownFlags is added, use it here.  Since
+     * snapshot shutdown safety was introduced before the new API, we
+     * don't have to go through quite as many contortions as
+     * cmdDestroy to check for snapshot safety.  */
+
+    /* Fall back to doing things piecewise.  */
+    if (has_snapshots) {
+        if (has_snapshots_metadata &&
+            !(snapshots_metadata || snapshots_full)) {
+            vshError(ctl,
+                     _("Refusing to shutdown while %d snapshots exist"),
+                     has_snapshots);
+            goto cleanup;
+        }
+
+        if ((snapshots_metadata || snapshots_full) &&
+            vshRemoveAllSnapshots(dom, has_snapshots, snapshots_full) < 0)
+            goto cleanup;
+    }
+
     if (virDomainShutdown(dom) == 0) {
         vshPrint(ctl, _("Domain %s is being shutdown\n"), name);
+        ret = true;
     } else {
         vshError(ctl, _("Failed to shutdown domain %s"), name);
-        ret = false;
     }

+cleanup:
     virDomainFree(dom);
     return ret;
 }
@@ -2565,6 +2768,10 @@ static const vshCmdInfo info_destroy[] = {

 static const vshCmdOptDef opts_destroy[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
+    {"snapshots-metadata", VSH_OT_BOOL, 0,
+     N_("remove all domain snapshot metadata, if transient")},
+    {"snapshots-full", VSH_OT_BOOL, 0,
+     N_("remove all domain snapshot contents, if transient")},
     {NULL, 0, 0, NULL}
 };

@@ -2572,8 +2779,29 @@ static bool
 cmdDestroy(vshControl *ctl, const vshCmd *cmd)
 {
     virDomainPtr dom;
-    bool ret = true;
+    bool ret = false;
     const char *name;
+    /* Flags to attempt.  */
+    int flags = 0;
+    /* User-requested actions.  */
+    bool snapshots_metadata = vshCommandOptBool(cmd, "snapshots-metadata");
+    bool snapshots_full = vshCommandOptBool(cmd, "snapshots-full");
+    /* Positive if these items exist.  */
+    int has_snapshots_metadata = 0;
+    int has_snapshots = 0;
+    /* True if undefine will not strand data, even on older servers.  */
+    bool snapshots_safe = false;
+    int rc = -1;
+    int persistent;
+
+    if (snapshots_metadata) {
+        flags |= VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA;
+        snapshots_safe = true;
+    }
+    if (snapshots_full) {
+        flags |= VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL;
+        snapshots_safe = true;
+    }

     if (!vshConnectionUsability(ctl, ctl->conn))
         return false;
@@ -2581,13 +2809,93 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd)
     if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
         return false;

-    if (virDomainDestroy(dom) == 0) {
+    /* Do some flag manipulation.  The goal here is to disable bits
+     * from flags to reduce the likelihood of a server rejecting
+     * unknown flag bits, as well as to track conditions which are
+     * safe by default for the given hypervisor and server version.  */
+    persistent = virDomainIsPersistent(dom);
+    if (persistent < 0) {
+        virshReportError(ctl);
+        goto cleanup;
+    }
+    if (!persistent) {
+        /* Snapshot safety is only needed for transient domains.  */
+        has_snapshots = virDomainSnapshotNum(dom, 0);
+        if (has_snapshots < 0) {
+            if (last_error->code != VIR_ERR_NO_SUPPORT) {
+                virshReportError(ctl);
+                goto cleanup;
+            }
+            virFreeError(last_error);
+            last_error = NULL;
+            has_snapshots = 0;
+        }
+        if (has_snapshots) {
+            has_snapshots_metadata
+                = virDomainSnapshotNum(dom, VIR_DOMAIN_SNAPSHOT_NUM_METADATA);
+            if (has_snapshots_metadata < 0) {
+                /* The server did not know the new flag, assume that all
+                   snapshots have metadata.  */
+                virFreeError(last_error);
+                last_error = NULL;
+                has_snapshots_metadata = has_snapshots;
+            } else {
+                /* The server knew the new flag, all aspects of
+                 * destroyFlags are safe.  */
+                snapshots_safe = true;
+            }
+        }
+    }
+    if (has_snapshots == 0) {
+        flags &= ~VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL;
+        snapshots_safe = true;
+    }
+    if (has_snapshots_metadata == 0) {
+        flags &= ~VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA;
+        snapshots_safe = true;
+    }
+
+    /* Generally we want to try the new API first.  However, while
+     * virDomainDestroyFlags was introduced in 0.9.4, the
+     * VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA flag was not present
+     * until 0.9.5; skip to piecewise emulation if we couldn't prove
+     * above that the new API is safe.  */
+    if (snapshots_safe) {
+        rc = virDomainDestroyFlags(dom, flags);
+        if (rc == 0 || (last_error->code != VIR_ERR_NO_SUPPORT &&
+                        last_error->code != VIR_ERR_INVALID_ARG))
+            goto out;
+        virFreeError(last_error);
+        last_error = NULL;
+    }
+
+    /* The new API is unsupported or unsafe; fall back to doing things
+     * piecewise.  */
+    if (has_snapshots) {
+        if (has_snapshots_metadata &&
+            !(snapshots_metadata || snapshots_full)) {
+            vshError(ctl,
+                     _("Refusing to destroy while %d snapshots exist"),
+                has_snapshots);
+            goto cleanup;
+        }
+
+        if ((snapshots_metadata || snapshots_full) &&
+            vshRemoveAllSnapshots(dom, has_snapshots, snapshots_full) < 0)
+            goto cleanup;
+    }
+
+    rc = virDomainDestroy(dom);
+
+out:
+    if (rc == 0) {
         vshPrint(ctl, _("Domain %s destroyed\n"), name);
+        ret = true;
     } else {
         vshError(ctl, _("Failed to destroy domain %s"), name);
-        ret = false;
     }

+cleanup:
     virDomainFree(dom);
     return ret;
 }
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 11a13fd..c27e99d 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -405,7 +405,7 @@ B<Example>
 Define a domain from an XML <file>. The domain definition is registered
 but not started.

-=item B<destroy> I<domain-id>
+=item B<destroy> I<domain-id> [I<--snapshots-full>] [I<--snapshots-metadata>]

 Immediately terminate the domain domain-id.  This doesn't give the domain
 OS any chance to react, and it's the equivalent of ripping the power
@@ -414,6 +414,15 @@ the B<shutdown> command instead.  However, this does not delete any
 storage volumes used by the guest, and if the domain is persistent, it
 can be restarted later.

+The I<--snapshots-full> and I<--snapshots-metadata> flags guarantee
+that any snapshots (see the B<snapshot-list> command) are also cleaned
+up when stopping a transient domain.  Without either flag, attempts
+to destroy a transient domain with snapshot metadata will fail.  The
+I<--snapshots-full> flag works with older servers, but loses all
+snapshot contents; the I<--snapshots-metadata> flag only removes
+metadata that can later be recreated, but requires newer servers.  If
+the domain is persistent, these flags are ignored.
+
 =item B<domblkstat> I<domain> I<block-device>

 Get device block stats for a running domain.
@@ -867,7 +876,7 @@ The I<--maximum> flag controls the maximum number of virtual cpus that can
 be hot-plugged the next time the domain is booted.  As such, it must only be
 used with the I<--config> flag, and not with the I<--live> flag.

-=item B<shutdown> I<domain-id>
+=item B<shutdown> I<domain-id> [I<--snapshots-full>] [I<--snapshots-metadata>]

 Gracefully shuts down a domain.  This coordinates with the domain OS
 to perform graceful shutdown, so there is no guarantee that it will
@@ -877,6 +886,15 @@ services must be shutdown in the domain.
 The exact behavior of a domain when it shuts down is set by the
 I<on_shutdown> parameter in the domain's XML definition.

+The I<--snapshots-full> and I<--snapshots-metadata> flags guarantee
+that any snapshots (see the B<snapshot-list> command) are also cleaned
+up when shutting down a transient domain.  Without either flag, attempts
+to shutdown a transient domain with snapshot metadata will fail.  The
+I<--snapshots-full> flag works with older servers, but loses all
+snapshot contents; the I<--snapshots-metadata> flag only removes
+metadata that can later be recreated, but requires newer servers.  If
+the domain is persistent, these flags are ignored.
+
 =item B<start> I<domain-name> [I<--console>] [I<--paused>] [I<--autodestroy>]
 [I<--bypass-cache>]

@@ -907,16 +925,26 @@ hypervisor.
 Output the device used for the TTY console of the domain. If the information
 is not available the processes will provide an exit code of 1.

-=item B<undefine> I<domain-id> [I<--managed-save>]
+=item B<undefine> I<domain-id> [I<--managed-save>] [I<--snapshots-full>]
+[I<--snapshots-metadata]

 Undefine a domain. If the domain is running, this converts it to a
 transient domain, without stopping it. If the domain is inactive,
 the domain configuration is removed.

-The I<--managed-save> flag guarantees that any managed save image(see
+The I<--managed-save> flag guarantees that any managed save image (see
 the B<managedsave> command) is also cleaned up.  Without the flag, attempts
 to undefine a domain with a managed save image will fail.

+The I<--snapshots-full> and I<--snapshots-metadata> flags guarantee
+that any snapshots (see the B<snapshot-list> command) are also cleaned
+up when undefining an inactive domain.  Without either flag, attempts
+to undefine an inactive domain with snapshot metadata will fail.  The
+I<--snapshots-full> flag works with older servers, but loses all
+snapshot contents; the I<--snapshots-metadata> flag only removes
+metadata that can later be recreated, but requires newer servers.  If
+the domain is active, these flags are ignored.
+
 NOTE: For an inactive domain, the domain name or UUID must be used as the
 I<domain-id>.

-- 
1.7.4.4


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]