Wire up everything in the server to query the export list from the plugin. Actual patches to let plugins and filters return a non-default list will come later, so for now, the behavior is unchanged: NBD_OPT_LIST still lists just "", and a client requesting "" still lets the plugin see an export name of "". Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- server/internal.h | 6 +++ server/backend.c | 22 ++++++++ server/filters.c | 9 ++++ server/plugins.c | 9 ++++ server/protocol-handshake-newstyle.c | 80 ++++++++++++++++------------ server/protocol-handshake.c | 24 +++++++++ 6 files changed, 116 insertions(+), 34 deletions(-) diff --git a/server/internal.h b/server/internal.h index 1acbbb06..32388d07 100644 --- a/server/internal.h +++ b/server/internal.h @@ -361,6 +361,8 @@ struct backend { void (*get_ready) (struct backend *); void (*after_fork) (struct backend *); int (*preconnect) (struct backend *, int readonly); + int (*list_exports) (struct backend *, int readonly, int default_only, + struct nbdkit_exports *exports); void *(*open) (struct backend *, int readonly, const char *exportname); int (*prepare) (struct backend *, void *handle, int readonly); int (*finalize) (struct backend *, void *handle); @@ -405,6 +407,10 @@ extern void backend_load (struct backend *b, const char *name, extern void backend_unload (struct backend *b, void (*unload) (void)) __attribute__((__nonnull__ (1))); +extern int backend_list_exports (struct backend *b, int readonly, + int default_only, + struct nbdkit_exports *exports) + __attribute__((__nonnull__ (1, 4))); /* exportname is only valid for this call and almost certainly will be * freed on return of this function, so backends must save the * exportname if they need to refer to it later. diff --git a/server/backend.c b/server/backend.c index d39fdeaf..315bd8d2 100644 --- a/server/backend.c +++ b/server/backend.c @@ -151,6 +151,28 @@ backend_unload (struct backend *b, void (*unload) (void)) free (b->name); } +int +backend_list_exports (struct backend *b, int readonly, int default_only, + struct nbdkit_exports *exports) +{ + GET_CONN; + struct handle *h = get_handle (conn, b->i); + int r; + + controlpath_debug ("%s: list_exports readonly=%d default_only=%d", + b->name, readonly, default_only); + + assert (h->handle == NULL); + assert ((h->state & HANDLE_OPEN) == 0); + r = b->list_exports (b, readonly, default_only, exports); + if (r == -1) + controlpath_debug ("%s: list_exports failed", b->name); + else + controlpath_debug ("%s: list_exports returned %zu names", b->name, + nbdkit_exports_count (exports)); + return r; +} + int backend_open (struct backend *b, int readonly, const char *exportname) { diff --git a/server/filters.c b/server/filters.c index 7d268096..e5b5b860 100644 --- a/server/filters.c +++ b/server/filters.c @@ -237,6 +237,14 @@ plugin_magic_config_key (struct backend *b) return b->next->magic_config_key (b->next); } +static int +filter_list_exports (struct backend *b, int readonly, int default_only, + struct nbdkit_exports *exports) +{ + /* XXX No filter override yet... */ + return backend_list_exports (b->next, readonly, default_only, exports); +} + static void * filter_open (struct backend *b, int readonly, const char *exportname) { @@ -540,6 +548,7 @@ static struct backend filter_functions = { .get_ready = filter_get_ready, .after_fork = filter_after_fork, .preconnect = filter_preconnect, + .list_exports = filter_list_exports, .open = filter_open, .prepare = filter_prepare, .finalize = filter_finalize, diff --git a/server/plugins.c b/server/plugins.c index 8449b1d9..8020046b 100644 --- a/server/plugins.c +++ b/server/plugins.c @@ -277,6 +277,14 @@ plugin_preconnect (struct backend *b, int readonly) return p->plugin.preconnect (readonly); } +static int +plugin_list_exports (struct backend *b, int readonly, int default_only, + struct nbdkit_exports *exports) +{ + /* XXX No plugin support yet, so for now just advertise "" */ + return nbdkit_add_export (exports, "", NULL); +} + static void * plugin_open (struct backend *b, int readonly, const char *exportname) { @@ -730,6 +738,7 @@ static struct backend plugin_functions = { .get_ready = plugin_get_ready, .after_fork = plugin_after_fork, .preconnect = plugin_preconnect, + .list_exports = plugin_list_exports, .open = plugin_open, .prepare = plugin_prepare, .finalize = plugin_finalize, diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c index 4025a645..8f41c7a8 100644 --- a/server/protocol-handshake-newstyle.c +++ b/server/protocol-handshake-newstyle.c @@ -75,44 +75,59 @@ send_newstyle_option_reply (uint32_t option, uint32_t reply) return 0; } -/* Reply to NBD_OPT_LIST with a single empty export name. - * TODO: Ask the plugin for the list of exports. +/* Reply to NBD_OPT_LIST with the plugin's list of export names. */ static int -send_newstyle_option_reply_exportname (uint32_t option, uint32_t reply) +send_newstyle_option_reply_exportnames (uint32_t option) { GET_CONN; struct nbd_fixed_new_option_reply fixed_new_option_reply; - const size_t name_len = 0; /* length of export name */ - uint32_t len; + size_t i; + CLEANUP_EXPORTS_FREE struct nbdkit_exports *exps = NULL; - fixed_new_option_reply.magic = htobe64 (NBD_REP_MAGIC); - fixed_new_option_reply.option = htobe32 (option); - fixed_new_option_reply.reply = htobe32 (reply); - fixed_new_option_reply.replylen = htobe32 (name_len + sizeof (len)); + exps = nbdkit_exports_new (false); + if (exps == NULL) + return send_newstyle_option_reply (option, NBD_REP_ERR_TOO_BIG); + if (backend_list_exports (top, read_only, false, exps) == -1) + return send_newstyle_option_reply (option, NBD_REP_ERR_PLATFORM); - if (conn->send (&fixed_new_option_reply, - sizeof fixed_new_option_reply, SEND_MORE) == -1) { - nbdkit_error ("write: %s: %m", name_of_nbd_opt (option)); - return -1; - } + for (i = 0; i < nbdkit_exports_count (exps); i++) { + const struct nbdkit_export export = nbdkit_get_export (exps, i); + size_t name_len = strlen (export.name); + size_t desc_len = export.description ? strlen (export.description) : 0; + uint32_t len; - len = htobe32 (name_len); - if (conn->send (&len, sizeof len, SEND_MORE) == -1) { - nbdkit_error ("write: %s: %s: %m", - name_of_nbd_opt (option), "sending length"); - return -1; - } -#if 0 - /* If we were sending a non-"" export name, this is what we'd use. */ - if (conn->send (exportname, name_len, 0) == -1) { - nbdkit_error ("write: %s: %s: %m", - name_of_nbd_opt (option), "sending export name"); - return -1; + fixed_new_option_reply.magic = htobe64 (NBD_REP_MAGIC); + fixed_new_option_reply.option = htobe32 (option); + fixed_new_option_reply.reply = htobe32 (NBD_REP_SERVER); + fixed_new_option_reply.replylen = htobe32 (name_len + sizeof (len) + + desc_len); + + if (conn->send (&fixed_new_option_reply, + sizeof fixed_new_option_reply, SEND_MORE) == -1) { + nbdkit_error ("write: %s: %m", name_of_nbd_opt (option)); + return -1; + } + + len = htobe32 (name_len); + if (conn->send (&len, sizeof len, SEND_MORE) == -1) { + nbdkit_error ("write: %s: %s: %m", + name_of_nbd_opt (option), "sending length"); + return -1; + } + if (conn->send (export.name, name_len, SEND_MORE) == -1) { + nbdkit_error ("write: %s: %s: %m", + name_of_nbd_opt (option), "sending export name"); + return -1; + } + if (conn->send (export.description, desc_len, 0) == -1) { + nbdkit_error ("write: %s: %s: %m", + name_of_nbd_opt (option), "sending export description"); + return -1; + } } -#endif - return 0; + return send_newstyle_option_reply (option, NBD_REP_ACK); } static int @@ -384,13 +399,10 @@ negotiate_handshake_newstyle_options (void) continue; } - /* Send back the exportname. */ - debug ("newstyle negotiation: %s: advertising export \"\"", + /* Send back the exportname list. */ + debug ("newstyle negotiation: %s: advertising exports", name_of_nbd_opt (option)); - if (send_newstyle_option_reply_exportname (option, NBD_REP_SERVER) == -1) - return -1; - - if (send_newstyle_option_reply (option, NBD_REP_ACK) == -1) + if (send_newstyle_option_reply_exportnames (option) == -1) return -1; break; diff --git a/server/protocol-handshake.c b/server/protocol-handshake.c index 80233713..afc954d2 100644 --- a/server/protocol-handshake.c +++ b/server/protocol-handshake.c @@ -59,6 +59,8 @@ protocol_handshake () } /* Common code used by oldstyle and newstyle protocols to: + * + * - canonicalize default export name of "" * * - call the backend .open method * @@ -79,6 +81,28 @@ protocol_common_open (uint64_t *exportsize, uint16_t *flags, int64_t size; uint16_t eflags = NBD_FLAG_HAS_FLAGS; int fl; + CLEANUP_FREE char *default_export = NULL; + + if (!*exportname) { + /* Map client's request for the default export ("") to plugin's + * canonical name. This is only a best-effort attempt, other + * than detection of malloc failure. + */ + CLEANUP_EXPORTS_FREE struct nbdkit_exports *exps = NULL; + + exps = nbdkit_exports_new (true); + if (exps == NULL) + return -1; + if (backend_list_exports (top, read_only, true, exps) == 0 && + nbdkit_exports_count (exps)) { + default_export = strdup (nbdkit_get_export (exps, 0).name); + if (default_export == NULL) { + nbdkit_error ("strdup: %m"); + return -1; + } + exportname = default_export; + } + } if (backend_open (top, read_only, exportname) == -1) return -1; -- 2.28.0

On 7/31/20 5:22 PM, Eric Blake wrote: I'm trying to figure out a situation where knowing whether we are running under 'nbdkit -r' (and thus all exports will be read-only) will affect the list advertised by the plugin. But then again, we _do_ pass the readonly flag to both .preconnect and .open, which are bookends around this call. That said, now's the time to decide whether to keep it or to simplify the interface by dropping it, before we bake in a stable release with the API. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

Exposing .list_exports through to shell scripts makes testing export listing a lot more feasible. The design chosen here is amenable to 'ls -1' or 'find' output provided there are no newlines or whitespace in the files being listed (the user gives up descriptions in that case), while also being flexible enough to support names or descriptions with arbitrary content. One idea that might be worth considering for future patches is parsing a magic header line describing how the rest of the input would be parsed, where the first line being unrecognized uses a default format, but other magic words can introduce alternative formats that might be easier to produce within scripts. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- RFC because the test is incomplete (in part because of libnbd bugs that I discovered which need fixing first). plugins/eval/nbdkit-eval-plugin.pod | 2 + plugins/sh/nbdkit-sh-plugin.pod | 31 ++++++++++ tests/Makefile.am | 2 + plugins/sh/methods.h | 4 +- plugins/eval/eval.c | 2 + plugins/sh/methods.c | 91 +++++++++++++++++++++++++++++ plugins/sh/sh.c | 1 + plugins/sh/example.sh | 8 +++ tests/test-eval-exports.sh | 65 +++++++++++++++++++++ 9 files changed, 205 insertions(+), 1 deletion(-) create mode 100755 tests/test-eval-exports.sh diff --git a/plugins/eval/nbdkit-eval-plugin.pod b/plugins/eval/nbdkit-eval-plugin.pod index 7e25a01f..7126c6de 100644 --- a/plugins/eval/nbdkit-eval-plugin.pod +++ b/plugins/eval/nbdkit-eval-plugin.pod @@ -108,6 +108,8 @@ features): =item B<is_rotational=>SCRIPT +=item B<list_exports=>SCRIPT + =item B<open=>SCRIPT =item B<pread=>SCRIPT diff --git a/plugins/sh/nbdkit-sh-plugin.pod b/plugins/sh/nbdkit-sh-plugin.pod index 771c6bc0..303e96a7 100644 --- a/plugins/sh/nbdkit-sh-plugin.pod +++ b/plugins/sh/nbdkit-sh-plugin.pod @@ -266,6 +266,37 @@ with status C<1>; unrecognized output is ignored. /path/to/script preconnect <readonly> <exportname> +=item C<list_exports> + + /path/to/script list_exports <readonly> <default_only> + +The C<readonly> parameter will be C<true> or C<false>. The +C<default_only> parameter will be C<true> if the caller is only +interested in the canonical name of the default export, or C<false> to +get a full list of export names; the script may safely ignore this +parameter and always provide a full list if desired. + +This must print, one per line on stdout, a list of zero or more export +declarations in the format: + + name [description...] + +which correspond to the inputs of the C C<nbdkit_add_export> function +(see L<nbdkit-plugin(3)>). Within the line, the sequence C<\n> is an +escape for a literal newline, and all other uses of backslash escape +the next character. The C<name> field is mandatory; if it begins with +a single quote C<'>, it ends on the first matching unescaped single +quote (with the quotes not part of the name), otherwise the name ends +at the first unescaped whitespace. Any text remaining on the line +after C<name> is treated as the C<description>, with unescaped leading +and trailing whitespace stripped. Note that if none of the files in a +directory contain backslash, single quote, or whitespace in their +names, then the output of C<ls> will list names without descriptions. + +This method is I<not> required; if it is absent, the list of exports +advertised by nbdkit will be the single export with the empty string +as a name and no description. + =item C<open> /path/to/script open <readonly> <exportname> diff --git a/tests/Makefile.am b/tests/Makefile.am index 129e0647..e3c42219 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -614,10 +614,12 @@ test_data_LDADD = libtest.la $(LIBGUESTFS_LIBS) TESTS += \ test-eval.sh \ test-eval-file.sh \ + test-eval-exports.sh \ $(NULL) EXTRA_DIST += \ test-eval.sh \ test-eval-file.sh \ + test-eval-exports.sh \ $(NULL) # file plugin test. diff --git a/plugins/sh/methods.h b/plugins/sh/methods.h index 08a5ed17..69017fa4 100644 --- a/plugins/sh/methods.h +++ b/plugins/sh/methods.h @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2018 Red Hat Inc. + * Copyright (C) 2018-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -44,6 +44,8 @@ extern int sh_thread_model (void); extern int sh_get_ready (void); extern int sh_after_fork (void); extern int sh_preconnect (int readonly); +extern int sh_list_exports (int readonly, int default_only, + struct nbdkit_exports *exports); extern void *sh_open (int readonly); extern void sh_close (void *handle); extern int64_t sh_get_size (void *handle); diff --git a/plugins/eval/eval.c b/plugins/eval/eval.c index 54c5029e..2bd5e79f 100644 --- a/plugins/eval/eval.c +++ b/plugins/eval/eval.c @@ -74,6 +74,7 @@ static const char *known_methods[] = { "get_ready", "get_size", "is_rotational", + "list_exports", "missing", "open", "pread", @@ -393,6 +394,7 @@ static struct nbdkit_plugin plugin = { .after_fork = sh_after_fork, .preconnect = sh_preconnect, + .list_exports = sh_list_exports, .open = sh_open, .close = sh_close, diff --git a/plugins/sh/methods.c b/plugins/sh/methods.c index 8257103e..a4422238 100644 --- a/plugins/sh/methods.c +++ b/plugins/sh/methods.c @@ -225,6 +225,97 @@ struct sh_handle { int can_zero; }; +static int +parse_exports (const char *script, + const char *s, size_t slen, struct nbdkit_exports *exports) +{ + FILE *fp = NULL; + CLEANUP_FREE char *line = NULL; + size_t linelen = 0; + ssize_t len; + int ret = -1; + + fp = fmemopen ((void *) s, slen, "r"); + if (!fp) { + nbdkit_error ("%s: fmemopen: %m", script); + goto out; + } + + while ((len = getline (&line, &linelen, fp)) != -1) { + bool quoted = *line == '\''; + char *p = line, *q = p + quoted, *desc; + + /* Modify line in-place with our parse of name */ + while (q - line < len && (quoted ? *q != '\'' : !ascii_isspace (*q))) { + if (*q == '\\') { + q++; + if (*q == 'n') + *q = '\n'; + } + *p++ = *q++; + } + /* Transition from name to description */ + *p++ = '\0'; + q++; + while (ascii_isspace (*q)) + q++; + p = desc = q; + while (q - line < len) { + if (*q == '\\') { + q++; + if (*q == 'n') + *q = '\n'; + } + *p++ = *q++; + } + while (p > desc && ascii_isspace (p[-1])) + p--; + *p = '\0'; + + nbdkit_debug ("%s: adding export '%s' '%s'", script, line, desc); + if (nbdkit_add_export (exports, line, desc) == -1) + goto out; + } + + ret = 0; + + out: + if (fp) + fclose (fp); + return ret; +} + +int +sh_list_exports (int readonly, int default_only, + struct nbdkit_exports *exports) +{ + const char *method = "list_exports"; + const char *script = get_script (method); + const char *args[] = { script, method, readonly ? "true" : "false", + default_only ? "true" : "false", NULL }; + CLEANUP_FREE char *s = NULL; + size_t slen; + + switch (call_read (&s, &slen, args)) { + case OK: + return parse_exports (script, s, slen, exports); + + case MISSING: + return nbdkit_add_export (exports, "", NULL); + + case ERROR: + return -1; + + case RET_FALSE: + nbdkit_error ("%s: %s method returned unexpected code (3/false)", + script, method); + errno = EIO; + return -1; + + default: abort (); + } +} + void * sh_open (int readonly) { diff --git a/plugins/sh/sh.c b/plugins/sh/sh.c index 9e484823..374888a4 100644 --- a/plugins/sh/sh.c +++ b/plugins/sh/sh.c @@ -300,6 +300,7 @@ static struct nbdkit_plugin plugin = { .after_fork = sh_after_fork, .preconnect = sh_preconnect, + .list_exports = sh_list_exports, .open = sh_open, .close = sh_close, diff --git a/plugins/sh/example.sh b/plugins/sh/example.sh index 99e4e890..a6550055 100755 --- a/plugins/sh/example.sh +++ b/plugins/sh/example.sh @@ -85,6 +85,14 @@ case "$1" in echo parallel ;; + list_exports) + # The following lists the names of all files in the current + # directory that do not contain whitespace, backslash, or single + # quotes. No description accompanies the export names. + # The first file listed is used when a client requests export ''. + find . -type f \! -name "*['\\\\[:space:]]*" + ;; + open) # Open a new client connection. diff --git a/tests/test-eval-exports.sh b/tests/test-eval-exports.sh new file mode 100755 index 00000000..e9f99565 --- /dev/null +++ b/tests/test-eval-exports.sh @@ -0,0 +1,65 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2018-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. + +# This is an example from the nbdkit-eval-plugin(1) manual page. +# Check here that it doesn't regress. + +source ./functions.sh +set -e +set -x + +requires nbdsh -c 'print (h.get_list_export_description)' +requires nbdinfo --help +requires jq --version + +files="eval-exports.list eval-exports.out" +rm -f $files +cleanup_fn rm -f $files + +# Control case: no list_exports +nbdkit -U - -v eval get_size='echo 0' \ + --run 'nbdinfo --list --json "$uri"' > eval-exports.out +cat eval-exports.out +diff -u <(jq -c '[.exports[] | [."export-name", .description]]' \ + eval-exports.out) - <<EOF +[["",null]] +EOF + +# Empty exports list produces 0 exports +: >eval-exports.list +nbdkit -U - -v eval list_exports='cat eval-exports.list' get_size='echo 0' \ + --run 'nbdinfo --list --json "$uri"' > eval-exports.out +cat eval-exports.out +diff -u <(jq -c '[.exports[] | [."export-name", .description]]' \ + eval-exports.out) - <<EOF +[] +EOF -- 2.28.0