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

Re: [libvirt] [PATCH v2 2/3] virsh: Pass vshControl to all vshCommandOpt*() calls.



On 22.05.2015 10:59, Andrea Bolognani wrote:
> This will allow us to use vshError() to report errors from inside
> vshCommandOpt*(), instead of replicating the same logic and error
> messages all over the place.
> 
> We also have more context inside the vshCommandOpt*() functions,
> for example the actual value used on the command line, which means
> we can produce more detailed error messages.
> ---
>  tools/virsh-domain-monitor.c |  90 +++----
>  tools/virsh-domain.c         | 598 +++++++++++++++++++++----------------------
>  tools/virsh-host.c           |  46 ++--
>  tools/virsh-interface.c      |  14 +-
>  tools/virsh-network.c        |  34 +--
>  tools/virsh-nodedev.c        |   6 +-
>  tools/virsh-pool.c           |  26 +-
>  tools/virsh-secret.c         |   8 +-
>  tools/virsh-snapshot.c       |  88 +++----
>  tools/virsh-volume.c         |  34 +--
>  tools/virsh.c                | 107 ++++----
>  tools/virsh.h                |  77 +++---
>  12 files changed, 574 insertions(+), 554 deletions(-)
> 
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index a42c150..db7ef8b 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -317,9 +317,9 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
>      bool ret = false;
>      int rv = 0;
>      int period = -1;
> -    bool config = vshCommandOptBool(cmd, "config");
> -    bool live = vshCommandOptBool(cmd, "live");
> -    bool current = vshCommandOptBool(cmd, "current");
> +    bool config = vshCommandOptBool(ctl, cmd, "config");

I don't think this is needed. vshCommandOptBool should never return an
error. Well, it's returning just if a flag was specified or not.

> +    bool live = vshCommandOptBool(ctl, cmd, "live");
> +    bool current = vshCommandOptBool(ctl, cmd, "current");
>      unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
>  
>      VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
> @@ -340,7 +340,7 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
>      /* Providing a period will adjust the balloon driver collection period.
>       * This is not really an unsigned long, but it
>       */
> -    if ((rv = vshCommandOptInt(cmd, "period", &period)) < 0) {
> +    if ((rv = vshCommandOptInt(ctl, cmd, "period", &period)) < 0) {

This is of course different, specified value may not be a number in
which case we want an error to be reported.

>          vshError(ctl,
>                   _("Numeric value for <%s> option is malformed or out of range"),
>                   "period");

ACK modulo the OptBool() change.

Michal


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