Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] block/nbd-client: use traces instead of noisy error_report_err

[Eric Blake]

On 12/18/18 3:23 PM, Eric Blake wrote:
> On 11/2/18 10:11 AM, Vladimir Sementsov-Ogievskiy wrote:
> > Reduce extra noise of nbd-client, change 083 correspondingly.
>
> This says what, but not why. The details from the cover letter are 
> important to include here, namely:
>
> > It was discussed, that error messages, produced by error_reprt_err's,
> > added in f140e300 are
> > 1. not really needed
> > 2. subject to race conditions
> >
> > And it was decided to drop them (switch to trace-points), look thread
> > https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg00833.html
> >
> > So, I've also dropped error_report_err, added earlier in be41c100c0d
> > and later in 78a33ab5878.

> > @@ -79,7 +81,9 @@ static coroutine_fn void nbd_read_reply_entry(void 
> > *opaque)
> >           assert(s->reply.handle == 0);
> >           ret = nbd_receive_reply(s->ioc, &s->reply, &local_err);
> >           if (local_err) {
> > -            error_report_err(local_err);
> > +            trace_nbd_read_reply_entry_fail(ret, 
> > error_get_pretty(local_err),
> > +                                            error_get_hint(local_err) 
> > ?: "");
>
> I'm not sold on the error hint being useful in the trace message.  For 
> that matter, even error_get_pretty() seems like it might be rather verbose.
>
> I do see why you are trying it, though: in nbd/client.c, 
> nbd_handle_reply_err(), we have:

Actually, on looking further, the ONLY use of error_append_hint() in 
nbd/client.c is when handling NBD_OPT_ which is synchronous when first 
establishing the connection; but all of the additions of trace_nbd_* 
calls in this file occur during transmission phase, where we don't have 
any hints appended.  I could NOT trigger any error path where a hint 
would be present in the first place (although I will admit that I may 
have missed a spot in my tracing).

> But instead of trying to make all the tracepoints in block/nbd-client.c 
> extract this information, we could just improve nbd/client.c to have a 
> tracepoint for any server-received error message at the same point where 
> it calls error_append_hint().

This part is still true, even if it is no longer relevant to this patch.

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