search for: nbd_config_complete

...of complaints] Signed-off-by: Eric Blake <eblake@redhat.com> --- plugins/nbd/nbd.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index 51de178..a4a1f12 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -100,7 +100,7 @@ nbd_config_complete (void) nbdkit_error ("you must supply the socket=<SOCKNAME> parameter after the plugin name on the command line"); return -1; } - if (strlen (sockname) >= sizeof sock.sun_path) { + if (strlen (sockname) > sizeof sock.sun_path) { nbdkit_error ("socket f...

There are a few other issues that Coverity found, but I believe all can be ignored ... except one: We don't set umask anywhere inside nbdkit. Coverity complains that this is a problem where we create temporary files, since the result of mkstemp depends implicitly on the umask value. I think we might consider setting umask anyway (eg. to 022) just to make plugin behaviour more predictable.

Accepting only Unix sockets can be a bit limiting; let's be more flexible. Eric Blake (2): nbd: Refactor Unix socket connection nbd: Support TCP socket plugins/nbd/nbdkit-nbd-plugin.pod | 36 ++++-- plugins/nbd/nbd.c | 175 ++++++++++++++++++++++-------- TODO | 3 - 3 files changed, 161 insertions(+), 53 deletions(-) -- 2.20.1

...*value) return -1; } } + else if (strcmp (key, "shared") == 0) { + r = nbdkit_parse_bool (value); + if (r == -1) + return -1; + shared = r; + } else { nbdkit_error ("unknown parameter '%s'", key); return -1; @@ -157,6 +204,9 @@ nbd_config_complete (void) if (!export) export = ""; + + if (shared && (shared_handle = nbd_open_handle (false)) == NULL) + return -1; return 0; } @@ -168,37 +218,6 @@ nbd_config_complete (void) #define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL -/* The per-transaction details */...

...69b 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -465,6 +465,7 @@ nbd_open (int readonly) h->fd = socket (AF_UNIX, SOCK_STREAM, 0); if (h->fd < 0) { nbdkit_error ("socket: %m"); + free (h); return NULL; } /* We already validated length during nbd_config_complete */ @@ -559,6 +560,7 @@ nbd_open (int readonly) err: close (h->fd); + free (h); return NULL; } -- 2.16.2

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

...index 35f2781..770fb71 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -44,6 +44,7 @@ #include <sys/socket.h> #include <sys/un.h> #include <assert.h> +#include <pthread.h> #include <nbdkit-plugin.h> #include "protocol.h" @@ -119,18 +120,23 @@ nbd_config_complete (void) /* The per-transaction details */ struct transaction { - /* TODO: protocol allows 64-bit handle, but until we allow - interleaved transactions, 31 bits with wraparound is plenty */ - int cookie; + union { + uint64_t cookie; + int fds[2]; + } u; void *buf; uint32_t coun...

.../plugins/nbd/nbd.c b/plugins/nbd/nbd.c index 9039e1b..b2f3446 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -48,6 +48,7 @@ #include <sys/un.h> #include <assert.h> #include <pthread.h> +#include <semaphore.h> #define NBDKIT_API_VERSION 2 @@ -155,10 +156,8 @@ nbd_config_complete (void) /* The per-transaction details */ struct transaction { - union { - uint64_t cookie; - int fds[2]; - } u; + uint64_t cookie; + sem_t sem; void *buf; uint64_t offset; uint32_t count; @@ -182,6 +181,7 @@ struct handle { pthread_mutex_t trans_lock; /* Covers access to a...

...if (!export) { + nbdkit_error ("memory failure: %m"); + return -1; + } + } + else { + nbdkit_error ("unknown parameter '%s'", key); + return -1; + } + + return 0; +} + +/* Check the user did pass a socket=<SOCKNAME> parameter. */ +static int +nbd_config_complete (void) +{ + struct sockaddr_un sock; + + if (sockname == NULL) { + nbdkit_error ("you must supply the socket=<SOCKNAME> parameter after the plugin name on the command line"); + return -1; + } + if (strlen (sockname) >= sizeof sock.sun_path) { + nbdkit_error ("so...

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

...t;) == 0) { - r = nbdkit_parse_bool (value); - if (r == -1) - return -1; - shared = r; - } - else { - nbdkit_error ("unknown parameter '%s'", key); - return -1; - } - - return 0; -} - -/* Check the user passed exactly one socket description. */ -static int -nbd_config_complete (void) -{ - int r; - - if (sockname) { - struct sockaddr_un sock; - - if (hostname || port) { - nbdkit_error ("cannot mix Unix socket and TCP hostname/port parameters"); - return -1; - } - if (strlen (sockname) > sizeof sock.sun_path) { - nbdkit_error (&quo...

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

...(strcmp (key, "export") == 0) export = value; else { @@ -87,29 +104,52 @@ nbd_config (const char *key, const char *value) return 0; } -/* Check the user did pass a socket=<SOCKNAME> parameter. */ +/* Check the user passed exactly one socket description. */ static int nbd_config_complete (void) { - struct sockaddr_un sock; + int r; - if (sockname == NULL) { - nbdkit_error ("you must supply the socket=<SOCKNAME> parameter " - "after the plugin name on the command line"); - return -1; + if (sockname) { + struct sockaddr_un soc...

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:

...if (!export) { + nbdkit_error ("memory failure: %m"); + return -1; + } + } + else { + nbdkit_error ("unknown parameter '%s'", key); + return -1; + } + + return 0; +} + +/* Check the user did pass a socket=<FILENAME> parameter. */ +static int +nbd_config_complete (void) +{ + struct sockaddr_un sock; + + if (filename == NULL) { + nbdkit_error ("you must supply the socket=<FILENAME> parameter after the plugin name on the command line"); + return -1; + } + if (strlen (filename) >= sizeof sock.sun_path) { + nbdkit_error ("so...

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