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. --- I went ahead and implemented a decent solution to the snapshot-create-as needing to provide multiple disk specs when generating xml. tests/virsh-optparse | 20 +++++++++++ tools/virsh.c | 88 +++++++++++++++++++++++++++++++++++++++++++------ tools/virsh.pod | 28 ++++++++++++++-- 3 files changed, 122 insertions(+), 14 deletions(-) diff --git a/tests/virsh-optparse b/tests/virsh-optparse index 7b3a25d..cd5e3eb 100755 --- a/tests/virsh-optparse +++ b/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<2</description> + <disks> + <disk name='vda' snapshot='external'> + <source file='a&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 diff --git a/tools/virsh.c b/tools/virsh.c index 648e62c..c430cfb 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12153,6 +12153,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} }; @@ -12173,6 +12174,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; @@ -12211,6 +12214,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")}, @@ -12224,6 +12283,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} }; @@ -12237,11 +12299,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; @@ -12261,6 +12326,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); @@ -12270,11 +12345,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; @@ -13327,12 +13397,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) { - vshError(ctl, _("variable argument <%s> " - "should not be used with --<%s>"), name, name); - return NULL; - } - *opts_seen |= 1 << i; + if (opt->type != VSH_OT_ARGV) + *opts_seen |= 1 << i; return opt; } } diff --git a/tools/virsh.pod b/tools/virsh.pod index 0d1ddd1..e0b9bef 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1706,11 +1706,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>. @@ -1718,6 +1719,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 @@ -1741,13 +1750,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 -- 1.7.4.4 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list