With this patch, it is hopefully a bit more obvious that for snapshot-create-as, a literal '--diskspec' is mandatory if name or description was omitted, but optional if all earlier options were provided. These all denote two diskspecs and a description: virsh snapshot-create-as dom name desc vda vdb virsh snapshot-create-as dom name desc --diskspec vda --diskspec vdb virsh snapshot-create-as dom name desc --diskspec vda vdb virsh snapshot-create-as dom name desc vda --diskspec vdb virsh snapshot-create-as dom name desc --diskspec vda --diskspec vdb This gives two diskspecs but no description: virsh snapshot-create-as dom name --diskspec vda --diskspec vdb And this treats 'vda' as the description, with only one diskspec: virsh snapshot-create-as dom name vda vdb The help output now shows: snapshot-create-as <domain> [<name>] [<description>] [--print-xml] [--no-metadata] [--halt] [--disk-only] [[--diskspec] <string>]... I also checked the help output for echo and send-key, which are two other variants of argv commands. * tools/virsh.pod (snapshot-create-as): Document when a literal --diskspec must preceed a diskspec argument. * tools/virsh.c (vshCmddefHelp): Update help output for argv when naming the option is useful. (vshCmddefGetData): Fix logic on when argv was seen. Reported by Nan Zhang. --- I think this is more comprehensive than Nan's patch, for nicer results. tools/virsh.c | 27 +++++++++++++++++++++------ tools/virsh.pod | 6 ++++-- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 3c6e65a..fdb503a 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13856,9 +13856,10 @@ vshCmddefGetData(const vshCmdDef *cmd, uint32_t *opts_need_arg, /* Grab least-significant set bit */ i = ffs(*opts_need_arg) - 1; opt = &cmd->opts[i]; - if (opt->type != VSH_OT_ARGV) + if (opt->type != VSH_OT_ARGV) { *opts_need_arg &= ~(1 << i); - *opts_seen |= 1 << i; + *opts_seen |= 1 << i; + } return opt; } @@ -13956,6 +13957,7 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname) char buf[256]; uint32_t opts_need_arg; uint32_t opts_required; + bool shortopt = false; /* true if 'arg' works instead of '--opt arg' */ if (vshCmddefOptParse(def, &opts_need_arg, &opts_required)) { vshError(ctl, _("internal error: bad options in command: '%s'"), @@ -13980,18 +13982,30 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname) /* xgettext:c-format */ fmt = ((opt->flags & VSH_OFLAG_REQ) ? "<%s>" : _("[--%s <number>]")); + if (!(opt->flags & VSH_OFLAG_REQ_OPT)) + shortopt = true; break; case VSH_OT_STRING: /* xgettext:c-format */ fmt = _("[--%s <string>]"); + if (!(opt->flags & VSH_OFLAG_REQ_OPT)) + shortopt = true; break; case VSH_OT_DATA: fmt = ((opt->flags & VSH_OFLAG_REQ) ? "<%s>" : "[<%s>]"); + if (!(opt->flags & VSH_OFLAG_REQ_OPT)) + shortopt = true; break; case VSH_OT_ARGV: /* xgettext:c-format */ - fmt = (opt->flags & VSH_OFLAG_REQ) ? _("<%s>...") - : _("[<%s>]..."); + if (shortopt) { + fmt = (opt->flags & VSH_OFLAG_REQ) + ? _("{[--%s] <string>}...") + : _("[[--%s] <string>]..."); + } else { + fmt = (opt->flags & VSH_OFLAG_REQ) ? _("<%s>...") + : _("[<%s>]..."); + } break; default: assert(0); @@ -14030,8 +14044,9 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname) opt->name); break; case VSH_OT_ARGV: - /* Not really an option. */ - snprintf(buf, sizeof(buf), _("<%s>"), opt->name); + snprintf(buf, sizeof(buf), + shortopt ? _("[--%s] <string>") : _("<%s>"), + opt->name); break; default: assert(0); diff --git a/tools/virsh.pod b/tools/virsh.pod index e82567d..126ace6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1773,7 +1773,7 @@ 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>]...] +[I<--disk-only> [[I<--diskspec>] B<diskspec>]... Create a snapshot for domain I<domain> with the given <name> and <description>; if either value is omitted, libvirt will choose a @@ -1788,7 +1788,9 @@ 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" +comma. A literal I<--diskspec> must preceed each B<diskspec> unless +all three of I<domain>, I<name>, and I<description> are also present. +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'/> -- 1.7.4.4 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list