Re: [PATCH 3/8] virsh-host: Refactor cmdFreecell

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/07/2013 05:53 AM, Peter Krempa wrote:
> Use the new helpers to determine mutually exclusive options and touch up
> some parts to simplify the code.
> ---
>  tools/virsh-host.c | 55 +++++++++++++++++++++++-------------------------------
>  1 file changed, 23 insertions(+), 32 deletions(-)
> 
> 
> -    if ((cell_given = vshCommandOptInt(cmd, "cellno", &cell)) < 0) {
> -        vshError(ctl, "%s", _("cell number has to be a number"));
> -        goto cleanup;
> -    }
> -    all_given = vshCommandOptBool(cmd, "all");
> +    VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno);
> 
> -    if (all_given && cell_given) {
> -        vshError(ctl, "%s", _("--cellno and --all are mutually exclusive. "
> -                              "Please choose only one."));
> -        goto cleanup;
> +    if (cellno && vshCommandOptInt(cmd, "cellno", &cell) < 0) {

The 'cellno &&' portion is extra; we didn't need it before, so I don't
know why you added it here.  vshCommandOptInt returns 0 if --cellno was
not provided, since it is not a mandatory option.

> @@ -237,23 +231,20 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
>          vshPrintExtra(ctl, "--------------------\n");
>          vshPrintExtra(ctl, "%5s: %10llu KiB\n", _("Total"), memory/1024);
>      } else {
> -        if (!cell_given) {
> -            memory = virNodeGetFreeMemory(ctl->conn);
> -            if (memory == 0)
> +       if (cellno) {

Indentation is off.

> +            if (virNodeGetCellsFreeMemory(ctl->conn, &memory, cell, 1) != 1)
>                  goto cleanup;
> -        } else {
> -            ret = virNodeGetCellsFreeMemory(ctl->conn, &memory, cell, 1);
> -            if (ret != 1)
> +
> +            vshPrint(ctl, "%d: %llu KiB\n", cell, (memory/1024));
> +       } else {

and again.

ACK if you either explain the added conjunct or remove it, and if you
fix the whitespace.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]