search for: nbd_aio_is_dead

Rich recently patched things to generate one man page per function rather than libnbd-api.3 (nice), but in doing so got stumped by a problem with a fresh git clone (automake fails for any 'include' directive that does not already exist). I've figured out how to hack around it, but the hack requires GNU make. We already use GNU make constructs elsewhere (such as $(wildcard)), but

...ketsize, packetsize, 0); if (handles[1] == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); goto error; } - in_flight++; /* Now wait for commands to retire, or for deadlock to occur */ - while (in_flight > 0) { + while (nbd_aio_in_flight (nbd) > 0) { if (nbd_aio_is_dead (nbd) || nbd_aio_is_closed (nbd)) { fprintf (stderr, "connection is dead or closed\n"); goto error; @@ -96,23 +92,20 @@ try_deadlock (void *arg) /* If a command is ready to retire, retire it. */ while ((done = nbd_aio_peek_command_completed (nbd)) > 0) { -...

...death + * above - depending on load and timing, nbd_poll may succeed or + * fail, and we may transition to either CLOSED (the state machine + * saw a clean EOF) or DEAD (the state machine saw a stranded + * transaction or POLLERR). + */ + while ((r = nbd_poll (nbd, 1000)) == 1) + if (nbd_aio_is_dead (nbd) || nbd_aio_is_closed (nbd)) + break; + if (!(nbd_aio_is_dead (nbd) || nbd_aio_is_closed (nbd))) { + fprintf (stderr, "%s: test failed: server death not detected\n", argv[0]); + goto fail; + } + + /* Proof that the read was stranded */ + if (nbd_aio_peek_command_comple...

...send NBD_CMD_DISC receive NBD_CMD_DISC shutdown(SHUT_WR) wake up on EOF nbd_aio_notify_read - move state to CLOSED nbd_aio_is_dead -> true break loop - poll pthread_join close fds nbd_close see EOF from client but sometimes things happen differently: nbd .close nbd reader server nbd_shutdown poll - send NBD_CMD_DISC - p...

...n { struct handle { /* These fields are read-only once initialized */ struct nbd_handle *nbd; - int fd; /* Cache of nbd_aio_get_fd */ int fds[2]; /* Pipe for kicking the reader thread */ bool readonly; pthread_t reader; @@ -316,7 +315,7 @@ nbdplug_reader (void *handle) while (!nbd_aio_is_dead (h->nbd) && !nbd_aio_is_closed (h->nbd)) { struct pollfd fds[2] = { - [0].fd = h->fd, + [0].fd = nbd_aio_get_fd (h->nbd), [1].fd = h->fds[0], [1].events = POLLIN, }; @@ -469,7 +468,6 @@ nbdplug_open_handle (int readonly) #endif retry: -...

Reverts commit 387cbe67c3db27e8a61117fedb6e7fad76e409ef. --- generator/generator | 18 +++++++++++++++++- lib/handle.c | 28 +++++++++++++++++++++++++++- tests/closure-lifetimes.c | 4 +++- 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/generator/generator b/generator/generator index 2cd83f1..25e4aa5 100755 --- a/generator/generator +++ b/generator/generator

...t;nbd_aio_is_created (h)" > - | Connecting -> "nbd_aio_is_connecting (h)" > - | Connected -> "nbd_aio_is_ready (h) || nbd_aio_is_processing (h)" > - | Closed -> "nbd_aio_is_closed (h)" > - | Dead -> "nbd_aio_is_dead (h)" > + | Created -> "nbd_internal_is_state_created (state)" > + | Connecting -> "nbd_internal_is_state_connecting (state)" > + | Connected -> "nbd_internal_is_state_ready (state) || nbd_internal_is_state_processing (state)...

...disconnect \ - nbd_aio_flush \ - nbd_aio_trim \ - nbd_aio_cache \ - nbd_aio_zero \ - nbd_aio_block_status \ - nbd_aio_get_fd \ - nbd_aio_get_direction \ - nbd_aio_notify_read \ - nbd_aio_notify_write \ - nbd_aio_is_created \ - nbd_aio_is_connecting \ - nbd_aio_is_ready \ - nbd_aio_is_processing \ - nbd_aio_is_dead \ - nbd_aio_is_closed \ - nbd_aio_command_completed \ - nbd_aio_peek_command_completed \ - nbd_aio_in_flight \ - nbd_connection_state \ - nbd_get_package_name \ - nbd_get_version \ - nbd_kill_command \ - nbd_supports_tls \ - nbd_supports_uri \ - $(NULL) diff --git a/generator/generator b/generator/...

...disconnect \ + nbd_aio_flush \ + nbd_aio_trim \ + nbd_aio_cache \ + nbd_aio_zero \ + nbd_aio_block_status \ + nbd_aio_get_fd \ + nbd_aio_get_direction \ + nbd_aio_notify_read \ + nbd_aio_notify_write \ + nbd_aio_is_created \ + nbd_aio_is_connecting \ + nbd_aio_is_ready \ + nbd_aio_is_processing \ + nbd_aio_is_dead \ + nbd_aio_is_closed \ + nbd_aio_command_completed \ + nbd_aio_peek_command_completed \ + nbd_aio_in_flight \ + nbd_connection_state \ + nbd_get_package_name \ + nbd_get_version \ + nbd_kill_subprocess \ + nbd_supports_tls \ + nbd_supports_uri \ + $(NULL) diff --git a/generator/generator b/generat...

...etsize, 0); + if (handles[1] == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + goto error; + } + fprintf (stderr, " * after aio_pwrite\n"); + in_flight++; + + /* Now wait for commands to retire, or for deadlock to occur */ + while (in_flight > 0) { + if (nbd_aio_is_dead (conn) || nbd_aio_is_closed (conn)) { + fprintf (stderr, "connection is dead or closed\n"); + goto error; + } + + fds[0].fd = nbd_aio_get_fd (conn); + fds[0].events = 0; + fds[0].revents = 0; + dir = nbd_aio_get_direction (conn); + if ((dir & LIBNBD_AIO_DIR...

...handles[1] = nbd_aio_pwrite (conn, out, packetsize, packetsize, 0); + if (handles[1] == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + goto error; + } + in_flight++; + + /* Now wait for commands to retire, or for deadlock to occur */ + while (in_flight > 0) { + if (nbd_aio_is_dead (conn) || nbd_aio_is_closed (conn)) { + fprintf (stderr, "connection is dead or closed\n"); + goto error; + } + + fds[0].fd = nbd_aio_get_fd (conn); + fds[0].events = 0; + fds[0].revents = 0; + dir = nbd_aio_get_direction (conn); + if ((dir & LIBNBD_AIO_DIR...

...| Created -> "nbd_aio_is_created (h)" - | Connecting -> "nbd_aio_is_connecting (h)" - | Connected -> "nbd_aio_is_ready (h) || nbd_aio_is_processing (h)" - | Closed -> "nbd_aio_is_closed (h)" - | Dead -> "nbd_aio_is_dead (h)" + | Created -> "nbd_internal_is_state_created (state)" + | Connecting -> "nbd_internal_is_state_connecting (state)" + | Connected -> "nbd_internal_is_state_ready (state) || nbd_internal_is_state_processing (state)" +...

...gaction (SIGUSR1, &act, NULL) == -1) { + nbdkit_error ("sigaction: %m"); + return -1; + } + + /* Check the user passed exactly one socket description. */ if (sockname) { struct sockaddr_un sock; @@ -306,37 +335,27 @@ nbdplug_reader (void *handle) int r; while (!nbd_aio_is_dead (h->nbd) && !nbd_aio_is_closed (h->nbd)) { - struct pollfd fds[2] = { - [0].fd = h->fd, - [1].fd = h->fds[0], - [1].events = POLLIN, - }; + struct pollfd fd; struct transaction *trans, **prev; int dir; - char c; + fd.fd = h->fd; d...

...read_mutex_t trans_lock; /* Covers access to trans list */ - struct transaction *trans; /* List of pending transactions */ }; /* Connect to server via absolute name of Unix socket */ @@ -306,7 +302,6 @@ void * nbdplug_reader (void *handle) { struct handle *h = handle; - int r; while (!nbd_aio_is_dead (h->nbd) && !nbd_aio_is_closed (h->nbd)) { struct pollfd fds[2] = { @@ -314,7 +309,6 @@ nbdplug_reader (void *handle) [1].fd = h->fds[0], [1].events = POLLIN, }; - struct transaction *trans, **prev; int dir; char c; @@ -343,61 +337,9 @@ nbdplug...

I fixed the deadlock - turned out to be an actual bug in the nbd plugin (see patch 8). I changed the command syntax so it's now: nbdkit nbd command=qemu arg=-f arg=qcow2 arg=/path/to/disk.qcow2 Nir wrote: 18:08 < nsoffer> rwmjones: regarding the nbd proxy patches, did you have specific flow that help us? 18:08 < nsoffer> rwmjones: or this is just a way to support qcow2 in the

...ng, to see whether I can: a) reproduce the hang with patch 2 not applied b) feel confident that patch 2 is sufficient to fix the race, or else determine that I also need to augment the loop condition in the reader thread to additionally break out of the loop when the pipe-to-self sees EOF even when nbd_aio_is_dead() has not yet been satisfied I'm also fairly confident that I'll need to patch libnbd to call shutdown(SHUT_WR) on plaintext and gnutls_bye(GNUTLS_SHUT_WR) on TLS connections after sending NBD_CMD_DISC, as that is something we lowt when the nbd plugin switched to libnbd, even after documen...

...EXIT_FAILURE); + } + + if (nbd_set_concurrent_writer (nbd, &writer_data, writer) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + + /* Issue commands. */ + in_flight = 0; + i = NR_CYCLES; + while (i > 0 || in_flight > 0) { + if (nbd_aio_is_dead (nbd) || nbd_aio_is_closed (nbd)) { + fprintf (stderr, "thread %zu: connection is dead or closed\n", + status->i); + goto error; + } + + /* If we can issue another request, do so. Note that we reuse the + * same buffer for multiple in-flight requests....

...EXIT_FAILURE); + } + + if (nbd_set_concurrent_writer (nbd, &writer_data, writer) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + + /* Issue commands. */ + in_flight = 0; + i = NR_CYCLES; + while (i > 0 || in_flight > 0) { + if (nbd_aio_is_dead (nbd) || nbd_aio_is_closed (nbd)) { + fprintf (stderr, "thread %zu: connection is dead or closed\n", + status->i); + goto error; + } + + /* Do we want to send another request and there's room to issue it + * and the connection is in the READY state...

I need to think about this patch series a bit more, but it does at least pass the tests. Rich.

This might not be the final solution, but it certainly seems to solve a deadlock for me that I could trigger by using 'nbdkit --filter=noparallel memory 512k' and calling nbd_aio_pread for a request larger than 256k (enough for the Linux kernel to block the server until libnbd read()s), immediately followed by nbd_aio_pwrite for a request larger than 256k (enough to block libnbd until the