Re: [Qemu-devel] [PATCH 17/19] nbd/common: nbd_tls_handshake: use error_reportf_err instead of TRACE

[Eric Blake]

On 05/30/2017 09:30 AM, Vladimir Sementsov-Ogievskiy wrote:
> Use error_reportf_err instead of TRACE in case of fail.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  nbd/common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/nbd/common.c b/nbd/common.c
> index 88e0297fb2..574a551abe 100644
> --- a/nbd/common.c
> +++ b/nbd/common.c
> @@ -102,7 +102,7 @@ void nbd_tls_handshake(QIOTask *task,
>      struct NBDTLSHandshakeData *data = opaque;
>  
>      if (qio_task_propagate_error(task, &data->error)) {
> -        TRACE("TLS failed %s", error_get_pretty(data->error));
> +        error_reportf_err(data->error, "TLS failed");

I don't think this is right. You already populated &data->error(), which
means you have the error message available to the caller, and should let
the caller handle the message rather than blindly reporting it here
yourself (especially since if the caller also reports it, you've now
doubled up error messages).  Converting this TRACE() into a proper
tracepoint may be okay, but it may also be sufficient to just delete
this TRACE() since the caller should already be handling the failure.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature