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(a)redhat.com&gt; --- 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"));

goto cleanup; break; -- 2.26.2

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(a)redhat.com&gt; > > --- > > 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 > >