[libvirt] [PATCH 1/8] virsh: blockjob: Extract block job info code into a separate function

[John Ferlan]

On 04/30/2015 10:45 AM, Peter Krempa wrote:
> cmdBlockJob will be converted to a hub that will call into the
> individual executor functions.
> ---
>  tools/virsh-domain.c | 93 +++++++++++++++++++++++++++++++---------------------
>  1 file changed, 56 insertions(+), 37 deletions(-)
> 

This doesn't compile for me... gcc (GCC) 4.9.2 20150212

> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 4b627e1..01c6b9e 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -2457,47 +2457,19 @@ vshDomainBlockJobToString(int type)
>      return str ? _(str) : _("Unknown job");
>  }
> 
> +
>  static bool
> -cmdBlockJob(vshControl *ctl, const vshCmd *cmd)
> +vshBlockJobInfo(vshControl *ctl,
> +                virDomainPtr dom,
> +                const char *path,
> +                bool raw,
> +                bool bytes)
>  {
>      virDomainBlockJobInfo info;
> -    bool ret = false;
> -    int rc = -1;

^^^

> -    bool raw = vshCommandOptBool(cmd, "raw");
> -    bool bytes = vshCommandOptBool(cmd, "bytes");
> -    bool abortMode = (vshCommandOptBool(cmd, "abort") ||
> -                      vshCommandOptBool(cmd, "async") ||
> -                      vshCommandOptBool(cmd, "pivot"));
> -    bool infoMode = vshCommandOptBool(cmd, "info") || raw;
> -    bool bandwidth = vshCommandOptBool(cmd, "bandwidth");
> -    virDomainPtr dom = NULL;
> -    const char *path;
> -    unsigned int flags = 0;
>      unsigned long long speed;
> -
> -    if (abortMode + infoMode + bandwidth > 1) {
> -        vshError(ctl, "%s",
> -                 _("conflict between abort, info, and bandwidth modes"));
> -        return false;
> -    }
> -    /* XXX also support --bytes with bandwidth mode */
> -    if (bytes && (abortMode || bandwidth)) {
> -        vshError(ctl, "%s", _("--bytes requires info mode"));
> -        return false;
> -    }
> -
> -    if (abortMode)
> -        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;
> -
> -    /* XXX Allow path to be optional to list info on all devices at once */
> -    if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
> -        goto cleanup;
> +    unsigned int flags = 0;
> +    bool ret = false;
> +    int rc;

^^^
virsh-domain.c: In function 'cmdBlockJob':
virsh-domain.c:2515:8: error: 'rc' may be used uninitialized in this
function [-Werror=maybe-uninitialized]
     if (rc == 0) {
        ^
virsh-domain.c:2472:9: note: 'rc' was declared here
     int rc;
         ^
virsh-domain.c:2532:13: error: 'speed' may be used uninitialized in this
function [-Werror=maybe-uninitialized]
             vshPrint(ctl, _("    Bandwidth limit: %llu bytes/s (%-.3lf
%s/s)"),
             ^
virsh-domain.c:2469:24: note: 'speed' was declared here
     unsigned long long speed;
                        ^
Initializing rc to -1 resolves the issue

ACK with the adjustment.

John

> 
>      /* If bytes were requested, or if raw mode is not forcing a MiB/s
>       * query and cache can't prove failure, then query bytes/sec.  */
> @@ -2562,7 +2534,54 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd)
>          }
>          vshPrint(ctl, "\n");
>      }
> +
>      ret = true;
> +
> + cleanup:
> +    return ret;
> +}
> +
> +
> +static bool
> +cmdBlockJob(vshControl *ctl, const vshCmd *cmd)
> +{
> +    bool ret = false;
> +    bool raw = vshCommandOptBool(cmd, "raw");
> +    bool bytes = vshCommandOptBool(cmd, "bytes");
> +    bool abortMode = (vshCommandOptBool(cmd, "abort") ||
> +                      vshCommandOptBool(cmd, "async") ||
> +                      vshCommandOptBool(cmd, "pivot"));
> +    bool infoMode = vshCommandOptBool(cmd, "info") || raw;
> +    bool bandwidth = vshCommandOptBool(cmd, "bandwidth");
> +    virDomainPtr dom = NULL;
> +    const char *path;
> +
> +    if (abortMode + infoMode + bandwidth > 1) {
> +        vshError(ctl, "%s",
> +                 _("conflict between abort, info, and bandwidth modes"));
> +        return false;
> +    }
> +    /* XXX also support --bytes with bandwidth mode */
> +    if (bytes && (abortMode || bandwidth)) {
> +        vshError(ctl, "%s", _("--bytes requires info mode"));
> +        return false;
> +    }
> +
> +    if (abortMode)
> +        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;
> +
> +    /* XXX Allow path to be optional to list info on all devices at once */
> +    if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
> +        goto cleanup;
> +
> +    ret = vshBlockJobInfo(ctl, dom, path, raw, bytes);
> +
>   cleanup:
>      if (dom)
>          virDomainFree(dom);
>