[libvirt] [PATCH 3/8] virsh: blockjob: Split out vshBlockJobSetSpeed from blockJobImpl

[John Ferlan]

On 04/30/2015 10:45 AM, Peter Krempa wrote:
> ---
>  tools/virsh-domain.c | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 720bc68..0d0e39e 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -1667,7 +1667,6 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
> 
>  typedef enum {
>      VSH_CMD_BLOCK_JOB_ABORT,
> -    VSH_CMD_BLOCK_JOB_SPEED,
>      VSH_CMD_BLOCK_JOB_PULL,
>      VSH_CMD_BLOCK_JOB_COMMIT,
>  } vshCmdBlockJobMode;
> @@ -1704,10 +1703,6 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
>          if (virDomainBlockJobAbort(dom, path, flags) < 0)
>              goto cleanup;
>          break;
> -    case VSH_CMD_BLOCK_JOB_SPEED:
> -        if (virDomainBlockJobSetSpeed(dom, path, bandwidth, 0) < 0)
> -            goto cleanup;
> -        break;
>      case VSH_CMD_BLOCK_JOB_PULL:
>          if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0)
>              goto cleanup;
> @@ -2543,6 +2538,26 @@ vshBlockJobInfo(vshControl *ctl,
> 
> 
>  static bool
> +vshBlockJobSetSpeed(vshControl *ctl,
> +                    const vshCmd *cmd,
> +                    virDomainPtr dom,
> +                    const char *path)
> +{
> +    unsigned long bandwidth;
> +
> +    if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) {
> +        vshError(ctl, "%s", _("bandwidth must be a number"));
> +        return false;
> +    }
> +
> +    if (virDomainBlockJobSetSpeed(dom, path, bandwidth, 0) < 0)
> +        return false;
> +
> +    return true;
> +}
> +
> +
> +static bool
>  cmdBlockJob(vshControl *ctl, const vshCmd *cmd)
>  {
>      bool ret = false;
> @@ -2574,10 +2589,7 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd)
> 
>      if (abort || pivot || async)
>          return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_ABORT, NULL);
> -    if (bandwidth)
> -        return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_SPEED, NULL);
> 
> -    /* Everything below here is for --info mode */
>      if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
>          goto cleanup;
> 
> @@ -2585,7 +2597,10 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd)
>      if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
>          goto cleanup;
> 
> -    ret = vshBlockJobInfo(ctl, dom, path, raw, bytes);
> +    if (bandwidth)
> +        ret = vshBlockJobSetSpeed(ctl, cmd, dom, path);

Since we're getting bandwidth a few lines above - why not just pass
that?  Especially considering what you do in patch 5 which only allows
using a scaled bandwidth for SetSpeed, but not Pull and Commit which
patch 5 implies to me in the virsh.pod can be used "in general".

If you pass the already parsed bandwidth here - then an ACK; otherwise,
I'd have to get more explanation...

John
> +    else
> +        ret = vshBlockJobInfo(ctl, dom, path, raw, bytes);
> 
>   cleanup:
>      if (dom)
>