search for: nbd_aio_is_closed

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

...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) { - for (i = 0; i < in_flig...

...nding 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_completed (nbd) != 0) { + fpri...

...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: - h->fd = -1; h->nbd = nbd_create...

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

...( > function > - | 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_st...

...io_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/generator index 257303...

...io_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/generator/generator index 6cc...

...[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_DIRECTION_READ) != 0) + fd...

...e (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_DIRECTION_READ) != 0) + fd...

...ests = List.map ( function - | 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) ||...

...{ + 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; dir = nbd_aio_get_direction (h->nbd);...

...s 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_reader (void *handle) break;...

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

...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. It doesn't matter +...

...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 so it can be used to +...

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

I'm not observing any noticeable performance differences, but I'm liking the diffstat. I can't push this patch until we release a new libnbd version with the _notify API addition, but am posting it now for playing with things. Eric Blake (2): nbd: Move transaction info from heap to stack nbd: Use nbdkit aio_*_notify variants plugins/nbd/nbd.c | 217

..."incorrect structured replies %" PRId64 ", expected 1\n", r); + exit (EXIT_FAILURE); + } + + /* Quitting negotiation should be graceful. */ + if (nbd_opt_abort (nbd) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (nbd_aio_is_closed (nbd) != true) { + fprintf (stderr, "unexpected state after abort\n"); + exit (EXIT_FAILURE); + } + + /* As negotiation never finished, we have no size. */ + if ((r = nbd_get_size (nbd)) != -1) { + fprintf (stderr, "%s: test failed: incorrect size, " + &q...