snapshot: virsh shorthand for operating on current snap

Rather than having to do:

$ virsh snapshot-revert dom $(virsh snapshot-current dom --name)

I thought it would be nice to do:

$ virsh snapshot-revert dom --current

I didn't add 'virsh snapshot-dumpxml --current' since we already have
'virsh snapshot-current' for the same task.  snapshot-list accepted
a name but did not require it, and that remains the case, with
--current serving in place of that name.  For all other commands,
name used to be required, and can now be replaced by --current;
I intentionally made it so that omitting both --current and a name
is an error (having the absence of a name imply --current seems
just a bit too magic, so --current must be explicit).  I also had
to keep snapshot-edit backwards-compatible, as the only command
that already had a --current argument alongside a name, which still
works to both edit a named snapshot and make it current.

* tools/virsh.c (vshLookupSnapshot): New helper function.
(cmdSnapshotEdit, cmdSnapshotList, cmdSnapshotParent)
(cmdSnapshotDelete, cmdDomainSnapshotRevert): Use it, adding an
option where needed.
* tools/virsh.pod (snapshot-delete, snapshot-edit)
(snapshot-list, snapshot-parent, snapshot-revert): Document
use of --current.
(snapshot-dumpxml): Mention alternative.

tools/virsh.c

@@ -12817,6 +12817,45 @@ cleanup:
     return ret;
 }
 
+/* Helper for resolving {--current | --ARG name} into a snapshot
+ * belonging to DOM.  If EXCLUSIVE, fail if both --current and arg are
+ * present.  On success, populate *SNAP and *NAME, before returning 0.
+ * On failure, return -1 after issuing an error message.  */
+static int
+vshLookupSnapshot(vshControl *ctl, const vshCmd *cmd,
+                  const char *arg, bool exclusive, virDomainPtr dom,
+                  virDomainSnapshotPtr *snap, const char **name)
+{
+    bool current = vshCommandOptBool(cmd, "current");
+    const char *snapname = NULL;
+
+    if (vshCommandOptString(cmd, arg, &snapname) < 0) {
+        vshError(ctl, _("invalid argument for --%s"), arg);
+        return -1;
+    }
+
+    if (exclusive && current && snapname) {
+        vshError(ctl, _("--%s and --current are mutually exclusive"), arg);
+        return -1;
+    }
+
+    if (snapname) {
+        *snap = virDomainSnapshotLookupByName(dom, snapname, 0);
+    } else if (current) {
+        *snap = virDomainSnapshotCurrent(dom, 0);
+    } else {
+        vshError(ctl, _("--%s or --current is required"), arg);
+        return -1;
+    }
+    if (!*snap) {
+        virshReportError(ctl);
+        return -1;
+    }
+
+    *name = virDomainSnapshotGetName(*snap);
+    return 0;
+}
+
 /*
  * "snapshot-edit" command
  */
@@ -12828,7 +12867,7 @@ static const vshCmdInfo info_snapshot_edit[] = {
 
 static const vshCmdOptDef opts_snapshot_edit[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
-    {"snapshotname", VSH_OT_DATA, VSH_OFLAG_REQ, N_("snapshot name")},
+    {"snapshotname", VSH_OT_DATA, 0, N_("snapshot name")},
     {"current", VSH_OT_BOOL, 0, N_("also set edited snapshot as current")},
     {"rename", VSH_OT_BOOL, 0, N_("allow renaming an existing snapshot")},
     {"clone", VSH_OT_BOOL, 0, N_("allow cloning to new name")},
@@ -12858,21 +12897,19 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd)
         return false;
     }
 
-    if (vshCommandOptBool(cmd, "current"))
+    if (vshCommandOptBool(cmd, "current") &&
+        vshCommandOptBool(cmd, "snapshotname"))
         define_flags |= VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT;
 
     if (!vshConnectionUsability(ctl, ctl->conn))
         return false;
 
-    if (vshCommandOptString(cmd, "snapshotname", &name) <= 0)
-        goto cleanup;
-
     dom = vshCommandOptDomain(ctl, cmd, NULL);
     if (dom == NULL)
         goto cleanup;
 
-    snapshot = virDomainSnapshotLookupByName(dom, name, 0);
-    if (snapshot == NULL)
+    if (vshLookupSnapshot(ctl, cmd, "snapshotname", false, dom,
+                          &snapshot, &name) < 0)
         goto cleanup;
 
     /* Get the XML configuration of the snapshot.  */
@@ -13147,6 +13184,8 @@ static const vshCmdOptDef opts_snapshot_list[] = {
      N_("list only snapshots that have metadata that would prevent undefine")},
     {"tree", VSH_OT_BOOL, 0, N_("list snapshots in a tree")},
     {"from", VSH_OT_DATA, 0, N_("limit list to children of given snapshot")},
