similar to: [nbdkit PATCH] nbd: Fix sporadic use-after-free

With this, I am finally able to get the nbd plugin to do out-of-order responses to the client. Once this series goes in, we should be ready for Rich to cut a release. Eric Blake (4): nbd: Split reading into separate thread nbd: Protect writes with mutex nbd: Enable parallel handling tests: Test parallel nbd behavior plugins/nbd/nbd.c | 217

In preparation for allowing interleaved response, refactor the nbd forwarder so that writes are still done from the thread handling the original request from the client, but all reads are done by a dedicated reader thread. Control between the two flags is gated by a mutex for storing the transaction information, coupled with a pipe for the reader thread to send the final status back to the

Time to enhance the nbd plugin to request structured replies from the server. For now, deal only with structured reads. The server can now return sparse reads, even though we need nbdkit version 3 before we can in turn return sparse reads back to the client. In general, we have to assume the server is malicious, and so we must sanity check that it sends replies we expect. Thus, we have a choice

I'm hoping to implement .extents for the nbd plugin; this is a prerequisite. I'm not sure about patch 3 - if we like it, I'll squash it to 2, if we don't, I think we are okay just dropping it. I'm also wondering if we have to worry about malicious plugins that don't populate the entire .pread buffer in an effort to get nbdkit to expose portions of the heap; my patch 7 loses

I ran some local testing on my Fedora 29 system via: $ ./nbdkit memory 1m $ for i in `seq 10`; do ./nbdkit -U - --filter=stats nbd \ statsappend=true statsfile=$file hostname=localhost port=10809 \ --run '~/libnbd/examples/threaded-reads-and-writes $unixsocket' done Pre-patch, the runs averaged 1.266s, 1.30E+08 bits/s Post-patch, the runs averaged 1.154s, 1.42E+08

I'm still working on the interleaving (and Rich reminded me on IRC that we still don't have THREAD_MODEL_PARALLEL working anywhere yet, anyways). Since nbdkit doesn't really have a parallel plugin yet, my testing on that front will have to use qemu-nbd as the original server, as well as qemu-io as the driver (qemu-io's aio_read and aio_write commands can be used to trigger

Updated based on other changes that have happened in the meantime: - rely more on cleanup.h (throughout) - split structured read for easier review (patch 2 and 3 were combined in v1) - rely on nbdkit not leaking a server's partial answer (patch 3) - add tests (patch 5) - other bug fixes I found while testing it - drop EOVERFLOW patch for now; it will be separate once upstream NBD protocol

Patch 1 played with an early draft of Rich's Fedora 30 libnbd package: https://bugzilla.redhat.com/show_bug.cgi?id=1713767#c17 Note that comment 21 provides a newer package 0.1.1-1 with a different API; and that libnbd has more unreleased API changes in the pipeline (whether that will be called 0.2 or 0.1.2); so we'll have to tweak things based on what is actually available in distros.

libnbd-0.1.2-1 is now available in Fedora 29/30 updates-testing, although it was not compiled against libxml2 so it lacks uri support (I ended up testing patch 4 with a self-built libnbd). Diffs since v1 - rebase to master, bump from libnbd 0.1 to 0.1.2, add URI support, better timing results Still not done - patch 5 needs associated tests Eric Blake (5): nbd: Check for libnbd nbd:

When we first used libnbd (nbdkit 1.14), it was brand new, so we left in the ability to build a limited nbd plugin without the use of libnbd for comparison. But now that libnbd has had more time under its belt, and we have learned that our nbd-standalone code is not getting much testing, it's time to follow through with our plan to finish off the old code, and now build the nbd plugin only

libnbd-0.1.4-1 is now available in Fedora 29/30 updates testing. Diffs since v2 - rebase to master, bump from libnbd 0.1.2 to 0.1.3+, add tests to TLS usage which flushed out the need to turn relative pathnames into absolute, doc tweaks Now that the testsuite covers TLS and libnbd has been fixed to provide the things I found lacking when developing v2, I'm leaning towards pushing this on

Use new 'name_of_*' functions to offer a bit more details about messages being forwarded on to the remote server. Signed-off-by: Eric Blake <eblake@redhat.com> --- plugins/nbd/nbd.c | 8 +++++--- plugins/nbd/Makefile.am | 3 ++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index 672f35b..6bd8861 100644 ---

On Fri, Dec 07, 2018 at 10:00:42AM -0600, Eric Blake wrote: > Use new 'name_of_*' functions to offer a bit more details about > messages being forwarded on to the remote server. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > plugins/nbd/nbd.c | 8 +++++--- > plugins/nbd/Makefile.am | 3 ++- > 2 files changed, 7 insertions(+), 4 deletions(-)

This is a minimal implementation of an NBD forwarder; it lets us convert between old and newstyle connections (great if a client expects one style but the real server only provides the other), or add TLS safety on top of a server without having to rewrite that server. Right now, the real server is expected to live on a named Unix socket with no encryption, and the transactions are serialized

With some crufty sed scripts we can generate functions that map from NBD protocol flags (eg. NBD_CMD_READ) to strings ("NBD_CMD_READ"). This works on GNU sed and with FreeBSD, also with GNU sed's --posix option, so I guess the sed code is POSIX-compatible. Rich.

Tested via: term1$ qemu-nbd -k $PWD/sock -t -f raw -x foo junk --trace=nbd_\* term2$ ./nbdkit -f -v -e bar nbd socket=$PWD/sock export=foo term3$ qemu-io -t none -f raw nbd://localhost:10809/bar --trace=nbd_\* and checking the traces to see that 'w 0 1' vs. 'w -f 0 1' was able to influence whether the FUA flag showed up at the server in term1. Still to go: figure out how to

When converting from a single transaction to a linked list, I forgot to free the storage for each member of the list. Reported-by: Richard W.M. Jones <rjones@redhat.com> Fixes: 7f5bb9bf13f041ea7702bda557d9dd668bc3423a Signed-off-by: Eric Blake <eblake@redhat.com> --- plugins/nbd/nbd.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/plugins/nbd/nbd.c

This is about the simplest implementation of NBD Structured Replies (SR) that we can do right now. It accepts NBD_OPT_STRUCTURED_REPLIES when negotiated by the client, but only sends back the simplest possible SR when required to by NBD_CMD_READ. The rest of the time it will send back simple replies as before. We do not modify the plugin API so plugins are unable to send complex SRs. Also we

On 4/22/19 7:50 PM, Eric Blake wrote: > Time to enhance the nbd plugin to request structured replies from the > server. For now, deal only with structured reads. The server can now > return sparse reads, even though we need nbdkit version 3 before we > can in turn return sparse reads back to the client. > > In general, we have to assume the server is malicious, and so we must

Nothing in the second loop of nbd_reader() was ever setting quit; the only way the loop could end was via SEGV for dereferencing NULL or abort() when attempting to write to an invalid fd. Finish the implementation to properly mop up stranded in-flight requests. The testsuite did not exercise the case of quitting nbdkit while a transaction was in flight, but was relying on SIGTERM exiting the