search for: nbd_shutdown

As mentioned in commits 176fc4ea and 609c25f0, our original plan in adding a flags argument to nbd_shutdown was to let us specify different behaviors at the libnbd level, rather than NBD protocol flags (for that, the user has nbd_aio_disconnect). But when we later parameterized OFlags to accept various bitmasks (commit f891340b), we failed to mark nbd_shutdown as using a different bitmask than NBD_CMD_F...

..._join() the reader thread, while the reader thread is itself blocked on a poll() that will never make additional progress. In hindsight, the problem is obvious: we used the wrong libnbd API (synchronous instead of async), which often results in: nbd .close nbd reader server nbd_shutdown poll - send NBD_CMD_DISC receive NBD_CMD_DISC shutdown(SHUT_WR) wake up on EOF nbd_aio_notify_read - move state to CLOSED...

On Fri, Sep 11, 2020 at 09:31:11AM -0500, Eric Blake wrote: > As mentioned in commits 176fc4ea and 609c25f0, our original plan in > adding a flags argument to nbd_shutdown was to let us specify > different behaviors at the libnbd level, rather than NBD protocol > flags (for that, the user has nbd_aio_disconnect). But when we later > parameterized OFlags to accept various bitmasks (commit f891340b), we > failed to mark nbd_shutdown as using a different bi...

...he client to the 'nbdkit nbd' process has cleanly exited, > things are stalled in .close where nbd is trying to pthread_join() the > reader thread, while the reader thread is itself blocked on a poll() > that will never make additional progress. Tracing the race is > difficult: nbd_shutdown() sends NBD_CMD_DISC to the server, and the > NBD protocol does not require the server to send a response but does > not forbid the server from using gnutls_bye to at least gracefully end > the TLS session. So where a plaintext server just closes the socket > (and the resulting EOF exi...

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

I still need more soak time on testing, 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

...where even though the client to the 'nbdkit nbd' process has cleanly exited, things are stalled in .close where nbd is trying to pthread_join() the reader thread, while the reader thread is itself blocked on a poll() that will never make additional progress. Tracing the race is difficult: nbd_shutdown() sends NBD_CMD_DISC to the server, and the NBD protocol does not require the server to send a response but does not forbid the server from using gnutls_bye to at least gracefully end the TLS session. So where a plaintext server just closes the socket (and the resulting EOF exits our poll()), it a...

On 9/17/20 8:22 AM, Richard W.M. Jones wrote: > On Fri, Sep 11, 2020 at 09:31:11AM -0500, Eric Blake wrote: >> As mentioned in commits 176fc4ea and 609c25f0, our original plan in >> adding a flags argument to nbd_shutdown was to let us specify >> different behaviors at the libnbd level, rather than NBD protocol >> flags (for that, the user has nbd_aio_disconnect). But when we later >> parameterized OFlags to accept various bitmasks (commit f891340b), we >> failed to mark nbd_shutdown as usin...

...e", cookie); + trans->cookie = cookie; if (write (h->fds[1], &c, 1) != 1 && errno != EAGAIN) nbdkit_debug ("failed to kick reader thread: %m"); @@ -504,7 +510,7 @@ nbdplug_open (int readonly) static void nbdplug_close_handle (struct handle *h) { - if (nbd_shutdown (h->nbd) == -1) + if (nbd_shutdown (h->nbd, 0) == -1) nbdkit_debug ("failed to clean up handle: %s", nbd_get_error ()); if ((errno = pthread_join (h->reader, NULL))) nbdkit_debug ("failed to join reader thread: %m"); diff --git a/README b/README index 2661...

On Thu, Jul 25, 2019 at 03:19:23PM -0500, Eric Blake wrote: > This looks a bit funny until I read the docs at [1]. When using > kill(2), I'm used to the function call 'kill(pid, 0)' probing for > process existence. But on the command line, kill(1) has the behavior of > sending SIGTERM by default when you omit a signal number (and not > serving as a process existence

...h allows the state machine to break during reply processing) means we definitely don't need threads. I imagine that two threads doing simultaneous send(2) and recv(2) calls could still improve performance (eg. having two cores copying skbs from userspace to and from kernel). * Should ‘nbd_shutdown’ take an extra parameter to indicate that it should be delayed until all commands in the queue are retired? Is there anything else? We could also consider doing a "soft stable API" release where we bump the version up to 0.9.x, announce that we're going to make the API stable soon...

