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

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



On Thu, May 21, 2015 at 11:40:26 +0200, 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                | 113 ++++----
>  tools/virsh.h                |  77 +++---
>  12 files changed, 571 insertions(+), 563 deletions(-)
> 

...

> diff --git a/tools/virsh.c b/tools/virsh.c
> index 4425774..9f94b75 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c

...

> -vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
> +vshCommandOpt(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd,
> +              const char *name, vshCmdOpt **opt,
>                bool needData)


So this helper is not using ctl nor reporting any errors. Even in the
next patch.

>  {
>      vshCmdOpt *candidate = cmd->opts;

...

> @@ -1829,11 +1831,12 @@ vshCommandOptScaledInt(const vshCmd *cmd, const char *name,
>   * option is present without actually using that data.
>   */
>  bool
> -vshCommandOptBool(const vshCmd *cmd, const char *name)
> +vshCommandOptBool(vshControl *ctl, const vshCmd *cmd,
> +                  const char *name)
>  {
>      vshCmdOpt *dummy;
>  
> -    return vshCommandOpt(cmd, name, &dummy, false) == 1;
> +    return vshCommandOpt(ctl, cmd, name, &dummy, false) == 1;

And vshCommandOptBool is designed not to report any error. I'm not a big
fan of changing half of virsh by changing the prototype and then not
using the parameter at all.

>  }
>  
>  /**

I didn't go through the rest of the changes, but reporting malformed
integers right in the parser makes sense to me.

Peter

Attachment: signature.asc
Description: Digital signature


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