[libvirt] [PATCH 1/2] virsh: make -h always give help
https://bugzilla.redhat.com/show_bug.cgi?id=817244 mentions that unlike most other tools, where --help or --version prevent all further parsing of all later options, virsh was strange in that --version stopped parsing but --help tried to plow on to the end. There was no rationale for this original implementation (since 2005!), so I think we can safely conform to common usage patterns. * tools/virsh.c (main): Drop useless 'help' variable. --- I think the intent might have been to someday allow: virsh -h foo to be short for virsh help foo but since that hasn't been implemented in 7 years, I think it is smarter to be consistent with other tools instead. tools/virsh.c | 15 ++------------- 1 files changed, 2 insertions(+), 13 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index e177684..7159744 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -20179,7 +20179,6 @@ vshAllowedEscapeChar(char c) static bool vshParseArgv(vshControl *ctl, int argc, char **argv) { - bool help = false; int arg, len; struct option opt[] = { {"debug", required_argument, NULL, 'd'}, @@ -20206,7 +20205,8 @@ vshParseArgv(vshControl *ctl, int argc, char **argv) } break; case 'h': - help = true; + vshUsage(); + exit(EXIT_SUCCESS); break; case 'q': ctl->quiet = true; @@ -20251,17 +20251,6 @@ vshParseArgv(vshControl *ctl, int argc, char **argv) } } - if (help) { - if (optind < argc) { - vshError(ctl, _("extra argument '%s'. See --help."), argv[optind]); - exit(EXIT_FAILURE); - } - - /* list all command */ - vshUsage(); - exit(EXIT_SUCCESS); - } - if (argc > optind) { /* parse command */ ctl->imode = false; -- 1.7.7.6
The recent push to use correct scaling terms (kB for 1000, KiB for 1024 - such as commit 9dfdead) missed some places in virsh. * tools/virsh.c (prettyCapacity, cmdDominfo, cmdFreecell) (cmdNodeinfo, cmdNodeMemStats): Use KiB, not kB, when referring to multiples of 1024. --- tools/virsh.c | 24 ++++++++++++------------ 1 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 7159744..2527d7b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -514,16 +514,16 @@ prettyCapacity(unsigned long long val, *unit = ""; return (double)val; } else if (val < (1024.0l * 1024.0l)) { - *unit = "KB"; + *unit = "KiB"; return (((double)val / 1024.0l)); } else if (val < (1024.0l * 1024.0l * 1024.0l)) { - *unit = "MB"; + *unit = "MiB"; return (double)val / (1024.0l * 1024.0l); } else if (val < (1024.0l * 1024.0l * 1024.0l * 1024.0l)) { - *unit = "GB"; + *unit = "GiB"; return (double)val / (1024.0l * 1024.0l * 1024.0l); } else { - *unit = "TB"; + *unit = "TiB"; return (double)val / (1024.0l * 1024.0l * 1024.0l * 1024.0l); } } @@ -4541,13 +4541,13 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) } if (info.maxMem != UINT_MAX) - vshPrint(ctl, "%-15s %lu kB\n", _("Max memory:"), + vshPrint(ctl, "%-15s %lu KiB\n", _("Max memory:"), info.maxMem); else vshPrint(ctl, "%-15s %s\n", _("Max memory:"), _("no limit")); - vshPrint(ctl, "%-15s %lu kB\n", _("Used memory:"), + vshPrint(ctl, "%-15s %lu KiB\n", _("Used memory:"), info.memory); } else { @@ -4823,13 +4823,13 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) memory = 0; for (cell = 0; cell < nodes_cnt; cell++) { - vshPrint(ctl, "%5lu: %10llu kB\n", nodes_id[cell], + vshPrint(ctl, "%5lu: %10llu KiB\n", nodes_id[cell], (nodes_free[cell]/1024)); memory += nodes_free[cell]; } vshPrintExtra(ctl, "--------------------\n"); - vshPrintExtra(ctl, "%5s: %10llu kB\n", _("Total"), memory/1024); + vshPrintExtra(ctl, "%5s: %10llu KiB\n", _("Total"), memory/1024); } else { if (!cell_given) { memory = virNodeGetFreeMemory(ctl->conn); @@ -4842,9 +4842,9 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) } if (cell == -1) - vshPrint(ctl, "%s: %llu kB\n", _("Total"), (memory/1024)); + vshPrint(ctl, "%s: %llu KiB\n", _("Total"), (memory/1024)); else - vshPrint(ctl, "%d: %llu kB\n", cell, (memory/1024)); + vshPrint(ctl, "%d: %llu KiB\n", cell, (memory/1024)); } func_ret = true; @@ -6513,7 +6513,7 @@ cmdNodeinfo(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) vshPrint(ctl, "%-20s %d\n", _("Core(s) per socket:"), info.cores); vshPrint(ctl, "%-20s %d\n", _("Thread(s) per core:"), info.threads); vshPrint(ctl, "%-20s %d\n", _("NUMA cell(s):"), info.nodes); - vshPrint(ctl, "%-20s %lu kB\n", _("Memory size:"), info.memory); + vshPrint(ctl, "%-20s %lu KiB\n", _("Memory size:"), info.memory); return true; } @@ -6699,7 +6699,7 @@ cmdNodeMemStats(vshControl *ctl, const vshCmd *cmd) } for (i = 0; i < nparams; i++) - vshPrint(ctl, "%-7s: %20llu kB\n", params[i].field, params[i].value); + vshPrint(ctl, "%-7s: %20llu KiB\n", params[i].field, params[i].value); ret = true; -- 1.7.7.6
On 04/30/2012 02:31 PM, Eric Blake wrote:
The recent push to use correct scaling terms (kB for 1000, KiB for 1024 - such as commit 9dfdead) missed some places in virsh.
* tools/virsh.c (prettyCapacity, cmdDominfo, cmdFreecell) (cmdNodeinfo, cmdNodeMemStats): Use KiB, not kB, when referring to multiples of 1024. --- tools/virsh.c | 24 ++++++++++++------------ 1 files changed, 12 insertions(+), 12 deletions(-)
Oops, should have tested 'make check'. v2 coming up. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The recent push to use correct scaling terms (kB for 1000, KiB for 1024 - such as commit 9dfdead) missed some places in virsh. * tools/virsh.c (prettyCapacity, cmdDominfo, cmdFreecell) (cmdNodeinfo, cmdNodeMemStats): Use KiB, not kB, when referring to multiples of 1024. * tests/virshtest.c: Update expected output to match. --- tests/virshtest.c | 8 ++++---- tools/virsh.c | 24 ++++++++++++------------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/virshtest.c b/tests/virshtest.c index 2a89866..72f2a1e 100644 --- a/tests/virshtest.c +++ b/tests/virshtest.c @@ -28,8 +28,8 @@ UUID: " DOM_UUID "\n\ OS Type: linux\n\ State: running\n\ CPU(s): 1\n\ -Max memory: 261072 kB\n\ -Used memory: 131072 kB\n\ +Max memory: 261072 KiB\n\ +Used memory: 131072 KiB\n\ Persistent: yes\n\ Autostart: disable\n\ Managed save: unknown\n\ @@ -122,7 +122,7 @@ CPU socket(s): 2\n\ Core(s) per socket: 2\n\ Thread(s) per core: 2\n\ NUMA cell(s): 2\n\ -Memory size: 3145728 kB\n\ +Memory size: 3145728 KiB\n\ \n"; return testCompareOutputLit(exp, NULL, argv); } @@ -141,7 +141,7 @@ CPU socket(s): 4\n\ Core(s) per socket: 4\n\ Thread(s) per core: 2\n\ NUMA cell(s): 4\n\ -Memory size: 8192000 kB\n\ +Memory size: 8192000 KiB\n\ \n"; return testCompareOutputLit(exp, NULL, argv); } diff --git a/tools/virsh.c b/tools/virsh.c index 7159744..2527d7b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -514,16 +514,16 @@ prettyCapacity(unsigned long long val, *unit = ""; return (double)val; } else if (val < (1024.0l * 1024.0l)) { - *unit = "KB"; + *unit = "KiB"; return (((double)val / 1024.0l)); } else if (val < (1024.0l * 1024.0l * 1024.0l)) { - *unit = "MB"; + *unit = "MiB"; return (double)val / (1024.0l * 1024.0l); } else if (val < (1024.0l * 1024.0l * 1024.0l * 1024.0l)) { - *unit = "GB"; + *unit = "GiB"; return (double)val / (1024.0l * 1024.0l * 1024.0l); } else { - *unit = "TB"; + *unit = "TiB"; return (double)val / (1024.0l * 1024.0l * 1024.0l * 1024.0l); } } @@ -4541,13 +4541,13 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) } if (info.maxMem != UINT_MAX) - vshPrint(ctl, "%-15s %lu kB\n", _("Max memory:"), + vshPrint(ctl, "%-15s %lu KiB\n", _("Max memory:"), info.maxMem); else vshPrint(ctl, "%-15s %s\n", _("Max memory:"), _("no limit")); - vshPrint(ctl, "%-15s %lu kB\n", _("Used memory:"), + vshPrint(ctl, "%-15s %lu KiB\n", _("Used memory:"), info.memory); } else { @@ -4823,13 +4823,13 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) memory = 0; for (cell = 0; cell < nodes_cnt; cell++) { - vshPrint(ctl, "%5lu: %10llu kB\n", nodes_id[cell], + vshPrint(ctl, "%5lu: %10llu KiB\n", nodes_id[cell], (nodes_free[cell]/1024)); memory += nodes_free[cell]; } vshPrintExtra(ctl, "--------------------\n"); - vshPrintExtra(ctl, "%5s: %10llu kB\n", _("Total"), memory/1024); + vshPrintExtra(ctl, "%5s: %10llu KiB\n", _("Total"), memory/1024); } else { if (!cell_given) { memory = virNodeGetFreeMemory(ctl->conn); @@ -4842,9 +4842,9 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) } if (cell == -1) - vshPrint(ctl, "%s: %llu kB\n", _("Total"), (memory/1024)); + vshPrint(ctl, "%s: %llu KiB\n", _("Total"), (memory/1024)); else - vshPrint(ctl, "%d: %llu kB\n", cell, (memory/1024)); + vshPrint(ctl, "%d: %llu KiB\n", cell, (memory/1024)); } func_ret = true; @@ -6513,7 +6513,7 @@ cmdNodeinfo(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) vshPrint(ctl, "%-20s %d\n", _("Core(s) per socket:"), info.cores); vshPrint(ctl, "%-20s %d\n", _("Thread(s) per core:"), info.threads); vshPrint(ctl, "%-20s %d\n", _("NUMA cell(s):"), info.nodes); - vshPrint(ctl, "%-20s %lu kB\n", _("Memory size:"), info.memory); + vshPrint(ctl, "%-20s %lu KiB\n", _("Memory size:"), info.memory); return true; } @@ -6699,7 +6699,7 @@ cmdNodeMemStats(vshControl *ctl, const vshCmd *cmd) } for (i = 0; i < nparams; i++) - vshPrint(ctl, "%-7s: %20llu kB\n", params[i].field, params[i].value); + vshPrint(ctl, "%-7s: %20llu KiB\n", params[i].field, params[i].value); ret = true; -- 1.7.7.6
On 05/01/2012 09:21 AM, Stefan Berger wrote:
On 04/30/2012 06:27 PM, Eric Blake wrote:
The recent push to use correct scaling terms (kB for 1000, KiB for 1024 - such as commit 9dfdead) missed some places in virsh.
ACK
Thanks; series pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 04/30/2012 04:31 PM, Eric Blake wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=817244 mentions that unlike most other tools, where --help or --version prevent all further parsing of all later options, virsh was strange in that --version stopped parsing but --help tried to plow on to the end. There was no rationale for this original implementation (since 2005!), so I think we can safely conform to common usage patterns.
* tools/virsh.c (main): Drop useless 'help' variable. --- ACK
participants (2)
-
Eric Blake -
Stefan Berger