snapshot: wire up disk-only flag to snapshot-create

Expose the disk-only flag through virsh.  Additionally, make
virsh snapshot-create-as take an arbitrary number of diskspecs,
which can be used to build up the xml for <domainsnapshot>.

* tools/virsh.c (cmdSnapshotCreate): Add --disk-only.
(cmdSnapshotCreateAs): Likewise, and add argv diskspec.
(vshParseSnapshotDiskspec): New helper function.
(vshCmddefGetOption): Allow naming of argv field.
* tools/virsh.pod (snapshot-create, snapshot-create-as): Document
them.
* tests/virsh-optparse: Test snapshot-create-as parsing.

tests/virsh-optparse

@@ -67,6 +67,26 @@ for args in \
   virsh -d0 -c $test_url setvcpus $args >out 2>>err || fail=1
   LC_ALL=C sort out | compare - exp-out || fail=1
 done
+
+# Another complex parsing example
+cat <<\EOF > exp-out || framework_failure
+<domainsnapshot>
+  <description>1&lt;2</description>
+  <disks>
+    <disk name='vda' snapshot='external'>
+      <source file='a&amp;b,c'/>
+    </disk>
+    <disk name='vdb'/>
+  </disks>
+</domainsnapshot>
+
+
+EOF
+virsh -c $test_url snapshot-create-as --print-xml test \
+  --diskspec 'vda,file=a&b,,c,snapshot=external' --description '1<2' \
+  --diskspec vdb >out 2>>err || fail=1
+compare out exp-out || fail=1
+
 test -s err && fail=1
 
 (exit $fail); exit $fail

tools/virsh.c

@@ -12166,6 +12166,7 @@ static const vshCmdOptDef opts_snapshot_create[] = {
     {"current", VSH_OT_BOOL, 0, N_("with redefine, set current snapshot")},
     {"no-metadata", VSH_OT_BOOL, 0, N_("take snapshot but create no metadata")},
     {"halt", VSH_OT_BOOL, 0, N_("halt domain after snapshot is created")},
+    {"disk-only", VSH_OT_BOOL, 0, N_("capture disk state but not vm state")},
     {NULL, 0, 0, NULL}
 };
 
@@ -12186,6 +12187,8 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA;
     if (vshCommandOptBool(cmd, "halt"))
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_HALT;
+    if (vshCommandOptBool(cmd, "disk-only"))
+        flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY;
 
     if (!vshConnectionUsability(ctl, ctl->conn))
         goto cleanup;
@@ -12224,6 +12227,62 @@ cleanup:
 /*
  * "snapshot-create-as" command
  */
+static int
+vshParseSnapshotDiskspec(vshControl *ctl, virBufferPtr buf, const char *str)
+{
+    int ret = -1;
+    char *name = NULL;
+    char *snapshot = NULL;
+    char *driver = NULL;
+    char *file = NULL;
+    char *spec = vshStrdup(ctl, str);
+    char *tmp = spec;
+    size_t len = strlen(str);
+
+    if (*str == ',')
+        goto cleanup;
+    name = tmp;
+    while ((tmp = strchr(tmp, ','))) {
+        if (tmp[1] == ',') {
+            /* Recognize ,, as an escape for a literal comma */
+            memmove(&tmp[1], &tmp[2], len - (tmp - spec) + 2);
+            len--;
+            tmp++;
+            continue;
+        }
+        /* Terminate previous string, look for next recognized one */
+        *tmp++ = '\0';
+        if (!snapshot && STRPREFIX(tmp, "snapshot="))
+            snapshot = tmp + strlen("snapshot=");
+        else if (!driver && STRPREFIX(tmp, "driver="))
+            driver = tmp + strlen("driver=");
+        else if (!file && STRPREFIX(tmp, "file="))
+            file = tmp + strlen("file=");
+        else
+            goto cleanup;
+    }
+
+    virBufferEscapeString(buf, "    <disk name='%s'", name);
+    if (snapshot)
+        virBufferAsprintf(buf, " snapshot='%s'", snapshot);
+    if (driver || file) {
+        virBufferAddLit(buf, ">\n");
+        if (driver)
+            virBufferAsprintf(buf, "      <driver type='%s'/>\n", driver);
+        if (file)
+            virBufferEscapeString(buf, "      <source file='%s'/>\n", file);
+        virBufferAddLit(buf, "    </disk>\n");
+    } else {
+        virBufferAddLit(buf, "/>\n");
+    }
+    ret = 0;
+cleanup:
+    if (ret < 0)
+        vshError(ctl, _("unable to parse diskspec: %s"), str);
+    VIR_FREE(spec);
+    return ret;
+}
+
 static const vshCmdInfo info_snapshot_create_as[] = {
     {"help", N_("Create a snapshot from a set of args")},
     {"desc", N_("Create a snapshot (disk and RAM) from arguments")},
@@ -12237,6 +12296,9 @@ static const vshCmdOptDef opts_snapshot_create_as[] = {
     {"print-xml", VSH_OT_BOOL, 0, N_("print XML document rather than create")},
     {"no-metadata", VSH_OT_BOOL, 0, N_("take snapshot but create no metadata")},
     {"halt", VSH_OT_BOOL, 0, N_("halt domain after snapshot is created")},
+    {"disk-only", VSH_OT_BOOL, 0, N_("capture disk state but not vm state")},
+    {"diskspec", VSH_OT_ARGV, 0,
+     N_("disk attributes: disk[,snapshot=type][,driver=type][,file=name]")},
     {NULL, 0, 0, NULL}
 };
 
@@ -12250,11 +12312,14 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
     const char *desc = NULL;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
     unsigned int flags = 0;
+    const vshCmdOpt *opt = NULL;
 
     if (vshCommandOptBool(cmd, "no-metadata"))
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA;
     if (vshCommandOptBool(cmd, "halt"))
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_HALT;
+    if (vshCommandOptBool(cmd, "disk-only"))
+        flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY;
 
     if (!vshConnectionUsability(ctl, ctl->conn))
         goto cleanup;
@@ -12274,6 +12339,16 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
         virBufferEscapeString(&buf, "  <name>%s</name>\n", name);
     if (desc)
         virBufferEscapeString(&buf, "  <description>%s</description>\n", desc);
+    if (vshCommandOptBool(cmd, "diskspec")) {
+        virBufferAddLit(&buf, "  <disks>\n");
+        while ((opt = vshCommandOptArgv(cmd, opt))) {
+            if (vshParseSnapshotDiskspec(ctl, &buf, opt->data) < 0) {
+                virBufferFreeAndReset(&buf);
+                goto cleanup;
+            }
+        }
+        virBufferAddLit(&buf, "  </disks>\n");
+    }
     virBufferAddLit(&buf, "</domainsnapshot>\n");
 
     buffer = virBufferContentAndReset(&buf);
@@ -12283,11 +12358,6 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
     }
 
     if (vshCommandOptBool(cmd, "print-xml")) {
-        if (vshCommandOptBool(cmd, "halt")) {
-            vshError(ctl, "%s",
-                     _("--print-xml and --halt are mutually exclusive"));
-            goto cleanup;
-        }
         vshPrint(ctl, "%s\n",  buffer);
         ret = true;
         goto cleanup;
@@ -13437,12 +13507,8 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char *name,
                 vshError(ctl, _("option --%s already seen"), name);
                 return NULL;
             }
