Re: [PATCH for-6.2 1/2] nbd/server: Don't complain on certain client disconnects

[Eric Blake]

On Wed, Nov 17, 2021 at 08:57:35PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 17.11.2021 20:02, Eric Blake wrote:

> > This patch fixes things to avoid uninitialized memory, and in general
> > avoids warning about a client that does a hard shutdown when not in
> > the middle of a packet.  A client that aborts mid-request, or which
> > does not read the full server's reply, can still result in warnings,
> > but those are indeed much more unusual situations.
> > 
> > CC: qemu-stable@nongnu.org
> > Fixes: f148ae7d36 (nbd/server: Quiesce coroutines on context switch)
> > Signed-off-by: Eric Blake <eblake@redhat.com>
> > ---
> >   nbd/server.c | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/nbd/server.c b/nbd/server.c
> > index d9164ee6d0da..85877f630533 100644
> > --- a/nbd/server.c
> > +++ b/nbd/server.c
> > @@ -1418,6 +1418,9 @@ static int nbd_receive_request(NBDClient *client, 
> > NBDRequest *request,
> >       if (ret < 0) {
> >           return ret;
> >       }
> > +    if (ret == 0) {
> > +        return -EIO;
> > +    }
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
> I'd prefer not include following hunks to the patch, as they are unrelated.
> 
> > 
> >       /* Request
> >          [ 0 ..  3]   magic   (NBD_REQUEST_MAGIC)
> > @@ -2285,7 +2288,7 @@ static int nbd_co_receive_request(NBDRequestData 
> > *req, NBDRequest *request,
> >       assert(client->recv_coroutine == qemu_coroutine_self());
> >       ret = nbd_receive_request(client, request, errp);
> >       if (ret < 0) {
> > -        return  ret;
> > +        return ret;
> >       }
> > 
> >       trace_nbd_co_receive_request_decode_type(request->handle, 
> > request->type,
> > @@ -2662,7 +2665,7 @@ static coroutine_fn void nbd_trip(void *opaque)
> >       }
> > 
> >       if (ret < 0) {
> > -        /* It wans't -EIO, so, according to nbd_co_receive_request()
> > +        /* It wasn't -EIO, so, according to nbd_co_receive_request()

Yeah, they were typo fixes I noticed while investigating the code; I
should have either called them out in the commit message, or as you
say, split them into a separate trivial patch (in which case they
aren't urgent for 6.2).  I'm happy to do the latter, since I'm working
on more patches for qemu 7.0 to add a new NBD protocol extension
NBD_OPT_EXTENDED_HEADERS to expose your work on 64-bit write-zero
support end-to-end.

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