[PATCH] virsh: cmdBlockcopy: Remove 'error:' prefix for an empty line

[Peter Krempa]

On Wed, Jun 17, 2020 at 14:52:38 +0200, Ján Tomko wrote:
> On a Wednesday in 2020, Peter Krempa wrote:
> > When a block copy job fails prior to reaching the synchronized phase
> > while we are waiting for the job to finish virsh would print the
> > following:
> > 
> > $ virsh blockcopy backup-test vda /tmp/dst.qcow2 --wait --reuse-external --transient-job
> > error:
> > Copy failed
> > 
> > The above message looks like we've forgot to print the error message
> > itself as the line ends after 'error:'. Unfortunately with the current
> > API design clients have no way of actually getting the error message as
> > the VIR_DOMAIN_EVENT_ID_BLOCK_JOB(_2) event only reports the status but
> > not an error and the job then vanishes.
> > 
> > Fix the expectations by using vshPrintExtra instead of vshError:
> > 
> > $ virsh blockcopy backup-test vda /tmp/dst.qcow2 --wait --reuse-external --transient-job
> > 
> > Copy failed
> > 
> > Note that the newline is required to avoid printing the 'Copy failed'
> > message on the same line when printing the job progress percentage.
> > 
> > Inspired by https://bugzilla.redhat.com/show_bug.cgi?id=1847867
> > 
> > Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> > ---
> > tools/virsh-domain.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index 5222949566..3597fb6272 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -2457,7 +2457,7 @@ cmdBlockcopy(vshControl *ctl, const vshCmd *cmd)
> >             break;
> > 
> >         case VIR_DOMAIN_BLOCK_JOB_FAILED:
> > -            vshError(ctl, "\n%s", _("Copy failed"));
> > +            vshPrintExtra(ctl, "\n%s", _("Copy failed"));
> 
> If the newline is only needed after the percentage, we should
> print it based on the verbose parameter, like virshBlockJobWait
> does with the percentage.

Do we really care that much to add pointless eye-candy code?

> Even though we don't have a meaningful error, we should write something
> generic instead of quietly exiting (although with an error code).

Well we print "Copy failed". I don't really have any better idea what to
print at this point when the error is not passed to the client.

> 
> Also, the other two functions calling virshBlockJobWait seem to be
> affected.
> 
> Jano
> 
> >             goto cleanup;
> >             break;
> > 
> > -- 
> > 2.26.2
> >