-            if (opt->type == VSH_OT_ARGV) {
+            if (opt->type != VSH_OT_ARGV)
-                vshError(ctl, _("variable argument <%s> "
+                *opts_seen |= 1 << i;
-                         "should not be used with --<%s>"), name, name);
-                return NULL;
-            }
-            *opts_seen |= 1 << i;
             return opt;
         }
     }

tools/virsh.pod

@@ -1718,11 +1718,12 @@ used to represent properties of snapshots.
 =over 4
 
 =item B<snapshot-create> I<domain> [I<xmlfile>] {[I<--redefine> [I<--current>]]
-| [I<--no-metadata>] [I<--halt>]}
+| [I<--no-metadata>] [I<--halt>] [I<--disk-only>]}
 
 Create a snapshot for domain I<domain> with the properties specified in
 I<xmlfile>.  Normally, the only properties settable for a domain snapshot
-are the <name> and <description> elements; the rest of the fields are
+are the <name> and <description> elements, as well as <disks> if
+I<--disk-only> is given; the rest of the fields are
 ignored, and automatically filled in by libvirt.  If I<xmlfile> is
 completely omitted, then libvirt will choose a value for all fields.
 The new snapshot will become current, as listed by B<snapshot-current>.
@@ -1730,6 +1731,14 @@ The new snapshot will become current, as listed by B<snapshot-current>.
 If I<--halt> is specified, the domain will be left in an inactive state
 after the snapshot is created.
 
+If I<--disk-only> is specified, the snapshot will only include disk
+state rather than the usual system checkpoint with vm state.  Disk
+snapshots are faster than full system checkpoints, but reverting to a
+disk snapshot may require fsck or journal replays, since it is like
+the disk state at the point when the power cord is abruptly pulled;
+and mixing I<--halt> and I<--disk-only> loses any data that was not
+flushed to disk at the time.
+
 If I<--redefine> is specified, then all XML elements produced by
 B<snapshot-dumpxml> are valid; this can be used to migrate snapshot
 hierarchy from one machine to another, to recreate hierarchy for the
@@ -1753,13 +1762,26 @@ by command such as B<destroy> or by internal guest action).
 
 =item B<snapshot-create-as> I<domain> {[I<--print-xml>]
 | [I<--no-metadata>] [I<--halt>]} [I<name>] [I<description>]
+[I<--disk-only> [I<diskspec>]...]
 
 Create a snapshot for domain I<domain> with the given <name> and
 <description>; if either value is omitted, libvirt will choose a
 value.  If I<--print-xml> is specified, then XML appropriate for
 I<snapshot-create> is output, rather than actually creating a snapshot.
 Otherwise, if I<--halt> is specified, the domain will be left in an
-inactive state after the snapshot is created.
+inactive state after the snapshot is created, and if I<--disk-only>
+is specified, the snapshot will not include vm state.
+
+The I<--disk-only> flag is used to request a disk-only snapshot.  When
+this flag is in use, the command can also take additional I<diskspec>
+arguments to add <disk> elements to the xml.  Each <diskspec> is in the
+form B<disk[,snapshot=type][,driver=type][,file=name]>.  To include a
+literal comma in B<disk> or in B<file=name>, escape it with a second
+comma.  For example, a diskspec of "vda,snapshot=external,file=/path/to,,new"
+results in the following XML:
+  <disk name='vda' snapshot='external'>
+    <source file='/path/to,new'/>
+  </disk>
 
 If I<--no-metadata> is specified, then the snapshot data is created,
 but any metadata is immediately discarded (that is, libvirt does not