This allows plugins (or filters) to read the export name which was passed to the server from the client. --- TODO | 8 +++++++ docs/nbdkit-plugin.pod | 29 ++++++++++++++++++++++ include/nbdkit-common.h | 1 + server/connections.c | 36 ++++++++++++++++++---------- server/internal.h | 1 + server/nbdkit.syms | 1 + server/protocol-handshake-newstyle.c | 31 ++++++++++++++---------- server/public.c | 13 ++++++++++ 8 files changed, 95 insertions(+), 25 deletions(-) diff --git a/TODO b/TODO index 49b60b1..2468d74 100644 --- a/TODO +++ b/TODO @@ -62,6 +62,12 @@ General ideas for improvements and also look at the implementation of the -swap option in nbd-client. +* Clients should be able to list export names supported by plugins. + Current behaviour is not really correct: We only list the -e + parameter from the command line, which is different from the export + name(s) that a plugin might want to support. Probably we should + deprecate the -e option entirely since it does nothing useful. + Suggestions for plugins ----------------------- @@ -190,3 +196,5 @@ using ‘#define NBDKIT_API_VERSION <version>’. value) strings. nbdkit should know the possible keys for the plugin and filters, and the type of the values, and both check and parse them for the plugin. + +* Modify open() API so it takes an export name parameter. diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index bb162e4..219906e 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -362,6 +362,32 @@ requested, or -1 after calling C<nbdkit_error> if there is no point in continuing the current command. Attempts to sleep more than C<INT_MAX> seconds are treated as an error. +=head1 EXPORT NAME + +If the client negotiated an NBD export name with nbdkit then plugins +may read this from any connected callbacks. Nbdkit's normal behaviour +is to accept any export name passed by the client, log it in debug +output, but otherwise ignore it. By using C<nbdkit_export_name> +plugins may choose to filter by export name or serve different +content. + +=head2 C<nbdkit_export_name> + + const char *nbdkit_export_name (void); + +Return the optional NBD export name if one was negotiated with the +current client (this uses thread-local magic so no parameter is +required). The returned string is only valid while the client is +connected, so if you need to store it in the plugin you must copy it. + +The export name is a free-form text string, it is not necessarily a +path or filename and it does not need to begin with a C<'/'> +character. The NBD protocol describes the empty string (C<"">) as a +representing a "default export" or to be used in cases where the +export name does not make sense. + +On error, nbdkit_error is called and the call returns C<NULL>. + =head1 CALLBACKS =head2 C<.name> @@ -709,6 +735,9 @@ request or setting C<NBDKIT_FLAG_FUA> on a request must be visible across all connections to the plugin before the plugin replies to that request. +Properly working clients should send the same export name for each of +these connections. + If you use Linux L<nbd-client(8)> option S<I<-C num>> with S<num E<gt> 1> then Linux checks this flag and will refuse to connect if C<.can_multi_conn> is false. diff --git a/include/nbdkit-common.h b/include/nbdkit-common.h index 42d94a1..acf0abd 100644 --- a/include/nbdkit-common.h +++ b/include/nbdkit-common.h @@ -86,6 +86,7 @@ extern int nbdkit_parse_bool (const char *str); extern int nbdkit_read_password (const char *value, char **password); extern char *nbdkit_realpath (const char *path); extern int nbdkit_nanosleep (unsigned sec, unsigned nsec); +extern const char *nbdkit_export_name (void); struct nbdkit_extents; extern int nbdkit_add_extent (struct nbdkit_extents *, diff --git a/server/connections.c b/server/connections.c index 8ef5e57..819f7b8 100644 --- a/server/connections.c +++ b/server/connections.c @@ -255,11 +255,18 @@ new_connection (int sockin, int sockout, int nworkers) perror ("malloc"); return NULL; } + + conn->status_pipe[0] = conn->status_pipe[1] = -1; + + conn->exportname = strdup (""); + if (conn->exportname == NULL) { + perror ("strdup"); + goto error; + } conn->handles = calloc (backend->i + 1, sizeof *conn->handles); if (conn->handles == NULL) { perror ("malloc"); - free (conn); - return NULL; + goto error; } conn->nr_handles = backend->i + 1; memset (conn->handles, -1, conn->nr_handles * sizeof *conn->handles); @@ -272,8 +279,7 @@ new_connection (int sockin, int sockout, int nworkers) #ifdef HAVE_PIPE2 if (pipe2 (conn->status_pipe, O_NONBLOCK | O_CLOEXEC)) { perror ("pipe2"); - free (conn); - return NULL; + goto error; } #else /* If we were fully parallel, then this function could be @@ -289,29 +295,24 @@ new_connection (int sockin, int sockout, int nworkers) lock_request (NULL); if (pipe (conn->status_pipe)) { perror ("pipe"); - free (conn); unlock_request (NULL); - return NULL; + goto error; } if (set_nonblock (set_cloexec (conn->status_pipe[0])) == -1) { perror ("fcntl"); close (conn->status_pipe[1]); - free (conn); unlock_request (NULL); - return NULL; + goto error; } if (set_nonblock (set_cloexec (conn->status_pipe[1])) == -1) { perror ("fcntl"); close (conn->status_pipe[0]); - free (conn); unlock_request (NULL); - return NULL; + goto error; } unlock_request (NULL); #endif } - else - conn->status_pipe[0] = conn->status_pipe[1] = -1; conn->sockin = sockin; conn->sockout = sockout; pthread_mutex_init (&conn->request_lock, NULL); @@ -329,6 +330,16 @@ new_connection (int sockin, int sockout, int nworkers) threadlocal_set_conn (conn); return conn; + + error: + if (conn->status_pipe[0] >= 0) + close (conn->status_pipe[0]); + if (conn->status_pipe[1] >= 0) + close (conn->status_pipe[1]); + free (conn->exportname); + free (conn->handles); + free (conn); + return NULL; } static void @@ -373,6 +384,7 @@ free_connection (struct connection *conn) pthread_mutex_destroy (&conn->status_lock); free (conn->handles); + free (conn->exportname); free (conn); } diff --git a/server/internal.h b/server/internal.h index ac5b894..1f72b01 100644 --- a/server/internal.h +++ b/server/internal.h @@ -181,6 +181,7 @@ struct connection { struct b_conn_handle *handles; size_t nr_handles; + char *exportname; uint32_t cflags; uint16_t eflags; bool using_tls; diff --git a/server/nbdkit.syms b/server/nbdkit.syms index 2a024ed..1fb1315 100644 --- a/server/nbdkit.syms +++ b/server/nbdkit.syms @@ -42,6 +42,7 @@ nbdkit_add_extent; nbdkit_debug; nbdkit_error; + nbdkit_export_name; nbdkit_extents_count; nbdkit_extents_free; nbdkit_extents_new; diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c index 75465b7..87e0bcd 100644 --- a/server/protocol-handshake-newstyle.c +++ b/server/protocol-handshake-newstyle.c @@ -272,11 +272,17 @@ negotiate_handshake_newstyle_options (struct connection *conn) if (conn_recv_full (conn, data, optlen, "read: %s: %m", name_of_nbd_opt (option)) == -1) return -1; - /* Apart from printing it, ignore the export name. */ + /* Print the export name and save it in the connection. */ data[optlen] = '\0'; - debug ("newstyle negotiation: %s: " - "client requested export '%s' (ignored)", + debug ("newstyle negotiation: %s: client requested export '%s'", name_of_nbd_opt (option), data); + free (conn->exportname); + conn->exportname = malloc (optlen+1); + if (conn->exportname == NULL) { + nbdkit_error ("malloc: %m"); + return -1; + } + strcpy (conn->exportname, data); /* We have to finish the handshake by sending handshake_finish. */ if (finish_newstyle_options (conn, &exportsize) == -1) @@ -388,7 +394,6 @@ negotiate_handshake_newstyle_options (struct connection *conn) uint16_t nrinfos; uint16_t info; size_t i; - CLEANUP_FREE char *requested_exportname = NULL; /* Validate the name length and number of INFO requests. */ memcpy (&exportnamelen, &data[0], 4); @@ -411,19 +416,19 @@ negotiate_handshake_newstyle_options (struct connection *conn) continue; } - /* As with NBD_OPT_EXPORT_NAME we print the export name and then - * ignore it. + /* As with NBD_OPT_EXPORT_NAME we print the export name and + * save it in the connection. */ - requested_exportname = malloc (exportnamelen+1); - if (requested_exportname == NULL) { + free (conn->exportname); + conn->exportname = malloc (exportnamelen+1); + if (conn->exportname == NULL) { nbdkit_error ("malloc: %m"); return -1; } - memcpy (requested_exportname, &data[4], exportnamelen); - requested_exportname[exportnamelen] = '\0'; - debug ("newstyle negotiation: %s: " - "client requested export '%s' (ignored)", - optname, requested_exportname); + memcpy (conn->exportname, &data[4], exportnamelen); + conn->exportname[exportnamelen] = '\0'; + debug ("newstyle negotiation: %s: client requested export '%s'", + optname, conn->exportname); /* The spec is confusing, but it is required that we send back * NBD_INFO_EXPORT, even if the client did not request it! diff --git a/server/public.c b/server/public.c index 630de9b..96ab353 100644 --- a/server/public.c +++ b/server/public.c @@ -379,3 +379,16 @@ nbdkit_nanosleep (unsigned sec, unsigned nsec) return 0; #endif } + +const char * +nbdkit_export_name (void) +{ + struct connection *conn = threadlocal_get_conn (); + + if (!conn) { + nbdkit_error ("no connection in this thread"); + return NULL; + } + + return conn->exportname; +} -- 2.23.0

