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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On a Wednesday in 2020, Peter Krempa wrote:
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@xxxxxxxxxx>
> ---
> 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?


That's the question you have to ask yourself. I don't really care about
anything lately.

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.


Unlike vshError, vshPrintExtra does not print anything in quiet mode.

Jano


Also, the other two functions calling virshBlockJobWait seem to be
affected.

Jano

>             goto cleanup;
>             break;
>
> --
> 2.26.2
>


Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux