[libvirt] [PATCH] virsh: Fix job status indicator for 0 length block jobs

[Michal Privoznik]

On 14.09.2015 09:22, Peter Krempa wrote:
> Although 0 length block jobs aren't entirely useful, the output of virsh
> blockjob is empty due to the condition that suppresses the output for
> migration jobs that did not start. Add a flag for virshPrintJobProgress
> that specifies that the job was started so that the function can be used
> both in migration where the job starts delayed and for block jobs where
> we already know that the job was started.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1196711
> ---
>  tools/virsh-domain.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index b029b65..7837974 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -1683,13 +1683,23 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
>  }
> 
> 
> +/**
> + * virshPrintJobProgress:
> + * @label: name of the block job
> + * @remaining: amount of remaining work
> + * @total: amount of total work
> + * @started: The block job was started already
> + *
> + * Prints a human friendly percentage string decribing the state of the given
> + * job. The output is omitted if the job was not started.
> + */
>  static void
>  virshPrintJobProgress(const char *label, unsigned long long remaining,
> -                      unsigned long long total)
> +                      unsigned long long total, bool started)
>  {
>      int progress;
> 

Since you are passing true almost anywhere except one place, can't we
just do ..

> -    if (total == 0)
> +    if (total == 0 && !started)
>          /* migration has not been started */
>          return;
> 
> @@ -1921,7 +1931,7 @@ virshBlockJobWait(virshBlockJobWaitDataPtr data)
> 
>          if (data->verbose)
>              virshPrintJobProgress(data->job_name, info.end - info.cur,
> -                                  info.end);
> +                                  info.end, true);
> 
>          if (data->timeout && virTimeMillisNow(&curr) < 0) {
>              vshSaveLibvirtError();
> @@ -1949,7 +1959,7 @@ virshBlockJobWait(virshBlockJobWaitDataPtr data)
>      if (data->verbose &&
>          (ret == VIR_DOMAIN_BLOCK_JOB_COMPLETED ||
>           ret == VIR_DOMAIN_BLOCK_JOB_READY))
> -        virshPrintJobProgress(data->job_name, 0, 1);
> +        virshPrintJobProgress(data->job_name, 0, 1, true);
> 
>      sigaction(SIGINT, &old_sig_action, NULL);
>      return ret;
> @@ -2621,7 +2631,7 @@ virshBlockJobInfo(vshControl *ctl,
>                   info.bandwidth, info.cur, info.end);
>      } else {
>          virshPrintJobProgress(virshDomainBlockJobToString(info.type),
> -                              info.end - info.cur, info.end);
> +                              info.end - info.cur, info.end, true);
>          if (speed) {
>              const char *unit;
>              double val = vshPrettyCapacity(speed, &unit);
> @@ -4356,7 +4366,7 @@ virshWatchJob(vshControl *ctl,
>                  retchar == '0') {
>                  if (verbose) {
>                      /* print [100 %] */
> -                    virshPrintJobProgress(label, 0, 1);
> +                    virshPrintJobProgress(label, 0, 1, true);
>                  }
>                  break;
>              }
> @@ -4392,7 +4402,7 @@ virshWatchJob(vshControl *ctl,
>              if (ret == 0) {
>                  if (verbose)

.. if (verbose && jobStarted) virshPrintJobProgress();

>                      virshPrintJobProgress(label, jobinfo.dataRemaining,
> -                                          jobinfo.dataTotal);
> +                                          jobinfo.dataTotal, false);

Moreover, this false looks spurious to me. It should be @jobStarted,
since @jobStarted would be false only on the first iteration over the loop.

> 
>                  if (!jobStarted &&
>                      (jobinfo.type == VIR_DOMAIN_JOB_BOUNDED ||
> 

Michal