Re: [Qemu-devel] [PATCH v2 2/6] nbd/server: use errp instead of LOG

[Vladimir Sementsov-Ogievskiy]

Markus, what do you think? I'll resend tomorrow with Eric's comments 
applied, should I change something with this errp?

29.06.2017 22:27, Eric Blake wrote:
> On 06/21/2017 10:34 AM, Vladimir Sementsov-Ogievskiy wrote:

[...]

> > -static int GCC_FMT_ATTR(4, 5)
> > +static int GCC_FMT_ATTR(5, 6)
> >   nbd_negotiate_send_rep_err(QIOChannel *ioc, uint32_t type,
> > -                           uint32_t opt, const char *fmt, ...)
> > +                           uint32_t opt, Error **errp, const char *fmt, ...)
> >   {
> Markus, this violates our usual idiom of errp last in code that is not
> directly operating on an Error (the actual Error implementation in
> error.c being the main obvious exception that lists errp first), but I
> don't see any better approach. Do you have any thoughts on it?
>
> > -    if (nbd_read(client->ioc, name, length, NULL) < 0) {
> > -        LOG("read failed");
> > +    if (nbd_read(client->ioc, name, length, errp) < 0) {
> > +        error_prepend(errp, "read failed: ");
> >           return -EINVAL;
> >       }

[...]

> Overall the change looks good to me, but I'll wait to see if Markus has
> any design advice before giving my R-b.

--
Best regards,
Vladimir