--- tests/Makefile.am | 3 ++ tests/test-export-name.sh | 86 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index b5806bb..f54597b 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -395,6 +395,9 @@ endif HAVE_LIBGUESTFS # Test export flags. TESTS += test-eflags.sh +# Test export name. +TESTS += test-export-name.sh + # common disk image shared with several tests if HAVE_GUESTFISH check_DATA += disk diff --git a/tests/test-export-name.sh b/tests/test-export-name.sh new file mode 100755 index 0000000..2f63911 --- /dev/null +++ b/tests/test-export-name.sh @@ -0,0 +1,86 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2019 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 + +requires nbdsh --version +requires stat --version + +sock=`mktemp -u` +files="$sock exportname.pid" +rm -f $files +cleanup_fn rm -f $files + +# Create a sh plugin which echos the export name back in the device. +start_nbdkit -P exportname.pid -U $sock \ + sh - <<'EOF' +case "$1" in + open) + h=`mktemp $tmpdir/disk-XXXXXX` + echo -n "$3" > $h + echo $h + ;; + get_size) + stat -c '%s' "$2" + ;; + pread) + dd if="$2" skip=$4 count=$3 iflag=skip_bytes,count_bytes + ;; + *) exit 2 ;; +esac +EOF + +# Try to read back various export names from the plugin. +for e in "" "test" "/" "//" " " \ + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" +do + export e sock + nbdsh -c ' +import os + +e = os.environ["e"] +h.set_export_name (e) +h.connect_unix (os.environ["sock"]) + +size = h.get_size () +print ("size=%r e=%r" % (size, e)) +assert size == len (e) + +# Zero-sized reads are not defined in the NBD protocol. +if size > 0: + buf = h.pread (size, 0) + print ("buf=%r e=%r" % (buf, e)) + assert buf.decode() == e +' +done -- 2.23.0