[libvirt] [PATCHv2] virsh: Make vshCommandOpt* report error

[Ján Tomko]

On 04/04/2014 03:50 PM, Michal Privoznik wrote:
> Currently, the virsh code is plenty of the following pattern:
> 
>   if (vshCommandOptUInt(..) < 0) {
>       vshError(...);
>       goto cleanup;
>   }
> 
> It doesn't make much sense to repeat the code everywhere.
> Moreover, some functions from the family already report
> error some of them don't.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  tests/vcpupin                |   2 +-
>  tools/virsh-domain-monitor.c |   7 +--
>  tools/virsh-domain.c         | 102 +++++++++++++++----------------------------
>  tools/virsh-host.c           |  25 +++--------
>  tools/virsh-interface.c      |   4 +-
>  tools/virsh-network.c        |   4 +-
>  tools/virsh-volume.c         |  16 ++-----
>  tools/virsh.c                |  99 +++++++++++++++++++++++++++++++----------
>  tools/virsh.h                |  24 +++++-----
>  9 files changed, 140 insertions(+), 143 deletions(-)
> 

> @@ -5819,9 +5806,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
>      query = !cpulist;
>  
>      /* In query mode, "vcpu" is optional */
> -    if (vshCommandOptInt(cmd, "vcpu", &vcpu) < !query) {
> -        vshError(ctl, "%s",
> -                 _("vcpupin: Invalid or missing vCPU number."));
> +    if (vshCommandOptInt(ctl, cmd, "vcpu", &vcpu) < !query) {

This should report an error for a missing option if query == false.

>          virDomainFree(dom);
>          return false;
>      }

> @@ -8123,9 +8099,11 @@ cmdQemuAttach(vshControl *ctl, const vshCmd *cmd)
>      bool ret = false;
>      unsigned int flags = 0;
>      unsigned int pid_value; /* API uses unsigned int, not pid_t */
> +    int rv;
>  
> -    if (vshCommandOptUInt(cmd, "pid", &pid_value) <= 0) {
> -        vshError(ctl, "%s", _("missing pid value"));
> +    if ((rv = vshCommandOptUInt(ctl, cmd, "pid", &pid_value)) <= 0) {
> +        if (rv == 0)
> +            vshError(ctl, "%s", _("missing pid value"));

pid has the VSH_OFLAG_REQ flag set, 0 shouldn't be returned here.


>          goto cleanup;
>      }
>  
> @@ -1469,6 +1469,7 @@ vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
>  
>  /**
>   * vshCommandOptInt:
> + * @ctl virsh control structure
>   * @cmd command reference
>   * @name option name
>   * @value result
> @@ -1480,7 +1481,10 @@ vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
>   * <0 in all other cases (@value untouched)
>   */
>  int
> -vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
> +vshCommandOptInt(vshControl *ctl,
> +                 const vshCmd *cmd,
> +                 const char *name,
> +                 int *value)
>  {
>      vshCmdOpt *arg;
>      int ret;
> @@ -1489,14 +1493,19 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
>      if (ret <= 0)

Here you don't report an error for -1,

>          return ret;
>  
> -    if (virStrToLong_i(arg->data, NULL, 10, value) < 0)
> +    if (virStrToLong_i(arg->data, NULL, 10, value) < 0) {
> +        vshError(ctl,
> +                 _("Unable to parse integer parameter to --%s"),
> +                 name);

here you do.

>          return -1;
> +    }
>      return 1;
>  }
>  

Jan


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