[libvirt] [PATCH v2 2/3] virfile: Report error in virFileWrapperFdFree()

[John Ferlan]

On 2/13/19 7:04 AM, Andrea Bolognani wrote:
> Logging the error is fine and all, but getting the information
> to the user directly is even better.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1578741
> Signed-off-by: Andrea Bolognani <abologna at redhat.com>
> ---
>  src/util/virfile.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 

Shall I assume this works because qemuProcessHandleDumpCompleted will
move this message now as opposed to some other message related to EPIPE
as noted in the bz response?

I think it's good to fill in pieces of the commit message at least so if
someone else ends up in this same code they have a few hints where to start.

My other throughts would be to note commit 1f25194ad and 34e8f63a3 as
the genesis of at least printing the message "somewhere"... Then commit
b0c3e931 at least made sure the message got printed in the event that
the *FdClose never occurred. Just saying "is fine and all" hand waves a
bit too much for me compared to what you got to figure out here ;-).

Reviewed-by: John Ferlan <jferlan at redhat.com>

John

> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 271bf5e796..30cad87df9 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -351,8 +351,12 @@ virFileWrapperFdFree(virFileWrapperFdPtr wfd)
>      if (!wfd)
>          return;
>  
> +    /* If the command used to process IO has produced errors, it's fair
> +     * to assume those will be more relevant to the user than whatever
> +     * eg. QEMU can figure out on its own, so it's okay if we end up
> +     * discarding an existing error */
>      if (wfd->err_msg && *wfd->err_msg)
> -        VIR_WARN("iohelper reports: %s", wfd->err_msg);
> +        virReportError(VIR_ERR_OPERATION_FAILED, "%s", wfd->err_msg);
>  
>      virCommandAbort(wfd->cmd);
>  
>