|
From: | Eric Blake |
Subject: | Re: [Qemu-block] [PATCH 2/6 for-3.1] nbd: stop waiting for a NBD response with NBD_CMD_DISC |
Date: | Mon, 19 Nov 2018 08:24:00 -0600 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.0 |
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
[Prev in Thread] | Current Thread | [Next in Thread] |