[libvirt] [PATCH] virsh: added --all flag to freecell command

[Eric Blake]

On 01/28/2011 09:42 AM, Michal Privoznik wrote:
> This will iterate over all NUMA nodes, showing
> free memory for each and sum at the end.
> Existing default behavior is not changed.
> ---
>  tools/virsh.c |   73 +++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 61 insertions(+), 12 deletions(-)
> 
>  static int
>  cmdFreecell(vshControl *ctl, const vshCmd *cmd)
>  {
> +    int func_ret = FALSE;
>      int ret;
>      int cell, cell_given;
>      unsigned long long memory;
> +    unsigned long long *nodes = NULL;
> +    int all_given;

At first, I thought to suggest s/int/bool/, given it's use

> +    virNodeInfo info;
> +
>  
>      if (!vshConnectionUsability(ctl, ctl->conn))
>          return FALSE;
>  
>      cell = vshCommandOptInt(cmd, "cellno", &cell_given);
> -    if (!cell_given) {
> -        memory = virNodeGetFreeMemory(ctl->conn);
> -        if (memory == 0)
> -            return FALSE;
> +    all_given = vshCommandOptBool(cmd, "all");

...but this (still) returns int instead of bool (we should probably fix
that, but that's a separate cleanup patch).

> +
> +    if (all_given && cell_given) {
> +        vshError(ctl, "%s", _("--cellno and --all are mutual exclusive. Please choose only one."));

s/mutual/mutually/

I'd drop the last sentence; it adds politeness, but not any new
information over the first sentence, and just makes the line longer than
80 columns

> +        goto cleanup;
> +    }
> +
> +    if (all_given) {
> +        if (virNodeGetInfo(ctl->conn, &info) < 0) {
> +            vshError(ctl, "%s", _("failed to get NUMA nodes count"));
> +            goto cleanup;
> +        }
> +
> +        if (!info.nodes) {
> +            vshError(ctl, "%s", _("no NUMA nodes presented"));

s/presented/present/

> +        memory = 0;
> +        for (cell = 0; cell < info.nodes; cell++) {
> +            vshPrint(ctl, "%d: %llu kB\n", cell, (nodes[cell]/1024));

Should we try to align columns here?

> +            memory += nodes[cell];
> +        }
> +
> +        vshPrintExtra(ctl, "--------------------\n");
> +        vshPrintExtra(ctl, "%s %llu kB\n",_("Total"), memory/1024);

and also here?

But I like the idea.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110128/4fd05bd2/attachment-0001.sig>