Re: [Qemu-devel] [PATCH 2/6 for-3.1] nbd: stop waiting for a NBD response with NBD_CMD_DISC

[Eric Blake]

On 11/19/18 4:23 AM, Daniel P. Berrangé wrote:

> > > Simply setting the 'quit' flag after sending NBD_CMD_DISC is enough to
> > > get the coroutine to stop waiting for a reply and thus supress the error
> > > message.
> >
> > Actually, it's not quite enough - once you actually start performing I/O,
> > enough coroutines are kicked off that the error still happens:
>
> Hmm, does the client not send requests synchronously  ? I expected that
> any other I/O operations would have already received their reply by the
> time we sent the DISC command.

During handshake phase (NBD_OPT_* messages), client sends requests 
synchronously (at most one pending read, in response to the most recent 
write). During transmission phase (NBD_CMD_* messages), the client is 
allowed to send messages asynchronously.  However, the NBD protocol 
recommends (but not requires) that the client never send NBD_CMD_DISC to 
disconnect until AFTER the client has collected all responses to any 
earlier in-flight messages. And if I'm reading the block/nbd-client.c 
code correctly, in general we don't try to shutdown a client while there 
are any pending commands with unread replies, except when we are 
shutting down due to detecting a protocol error (at which point we are 
already out of sync with the server and waiting for replies is not going 
to be helpful).


> > But I want to do more testing to make sure I'm not missing out on reporting
> > an actual error if I add that.
>
> Yes, I'd be a little concerned about missing reporting of an error from
> a command other than NBD_CMD_DISC if we did this.

But it looks like your approach of making the qio code behave more 
nicely is a smarter cleanup than my idea, so I'm willing to ditch my 
code in favor of yours.  Testing your approach now :)

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