[Libguestfs] [nbdkit PATCH 2/3] nbd: Add dynamic-export=true option

Now that nbdkit gives us the ability to know what export name the client wants, we can pass that name on to the server. This only works for non-shared connections (in a shared connection, we only connect to the server once, but we can't differentiate content without different connections); and when uri is specified, it requires libnbd 1.4 (as we need opt_mode to override the export name from the uri with the export name from the client). libnbd 1.4 also lets us report the canonical name of the "" export according to the server. With this patch, it will merely change our advertisement from [""] to ["name"], but the next patch will further add .list_exports so that a client can see the same advertisement that the server sent. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- plugins/nbd/nbdkit-nbd-plugin.pod | 23 +++- tests/Makefile.am | 2 + plugins/nbd/nbd.c | 173 +++++++++++++++++++++++------- tests/test-nbd-dynamic-content.sh | 75 +++++++++++++ 4 files changed, 233 insertions(+), 40 deletions(-) create mode 100755 tests/test-nbd-dynamic-content.sh diff --git a/plugins/nbd/nbdkit-nbd-plugin.pod b/plugins/nbd/nbdkit-nbd-plugin.pod index 0c3271e2..55f6fff0 100644 --- a/plugins/nbd/nbdkit-nbd-plugin.pod +++ b/plugins/nbd/nbdkit-nbd-plugin.pod @@ -10,7 +10,7 @@ nbdkit-nbd-plugin - proxy / forward to another NBD server socket=SOCKNAME | socket-fd=FD | [uri=]URI } - [export=NAME] [retry=N] [shared=BOOL] + [dynamic-export=BOOL] [export=NAME] [retry=N] [shared=BOOL] [tls=MODE] [tls-certificates=DIR] [tls-verify=BOOL] [tls-username=NAME] [tls-psk=FILE] @@ -112,6 +112,9 @@ The C<uri> parameter is only available when the plugin was compiled against libnbd with URI support; C<nbdkit --dump-plugin nbd> will contain C<libnbd_uri=1> if this is the case. +The export portion of the URI is ignored when using +C<dynamic-export=true>. + C<uri=> is a magic config key and may be omitted in most cases. See L<nbdkit(1)/Magic parameters>. @@ -126,7 +129,8 @@ Other parameters control the NBD connection: 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). If C<uri> is supplied, the export name should be -embedded in the URI instead. +embedded in the URI instead. This is incompatible with +C<dynamic-export=true>. =item B<retry=>N @@ -149,6 +153,20 @@ If this parameter is set to C<true>, the plugin will open a single connection to the server when nbdkit is first started (the C<retry> parameter may be necessary to coordinate timing of the remote server startup), and all clients to nbdkit will share that single connection. +This mode is incompatible with B<dynamic-export=true>. + +=item B<dynamic-export=false> + +=item B<dynamic-export=true> + +This parameter defaults to false, in which case all clients to nbdkit +use the same export of the server, as set by C<export> or C<uri>, +regardless of the client's export name request. If it is set to true, +nbdkit will pass the client's requested export name over to the final +NBD server, which means clients requesting different export names can +see different content when the server differentiates content by export +name. Dynamic exports prevent the use of C<shared> mode, and thus are +not usable with C<command> or C<socket-fd>. =item B<tls=off> @@ -283,6 +301,7 @@ C<nbdkit-nbd-plugin> first appeared in nbdkit 1.2. L<nbdkit(1)>, L<nbdkit-captive(1)>, L<nbdkit-filter(3)>, +L<nbdkit-exportname-filter(1)>, L<nbdkit-tls(1)>, L<nbdkit-plugin(3)>, L<libnbd(3)>, diff --git a/tests/Makefile.am b/tests/Makefile.am index 911fa2b3..14e9abdb 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -762,6 +762,7 @@ if HAVE_LIBNBD # nbd plugin test. LIBGUESTFS_TESTS += test-nbd TESTS += \ + test-nbd-dynamic-content.sh \ test-nbd-extents.sh \ test-nbd-qcow2.sh \ test-nbd-tls.sh \ @@ -769,6 +770,7 @@ TESTS += \ test-nbd-vsock.sh \ $(NULL) EXTRA_DIST += \ + test-nbd-dynamic-content.sh \ test-nbd-extents.sh \ test-nbd-qcow2.sh \ test-nbd-tls.sh \ diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index e8ce4124..7389b6d9 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -109,8 +109,11 @@ static string_vector command = empty_vector; /* Connect to a socket file descriptor. */ static int socket_fd = -1; -/* Name of export on remote server, default '', ignored for oldstyle */ +/* Name of export on remote server, default '', ignored for oldstyle, + * NULL if dynamic. + */ static const char *export; +static bool dynamic_export; /* Number of retries */ static unsigned retry; @@ -126,7 +129,8 @@ static int tls_verify = -1; static const char *tls_username; static char *tls_psk; -static struct handle *nbdplug_open_handle (int readonly); +static struct handle *nbdplug_open_handle (int readonly, + const char *client_export); static void nbdplug_close_handle (struct handle *h); static void @@ -180,6 +184,12 @@ nbdplug_config (const char *key, const char *value) } else if (strcmp (key, "export") == 0) export = value; + else if (strcmp (key, "dynamic-export") == 0) { + r = nbdkit_parse_bool (value); + if (r == -1) + return -1; + dynamic_export = r; + } else if (strcmp (key, "retry") == 0) { if (nbdkit_parse_unsigned ("retry", value, &retry) == -1) return -1; @@ -308,10 +318,31 @@ nbdplug_config_complete (void) abort (); /* can't happen, if checks above were correct */ } - /* Check the other parameters. */ - if (!export) + /* Can't mix dynamic-export with export or shared (including + * connection modes that imply shared). Also, it requires + * new-enough libnbd if uri was used. + */ + if (dynamic_export) { + if (export) { + nbdkit_error ("cannot mix 'dynamic-export' with explicit export name"); + return -1; + } + if (shared) { + nbdkit_error ("cannot use 'dynamic-export' with shared connection"); + return -1; + } +#if !LIBNBD_HAVE_NBD_SET_OPT_MODE + if (uri) { + nbdkit_error ("libnbd too old to support 'dynamic-export' with uri " + "connection"); + return -1; + } +#endif + } + else if (!export) export = ""; + /* Check the other parameters. */ if (tls == -1) tls = (tls_certificates || tls_verify >= 0 || tls_username || tls_psk) ? LIBNBD_TLS_ALLOW : LIBNBD_TLS_DISABLE; @@ -338,7 +369,7 @@ nbdplug_config_complete (void) static int nbdplug_after_fork (void) { - if (shared && (shared_handle = nbdplug_open_handle (false)) == NULL) + if (shared && (shared_handle = nbdplug_open_handle (false, NULL)) == NULL) return -1; return 0; } @@ -353,6 +384,7 @@ nbdplug_after_fork (void) "arg=<ARG> Parameters for command.\n" \ "socket-fd=<FD> Socket file descriptor to connect to.\n" \ "export=<NAME> Export name to connect to (default \"\").\n" \ + "dynamic-export=<BOOL> True to enable export name pass-through.\n" \ "retry=<N> Retry connection up to N seconds (default 0).\n" \ "shared=<BOOL> True to share one server connection among all clients,\n" \ " rather than a connection per client (default false).\n" \ @@ -510,12 +542,46 @@ nbdplug_reply (struct handle *h, struct transaction *trans) return err ? -1 : 0; } +/* Move an nbd handle from created to negotiating/ready. Error reporting + * is left to the caller. + */ +static int +nbdplug_connect (struct nbd_handle *nbd) +{ + if (tls_certificates && + nbd_set_tls_certificates (nbd, tls_certificates) == -1) + return -1; + if (tls_verify >= 0 && nbd_set_tls_verify_peer (nbd, tls_verify) == -1) + return -1; + if (tls_username && nbd_set_tls_username (nbd, tls_username) == -1) + return -1; + if (tls_psk && nbd_set_tls_psk_file (nbd, tls_psk) == -1) + return -1; + if (uri) + return nbd_connect_uri (nbd, uri); + else if (sockname) + return nbd_connect_unix (nbd, sockname); + else if (hostname) + return nbd_connect_tcp (nbd, hostname, port); + else if (raw_cid) +#if !USE_VSOCK + abort (); +#else + return nbd_connect_vsock (nbd, cid, vport); +#endif + else if (command.size > 0) + return nbd_connect_systemd_socket_activation (nbd, (char **) command.ptr); + else if (socket_fd >= 0) + return nbd_connect_socket (nbd, socket_fd); + else + abort (); +} + /* Create the shared or per-connection handle. */ static struct handle * -nbdplug_open_handle (int readonly) +nbdplug_open_handle (int readonly, const char *client_export) { struct handle *h; - int r; unsigned long retries = retry; h = calloc (1, sizeof *h); @@ -550,11 +616,16 @@ nbdplug_open_handle (int readonly) } #endif + if (dynamic_export) + assert (client_export); + else + client_export = export; + retry: h->nbd = nbd_create (); if (!h->nbd) goto errnbd; - if (nbd_set_export_name (h->nbd, export) == -1) + if (nbd_set_export_name (h->nbd, client_export) == -1) goto errnbd; if (nbd_add_meta_context (h->nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1) goto errnbd; @@ -562,36 +633,17 @@ nbdplug_open_handle (int readonly) if (nbd_set_full_info (h->nbd, 1) == -1) goto errnbd; #endif + if (dynamic_export && uri) { +#if LIBNBD_HAVE_NBD_SET_OPT_MODE + if (nbd_set_opt_mode (h->nbd, 1) == -1) + goto errnbd; +#else + abort (); /* Prevented by .config_complete */ +#endif + } if (nbd_set_tls (h->nbd, tls) == -1) goto errnbd; - if (tls_certificates && - nbd_set_tls_certificates (h->nbd, tls_certificates) == -1) - goto errnbd; - if (tls_verify >= 0 && nbd_set_tls_verify_peer (h->nbd, tls_verify) == -1) - goto errnbd; - if (tls_username && nbd_set_tls_username (h->nbd, tls_username) == -1) - goto errnbd; - if (tls_psk && nbd_set_tls_psk_file (h->nbd, tls_psk) == -1) - goto errnbd; - if (uri) - r = nbd_connect_uri (h->nbd, uri); - else if (sockname) - r = nbd_connect_unix (h->nbd, sockname); - else if (hostname) - r = nbd_connect_tcp (h->nbd, hostname, port); - else if (raw_cid) -#if !USE_VSOCK - abort (); -#else - r = nbd_connect_vsock (h->nbd, cid, vport); -#endif - else if (command.size > 0) - r = nbd_connect_systemd_socket_activation (h->nbd, (char **) command.ptr); - else if (socket_fd >= 0) - r = nbd_connect_socket (h->nbd, socket_fd); - else - abort (); - if (r == -1) { + if (nbdplug_connect (h->nbd) == -1) { if (retries--) { nbdkit_debug ("connect failed; will try again: %s", nbd_get_error ()); nbd_close (h->nbd); @@ -601,6 +653,16 @@ nbdplug_open_handle (int readonly) goto errnbd; } +#if LIBNBD_HAVE_NBD_SET_OPT_MODE + /* Oldstyle servers can't change export name, but that's okay. */ + if (uri && dynamic_export && nbd_aio_is_negotiating (h->nbd)) { + if (nbd_set_export_name (h->nbd, client_export) == -1) + goto errnbd; + if (nbd_opt_go (h->nbd) == -1) + goto errnbd; + } +#endif + if (readonly) h->readonly = true; @@ -627,7 +689,42 @@ nbdplug_open_handle (int readonly) static const char * nbdplug_default_export (int readonly, int is_tls) { - return export; + const char *ret = ""; + CLEANUP_FREE char *name = NULL; + + if (!dynamic_export) + return export; +#if LIBNBD_HAVE_NBD_SET_FULL_INFO + /* Best effort determination of server's canonical name. If it + * fails, we're fine using the default name on our end (NBD_OPT_GO + * might still work on "" later on). + */ + struct nbd_handle *nbd = nbd_create (); + + if (!nbd) + return ""; + if (nbd_set_full_info (nbd, 1) == -1) + goto out; + if (nbd_set_opt_mode (nbd, 1) == -1) + goto out; + if (nbdplug_connect (nbd) == -1) + goto out; + if (nbd_set_export_name (nbd, "") == -1) + goto out; + if (nbd_opt_info (nbd) == -1) + goto out; + name = nbd_get_canonical_export_name (nbd); + if (name) + ret = nbdkit_strdup_intern (name); + + out: + if (nbd_aio_is_negotiating (nbd)) + nbd_opt_abort (nbd); + else if (nbd_aio_is_ready (nbd)) + nbd_shutdown (nbd, 0); + nbd_close (nbd); +#endif + return ret; } /* Create the per-connection handle. */ @@ -636,7 +733,7 @@ nbdplug_open (int readonly) { if (shared) return shared_handle; - return nbdplug_open_handle (readonly); + return nbdplug_open_handle (readonly, nbdkit_export_name ()); } /* Free up the shared or per-connection handle. */ diff --git a/tests/test-nbd-dynamic-content.sh b/tests/test-nbd-dynamic-content.sh new file mode 100755 index 00000000..bbd889e4 --- /dev/null +++ b/tests/test-nbd-dynamic-content.sh @@ -0,0 +1,75 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2019-2020 Red Hat Inc. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# * Neither the name of Red Hat nor the names of its contributors may be +# used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF +# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +source ./functions.sh +set -e +set -x + +# This test works with older libnbd, showing that dynamic mode affects +# content. XXX Also write a test, requiring newer libnbd, to show export list +requires_plugin info +requires nbdsh --version + +sock1=`mktemp -u` +export sock2=`mktemp -u` +pid1="test-nbd-dynamic-content.pid1" +pid2="test-nbd-dynamic-content.pid2" +files="$sock1 $sock2 $pid1 $pid2" +rm -f $files +cleanup_fn rm -f $files + +# Long-running info server, for content sensitive to export name +start_nbdkit -P $pid1 -U $sock1 info exportname + +# Long-running nbd bridge, which should pass export names through +start_nbdkit -P $pid2 -U $sock2 nbd socket=$sock1 dynamic-export=true + +long=$(printf %04096d 1) +export e +for e in "" "test" "/" "//" " " "/ " "?" "テスト" "-n" '\\' $'\n' "%%" "$long" +do + nbdsh -c ' +import os + +e = os.environ["e"] +h.set_export_name (e) +h.connect_unix (os.environ["sock2"]) + +size = h.get_size () +assert size == len (e.encode("utf-8")) + +# Zero-sized reads are not defined in the NBD protocol. +if size > 0: + buf = h.pread (size, 0) + assert buf == e.encode("utf-8") +' +done -- 2.28.0