search for: nbd_request_raw

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

...>trans; + while ((trans = *ptr) != NULL) { + if (cookie == trans->u.cookie) + break; + ptr = &trans->next; + } + if (trans) + *ptr = trans->next; + nbd_unlock (h); + return trans; +} + /* Send a request, return 0 on success or -1 on write failure. */ static int nbd_request_raw (struct handle *h, uint32_t type, uint64_t offset, @@ -260,6 +280,8 @@ nbd_request_full (struct handle *h, uint32_t type, uint64_t offset, { int err; struct transaction *trans; + int fd; + uint64_t cookie; trans = calloc (1, sizeof *trans); if (!trans) { @@ -282,9 +304,14 @@ nbd_req...

...} else if (!err) errno = ESHUTDOWN; + nbd_unlock (h); /* NBD only accepts a limited set of errno values over the wire, and nbdkit converts all other values to EINVAL. If we died due to an errno value that cannot transmit over the wire, translate it to @@ -223,22 +243,34 @@ nbd_request_raw (struct handle *h, uint32_t type, uint64_t offset, } /* Perform the request half of a transaction. On success, return the - non-negative cookie to match to the reply; on error return -1. */ + non-negative fd for reading the reply; on error return -1. */ static int nbd_request_full (struct...

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

...possibly reused by Thread 3 in the meantime). plugins/nbd/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index b9a4523..9130642 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -575,9 +575,9 @@ nbd_close (void *handle) 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"); + close (h->fd); pthread_mutex_destroy (&h->write_lock); pthread_m...

...int64_t cookie, bool remove) ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&h->trans_lock); ptr = &h->trans; while ((trans = *ptr) != NULL) { - if (cookie == trans->u.cookie) + if (cookie == trans->cookie) break; ptr = &trans->next; } @@ -300,28 +300,27 @@ nbd_request_raw (struct handle *h, uint16_t flags, uint16_t type, } /* Perform the request half of a transaction. On success, return the - non-negative fd for reading the reply; on error return -1. */ -static int + transaction; on error return NULL. */ +static struct transaction * nbd_request_full (struct...

...INVAL. If we died due to an + errno value that cannot transmit over the wire, translate it to + ESHUTDOWN instead. */ + if (err == EPIPE || err == EBADMSG) + nbdkit_set_error (ESHUTDOWN); + return -1; +} + +/* Send a request, return 0 on success or -1 on write failure. */ +static int +nbd_request_raw (struct handle *h, uint32_t type, uint64_t offset, + uint32_t count, uint64_t cookie) +{ + struct request req = { + .magic = htobe32 (NBD_REQUEST_MAGIC), + /* TODO nbdkit should have a way to pass flags, separate from cmd type */ + .type = htobe32 (type), + .handle = co...

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

...lt;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 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -267,8 +267,9 @@ nbd_request_raw (struct handle *h, uint16_t flags, uint16_t type, int r; pthread_mutex_lock (&h->write_lock); - nbdkit_debug ("sending request with type %d and cookie %#" PRIx64, type, - cookie); + nbdkit_debug ("sending request type %d (%s), flags %#x, offset %#"...

...d/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 > --- a/plugins/nbd/nbd.c > +++ b/plugins/nbd/nbd.c > @@ -267,8 +267,9 @@ nbd_request_raw (struct handle *h, uint16_t flags, uint16_t type, > int r; > > pthread_mutex_lock (&h->write_lock); > - nbdkit_debug ("sending request with type %d and cookie %#" PRIx64, type, > - cookie); > + nbdkit_debug ("sending request type %d (%...

Hi Eric, There's a memory leak in the nbd client. The message (below) is not very useful because somehow debuginfo is missing in the plugin. However it's easily reproducible by doing: make check-valgrind TESTS=test-nbd I tried the attached patch to fix what I thought was the bug, but sadly the fix doesn't work for me :-( So I guess something else is going on, but it does look as

...void * +nbd_open (int readonly) +{ + if (shared) + return shared_handle; + return nbd_open_handle (readonly); +} + /* Free up the per-connection handle. */ static void -nbd_close (void *handle) +nbd_close_handle (struct handle *h) { - struct handle *h = handle; - if (!h->dead) { nbd_request_raw (h, 0, NBD_CMD_DISC, 0, 0, 0, NULL); shutdown (h->fd, SHUT_WR); @@ -1109,6 +1135,16 @@ nbd_close (void *handle) free (h); } +/* Free up the per-connection handle. */ +static void +nbd_close (void *handle) +{ + struct handle *h = handle; + + if (!shared) + nbd_close_handle (h); +}...

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.

...f25a67 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -50,6 +50,7 @@ #include <nbdkit-plugin.h> #include "protocol.h" #include "byte-swapping.h" +#include "cleanup.h" static char *sockname = NULL; static char *export = NULL; @@ -266,14 +267,13 @@ nbd_request_raw (struct handle *h, uint16_t flags, uint16_t type, }; int r; - pthread_mutex_lock (&h->write_lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&h->write_lock); nbdkit_debug ("sending request type %d (%s), flags %#x, offset %#" PRIx64 ", count %#x, coo...

Commit 9e6d990f exposed a pre-existing deadlock between the nbd plugin as client and parallel nbdkit as server. Prior to that commit, the deadlock was "resolved" because we unloaded the .so in parallel to a .close callback that never completed (yes, it's nasty that it usually? let the testsuite pass), but now we correctly refuse to unload a plugin that has not returned from .close,

...;h->trans; - while ((trans = *ptr) != NULL) { - if (cookie == trans->cookie) - break; - ptr = &trans->next; - } - if (trans && remove) - *ptr = trans->next; - return trans; -} - -/* Send a request, return 0 on success or -1 on write failure. */ -static int -nbd_request_raw (struct handle *h, uint16_t flags, uint16_t type, - uint64_t offset, uint32_t count, uint64_t cookie, - const void *buf) -{ - struct nbd_request req = { - .magic = htobe32 (NBD_REQUEST_MAGIC), - .flags = htobe16 (flags), - .type = htobe16 (type), - .hand...

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.

...flags, uint16_t type, } trans->buf = rep_buf; trans->count = rep_buf ? count : 0; + trans->offset = offset; nbd_lock (h); if (h->dead) { nbd_unlock (h); @@ -315,7 +320,7 @@ nbd_request_full (struct handle *h, uint16_t flags, uint16_t type, nbd_unlock (h); if (nbd_request_raw (h, flags, type, offset, count, cookie, req_buf) == 0) return fd; - trans = find_trans_by_cookie (h, cookie); + trans = find_trans_by_cookie (h, cookie, true); err: err = errno; @@ -340,35 +345,196 @@ nbd_request (struct handle *h, uint16_t flags, uint16_t type, uint64_t offset, /* R...

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:

I got annoyed by qemu-nbd's default of only allowing a single connection; combine that with nbdkit's nbd plugin, and even 'qemu-nbd --list' of nbdkit counts as the single connection and immediately hangs up. If we introduce a shared mode, then 'qemu-nbd --list' can connect as many times as it wants without killing the original qemu-nbd wrapped by nbdkit. But this in turn