Eric Blake
2019-May-25 22:32 UTC
[Libguestfs] [RFC nbdkit PATCH 0/2] Add 'nbdkit nbd shared=1' mode
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 required a way to wait for the socket to appear (as nbdkit --exit-with-parent has to be invoked before exec qemu-nbd, but the socket isn't created until the latter case). RFC in part because I wonder if nbdkit should offer a shared mode to ALL plugins, rather than just the nbd plugin providing its own. Eric Blake (2): nbd: Add retry=N parameter nbd: Add shared=true parameter plugins/nbd/nbdkit-nbd-plugin.pod | 22 ++++- plugins/nbd/nbd.c | 134 +++++++++++++++++++++--------- 2 files changed, 115 insertions(+), 41 deletions(-) -- 2.20.1
Eric Blake
2019-May-25 22:32 UTC
[Libguestfs] [nbdkit PATCH 1/2] nbd: Add retry=N parameter
A common command line use case for the nbd plugin is ( sock=`mktemp -u` && nbdkit --exit-with-parent nbd socket=$sock & exec /path/to/server -k $sock) As long as the first client to nbdkit doesn't try to connect until after /path/to/server has created $sock, we are fine. But on a heavily loaded system, it would be nice to give nbdkit some leeway to try again to connect to the socket if a client connects quickly to nbdkit. This becomes even more useful in the next patch when adding an option for the nbd plugin to connect during .config_complete rather than during .open, as the window of time for the external server to make the socket available is no longer dependent on when nbdkit's first client arrives. Signed-off-by: Eric Blake <eblake@redhat.com> --- plugins/nbd/nbdkit-nbd-plugin.pod | 6 ++++++ plugins/nbd/nbd.c | 24 ++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/plugins/nbd/nbdkit-nbd-plugin.pod b/plugins/nbd/nbdkit-nbd-plugin.pod index 913d1e5..4f9efa1 100644 --- a/plugins/nbd/nbdkit-nbd-plugin.pod +++ b/plugins/nbd/nbdkit-nbd-plugin.pod @@ -5,6 +5,7 @@ nbdkit-nbd-plugin - nbdkit nbd plugin =head1 SYNOPSIS nbdkit nbd { socket=SOCKNAME | hostname=HOST [port=PORT] } [export=NAME] + [retry=N] =head1 DESCRIPTION @@ -50,6 +51,11 @@ If this parameter is given, and the server speaks new style protocol, then connect to the named export instead of the default export (the empty string). +=item B<retry=>N + +If the initial connection attempt to the server fails, retry up to +B<N> times more after a second delay between tries (default 0). + =back =head1 EXAMPLES diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index 1195eb8..6665cd9 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -70,6 +70,9 @@ static char *servname; /* Name of export on remote server, default '', ignored for oldstyle */ static const char *export; +/* Number of retries */ +static unsigned long retry; + static void nbd_unload (void) { @@ -79,11 +82,14 @@ nbd_unload (void) /* Called for each key=value passed on the command line. This plugin * accepts socket=<sockname> or hostname=<hostname>/port=<port> - * (exactly one connection required) and export=<name> (optional). + * (exactly one connection required), and optional parameters + * export=<name>, retry=<n>. */ static int nbd_config (const char *key, const char *value) { + char *end; + if (strcmp (key, "socket") == 0) { /* See FILENAMES AND PATHS in nbdkit-plugin(3) */ free (sockname); @@ -97,6 +103,14 @@ nbd_config (const char *key, const char *value) port = value; else if (strcmp (key, "export") == 0) export = value; + else if (strcmp (key, "retry") == 0) { + errno = 0; + retry = strtoul (value, &end, 0); + if (value == end || errno) { + nbdkit_error ("could not parse retry as integer (%s)", value); + return -1; + } + } else { nbdkit_error ("unknown parameter '%s'", key); return -1; @@ -960,12 +974,18 @@ nbd_open (int readonly) return NULL; } + retry: if (sockname) h->fd = nbd_connect_unix (); else h->fd = nbd_connect_tcp (); - if (h->fd == -1) + if (h->fd == -1) { + if (retry--) { + sleep (1); + goto retry; + } goto err; + } /* old and new handshake share same meaning of first 16 bytes */ if (read_full (h->fd, &old, offsetof (struct old_handshake, exportsize))) { -- 2.20.1
Eric Blake
2019-May-25 22:32 UTC
[Libguestfs] [nbdkit PATCH 2/2] nbd: Add shared=true parameter
qemu-nbd defaults to permitting only a single NBD client. Of course, you can run qemu-nbd -t to work around it, but other servers may have similar restrictions, at which point nbdkit's nbd plugin can provide the automatic fanout to multiple clients via a single server link. Note that the shared=true parameter makes the previously-added retry=N parameter more useful, as it is much easier to run into the scenario where the server has not created the socket in time for .config_complete, when compared to a later .open from nbdkit's first client. Signed-off-by: Eric Blake <eblake@redhat.com> --- plugins/nbd/nbdkit-nbd-plugin.pod | 18 ++++- plugins/nbd/nbd.c | 110 ++++++++++++++++++++---------- 2 files changed, 88 insertions(+), 40 deletions(-) diff --git a/plugins/nbd/nbdkit-nbd-plugin.pod b/plugins/nbd/nbdkit-nbd-plugin.pod index 4f9efa1..8dd8681 100644 --- a/plugins/nbd/nbdkit-nbd-plugin.pod +++ b/plugins/nbd/nbdkit-nbd-plugin.pod @@ -5,7 +5,7 @@ nbdkit-nbd-plugin - nbdkit nbd plugin =head1 SYNOPSIS nbdkit nbd { socket=SOCKNAME | hostname=HOST [port=PORT] } [export=NAME] - [retry=N] + [retry=N] [shared=BOOL] =head1 DESCRIPTION @@ -56,6 +56,15 @@ empty string). If the initial connection attempt to the server fails, retry up to B<N> times more after a second delay between tries (default 0). +=item B<shared=>BOOL + +If this parameter is false (default), the plugin will open a distinct +connection to the server for each client making a connection to +nbdkit, and the remote server need not be running at the time nbdkit +is started. If this parameter is set to true, the plugin will open a +single connection to the server when nbdkit is first started, and all +clients to nbdkit will share that single connection. + =back =head1 EXAMPLES @@ -76,10 +85,13 @@ that the old server exits. Combine nbdkit's partition filter with qemu-nbd's ability to visit qcow2 files (nbdkit does not have a native qcow2 plugin), performing the same task as the deprecated C<qemu-nbd -P 1 -f qcow2 -/path/to/image.qcow2> command: +/path/to/image.qcow2> command. Allow multiple clients, even though +C<qemu-nbd> without B<-t> normally quits after the first client, and +utilize a 5-second retry to give qemu-nbd time to create the socket: ( sock=`mktemp -u` - nbdkit --exit-with-parent --filter=partition nbd socket=$sock partition=1 & + nbdkit --exit-with-parent --filter=partition nbd socket=$sock \ + shared=1 retry=5 partition=1 & exec qemu-nbd -k $sock -f qcow2 /path/to/image.qcow2 ) Conversely, expose the contents of export I<foo> from a new style diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index 6665cd9..9389a55 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -57,6 +57,37 @@ #include "byte-swapping.h" #include "cleanup.h" +/* The per-transaction details */ +struct transaction { + uint64_t cookie; + sem_t sem; + void *buf; + uint64_t offset; + uint32_t count; + uint32_t err; + struct nbdkit_extents *extents; + struct transaction *next; +}; + +/* The per-connection handle */ +struct handle { + /* These fields are read-only once initialized */ + int fd; + int flags; + int64_t size; + bool structured; + bool extents; + pthread_t reader; + + /* Prevents concurrent threads from interleaving writes to server */ + pthread_mutex_t write_lock; + + pthread_mutex_t trans_lock; /* Covers access to all fields below */ + struct transaction *trans; + uint64_t unique; + bool dead; +}; + /* Connect to server via absolute name of Unix socket */ static char *sockname; @@ -73,9 +104,18 @@ static const char *export; /* Number of retries */ static unsigned long retry; +/* True to share single server connection among all clients */ +static bool shared; +static struct handle *shared_handle; + +static struct handle *nbd_open_handle (int readonly); +static void nbd_close_handle (struct handle *h); + static void nbd_unload (void) { + if (shared) + nbd_close_handle (shared_handle); free (sockname); free (servname); } @@ -89,6 +129,7 @@ static int nbd_config (const char *key, const char *value) { char *end; + int r; if (strcmp (key, "socket") == 0) { /* See FILENAMES AND PATHS in nbdkit-plugin(3) */ @@ -111,6 +152,12 @@ nbd_config (const char *key, const char *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 */ -struct transaction { - uint64_t cookie; - sem_t sem; - void *buf; - uint64_t offset; - uint32_t count; - uint32_t err; - struct nbdkit_extents *extents; - struct transaction *next; -}; - -/* The per-connection handle */ -struct handle { - /* These fields are read-only once initialized */ - int fd; - int flags; - int64_t size; - bool structured; - bool extents; - pthread_t reader; - - /* Prevents concurrent threads from interleaving writes to server */ - pthread_mutex_t write_lock; - - pthread_mutex_t trans_lock; /* Covers access to all fields below */ - struct transaction *trans; - uint64_t unique; - bool dead; -}; - /* Read an entire buffer, returning 0 on success or -1 with errno set. */ static int read_full (int fd, void *buf, size_t len) @@ -960,9 +979,9 @@ nbd_connect_tcp (void) return fd; } -/* Create the per-connection handle. */ -static void * -nbd_open (int readonly) +/* Create the shared or per-connection handle. */ +static struct handle * +nbd_open_handle (int readonly) { struct handle *h; struct old_handshake old; @@ -1091,12 +1110,19 @@ nbd_open (int readonly) return NULL; } +/* Create the per-connection handle. */ +static 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); +} + /* Get the file size. */ static int64_t nbd_get_size (void *handle) -- 2.20.1
Richard W.M. Jones
2019-May-26 12:49 UTC
Re: [Libguestfs] [nbdkit PATCH 2/2] nbd: Add shared=true parameter
Looks reasonable, ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Reasonably Related Threads
- [nbdkit PATCH 0/4] Play with libnbd for nbdkit-add
- [nbdkit PATCH v2 0/5] Play with libnbd for nbdkit-nbd
- [nbdkit PATCH v3 0/5] Play with libnbd for nbdkit-nbd
- [nbdkit PATCH 0/2] Let nbd plugin connect to TCP socket
- [nbdkit PATCH 2/2] nbd: Add shared=true parameter