[Libguestfs] [nbdkit PATCH 1/2] nbd: Inform server of intent to quit
During .close, the nbd plugin waits for pthread_join() to
collect the reader thread, ensuring we have read any remaining
replies from the server prior to giving up control. However,
after we have sent NBD_CMD_DISC, we end up in a blocking read()
that won't progress until the server hangs up its end (the NBD
protocol says the server won't reply to NBD_CMD_DISC, but it
may still be flushing replies to other pending requests). But
if the server is likewise stuck in a read() to see if we
(erroneously) send anything after NBD_CMD_DISC (which is the
case when nbdkit is the remote server, using parallel threads
where the next read() is started before processing the command
that was just read off the wire), then our NBD plugin and the
remote NBD server are deadlocked with both sides trying to read
from the other.
Prior to commit 9e6d990f, this deadlock was (typically?) not
visible in the testsuite: after the client quits, nbdkit code
was unloading the nbd plugin without waiting for .close to
complete, at which point process exit() breaks the deadlock
(unloading the .so while code is still running might also
explain some harder-to-reproduce crashes that also sometimes
plagued the testsuite). But now that an ongoing .close
inhibits an early unload (which is a GOOD thing), the deadlock
is hanging the testsuite on tests involving the nbd plugin
when the server doesn't immediately hang up after NBD_CMD_DISC.
Using nonblocking reads with a poll() loop that handles both
reads and writes from a single thread could probably solve the
issue (as then, nbd_close wouldn't have to wait to join a
thread stuck on blocking I/O); but refactoring the code to
support nonblocking I/O is rather invasive.
Instead, we can use shutdown() to inform the remote server that
we will not be writing any more data to the socket; if the
remote server is blocked on read(), it will now see an immediate
EOF rather than waiting forever for something we will never
write, and can proceed on its cleanup paths to eventually close
its end, so that our NBD reader will likewise complete quickly
and let us leave nbd_close(); then the .so can be unloaded with
no plugin code running.
Note that the deadlock also goes away if the server hangs up
automatically after flushing all prior pending requests when
NBD_CMD_DISC is received, and that nbdkit will be patched
along those lines in the next patch. But while that fixes
our testsuite in our own setup, it's better to fix our plugin
to not be reliant on the server behavior, since we can't
guarantee that all other servers behave the same way as nbdkit.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
plugins/nbd/nbd.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
index a4a1f12..b9e72bc 100644
--- a/plugins/nbd/nbd.c
+++ b/plugins/nbd/nbd.c
@@ -568,8 +568,10 @@ nbd_close (void *handle)
{
struct handle *h = handle;
- if (!h->dead)
+ if (!h->dead) {
nbd_request_raw (h, 0, NBD_CMD_DISC, 0, 0, 0, NULL);
+ shutdown (h->fd, SHUT_WR);
+ }
close (h->fd);
if ((errno = pthread_join (h->reader, NULL)))
nbdkit_debug ("failed to join reader thread: %m");
--
2.14.3