...fter that point even while still allowing replies from the server for existing in-flight commands. The protocol also recommends that NBD_CMD_DISC not be sent until there are no other pending in-flight commands, but at the moment, we place that burden on the client. Perhaps we should add a knob to nbd_shutdown and/or add a new API nbd_aio_in_flight returning the number of in-flight commands, to make things easier? --- lib/disconnect.c | 6 ++++-- lib/internal.h | 2 ++ lib/rw.c | 5 +++++ tests/errors.c | 9 ++++++--- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/discon...

...nbd_set_opt_mode (nbd, true); - if (!map && !size_only) nbd_set_full_info (nbd, true); + } if (map) nbd_add_meta_context (nbd, map); @@ -320,10 +326,32 @@ main (int argc, char *argv[]) } free (export_list.names); free (export_list.descs); + nbd_opt_abort (nbd); + nbd_shutdown (nbd, 0); nbd_close (nbd); exit (EXIT_SUCCESS); } +static int +collect_context (void *opaque, const char *name) +{ + struct context_list **head = opaque; + struct context_list *next = malloc (sizeof *next); + + if (!next) { + perror ("malloc"); + exit (EXIT_FAILURE); + }...

...l there are no other commands in flight +(see C<nbd_aio_in_flight>), to give the server a better chance at a +clean shutdown. + The C<flags> parameter must be C<0> for now (it exists for future NBD protocol extensions). There is no direct synchronous counterpart; however, C<nbd_shutdown> will call this function if appropriate."; @@ -1867,16 +1873,16 @@ you would set C<events = POLLIN>. If C<revents> returns C<POLLIN> or C<POLLHUP> you would then call C<nbd_aio_notify_read>. Note that once libnbd reaches C<nbd_aio_is_ready>, this dire...

This patch is a straightforward refactoring of libnbd?s nbdcopy program, and not very interesting. However I have plans for nbdcopy (see full todo below). I would like to use this utility for virt-v2v as a replacement for ?qemu-img convert?. qemu-img has caused us a series of problems: - change in zeroing behaviour caused a big performance regression - qemu-img reads extents up-front which

...have non-blocking I/O, I'm not seeing that it will make any noticeable difference. That said, I do know that you were experimenting at one point about adding a way to offload writing to a user-controlled thread, and maybe it's still worth playing with that a bit more. > > * Should ‘nbd_shutdown’ take an extra parameter to indicate that it > should be delayed until all commands in the queue are retired? That may still be worthwhile to pursue. > > Is there anything else? Do we like the signature of all the callbacks? Right now, there is a slight inconsistency in that the '...

...send a default. Hopefully people will read the docs :-) Your explanation is good enough for me :) > > I pushed it with s/signal/signum/. > > Are we waiting on any other ABI breaks or would now be a good time to > do another 0.x release? I know we've debated about a flag to nbd_shutdown to give the user more control over whether to wait on in-flight commands to settle, but without code for that, I don't see it stalling another 0.x release. In the meantime, we've definitely broken API/ABI, so another release makes it easier to test against. Still, I'd appreciate a few...

These patches aren't related to each other, but both are quite simple. The second one requires particular attention - it's my experience that printing out the state transitions in debug mode has never helped me to diagnose a bug, but it has made the debug logs huge and hard to follow. However that might just be me! Has it helped anyone else? Also I'm open to the concept of debug

...u-img convert). -NBD_OPT_INFO mode (like qemu-nbd -L). - NBD resize extension. -NBD_INFO_BLOCK_SIZE. - TLS should properly shut down the session (calling gnutls_bye). Performance: Chart it over various buffer sizes and threads, as that @@ -71,12 +67,3 @@ Suggested API improvements: maybe nbd_shutdown should wait for the subprocess or there should be another API to do this - capture error message when nbd_connect_command fails - - list exports - - It should be possible to get details from each export without - needing to reconnect. This would make nbdinfo --list much more - effi...

...| 66 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 84 insertions(+), 2 deletions(-) diff --git a/TODO b/TODO index 6c03736..2f23a34 100644 --- a/TODO +++ b/TODO @@ -57,3 +57,4 @@ Suggested API improvements: - it should be possible to use nbd_close and never block, so maybe nbd_shutdown should wait for the subprocess or there should be another API to do this + - capture error message when nbd_connect_command fails diff --git a/configure.ac b/configure.ac index 6296ccf..35a4fed 100644 --- a/configure.ac +++ b/configure.ac @@ -77,6 +77,16 @@ AC_CHECK_HEADERS([\ stdatomic....