[libvirt] clarify freecell documentation
There were a couple of minor inaccuracies in the freecell manpage and virsh help. The first patch fixes the manpage and the second, to virsh.c, attempts to fix the help output. The virsh.c patch appears to produce the correct output, but is pure cargo cult programming, so it should be very carefully reviewed. Dave
--- tools/virsh.pod | 23 ++++++++++++++++++----- 1 files changed, 18 insertions(+), 5 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index f0df4fd..b4deae8 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -401,11 +401,24 @@ B<virsh> list --title 0 Domain-0 running Mailserver 1 2 fedora paused -=item B<freecell> [B<cellno> | I<--all>] - -Prints the available amount of memory on the machine or within a -NUMA cell if I<cellno> is provided. If I<--all> is provided instead -of I<--cellno>, then show the information on all NUMA cells. +=item B<freecell> [{ [I<--cellno>] B<cellno> | I<--all> }] + +Prints the available amount of memory on the machine or within a NUMA +cell. The freecell command can provide one of three different +displays of available memory on the machine depending on the options +specified. With no options, it displays the total free memory on the +machine. With the --all option, it displays the free memory on each +node and the total free memory on the machine. Finally, with a +numeric argument or with --cellno it will display the free memory for +that node only. + + For example: + + freecell => total free memory on the machine + freecell --all => free memory on each node, and the total + freecell --cellno 0 => free memory on node 0 only + freecell 0 => free memory on node 0 only + freecell --cellno=0 => free memory on node 0 only =item B<cpu-baseline> I<FILE> -- 1.7.7.6
On 03/22/2012 01:59 PM, Dave Allan wrote:
--- tools/virsh.pod | 23 ++++++++++++++++++----- 1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod index f0df4fd..b4deae8 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -401,11 +401,24 @@ B<virsh> list --title 0 Domain-0 running Mailserver 1 2 fedora paused
-=item B<freecell> [B<cellno> | I<--all>] - -Prints the available amount of memory on the machine or within a -NUMA cell if I<cellno> is provided. If I<--all> is provided instead -of I<--cellno>, then show the information on all NUMA cells. +=item B<freecell> [{ [I<--cellno>] B<cellno> | I<--all> }] + +Prints the available amount of memory on the machine or within a NUMA +cell. The freecell command can provide one of three different +displays of available memory on the machine depending on the options +specified. With no options, it displays the total free memory on the +machine. With the --all option, it displays the free memory on each +node and the total free memory on the machine. Finally, with a +numeric argument or with --cellno it will display the free memory for +that node only.
I like this part.
+ + For example: + + freecell => total free memory on the machine + freecell --all => free memory on each node, and the total + freecell --cellno 0 => free memory on node 0 only + freecell 0 => free memory on node 0 only + freecell --cellno=0 => free memory on node 0 only
This may be a bit verbose; we aren't giving this many examples elsewhere in the man page. I'm not entirely opposed to verbosity, if it helps silence confusion, but if others agree to keep this many examples, can we at least add some variety, as in: freecell 1 => free memory on node 1 only -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Mar 22, 2012 at 02:13:07PM -0600, Eric Blake wrote:
On 03/22/2012 01:59 PM, Dave Allan wrote:
--- tools/virsh.pod | 23 ++++++++++++++++++----- 1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod index f0df4fd..b4deae8 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -401,11 +401,24 @@ B<virsh> list --title 0 Domain-0 running Mailserver 1 2 fedora paused
-=item B<freecell> [B<cellno> | I<--all>] - -Prints the available amount of memory on the machine or within a -NUMA cell if I<cellno> is provided. If I<--all> is provided instead -of I<--cellno>, then show the information on all NUMA cells. +=item B<freecell> [{ [I<--cellno>] B<cellno> | I<--all> }] + +Prints the available amount of memory on the machine or within a NUMA +cell. The freecell command can provide one of three different +displays of available memory on the machine depending on the options +specified. With no options, it displays the total free memory on the +machine. With the --all option, it displays the free memory on each +node and the total free memory on the machine. Finally, with a +numeric argument or with --cellno it will display the free memory for +that node only.
I like this part.
+ + For example: + + freecell => total free memory on the machine + freecell --all => free memory on each node, and the total + freecell --cellno 0 => free memory on node 0 only + freecell 0 => free memory on node 0 only + freecell --cellno=0 => free memory on node 0 only
This may be a bit verbose; we aren't giving this many examples elsewhere in the man page. I'm not entirely opposed to verbosity, if it helps silence confusion, but if others agree to keep this many examples, can we at least add some variety, as in:
That's a good point, and I think it's clear enough without it; I'll drop that bit and resubmit. Dave
freecell 1 => free memory on node 1 only
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- tools/virsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 9e5c9b2..d9cff0c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4742,7 +4742,7 @@ static const vshCmdInfo info_freecell[] = { }; static const vshCmdOptDef opts_freecell[] = { - {"cellno", VSH_OT_INT, 0, N_("NUMA cell number")}, + {"cellno", VSH_OT_INT, VSH_OFLAG_REQ, N_("NUMA cell number")}, {"all", VSH_OT_BOOL, 0, N_("show free memory for all NUMA cells")}, {NULL, 0, 0, NULL} }; -- 1.7.7.6
On 03/22/2012 01:59 PM, Dave Allan wrote:
--- tools/virsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 9e5c9b2..d9cff0c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4742,7 +4742,7 @@ static const vshCmdInfo info_freecell[] = { };
static const vshCmdOptDef opts_freecell[] = { - {"cellno", VSH_OT_INT, 0, N_("NUMA cell number")}, + {"cellno", VSH_OT_INT, VSH_OFLAG_REQ, N_("NUMA cell number")},
NACK. VSH_OFLAG_REQ means required, absence of that flag (ie. using 0 for the flag) means optional. This patch would break the command by requiring a --cellno argument, even with --all. This is the current 'virsh help freecell' output, without your patch: $ virsh help freecell NAME freecell - NUMA free memory SYNOPSIS freecell [--cellno <number>] [--all] DESCRIPTION display available free memory for the NUMA cell. OPTIONS --cellno <number> NUMA cell number --all show free memory for all NUMA cells It shows that both --cellno and --all are optional; however, what it does not show (and cannot show, without a lot more work throughout virsh), is the notion of mutual exclusion (that is, there is no trivial way to make virsh help output the {} operators to show the alternation that the virsh.pod has by hand). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Mar 22, 2012 at 02:10:30PM -0600, Eric Blake wrote:
On 03/22/2012 01:59 PM, Dave Allan wrote:
--- tools/virsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 9e5c9b2..d9cff0c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4742,7 +4742,7 @@ static const vshCmdInfo info_freecell[] = { };
static const vshCmdOptDef opts_freecell[] = { - {"cellno", VSH_OT_INT, 0, N_("NUMA cell number")}, + {"cellno", VSH_OT_INT, VSH_OFLAG_REQ, N_("NUMA cell number")},
NACK. VSH_OFLAG_REQ means required, absence of that flag (ie. using 0 for the flag) means optional. This patch would break the command by requiring a --cellno argument, even with --all.
This is the current 'virsh help freecell' output, without your patch:
$ virsh help freecell NAME freecell - NUMA free memory
SYNOPSIS freecell [--cellno <number>] [--all]
DESCRIPTION display available free memory for the NUMA cell.
OPTIONS --cellno <number> NUMA cell number --all show free memory for all NUMA cells
It shows that both --cellno and --all are optional; however, what it does not show (and cannot show, without a lot more work throughout virsh), is the notion of mutual exclusion (that is, there is no trivial way to make virsh help output the {} operators to show the alternation that the virsh.pod has by hand).
Ok, that's what I was afraid someone was going to say. We can kill that patch as far as I'm concerned. Dave
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Dave Allan -
Eric Blake