+    {"current", VSH_OT_BOOL, 0,
+     N_("limit list to children of current snapshot")},
     {"descendants", VSH_OT_BOOL, 0, N_("with --from, list all descendants")},
     {NULL, 0, 0, NULL}
 };
@@ -13180,10 +13219,17 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
     int start_index = -1;
     bool descendants = false;
 
-    if (vshCommandOptString(cmd, "from", &from) < 0) {
-        vshError(ctl, _("invalid from argument '%s'"), from);
+    if (!vshConnectionUsability(ctl, ctl->conn))
+        goto cleanup;
+
+    dom = vshCommandOptDomain(ctl, cmd, NULL);
+    if (dom == NULL)
+        goto cleanup;
+
+    if ((vshCommandOptBool(cmd, "from") ||
+         vshCommandOptBool(cmd, "current")) &&
+        vshLookupSnapshot(ctl, cmd, "from", true, dom, &start, &from) < 0)
         goto cleanup;
-    }
 
     if (vshCommandOptBool(cmd, "parent")) {
         if (vshCommandOptBool(cmd, "roots")) {
@@ -13214,18 +13260,8 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
         flags |= VIR_DOMAIN_SNAPSHOT_LIST_METADATA;
     }
 
-    if (!vshConnectionUsability(ctl, ctl->conn))
-        goto cleanup;
-
-    dom = vshCommandOptDomain(ctl, cmd, NULL);
-    if (dom == NULL)
-        goto cleanup;
-
     if (from) {
         descendants = vshCommandOptBool(cmd, "descendants");
-        start = virDomainSnapshotLookupByName(dom, from, 0);
-        if (!start)
-            goto cleanup;
         if (descendants || tree) {
             flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
         }
@@ -13556,7 +13592,8 @@ static const vshCmdInfo info_snapshot_parent[] = {
 
 static const vshCmdOptDef opts_snapshot_parent[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
-    {"snapshotname", VSH_OT_DATA, VSH_OFLAG_REQ, N_("snapshot name")},
+    {"snapshotname", VSH_OT_DATA, 0, N_("find parent of snapshot name")},
+    {"current", VSH_OT_BOOL, 0, N_("find parent of current snapshot")},
     {NULL, 0, 0, NULL}
 };
 
@@ -13576,11 +13613,8 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd)
     if (dom == NULL)
         goto cleanup;
 
-    if (vshCommandOptString(cmd, "snapshotname", &name) <= 0)
-        goto cleanup;
-
-    snapshot = virDomainSnapshotLookupByName(dom, name, 0);
-    if (snapshot == NULL)
+    if (vshLookupSnapshot(ctl, cmd, "snapshotname", true, dom,
+                          &snapshot, &name) < 0)
         goto cleanup;
 
     if (vshGetSnapshotParent(ctl, snapshot, &parent) < 0)
@@ -13615,7 +13649,8 @@ static const vshCmdInfo info_snapshot_revert[] = {
 
 static const vshCmdOptDef opts_snapshot_revert[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
-    {"snapshotname", VSH_OT_DATA, VSH_OFLAG_REQ, N_("snapshot name")},
+    {"snapshotname", VSH_OT_DATA, 0, N_("snapshot name")},
+    {"current", VSH_OT_BOOL, 0, N_("revert to current snapshot")},
     {"running", VSH_OT_BOOL, 0, N_("after reverting, change state to running")},
     {"paused", VSH_OT_BOOL, 0, N_("after reverting, change state to paused")},
     {"force", VSH_OT_BOOL, 0, N_("try harder on risky reverts")},
@@ -13651,11 +13686,8 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd)
     if (dom == NULL)
         goto cleanup;
 
-    if (vshCommandOptString(cmd, "snapshotname", &name) <= 0)
-        goto cleanup;
-
-    snapshot = virDomainSnapshotLookupByName(dom, name, 0);
-    if (snapshot == NULL)
+    if (vshLookupSnapshot(ctl, cmd, "snapshotname", true, dom,
+                          &snapshot, &name) < 0)
         goto cleanup;
 
     result = virDomainRevertToSnapshot(snapshot, flags);
@@ -13691,7 +13723,8 @@ static const vshCmdInfo info_snapshot_delete[] = {
 
 static const vshCmdOptDef opts_snapshot_delete[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
-    {"snapshotname", VSH_OT_DATA, VSH_OFLAG_REQ, N_("snapshot name")},
+    {"snapshotname", VSH_OT_DATA, 0, N_("snapshot name")},
+    {"current", VSH_OT_BOOL, 0, N_("delete current snapshot")},
     {"children", VSH_OT_BOOL, 0, N_("delete snapshot and all children")},
     {"children-only", VSH_OT_BOOL, 0, N_("delete children but not snapshot")},
     {"metadata", VSH_OT_BOOL, 0,
@@ -13715,7 +13748,8 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd)
     if (dom == NULL)
         goto cleanup;
 
-    if (vshCommandOptString(cmd, "snapshotname", &name) <= 0)
+    if (vshLookupSnapshot(ctl, cmd, "snapshotname", true, dom,
+                          &snapshot, &name) < 0)
         goto cleanup;
 
     if (vshCommandOptBool(cmd, "children"))
@@ -13725,10 +13759,6 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd)
     if (vshCommandOptBool(cmd, "metadata"))
         flags |= VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY;
 
-    snapshot = virDomainSnapshotLookupByName(dom, name, 0);
-    if (snapshot == NULL)
-        goto cleanup;
-
     /* XXX If we wanted, we could emulate DELETE_CHILDREN_ONLY even on
      * older servers that reject the flag, by manually computing the
      * list of descendants.  But that's a lot of code to maintain.  */

tools/virsh.pod

@@ -1951,12 +1951,14 @@ the XML.
 With I<snapshotname>, this is a request to make the existing named
 snapshot become the current snapshot, without reverting the domain.
 
-=item B<snapshot-edit> I<domain> I<snapshotname> [I<--current>]
+=item B<snapshot-edit> I<domain> [I<snapshotname>] [I<--current>]
 {[I<--rename>] | [I<--clone>]}
 
 Edit the XML configuration file for I<snapshotname> of a domain.  If
-I<--current> is specified, also force the edited snapshot to become
-the current snapshot.
+both I<snapshotname> and I<--current> are specified, also force the
+edited snapshot to become the current snapshot.  If I<snapshotname>
+is omitted, then I<--current> must be supplied, to edit the current
+snapshot.
 
 This is equivalent to:
 
@@ -1978,7 +1980,7 @@ snapshots, such as internal snapshots within a single qcow2 file, are
 accessible only from the original name.
 
 =item B<snapshot-list> I<domain> [{I<--parent> | I<--roots> | I<--tree>}]
-[I<--metadata>] [[I<--from>] B<snapshot> [I<--descendants>]]
+[I<--metadata>] [{[I<--from>] B<snapshot> | I<--current>} [I<--descendants>]]
 
 List all of the available snapshots for the given domain, defaulting
 to show columns for the snapshot name, creation time, and domain state.
@@ -1990,7 +1992,8 @@ If I<--tree> is specified, the output will be in a tree format, listing
 just snapshot names.  These three options are mutually exclusive.
 
 If I<--from> is provided, filter the list to snapshots which are
-children of the given B<snapshot>.  When used in isolation or with
+children of the given B<snapshot>; or if I<--current> is provided,
+start at the current snapshot.  When used in isolation or with
 I<--parent>, the list is limited to direct children unless
 I<--descendants> is also present.  When used with I<--tree>, the
 use of I<--descendants> is implied.  This option is not compatible
@@ -2005,15 +2008,18 @@ a transient domain.
 
 Output the snapshot XML for the domain's snapshot named I<snapshot>.
 Using I<--security-info> will also include security sensitive information.
+Use B<snapshot-current> to easily access the XML of the current snapshot.
 
-=item B<snapshot-parent> I<domain> I<snapshot>
+=item B<snapshot-parent> I<domain> {I<snapshot> | I<--current>}
 
-Output the name of the parent snapshot for the given I<snapshot>, if any.
+Output the name of the parent snapshot, if any, for the given
+I<snapshot>, or for the current snapshot with I<--current>.
 
-=item B<snapshot-revert> I<domain> I<snapshot> [{I<--running> | I<--paused>}]
-[I<--force>]
+=item B<snapshot-revert> I<domain> {I<snapshot> | I<--current>}
+[{I<--running> | I<--paused>}] [I<--force>]
 
-Revert the given domain to the snapshot specified by I<snapshot>.  Be aware
+Revert the given domain to the snapshot specified by I<snapshot>, or to
+the current snapshot with I<--current>.  Be aware
 that this is a destructive action; any changes in the domain since the last
 snapshot was taken will be lost.  Also note that the state of the domain after
 snapshot-revert is complete will be the state of the domain at the time
@@ -2043,10 +2049,11 @@ snapshot that uses a provably incompatible configuration, as well as
 with an inactive snapshot that is combined with the I<--start> or
 I<--pause> flag.
 
-=item B<snapshot-delete> I<domain> I<snapshot> [I<--metadata>]
+=item B<snapshot-delete> I<domain> {I<snapshot> | I<--current>} [I<--metadata>]
 [{I<--children> | I<--children-only>}]
 
-Delete the snapshot for the domain named I<snapshot>.  If this snapshot
+Delete the snapshot for the domain named I<snapshot>, or the current
+snapshot with I<--current>.  If this snapshot
 has child snapshots, changes from this snapshot will be merged into the
 children.  If I<--children> is passed, then delete this snapshot and any
 children of this snapshot.  If I<--children-only> is passed, then delete