[libvirt] [PATCH] virsh: consolidate memtune docs
* tools/virsh.pod (memtune): Drop second copy, fill to 80 columns, enhance wording. --- Seeing memtune twice in the man page is a bit confusing. This patch doesn't touch the pending issue of whether --min_guarantee makes sense, or should be deleted before it becomes a pre-mature API addition. Also, should memtune be changed to use VSH_OT_INT rather than VSH_OT_STRING? The current 'virsh help memtune' output looks weird: OPTIONS [--domain] <string> domain name, id or uuid --hard_limit <string> Max memory in kilobytes --soft_limit <string> Memory during contention in kilobytes --swap_hard_limit <string> Max swap in kilobytes --min_guarantee <string> Min guaranteed memory in kilobytes tools/virsh.pod | 15 ++++++++------- 1 files changed, 8 insertions(+), 7 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 943a563..bc68146 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -513,13 +513,14 @@ Change the maximum memory allocation limit in the guest domain. This should not change the current memory use. The memory limit is specified in kilobytes. -=item B<memtune> I<domain-id> - -Displays the domain memory parameters. - -=item B<memtune> I<domain-id> optional I<--hard-limit> B<kilobytes> optional I<--soft-limit> B<kilobytes> optional I<--swap-hard-limit> B<kilobytes> -I<--min-guarantee> B<kilobytes> - -Allows you to set the domain memory parameters. LXC and QEMU/KVM supports these parameters. +=item B<memtune> I<domain-id> optional I<--hard-limit> B<kilobytes> +optional I<--soft-limit> B<kilobytes> optional I<--swap-hard-limit> +B<kilobytes> -I<--min-guarantee> B<kilobytes> + +Allows you to display or set the domain memory parameters. Without +flags, the current settings are displayed; with a flag, the +appropriate limit is adjusted if supported by the hypervisor. LXC and +QEMU/KVM supports I<--hard-limit>, I<--soft-limit>, and I<--swap-hard-limit>. =item B<setvcpus> I<domain-id> I<count> -- 1.7.2.3
On Tue, Oct 19, 2010 at 09:38:05AM -0600, Eric Blake wrote:
* tools/virsh.pod (memtune): Drop second copy, fill to 80 columns, enhance wording. ---
Seeing memtune twice in the man page is a bit confusing.
ACK to the cleanup , I didn't realized it went to a different part ...
This patch doesn't touch the pending issue of whether --min_guarantee makes sense, or should be deleted before it becomes a pre-mature API addition.
Also, should memtune be changed to use VSH_OT_INT rather than VSH_OT_STRING? The current 'virsh help memtune' output looks weird:
OPTIONS [--domain] <string> domain name, id or uuid --hard_limit <string> Max memory in kilobytes --soft_limit <string> Memory during contention in kilobytes --swap_hard_limit <string> Max swap in kilobytes --min_guarantee <string> Min guaranteed memory in kilobytes
hum, probably, yes :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
* tools/virsh.c (opts_freecell, opts_memtune, opts_vcpupin) (opts_setvcpus, opts_setmaxmem, opts_setmem) (opts_migrate_setmaxdowntime): Use VSH_OT_INT when only an integer is expected. --- I've pushed the virsh.pod cleanup.
Also, should memtune be changed to use VSH_OT_INT rather than VSH_OT_STRING? The current 'virsh help memtune' output looks weird: hum, probably, yes :-)
Is this okay to push? tools/virsh.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 0d078b0..cd4485f 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2247,7 +2247,7 @@ static const vshCmdInfo info_freecell[] = { }; static const vshCmdOptDef opts_freecell[] = { - {"cellno", VSH_OT_DATA, 0, N_("NUMA cell number")}, + {"cellno", VSH_OT_INT, 0, N_("NUMA cell number")}, {NULL, 0, 0, NULL} }; @@ -2583,7 +2583,7 @@ static const vshCmdInfo info_vcpupin[] = { static const vshCmdOptDef opts_vcpupin[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"vcpu", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vcpu number")}, + {"vcpu", VSH_OT_INT, VSH_OFLAG_REQ, N_("vcpu number")}, {"cpulist", VSH_OT_DATA, VSH_OFLAG_REQ, N_("host cpu number(s) (comma separated)")}, {NULL, 0, 0, NULL} }; @@ -2719,7 +2719,7 @@ static const vshCmdInfo info_setvcpus[] = { static const vshCmdOptDef opts_setvcpus[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"count", VSH_OT_DATA, VSH_OFLAG_REQ, N_("number of virtual CPUs")}, + {"count", VSH_OT_INT, VSH_OFLAG_REQ, N_("number of virtual CPUs")}, {"maximum", VSH_OT_BOOL, 0, N_("set maximum limit on next boot")}, {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, @@ -2772,7 +2772,7 @@ static const vshCmdInfo info_setmem[] = { static const vshCmdOptDef opts_setmem[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"kilobytes", VSH_OT_DATA, VSH_OFLAG_REQ, N_("number of kilobytes of memory")}, + {"kilobytes", VSH_OT_INT, VSH_OFLAG_REQ, N_("number of kilobytes of memory")}, {NULL, 0, 0, NULL} }; @@ -2829,7 +2829,7 @@ static const vshCmdInfo info_setmaxmem[] = { static const vshCmdOptDef opts_setmaxmem[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"kilobytes", VSH_OT_DATA, VSH_OFLAG_REQ, N_("maximum memory limit in kilobytes")}, + {"kilobytes", VSH_OT_INT, VSH_OFLAG_REQ, N_("maximum memory limit in kilobytes")}, {NULL, 0, 0, NULL} }; @@ -3453,7 +3453,7 @@ static const vshCmdInfo info_migrate_setmaxdowntime[] = { static const vshCmdOptDef opts_migrate_setmaxdowntime[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"downtime", VSH_OT_DATA, VSH_OFLAG_REQ, N_("maximum tolerable downtime (in milliseconds) for migration")}, + {"downtime", VSH_OT_INT, VSH_OFLAG_REQ, N_("maximum tolerable downtime (in milliseconds) for migration")}, {NULL, 0, 0, NULL} }; -- 1.7.2.3
On 10/19/2010 10:40 AM, Eric Blake wrote:
* tools/virsh.c (opts_freecell, opts_memtune, opts_vcpupin) (opts_setvcpus, opts_setmaxmem, opts_setmem) (opts_migrate_setmaxdowntime): Use VSH_OT_INT when only an integer is expected. ---
@@ -2583,7 +2583,7 @@ static const vshCmdInfo info_vcpupin[] = {
static const vshCmdOptDef opts_vcpupin[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"vcpu", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vcpu number")}, + {"vcpu", VSH_OT_INT, VSH_OFLAG_REQ, N_("vcpu number")}, {"cpulist", VSH_OT_DATA, VSH_OFLAG_REQ, N_("host cpu number(s) (comma separated)")}, {NULL, 0, 0, NULL}
Self-NAK - this causes 'make check; to fail: +++ out 2010-10-19 10:52:26.575490946 -0600 @@ -1,2 +1 @@ -error: vcpupin: Invalid or missing vCPU number. - +error: unexpected data '0,1' That is, $abs_top_builddir/tools/virsh --connect test:///default vcpupin test a 0,1 > out 2>&1 gives worse error output than before. v2 coming up. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
* tools/virsh.c (opts_freecell, opts_memtune, opts_vcpupin) (opts_setvcpus, opts_setmaxmem, opts_setmem) (opts_migrate_setmaxdowntime): Use VSH_OT_INT when only an integer is expected. (vshCmddefHelp, vshCmddefGetData): Allow mandatory VSH_OT_INT arguments. --- changes in v2: Update some infrastructure fo 'make check' passes again. tools/virsh.c | 37 +++++++++++++++++++++---------------- 1 files changed, 21 insertions(+), 16 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 3e37b06..ca9a61e 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -119,9 +119,9 @@ typedef enum { * vshCmdOptType - command option type */ typedef enum { - VSH_OT_BOOL, /* boolean option */ - VSH_OT_STRING, /* string option */ - VSH_OT_INT, /* int option */ + VSH_OT_BOOL, /* optional boolean option */ + VSH_OT_STRING, /* optional string option */ + VSH_OT_INT, /* optional or mandatory int option */ VSH_OT_DATA, /* string data (as non-option) */ VSH_OT_ARGV /* remaining arguments, opt->name should be "" */ } vshCmdOptType; @@ -2247,7 +2247,7 @@ static const vshCmdInfo info_freecell[] = { }; static const vshCmdOptDef opts_freecell[] = { - {"cellno", VSH_OT_DATA, 0, N_("NUMA cell number")}, + {"cellno", VSH_OT_INT, 0, N_("NUMA cell number")}, {NULL, 0, 0, NULL} }; @@ -2583,7 +2583,7 @@ static const vshCmdInfo info_vcpupin[] = { static const vshCmdOptDef opts_vcpupin[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"vcpu", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vcpu number")}, + {"vcpu", VSH_OT_INT, VSH_OFLAG_REQ, N_("vcpu number")}, {"cpulist", VSH_OT_DATA, VSH_OFLAG_REQ, N_("host cpu number(s) (comma separated)")}, {NULL, 0, 0, NULL} }; @@ -2719,7 +2719,7 @@ static const vshCmdInfo info_setvcpus[] = { static const vshCmdOptDef opts_setvcpus[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"count", VSH_OT_DATA, VSH_OFLAG_REQ, N_("number of virtual CPUs")}, + {"count", VSH_OT_INT, VSH_OFLAG_REQ, N_("number of virtual CPUs")}, {"maximum", VSH_OT_BOOL, 0, N_("set maximum limit on next boot")}, {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, @@ -2772,7 +2772,7 @@ static const vshCmdInfo info_setmem[] = { static const vshCmdOptDef opts_setmem[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"kilobytes", VSH_OT_DATA, VSH_OFLAG_REQ, N_("number of kilobytes of memory")}, + {"kilobytes", VSH_OT_INT, VSH_OFLAG_REQ, N_("number of kilobytes of memory")}, {NULL, 0, 0, NULL} }; @@ -2829,7 +2829,7 @@ static const vshCmdInfo info_setmaxmem[] = { static const vshCmdOptDef opts_setmaxmem[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"kilobytes", VSH_OT_DATA, VSH_OFLAG_REQ, N_("maximum memory limit in kilobytes")}, + {"kilobytes", VSH_OT_INT, VSH_OFLAG_REQ, N_("maximum memory limit in kilobytes")}, {NULL, 0, 0, NULL} }; @@ -2890,13 +2890,13 @@ static const vshCmdInfo info_memtune[] = { static const vshCmdOptDef opts_memtune[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {VIR_DOMAIN_MEMORY_HARD_LIMIT, VSH_OT_STRING, VSH_OFLAG_NONE, + {VIR_DOMAIN_MEMORY_HARD_LIMIT, VSH_OT_INT, VSH_OFLAG_NONE, N_("Max memory in kilobytes")}, - {VIR_DOMAIN_MEMORY_SOFT_LIMIT, VSH_OT_STRING, VSH_OFLAG_NONE, + {VIR_DOMAIN_MEMORY_SOFT_LIMIT, VSH_OT_INT, VSH_OFLAG_NONE, N_("Memory during contention in kilobytes")}, - {VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, VSH_OT_STRING, VSH_OFLAG_NONE, + {VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, VSH_OT_INT, VSH_OFLAG_NONE, N_("Max swap in kilobytes")}, - {VIR_DOMAIN_MEMORY_MIN_GUARANTEE, VSH_OT_STRING, VSH_OFLAG_NONE, + {VIR_DOMAIN_MEMORY_MIN_GUARANTEE, VSH_OT_INT, VSH_OFLAG_NONE, N_("Min guaranteed memory in kilobytes")}, {NULL, 0, 0, NULL} }; @@ -3458,7 +3458,7 @@ static const vshCmdInfo info_migrate_setmaxdowntime[] = { static const vshCmdOptDef opts_migrate_setmaxdowntime[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"downtime", VSH_OT_DATA, VSH_OFLAG_REQ, N_("maximum tolerable downtime (in milliseconds) for migration")}, + {"downtime", VSH_OT_INT, VSH_OFLAG_REQ, N_("maximum tolerable downtime (in milliseconds) for migration")}, {NULL, 0, 0, NULL} }; @@ -10006,7 +10006,8 @@ vshCmddefGetData(const vshCmdDef * cmd, int data_ct) const vshCmdOptDef *opt; for (opt = cmd->opts; opt && opt->name; opt++) { - if (opt->type >= VSH_OT_DATA) { + if (opt->type >= VSH_OT_DATA || + (opt->type == VSH_OT_INT && (opt->flag & VSH_OFLAG_REQ))) { if (data_ct == 0 || opt->type == VSH_OT_ARGV) return opt; else @@ -10089,7 +10090,8 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname) break; case VSH_OT_INT: /* xgettext:c-format */ - fmt = _("[--%s <number>]"); + fmt = ((opt->flag & VSH_OFLAG_REQ) ? "<%s>" + : _("[--%s <number>]")); break; case VSH_OT_STRING: /* xgettext:c-format */ @@ -10126,9 +10128,12 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname) snprintf(buf, sizeof(buf), "--%s", opt->name); break; case VSH_OT_INT: - snprintf(buf, sizeof(buf), _("--%s <number>"), opt->name); + snprintf(buf, sizeof(buf), + (opt->flag & VSH_OFLAG_REQ) ? _("[--%s] <number>") + : _("--%s <number>"), opt->name); break; case VSH_OT_STRING: + /* OT_STRING should never be VSH_OFLAG_REQ */ snprintf(buf, sizeof(buf), _("--%s <string>"), opt->name); break; case VSH_OT_DATA: -- 1.7.2.3
* tools/virsh.c (opts_freecell, opts_memtune, opts_vcpupin) (opts_setvcpus, opts_setmaxmem, opts_setmem) (opts_migrate_setmaxdowntime): Use VSH_OT_INT when only an integer is expected. (vshCmddefHelp, vshCmddefGetData): Allow mandatory VSH_OT_INT arguments. ---
changes in v2: Update some infrastructure fo 'make check' passes again.
tools/virsh.c | 37 +++++++++++++++++++++---------------- 1 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 3e37b06..ca9a61e 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -119,9 +119,9 @@ typedef enum { * vshCmdOptType - command option type */ typedef enum { - VSH_OT_BOOL, /* boolean option */ - VSH_OT_STRING, /* string option */ - VSH_OT_INT, /* int option */ + VSH_OT_BOOL, /* optional boolean option */ + VSH_OT_STRING, /* optional string option */ + VSH_OT_INT, /* optional or mandatory int option */ VSH_OT_DATA, /* string data (as non-option) */ VSH_OT_ARGV /* remaining arguments, opt->name should be "" */ } vshCmdOptType; @@ -2247,7 +2247,7 @@ static const vshCmdInfo info_freecell[] = { };
static const vshCmdOptDef opts_freecell[] = { - {"cellno", VSH_OT_DATA, 0, N_("NUMA cell number")}, + {"cellno", VSH_OT_INT, 0, N_("NUMA cell number")}, {NULL, 0, 0, NULL} };
@@ -2583,7 +2583,7 @@ static const vshCmdInfo info_vcpupin[] = {
static const vshCmdOptDef opts_vcpupin[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"vcpu", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vcpu number")}, + {"vcpu", VSH_OT_INT, VSH_OFLAG_REQ, N_("vcpu number")}, {"cpulist", VSH_OT_DATA, VSH_OFLAG_REQ, N_("host cpu number(s) (comma separated)")}, {NULL, 0, 0, NULL} }; @@ -2719,7 +2719,7 @@ static const vshCmdInfo info_setvcpus[] = {
static const vshCmdOptDef opts_setvcpus[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"count", VSH_OT_DATA, VSH_OFLAG_REQ, N_("number of virtual CPUs")}, + {"count", VSH_OT_INT, VSH_OFLAG_REQ, N_("number of virtual CPUs")}, {"maximum", VSH_OT_BOOL, 0, N_("set maximum limit on next boot")}, {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, @@ -2772,7 +2772,7 @@ static const vshCmdInfo info_setmem[] = {
static const vshCmdOptDef opts_setmem[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"kilobytes", VSH_OT_DATA, VSH_OFLAG_REQ, N_("number of kilobytes of memory")}, + {"kilobytes", VSH_OT_INT, VSH_OFLAG_REQ, N_("number of kilobytes of memory")}, {NULL, 0, 0, NULL} };
@@ -2829,7 +2829,7 @@ static const vshCmdInfo info_setmaxmem[] = {
static const vshCmdOptDef opts_setmaxmem[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"kilobytes", VSH_OT_DATA, VSH_OFLAG_REQ, N_("maximum memory limit in kilobytes")}, + {"kilobytes", VSH_OT_INT, VSH_OFLAG_REQ, N_("maximum memory limit in kilobytes")}, {NULL, 0, 0, NULL} };
@@ -2890,13 +2890,13 @@ static const vshCmdInfo info_memtune[] = {
static const vshCmdOptDef opts_memtune[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {VIR_DOMAIN_MEMORY_HARD_LIMIT, VSH_OT_STRING, VSH_OFLAG_NONE, + {VIR_DOMAIN_MEMORY_HARD_LIMIT, VSH_OT_INT, VSH_OFLAG_NONE, N_("Max memory in kilobytes")}, - {VIR_DOMAIN_MEMORY_SOFT_LIMIT, VSH_OT_STRING, VSH_OFLAG_NONE, + {VIR_DOMAIN_MEMORY_SOFT_LIMIT, VSH_OT_INT, VSH_OFLAG_NONE, N_("Memory during contention in kilobytes")}, - {VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, VSH_OT_STRING, VSH_OFLAG_NONE, + {VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, VSH_OT_INT, VSH_OFLAG_NONE, N_("Max swap in kilobytes")}, - {VIR_DOMAIN_MEMORY_MIN_GUARANTEE, VSH_OT_STRING, VSH_OFLAG_NONE, + {VIR_DOMAIN_MEMORY_MIN_GUARANTEE, VSH_OT_INT, VSH_OFLAG_NONE, N_("Min guaranteed memory in kilobytes")}, {NULL, 0, 0, NULL} }; @@ -3458,7 +3458,7 @@ static const vshCmdInfo info_migrate_setmaxdowntime[] = {
static const vshCmdOptDef opts_migrate_setmaxdowntime[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"downtime", VSH_OT_DATA, VSH_OFLAG_REQ, N_("maximum tolerable downtime (in milliseconds) for migration")}, + {"downtime", VSH_OT_INT, VSH_OFLAG_REQ, N_("maximum tolerable downtime (in milliseconds) for migration")}, {NULL, 0, 0, NULL} };
@@ -10006,7 +10006,8 @@ vshCmddefGetData(const vshCmdDef * cmd, int data_ct) const vshCmdOptDef *opt;
for (opt = cmd->opts; opt&& opt->name; opt++) { - if (opt->type>= VSH_OT_DATA) { + if (opt->type>= VSH_OT_DATA || + (opt->type == VSH_OT_INT&& (opt->flag& VSH_OFLAG_REQ))) { if (data_ct == 0 || opt->type == VSH_OT_ARGV) return opt; else @@ -10089,7 +10090,8 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname) break; case VSH_OT_INT: /* xgettext:c-format */ - fmt = _("[--%s<number>]"); + fmt = ((opt->flag& VSH_OFLAG_REQ) ? "<%s>" + : _("[--%s<number>]")); break; case VSH_OT_STRING: /* xgettext:c-format */ @@ -10126,9 +10128,12 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname) snprintf(buf, sizeof(buf), "--%s", opt->name); break; case VSH_OT_INT: - snprintf(buf, sizeof(buf), _("--%s<number>"), opt->name); + snprintf(buf, sizeof(buf), + (opt->flag& VSH_OFLAG_REQ) ? _("[--%s]<number>") + : _("--%s<number>"), opt->name); Looks to me like the first string (VSH_OFLAG_REQ set) should not have
On 10/20/2010 02:30 PM, Eric Blake wrote: the [--%s] and the second one should have it, no?
break; case VSH_OT_STRING: + /* OT_STRING should never be VSH_OFLAG_REQ */ snprintf(buf, sizeof(buf), _("--%s<string>"), opt->name); break; case VSH_OT_DATA:
ACK. Stefan
On 10/25/2010 05:51 PM, Stefan Berger wrote:
case VSH_OT_INT: - snprintf(buf, sizeof(buf), _("--%s<number>"), opt->name); + snprintf(buf, sizeof(buf), + (opt->flag& VSH_OFLAG_REQ) ? _("[--%s]<number>") + : _("--%s<number>"), opt->name); Looks to me like the first string (VSH_OFLAG_REQ set) should not have the [--%s] and the second one should have it, no?
It was correct as-is. When you use VSH_OFLAG_REQ, then the option must appear, so the parser is generous and lets you omit the --option prefix. For example, with setvcpus, you can do: virsh setvcpus dom 1 instead of virsh setvcpus --domain dom --count 1 so the help text shows up as: setvcpus <domain> <count> [--maximum] [--config] [--live] ... [--domain] <string> domain name, id or uuid [--count] <number> number of virtual CPUs On the other hand, with memtune, the VSH_OFLAG_REQ is not set, so the parser cannot tell which option the argument appear with unless the option name is also present. Thus: memtune dom 1024 is an error, and you have to do: memtune dome --hard_limit 1024 (hmm; from a cli perspective, it's better to name options with - than _; I'll propose a followup patch to name it hard-limit). The help for memtune is also correct: memtune <domain> [--hard_limit <number>] [--soft_limit <number>] [--swap_hard_limit <number>] [--min_guarantee <number>] ... [--domain] <string> domain name, id or uuid --hard_limit <number> Max memory in kilobytes
break; case VSH_OT_STRING: + /* OT_STRING should never be VSH_OFLAG_REQ */ snprintf(buf, sizeof(buf), _("--%s<string>"), opt->name); break; case VSH_OT_DATA:
ACK.
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Wed, Oct 20, 2010 at 12:30:23PM -0600, Eric Blake wrote:
* tools/virsh.c (opts_freecell, opts_memtune, opts_vcpupin) (opts_setvcpus, opts_setmaxmem, opts_setmem) (opts_migrate_setmaxdowntime): Use VSH_OT_INT when only an integer is expected. (vshCmddefHelp, vshCmddefGetData): Allow mandatory VSH_OT_INT arguments. ---
changes in v2: Update some infrastructure fo 'make check' passes again.
Looks fine, ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel Veillard -
Eric Blake -
Stefan Berger