I'm about to add an 'exportname' filter, and in the process, I noticed a few shortcomings in our API. Overloading .list_exports in order to determine a canonical export name at .open time is awkward; the two uses (answering NBD_OPT_LIST for a full list, vs. remapping a client's "" into a canonical name during .open) are orthogonal enough to warrant separate plugin callbacks. This will also make it easier to express the notion of no default export (connecting to "" is an error) at the same time as listing other exports. Another consideration is that when tls=1, the choice of export to expose pre-TLS vs. post-TLS may differ, but without a call to .open yet, our just-added nbdkit_is_tls() does not fit our preferred lifecycle, so this has to be a parameter to the new .default_export. We will alter the signature of .list_exports soon; in the meantime, the bool default_only parameter is now ignored. Adding .default_export support to sh/eval is big enough for a separate patch. The ondemand plugin continues to advertise "" (and not "default") in its list of exports, but now reports a canonical name of "default" when connecting to it. The cc plugin gets .default_export now; it will get .list_exports when we fix that signature. The tls-fallback filter can now avoid leaking a default export name through an insecure connection. In the testsuite, test-layers shows that a connection to export "" invokes the new callback through the chain of backends; while test-eval-exports is temporarily weakened until sh support is enhanced later. --- docs/nbdkit-filter.pod | 46 ++++++++++++++++--------- docs/nbdkit-plugin.pod | 35 +++++++++++++++++++ include/nbdkit-filter.h | 9 +++-- include/nbdkit-plugin.h | 1 + server/internal.h | 7 ++-- server/backend.c | 51 ++++++++++++++++++++-------- server/exports.c | 8 +---- server/filters.c | 12 +++++++ server/plugins.c | 14 +++++++- server/protocol-handshake-newstyle.c | 9 ++++- plugins/cc/cc.c | 9 +++++ plugins/ondemand/ondemand.c | 11 ++++-- filters/tls-fallback/tls-fallback.c | 12 ++++++- tests/test-eval-exports.sh | 3 +- tests/test-layers-filter.c | 9 +++++ tests/test-layers-plugin.c | 8 +++++ tests/test-layers.c | 12 +++++++ 17 files changed, 208 insertions(+), 48 deletions(-) diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod index 9c15ee9b..cd6d8fef 100644 --- a/docs/nbdkit-filter.pod +++ b/docs/nbdkit-filter.pod @@ -129,17 +129,19 @@ which is required. F<nbdkit-filter.h> defines some function types (C<nbdkit_next_config>, C<nbdkit_next_config_complete>, C<nbdkit_next_get_ready>, C<nbdkit_next_after_fork>, C<nbdkit_next_preconnect>, -C<nbdkit_next_list_exports>, C<nbdkit_next_open>) and a structure -called C<struct nbdkit_next_ops>. These abstract the next plugin or -filter in the chain. There is also an opaque pointer C<nxdata> which -must be passed along when calling these functions. The value of -C<nxdata> passed to C<.open> has a stable lifetime that lasts to the -corresponding C<.close>, with all intermediate functions (such as -C<.pread>) receiving the same value for convenience; the only -exceptions where C<nxdata> is not reused are C<.config>, -C<.config_complete>, C<.get_ready>, C<.after_fork>, C<.preconnect> and -C<.list_exports>, which are called outside the lifetime of a -connection. +C<nbdkit_next_list_exports>, C<nbdkit_next_default_export>, +C<nbdkit_next_open>) and a structure called C<struct nbdkit_next_ops>. +These abstract the next plugin or filter in the chain. There is also +an opaque pointer C<nxdata> which must be passed along when calling +these functions. The value of C<nxdata> passed to C<.open> has a +stable lifetime that lasts to the corresponding C<.close>, with all +intermediate functions (such as C<.pread>) receiving the same value +for convenience. Functions where C<nxdata> is not reused are +C<.config>, C<.config_complete>, C<.get_ready>, and C<.after_fork>, +which are called during initialization outside any connections, and +C<.preconnect>, C<.list_exports>, and C<.default_export>, which are +called based on client connections but prior to the stable lifetime of +C<.open>. =head2 Next config, open and close @@ -356,7 +358,7 @@ from the layer below. Without error checking it would look like this: struct nbdkit_export e; char *name, *desc; - exports2 = nbdkit_exports_new (default_only); + exports2 = nbdkit_exports_new (); next_list_exports (nxdata, readonly, default_only, exports); for (i = 0; i < nbdkit_exports_count (exports2); ++i) { e = nbdkit_get_export (exports2, i); @@ -377,11 +379,9 @@ an error message and return C<-1>. Two functions are provided to filters only for allocating and freeing the list: - struct nbdkit_exports *nbdkit_exports_new (int default_only); + struct nbdkit_exports *nbdkit_exports_new (void); -Allocates and returns a new, empty exports list. The C<default_only> -parameter should match whether the list is intended to grab the -canonical name of the default export, or all exports. +Allocates and returns a new, empty exports list. On error this function can return C<NULL>. In this case it calls C<nbdkit_error> as required. C<errno> will be set to a suitable @@ -409,6 +409,20 @@ Returns the number of exports in the list. Returns a copy of the C<i>'th export. +=head2 C<.default_export> + + const char *default_export (nbdkit_next_default_export *next, void *nxdata, + int readonly, int is_tls) + +This intercepts the plugin C<.default_export> method and can be used to +alter the canonical export name used in place of the default C<"">. + +The C<readonly> parameter matches what is passed to <.preconnect> and +C<.open>, and may be changed by the filter when calling into the +plugin. The C<is_tls> parameter informs the filter whether TLS +negotiation has been completed by the client, but is not passed on to +C<next> because it cannot be altered. + =head2 C<.open> void * (*open) (nbdkit_next_open *next, void *nxdata, diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index ab1e40cb..180c9daa 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -245,6 +245,12 @@ Early in option negotiation the client may try to list the exports served by the plugin, and plugins can optionally implement this callback to answer the client. See L</EXPORT NAME> below. +=item C<.default_export> + +During option negotiation, if the client requests the default export +name (C<"">), this optional callback provides a canonical name to +use in its place prior to calling C<.open>. + =item C<.open> A new client has connected and finished the NBD handshake. TLS @@ -709,6 +715,35 @@ Returning C<0> will send the list of exports back to the client. If there is an error, C<.list_exports> should call C<nbdkit_error> with an error message and return C<-1>. +=head2 C<.default_export> + + const char *default_export (int readonly, int is_tls); + +This optional callback is called if the client tries to connect to the +default export C<"">, where the plugin provides a UTF-8 string between +0 and 4096 bytes. If the plugin does not supply this callback, the +connection continues with the empty name; if the plugin returns a +valid string, nbdkit behaves as if the client had passed that string +instead of an empty name. + +The C<readonly> flag informs the plugin that the server was started +with the I<-r> flag on the command line, which is the same value +passed to C<.preconnect> and C<.open>. However, the NBD protocol does +not yet have a way to let the client advertise an intent to be +read-only even when the server allows writes, so this parameter may +not be as useful as it appears. + +The C<is_tls> flag informs the plugin whether the canonical name for +the default export is being requested after the client has completed +TLS negotiation. When running the server in a mode that permits but +does not require TLS, be careful that a default export name does not +leak unintended information. + +If the plugin returns C<NULL> or an invalid string (such as longer +than 4096 bytes), the client is not permitted to connect to the +default export. However, this is not an error in the protocol, so it +is not necessary to call C<nbdkit_error>. + =head2 C<.open> void *open (int readonly); diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h index b4024ae5..2c5b36be 100644 --- a/include/nbdkit-filter.h +++ b/include/nbdkit-filter.h @@ -66,8 +66,10 @@ typedef int nbdkit_next_get_ready (nbdkit_backend *nxdata); typedef int nbdkit_next_after_fork (nbdkit_backend *nxdata); typedef int nbdkit_next_preconnect (nbdkit_backend *nxdata, int readonly); typedef int nbdkit_next_list_exports (nbdkit_backend *nxdata, int readonly, - int default_only, + int ignored, struct nbdkit_exports *exports); +typedef const char *nbdkit_next_default_export (nbdkit_backend *nxdata, + int readonly); typedef int nbdkit_next_open (nbdkit_backend *nxdata, int readonly, const char *exportname); @@ -136,7 +138,7 @@ struct nbdkit_export { }; NBDKIT_EXTERN_DECL (struct nbdkit_exports *, nbdkit_exports_new, - (int default_only)); + (void)); NBDKIT_EXTERN_DECL (void, nbdkit_exports_free, (struct nbdkit_exports *)); NBDKIT_EXTERN_DECL (size_t, nbdkit_exports_count, (const struct nbdkit_exports *)); @@ -179,6 +181,9 @@ struct nbdkit_filter { int (*list_exports) (nbdkit_next_list_exports *next, nbdkit_backend *nxdata, int readonly, int default_only, struct nbdkit_exports *exports); + const char * (*default_export) (nbdkit_next_default_export *next, + nbdkit_backend *nxdata, + int readonly, int is_tls); void * (*open) (nbdkit_next_open *next, nbdkit_backend *nxdata, int readonly, const char *exportname, int is_tls); diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h index a5d85411..28e83757 100644 --- a/include/nbdkit-plugin.h +++ b/include/nbdkit-plugin.h @@ -142,6 +142,7 @@ struct nbdkit_plugin { int (*list_exports) (int readonly, int default_only, struct nbdkit_exports *exports); + const char * (*default_export) (int readonly, int is_tls); }; NBDKIT_EXTERN_DECL (void, nbdkit_set_error, (int err)); diff --git a/server/internal.h b/server/internal.h index 27871520..dfb4da31 100644 --- a/server/internal.h +++ b/server/internal.h @@ -386,8 +386,9 @@ 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, + int (*list_exports) (struct backend *, int readonly, int ignored, struct nbdkit_exports *exports); + const char *(*default_export) (struct backend *, int readonly, int is_tls); void *(*open) (struct backend *, int readonly, const char *exportname, int is_tls); int (*prepare) (struct backend *, void *handle, int readonly); @@ -434,9 +435,11 @@ 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, + int ignored, struct nbdkit_exports *exports) __attribute__((__nonnull__ (1, 4))); +extern const char *backend_default_export (struct backend *b, int readonly) + __attribute__((__nonnull__ (1))); /* 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 6c8871f9..6e82629c 100644 --- a/server/backend.c +++ b/server/backend.c @@ -166,13 +166,12 @@ backend_list_exports (struct backend *b, int readonly, int default_only, struct handle *h = get_handle (conn, b->i); int r; + assert (!default_only); /* XXX Switch to is_tls... */ 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); - if (default_only && h->default_exportname) - return nbdkit_add_export (exports, h->default_exportname, NULL); r = b->list_exports (b, readonly, default_only, exports); if (r == -1) @@ -180,13 +179,40 @@ backend_list_exports (struct backend *b, int readonly, int default_only, else { size_t count = nbdkit_exports_count (exports); controlpath_debug ("%s: list_exports returned %zu names", b->name, count); - /* Best effort caching of default export name */ - if (!h->default_exportname && count) - h->default_exportname = strdup (nbdkit_get_export (exports, 0).name); } return r; } +const char * +backend_default_export (struct backend *b, int readonly) +{ + GET_CONN; + struct handle *h = get_handle (conn, b->i); + const char *s; + + controlpath_debug ("%s: default_export readonly=%d tls=%d", + b->name, readonly, conn->using_tls); + + if (h->default_exportname == NULL) { + assert (h->handle == NULL); + assert ((h->state & HANDLE_OPEN) == 0); + s = b->default_export (b, readonly, conn->using_tls); + /* Ignore over-length strings. XXX Also ignore non-UTF8? */ + if (s && strnlen (s, NBD_MAX_STRING + 1) > NBD_MAX_STRING) { + controlpath_debug ("%s: default_export: ignoring invalid string", + b->name); + s = NULL; + } + if (s) { + /* Best effort caching */ + h->default_exportname = strdup (s); + if (h->default_exportname == NULL) + return s; + } + } + return h->default_exportname; +} + int backend_open (struct backend *b, int readonly, const char *exportname) { @@ -202,18 +228,13 @@ backend_open (struct backend *b, int readonly, const char *exportname) if (readonly) h->can_write = 0; - /* Best-effort determination of the canonical name for default export */ + /* Determine the canonical name for default export */ if (!*exportname) { - if (!h->default_exportname) { - CLEANUP_EXPORTS_FREE struct nbdkit_exports *exps = NULL; - - exps = nbdkit_exports_new (true); - if (exps && b->list_exports (b, readonly, true, exps) == 0 && - nbdkit_exports_count (exps)) - h->default_exportname = strdup (nbdkit_get_export (exps, 0).name); + exportname = backend_default_export (b, readonly); + if (exportname == NULL) { + nbdkit_error ("default export (\"\") not permitted"); + return -1; } - if (h->default_exportname) - exportname = h->default_exportname; } /* Most filters will call next_open first, resulting in diff --git a/server/exports.c b/server/exports.c index 3f819622..8d3faec4 100644 --- a/server/exports.c +++ b/server/exports.c @@ -52,12 +52,10 @@ DEFINE_VECTOR_TYPE(exports, struct nbdkit_export); struct nbdkit_exports { exports exports; - - bool default_only; }; struct nbdkit_exports * -nbdkit_exports_new (int default_only) +nbdkit_exports_new (void) { struct nbdkit_exports *r; @@ -67,7 +65,6 @@ nbdkit_exports_new (int default_only) return NULL; } r->exports = (exports) empty_vector; - r->default_only = default_only != 0; return r; } @@ -107,9 +104,6 @@ nbdkit_add_export (struct nbdkit_exports *exps, { struct nbdkit_export e = { NULL, NULL }; - if (exps->default_only && exps->exports.size == 1) - return 0; - if (exps->exports.size == MAX_EXPORTS) { nbdkit_error ("nbdkit_add_export: too many exports"); errno = EINVAL; diff --git a/server/filters.c b/server/filters.c index 0cfae344..bb22be76 100644 --- a/server/filters.c +++ b/server/filters.c @@ -249,6 +249,17 @@ filter_list_exports (struct backend *b, int readonly, int default_only, return backend_list_exports (b->next, readonly, default_only, exports); } +static const char * +filter_default_export (struct backend *b, int readonly, int is_tls) +{ + struct backend_filter *f = container_of (b, struct backend_filter, backend); + + if (f->filter.default_export) + return f->filter.default_export (backend_default_export, b->next, + readonly, is_tls); + return backend_default_export (b->next, readonly); +} + static void * filter_open (struct backend *b, int readonly, const char *exportname, int is_tls) @@ -555,6 +566,7 @@ static struct backend filter_functions = { .after_fork = filter_after_fork, .preconnect = filter_preconnect, .list_exports = filter_list_exports, + .default_export = filter_default_export, .open = filter_open, .prepare = filter_prepare, .finalize = filter_finalize, diff --git a/server/plugins.c b/server/plugins.c index cc9a1b45..4cd93efc 100644 --- a/server/plugins.c +++ b/server/plugins.c @@ -166,6 +166,7 @@ plugin_dump_fields (struct backend *b) HAS (after_fork); HAS (preconnect); HAS (list_exports); + HAS (default_export); HAS (open); HAS (close); @@ -288,7 +289,6 @@ static int plugin_list_exports (struct backend *b, int readonly, int default_only, struct nbdkit_exports *exports) { - GET_CONN; struct backend_plugin *p = container_of (b, struct backend_plugin, backend); if (!p->plugin.list_exports) @@ -297,6 +297,17 @@ plugin_list_exports (struct backend *b, int readonly, int default_only, return p->plugin.list_exports (readonly, default_only, exports); } +static const char * +plugin_default_export (struct backend *b, int readonly, int is_tls) +{ + struct backend_plugin *p = container_of (b, struct backend_plugin, backend); + + if (!p->plugin.default_export) + return ""; + + return p->plugin.default_export (readonly, is_tls); +} + static void * plugin_open (struct backend *b, int readonly, const char *exportname, int is_tls) @@ -759,6 +770,7 @@ static struct backend plugin_functions = { .after_fork = plugin_after_fork, .preconnect = plugin_preconnect, .list_exports = plugin_list_exports, + .default_export = plugin_default_export, .open = plugin_open, .prepare = plugin_prepare, .finalize = plugin_finalize, diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c index 8f41c7a8..6e5f3822 100644 --- a/server/protocol-handshake-newstyle.c +++ b/server/protocol-handshake-newstyle.c @@ -85,7 +85,7 @@ send_newstyle_option_reply_exportnames (uint32_t option) size_t i; CLEANUP_EXPORTS_FREE struct nbdkit_exports *exps = NULL; - exps = nbdkit_exports_new (false); + exps = nbdkit_exports_new (); if (exps == NULL) return send_newstyle_option_reply (option, NBD_REP_ERR_TOO_BIG); if (backend_list_exports (top, read_only, false, exps) == -1) @@ -301,6 +301,7 @@ negotiate_handshake_newstyle_options (void) struct nbd_export_name_option_reply handshake_finish; const char *optname; uint64_t exportsize; + struct backend *b; for (nr_options = 0; nr_options < MAX_NR_OPTIONS; ++nr_options) { CLEANUP_FREE char *data = NULL; @@ -445,6 +446,12 @@ negotiate_handshake_newstyle_options (void) return -1; conn->using_tls = true; debug ("using TLS on this connection"); + /* Wipe out any cached default export name. */ + for_each_backend (b) { + struct handle *h = get_handle (conn, b->i); + free (h->default_exportname); + h->default_exportname = NULL; + } } break; diff --git a/plugins/cc/cc.c b/plugins/cc/cc.c index 807ffcb6..1d688056 100644 --- a/plugins/cc/cc.c +++ b/plugins/cc/cc.c @@ -348,6 +348,14 @@ cc_preconnect (int readonly) return 0; } +static const char * +cc_default_export (int readonly, int is_tls) +{ + if (subplugin.default_export) + return subplugin.default_export (readonly, is_tls); + return ""; +} + static void * cc_open (int readonly) { @@ -563,6 +571,7 @@ static struct nbdkit_plugin plugin = { .after_fork = cc_after_fork, .preconnect = cc_preconnect, + .default_export = cc_default_export, .open = cc_open, .close = cc_close, diff --git a/plugins/ondemand/ondemand.c b/plugins/ondemand/ondemand.c index 393561e1..6998d8e9 100644 --- a/plugins/ondemand/ondemand.c +++ b/plugins/ondemand/ondemand.c @@ -223,6 +223,13 @@ ondemand_list_exports (int readonly, int default_only, return 0; } +static const char * +ondemand_default_export (int readonly, int is_tls) +{ + /* We always accept "" as an export name; canonicalize it to "default". */ + return "default"; +} + struct handle { int fd; int64_t size; @@ -366,8 +373,7 @@ ondemand_open (int readonly) nbdkit_error ("internal error: expected nbdkit_export_name () != NULL"); goto error; } - if (strcmp (h->exportname, "") == 0) - h->exportname = "default"; + assert (strcmp (h->exportname, "") != 0); /* see .default_export */ /* Verify that the export name is valid. */ if (strlen (h->exportname) > NAME_MAX || @@ -625,6 +631,7 @@ static struct nbdkit_plugin plugin = { .get_ready = ondemand_get_ready, .list_exports = ondemand_list_exports, + .default_export = ondemand_default_export, .can_multi_conn = ondemand_can_multi_conn, .can_trim = ondemand_can_trim, diff --git a/filters/tls-fallback/tls-fallback.c b/filters/tls-fallback/tls-fallback.c index 822748b4..0fcc2bcf 100644 --- a/filters/tls-fallback/tls-fallback.c +++ b/filters/tls-fallback/tls-fallback.c @@ -76,6 +76,15 @@ tls_fallback_get_ready (nbdkit_next_get_ready *next, void *nxdata, /* TODO: list_exports needs is_tls parameter */ +static const char * +tls_fallback_default_export (nbdkit_next_default_export *next, void *nxdata, + int readonly, int is_tls) +{ + if (!is_tls) + return ""; + return next (nxdata, readonly); +} + /* Helper for determining if this connection is insecure. This works * because we can treat all handles on a binary basis: secure or * insecure, which lets .open get away without allocating a more @@ -183,7 +192,8 @@ static struct nbdkit_filter filter = { .config = tls_fallback_config, .config_help = tls_fallback_config_help, .get_ready = tls_fallback_get_ready, - /* XXX .init_exports needs is_tls parameter */ + /* XXX .list_exports needs is_tls parameter */ + .default_export = tls_fallback_default_export, .open = tls_fallback_open, .get_size = tls_fallback_get_size, .can_write = tls_fallback_can_write, diff --git a/tests/test-eval-exports.sh b/tests/test-eval-exports.sh index aa694aaa..7c946378 100755 --- a/tests/test-eval-exports.sh +++ b/tests/test-eval-exports.sh @@ -67,7 +67,8 @@ do_nbdkit () # Check how the default export name is handled # nbdinfo currently makes multiple connections, so we can't use the # long-running server for validating default export name. - nbdkit -U - -v eval list_exports="cat '$PWD/eval-exports.list'" \ + # XXX FIXME: requires .default_export in eval + : || nbdkit -U - -v eval list_exports="cat '$PWD/eval-exports.list'" \ open='[ "$3" = "'"$1"'" ] || { echo EINVAL wrong export >&2; exit 1; }' \ get_size='echo 0' --run 'nbdsh -u "$uri" -c "exit()"' # Check what exports are listed diff --git a/tests/test-layers-filter.c b/tests/test-layers-filter.c index 00e5ccf3..2d14a37e 100644 --- a/tests/test-layers-filter.c +++ b/tests/test-layers-filter.c @@ -120,6 +120,14 @@ test_layers_filter_list_exports (nbdkit_next_list_exports *next, void *nxdata, return next (nxdata, readonly, default_only, exports); } +static const char * +test_layers_filter_default_export (nbdkit_next_default_export *next, + void *nxdata, int readonly, int is_tls) +{ + DEBUG_FUNCTION; + return next (nxdata, readonly); +} + static void * test_layers_filter_open (nbdkit_next_open *next, void *nxdata, int readonly, const char *exportname, int is_tls) @@ -397,6 +405,7 @@ static struct nbdkit_filter filter = { .after_fork = test_layers_filter_after_fork, .preconnect = test_layers_filter_preconnect, .list_exports = test_layers_filter_list_exports, + .default_export = test_layers_filter_default_export, .open = test_layers_filter_open, .close = test_layers_filter_close, .prepare = test_layers_filter_prepare, diff --git a/tests/test-layers-plugin.c b/tests/test-layers-plugin.c index f43c60d5..856e4031 100644 --- a/tests/test-layers-plugin.c +++ b/tests/test-layers-plugin.c @@ -108,6 +108,13 @@ test_layers_plugin_list_exports (int readonly, int default_only, return nbdkit_add_export (exports, "", NULL); } +static const char * +test_layers_plugin_default_export (int readonly, int is_tls) +{ + DEBUG_FUNCTION; + return ""; +} + static void * test_layers_plugin_open (int readonly) { @@ -272,6 +279,7 @@ static struct nbdkit_plugin plugin = { .after_fork = test_layers_plugin_after_fork, .preconnect = test_layers_plugin_preconnect, .list_exports = test_layers_plugin_list_exports, + .default_export = test_layers_plugin_default_export, .open = test_layers_plugin_open, .close = test_layers_plugin_close, .get_size = test_layers_plugin_get_size, diff --git a/tests/test-layers.c b/tests/test-layers.c index 0c81d488..cd5d73bc 100644 --- a/tests/test-layers.c +++ b/tests/test-layers.c @@ -346,6 +346,18 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } + /* default_export called in outer-to-inner order. */ + log_verify_seen_in_order + ("testlayersfilter3: default_export", + "filter3: test_layers_filter_default_export", + "testlayersfilter2: default_export", + "filter2: test_layers_filter_default_export", + "testlayersfilter1: default_export", + "filter1: test_layers_filter_default_export", + "testlayersplugin: default_export", + "test_layers_plugin_default_export", + NULL); + /* open methods called in outer-to-inner order, but thanks to next * pointer, complete in inner-to-outer order. */ log_verify_seen_in_order -- 2.28.0

The NBD spec says that additional information in response to NBD_OPT_GO or NBD_OPT_INFO, such as NBD_INFO_NAME, is optional, but also recommends that if a client hints about wanting a particular piece of information, that it is better for the server to reply to those known hints. This is also the the only way for a client to learn the canonical name that is used when connecting to the default export name. libnbd 1.4 added new APIs to request the client hint, and to track what the server replies with, making this easy enough to test. However, at present, the ondemand plugin is the only one to advertise an alternate name for the default export, so the test will be further enhanced when .default_export is plumbed through the sh plugin. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- docs/nbdkit-plugin.pod | 8 +- tests/Makefile.am | 4 +- server/protocol-handshake-newstyle.c | 60 +++++++++++++- tests/test-export-info.sh | 112 +++++++++++++++++++++++++++ 4 files changed, 175 insertions(+), 9 deletions(-) create mode 100755 tests/test-export-info.sh diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 5962b1b8..30fc0ecf 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -740,9 +740,11 @@ default export C<"">, where the plugin provides a UTF-8 string between 0 and 4096 bytes. If the plugin does not supply this callback, the connection continues with the empty name; if the plugin returns a valid string, nbdkit behaves as if the client had passed that string -instead of an empty name. Similarly, if the plugin does not supply a -C<.list_exports> callback, the result of this callback determines what -export name to advertise to a client requesting an export list. +instead of an empty name, and returns that name to clients that +support it (see the C<NBD_INFO_NAME> response to C<NBD_OPT_GO>). +Similarly, if the plugin does not supply a C<.list_exports> callback, +the result of this callback determines what export name to advertise +to a client requesting an export list. The C<readonly> flag informs the plugin that the server was started with the I<-r> flag on the command line, which is the same value diff --git a/tests/Makefile.am b/tests/Makefile.am index 88b4cb42..d662daf6 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -537,8 +537,8 @@ TESTS += test-eflags.sh EXTRA_DIST += test-eflags.sh # Test export name. -TESTS += test-export-name.sh -EXTRA_DIST += test-export-name.sh +TESTS += test-export-name.sh test-export-info.sh +EXTRA_DIST += test-export-name.sh test-export-info.sh # cdi plugin test. TESTS += test-cdi.sh diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c index 6e5f3822..dd4ca601 100644 --- a/server/protocol-handshake-newstyle.c +++ b/server/protocol-handshake-newstyle.c @@ -156,6 +156,37 @@ send_newstyle_option_reply_info_export (uint32_t option, uint32_t reply, return 0; } +/* Can be used for NBD_INFO_NAME and NBD_INFO_DESCRIPTION. */ +static int +send_newstyle_option_reply_info_str (uint32_t option, uint32_t reply, + uint16_t info, const char *str, + size_t len) +{ + GET_CONN; + struct nbd_fixed_new_option_reply fixed_new_option_reply; + struct nbd_fixed_new_option_reply_info_name_or_desc name; + + if (len == -1) + len = strlen (str); + assert (len <= NBD_MAX_STRING); + + 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 (sizeof info + len); + name.info = htobe16 (info); + + if (conn->send (&fixed_new_option_reply, + sizeof fixed_new_option_reply, SEND_MORE) == -1 || + conn->send (&name, sizeof name, SEND_MORE) == -1 || + conn->send (str, len, 0) == -1) { + nbdkit_error ("write: %s: %m", name_of_nbd_opt (option)); + return -1; + } + + return 0; +} + static int send_newstyle_option_reply_meta_context (uint32_t option, uint32_t reply, uint32_t context_id, @@ -533,16 +564,37 @@ negotiate_handshake_newstyle_options (void) exportsize) == -1) return -1; - /* For now we ignore all other info requests (but we must - * ignore NBD_INFO_EXPORT if it was requested, because we - * replied already above). Therefore this loop doesn't do - * much at the moment. + /* For now we send NBD_INFO_NAME if requested, and ignore all + * other info requests (including NBD_INFO_EXPORT if it was + * requested, because we replied already above). + * XXX NBD_INFO_DESCRIPTION is easy once we add .export_description. */ for (i = 0; i < nrinfos; ++i) { memcpy (&info, &data[4 + exportnamelen + 2 + i*2], 2); info = be16toh (info); switch (info) { case NBD_INFO_EXPORT: /* ignore - reply sent above */ break; + case NBD_INFO_NAME: + { + const char *name = &data[4]; + size_t namelen = exportnamelen; + + if (exportnamelen == 0) { + name = backend_default_export (top, read_only); + if (!name) { + debug ("newstyle negotiation: %s: " + "NBD_INFO_NAME: no name to send", optname); + break; + } + namelen = -1; + } + if (send_newstyle_option_reply_info_str (option, + NBD_REP_INFO, + NBD_INFO_NAME, + name, namelen) == -1) + return -1; + } + break; default: debug ("newstyle negotiation: %s: " "ignoring NBD_INFO_* request %u (%s)", diff --git a/tests/test-export-info.sh b/tests/test-export-info.sh new file mode 100755 index 00000000..f94b69e6 --- /dev/null +++ b/tests/test-export-info.sh @@ -0,0 +1,112 @@ +#!/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 + +requires_plugin sh +requires nbdsh -c 'print(h.set_full_info)' + +export sock=`mktemp -u` +files="$sock export-info.pid" +rm -f $files +cleanup_fn rm -f $files + +# Create an nbdkit sh plugin for checking NBD_INFO replies to NBD_OPT_GO. +# XXX Update when .default_export and .export_description are implemented in sh +start_nbdkit -P export-info.pid -U $sock \ + sh - <<'EOF' +case "$1" in + default_export) echo hello ;; + open) echo "$3" ;; + export_description) echo "$2 world" ;; + get_size) echo 0 ;; + *) exit 2 ;; +esac +EOF + +# Without client request, nothing is advertised +nbdsh -c ' +import os + +h.set_opt_mode(True) +h.connect_unix(os.environ["sock"]) + +h.set_export_name("a") +h.opt_info() +try: + h.get_canonical_export_name() + assert False +except nbd.Error as ex: + pass + +h.set_export_name("") +h.opt_info() +try: + h.get_canonical_export_name() + assert False +except nbd.Error as ex: + pass + +h.opt_go() +try: + h.get_canonical_export_name() + assert False +except nbd.Error as ex: + pass + +h.shutdown() +' + +# With client request, reflect the export name back +nbdsh -c ' +import os + +h.set_opt_mode(True) +h.set_full_info(True) +h.connect_unix(os.environ["sock"]) + +h.set_export_name("a") +h.opt_info() +assert h.get_canonical_export_name() == "a" + +h.set_export_name("") +h.opt_info() +# XXX Adjust once default export works +assert h.get_canonical_export_name() == "" + +h.opt_go() +assert h.get_canonical_export_name() == "" + +h.shutdown() +' -- 2.28.0

Use the recently added nbdkit_strndup_intern to make this possible. Testsuite coverage is added in both tls-fallback as well as a cleanup to test-eval-exports.sh. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- plugins/eval/nbdkit-eval-plugin.pod | 4 +++ plugins/sh/nbdkit-sh-plugin.pod | 20 +++++++++++-- plugins/sh/methods.h | 1 + plugins/eval/eval.c | 2 ++ plugins/sh/methods.c | 45 ++++++++++++++++++++++++++++- plugins/sh/sh.c | 1 + plugins/sh/example.sh | 2 +- tests/test-eval-exports.sh | 42 +++++++++++++++++++-------- tests/test-export-info.sh | 7 ++--- tests/test-tls-fallback.sh | 11 ++++--- 10 files changed, 111 insertions(+), 24 deletions(-) diff --git a/plugins/eval/nbdkit-eval-plugin.pod b/plugins/eval/nbdkit-eval-plugin.pod index 7126c6de..eb3c7f5a 100644 --- a/plugins/eval/nbdkit-eval-plugin.pod +++ b/plugins/eval/nbdkit-eval-plugin.pod @@ -96,6 +96,8 @@ features): =item B<config_complete=>SCRIPT +=item B<default_export=>SCRIPT + =item B<dump_plugin=>SCRIPT =item B<extents=>SCRIPT @@ -110,6 +112,8 @@ features): =item B<list_exports=>SCRIPT +=item B<default_export=>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 f63114d4..f3a6147c 100644 --- a/plugins/sh/nbdkit-sh-plugin.pod +++ b/plugins/sh/nbdkit-sh-plugin.pod @@ -315,8 +315,24 @@ export name or description, although this could be possible under a new mode supporting escape sequences. 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. +advertised by nbdkit will be the single name result of C<default_export> +and no description. + +=item C<default_export> + + /path/to/script default_export <readonly> <tls> + +The C<readonly> and C<tls> parameters will be C<true> or C<false>. + +On success this should print a name on stdout to use in place of the +default export C<"">, then exit with code C<0>. For convenience, the +output can be any of the list forms recognized by C<list_exports>, in +which case the first listed export name is used, and where an empty +list uses C<"">. Given the current set of recognized export lists, it +is not possible for the resulting name to include a newline. + +This method is I<not> required; if it is absent, the default export +name will be the empty string, C<"">. =item C<open> diff --git a/plugins/sh/methods.h b/plugins/sh/methods.h index 69017fa4..3fd4ef42 100644 --- a/plugins/sh/methods.h +++ b/plugins/sh/methods.h @@ -46,6 +46,7 @@ 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 const char *sh_default_export (int readonly, int is_tls); 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 2bd5e79f..a123734e 100644 --- a/plugins/eval/eval.c +++ b/plugins/eval/eval.c @@ -68,6 +68,7 @@ static const char *known_methods[] = { "close", "config", "config_complete", + "default_export", "dump_plugin", "extents", "flush", @@ -395,6 +396,7 @@ static struct nbdkit_plugin plugin = { .preconnect = sh_preconnect, .list_exports = sh_list_exports, + .default_export = sh_default_export, .open = sh_open, .close = sh_close, diff --git a/plugins/sh/methods.c b/plugins/sh/methods.c index 23e4f084..15e0d053 100644 --- a/plugins/sh/methods.c +++ b/plugins/sh/methods.c @@ -240,7 +240,9 @@ parse_exports (const char *script, { const char *n, *d, *p, *q; - /* The first line determines how to parse the rest of s */ + /* The first line determines how to parse the rest of s. Keep + * sh_default_export in sync with this. + */ if ((p = skip_prefix (s, "INTERLEAVED\n")) != NULL) { n = p; while ((d = strchr (n, '\n')) != NULL) { @@ -330,6 +332,47 @@ sh_list_exports (int readonly, int default_only, } } +const char * +sh_default_export (int readonly, int is_tls) +{ + const char *method = "default_export"; + const char *script = get_script (method); + const char *args[] = { script, method, readonly ? "true" : "false", + is_tls ? "true" : "false", NULL }; + CLEANUP_FREE char *s = NULL; + size_t slen; + const char *p, *n; + + switch (call_read (&s, &slen, args)) { + case OK: + /* The first line determines how to parse the rest of s. For now, + * all export modes treat the next line as the first export. + */ + if ((p = skip_prefix (s, "INTERLEAVED\n")) != NULL || + (p = skip_prefix (s, "NAMES+DESCRIPTIONS\n")) != NULL || + (p = skip_prefix (s, "NAMES\n")) != NULL) + ; + else + p = s; + n = strchr (p, '\n') ?: s + slen; + return nbdkit_strndup_intern (p, n - p); + + case MISSING: + return ""; + + case ERROR: + return NULL; + + case RET_FALSE: + nbdkit_error ("%s: %s method returned unexpected code (3/false)", + script, method); + errno = EIO; + return NULL; + + default: abort (); + } +} + void * sh_open (int readonly) { diff --git a/plugins/sh/sh.c b/plugins/sh/sh.c index 374888a4..4fcc2a5a 100644 --- a/plugins/sh/sh.c +++ b/plugins/sh/sh.c @@ -301,6 +301,7 @@ static struct nbdkit_plugin plugin = { .preconnect = sh_preconnect, .list_exports = sh_list_exports, + .default_export = sh_default_export, .open = sh_open, .close = sh_close, diff --git a/plugins/sh/example.sh b/plugins/sh/example.sh index 4f547db0..e0e1b773 100755 --- a/plugins/sh/example.sh +++ b/plugins/sh/example.sh @@ -85,7 +85,7 @@ case "$1" in echo parallel ;; - list_exports) + list_exports | default_export) # 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. diff --git a/tests/test-eval-exports.sh b/tests/test-eval-exports.sh index 7c946378..c56f5ac4 100755 --- a/tests/test-eval-exports.sh +++ b/tests/test-eval-exports.sh @@ -37,8 +37,8 @@ source ./functions.sh set -e set -x -# requires nbdinfo --version # nbdinfo 1.3.9 was broken, so check this instead: -requires nbdkit -U - memory 1 --run 'nbdinfo --size --json "$uri"' +requires nbdinfo --version +requires nbdsh -c 'print(h.set_full_info)' requires jq --version sock=$(mktemp -u) @@ -57,20 +57,19 @@ diff -u <(jq -c \ '[.exports[] | [."export-name", .description, ."export-size"]]' \ eval-exports.out) <(printf %s\\n '[["",null,0]]') -# Start a long-running server with .list_exports set to varying contents +# Start a long-running server with .list_exports and .default_export +# set to varying contents start_nbdkit -P eval-exports.pid -U $sock eval get_size='echo "$2"|wc -c' \ - open='echo "$3"' list_exports="cat '$PWD/eval-exports.list'" + open='echo "$3"' list_exports="cat '$PWD/eval-exports.list'" \ + default_export="cat '$PWD/eval-exports.list'" # do_nbdkit EXPNAME EXPOUT do_nbdkit () { # Check how the default export name is handled - # nbdinfo currently makes multiple connections, so we can't use the - # long-running server for validating default export name. - # XXX FIXME: requires .default_export in eval - : || nbdkit -U - -v eval list_exports="cat '$PWD/eval-exports.list'" \ - open='[ "$3" = "'"$1"'" ] || { echo EINVAL wrong export >&2; exit 1; }' \ - get_size='echo 0' --run 'nbdsh -u "$uri" -c "exit()"' + nbdinfo --no-content nbd+unix://\?socket=$sock >eval-exports.out + diff -u <(sed -n 's/export="\(.*\)":/\1/p' eval-exports.out) \ + <(printf %s\\n "$1") # Check what exports are listed nbdinfo --list --json nbd+unix://\?socket=$sock >eval-exports.out cat eval-exports.out @@ -79,9 +78,28 @@ do_nbdkit () eval-exports.out) <(printf %s\\n "$2") } -# With no file, .list_exports fails, but connecting works +# With no file, .list_exports and .default_export both fail, preventing +# connection to the default export, but not other exports nbdinfo --list --json nbd+unix://\?socket=$sock && fail=1 -nbdsh -u nbd+unix://\?socket=$sock -c 'quit()' +nbdsh -c ' +import os +try: + h.connect_uri("nbd+unix://?socket='"$sock"'") + exit(1) +except nbd.Error: + pass +' || fail=1 +nbdsh -u nbd+unix:///name\?socket=$sock -c 'quit()' + +# Setting .default_export but not .list_exports advertises the canonical name +nbdkit -U - eval default_export='echo hello' get_size='echo 0' \ + --run 'nbdinfo --list "$uri"' >eval-exports.out +diff -u <(grep '^export=' eval-exports.out) <(echo 'export="hello":') + +# Failing .default_export without .list_exports results in an empty list +nbdkit -U - eval default_export='echo ENOENT >&2; exit 1' get_size='echo 0' \ + --run 'nbdinfo --list "$uri"' >eval-exports.out +diff -u <(grep '^export=' eval-exports.out) /dev/null # Various spellings of empty lists, producing 0 exports for fmt in '' 'NAMES\n' 'INTERLEAVED\n' 'NAMES+DESCRIPTIONS\n'; do diff --git a/tests/test-export-info.sh b/tests/test-export-info.sh index f94b69e6..3b61979f 100755 --- a/tests/test-export-info.sh +++ b/tests/test-export-info.sh @@ -43,7 +43,7 @@ rm -f $files cleanup_fn rm -f $files # Create an nbdkit sh plugin for checking NBD_INFO replies to NBD_OPT_GO. -# XXX Update when .default_export and .export_description are implemented in sh +# XXX Update when .export_description is implemented in sh start_nbdkit -P export-info.pid -U $sock \ sh - <<'EOF' case "$1" in @@ -102,11 +102,10 @@ assert h.get_canonical_export_name() == "a" h.set_export_name("") h.opt_info() -# XXX Adjust once default export works -assert h.get_canonical_export_name() == "" +assert h.get_canonical_export_name() == "hello" h.opt_go() -assert h.get_canonical_export_name() == "" +assert h.get_canonical_export_name() == "hello" h.shutdown() ' diff --git a/tests/test-tls-fallback.sh b/tests/test-tls-fallback.sh index fe3b84de..f3c4b14b 100755 --- a/tests/test-tls-fallback.sh +++ b/tests/test-tls-fallback.sh @@ -60,12 +60,15 @@ cleanup_fn rm -f $files # Run dual-mode server start_nbdkit -P $pid -U $sock --tls=on --tls-psk=keys.psk \ --filter=tls-fallback sh - tlsreadme=$'dummy\n' <<\EOF +check () { + if test "$1" != true; then + echo 'EINVAL unexpected tls mode' 2>&1; exit 1 + fi +} case $1 in list_exports) echo NAMES; echo hello; echo world ;; - open) if test "$4" != true; then - echo 'EINVAL unexpected tls mode' 2>&1; exit 1 - fi - echo $3 ;; + default_export) check "$3"; echo hello ;; + open) check "$4"; echo $3 ;; get_size) echo "$2" | wc -c ;; pread) echo "$2" | dd skip=$4 count=$3 iflag=skip_bytes,count_bytes ;; can_write | can_trim) exit 0 ;; -- 2.28.0

I'm about to add an 'exportname' filter, and in the process, I noticed a few shortcomings in our API. Having .default_export makes it easy to answer NBD_INFO_NAME in response to a client request during NBD_OPT_GO, but answering NBD_INFO_DESCRIPTION is awkward - there's no guarantee the export name was given with a description in .list_exports. Note, however, that while we map "" into a canonical name prior to .open (as the plugin is easier to write if it can just assume the client already passed in the canonical name), we don't need a description until after .open, alongside .get_size. Note that in general, we cannot require the description given (or omitted) in .list_exports to match the description returned by .export_description. Whether a client asks for a listing of available exports is orthogonal to whether a client requests a description on a single export. The recent libnbd 1.4 release has added 'nbd_set_full_info()' as the knob for whether a client will request name and description during NBD_OPT_GO; adding this functionality in nbdkit makes it easier to test that API in libnbd. Also note that 'nbdinfo --list' released with libnbd 1.4 does NOT query descriptions during NBD_OPT_INFO (it assumes that the description during NBD_OPT_LIST is sufficient); but later libnbd does as a fallback for when the list description is empty, making it trickier to write a test that passes across libnbd versions. --- docs/nbdkit-filter.pod | 9 ++++ docs/nbdkit-plugin.pod | 20 ++++++++ docs/nbdkit-protocol.pod | 14 ++++++ plugins/eval/nbdkit-eval-plugin.pod | 2 + plugins/sh/nbdkit-sh-plugin.pod | 11 +++++ include/nbdkit-filter.h | 3 ++ include/nbdkit-plugin.h | 1 + server/internal.h | 3 ++ server/backend.c | 22 +++++++++ server/filters.c | 13 +++++ server/plugins.c | 13 +++++ server/protocol-handshake-newstyle.c | 25 ++++++++-- plugins/sh/methods.h | 1 + plugins/cc/cc.c | 59 +++++++++++++---------- plugins/eval/eval.c | 70 ++++++++++++++------------- plugins/sh/methods.c | 32 +++++++++++++ plugins/sh/sh.c | 71 ++++++++++++++-------------- filters/tls-fallback/tls-fallback.c | 42 +++++++++------- tests/test-export-info.sh | 34 ++++++------- tests/test-tls-fallback.sh | 6 +++ TODO | 5 -- 21 files changed, 320 insertions(+), 136 deletions(-) diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod index 916d62dd..652925d6 100644 --- a/docs/nbdkit-filter.pod +++ b/docs/nbdkit-filter.pod @@ -553,6 +553,15 @@ This function is only called once per connection and cached by nbdkit. Similarly, repeated calls to C<next_ops-E<gt>get_size> will return a cached value. +=head2 C<.export_description> + + const char *export_description (struct nbdkit_next_ops *next_ops, + void *nxdata, void *handle); + +This intercepts the plugin C<.export_description> method and can be +used to read or modify the export description that the NBD client +will see. + =head2 C<.can_write> =head2 C<.can_flush> diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index f1476032..62feae47 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -825,6 +825,26 @@ to get the size (in bytes) of the block device being exported. The returned size must be E<ge> 0. If there is an error, C<.get_size> should call C<nbdkit_error> with an error message and return C<-1>. +=head2 C<.export_description> + + const char *export_description (void *handle); + +This is called during the option negotiation phase only if the +client specifically requested an export description (see the +C<NBD_INFO_DESCRIPTION> response to C<NBD_OPT_GO>). Any +description provided must be human-readable UTF-8, no longer +than 4096 bytes. Ideally, this description should match any +description set during C<.list_exports>, but that is not +enforced. + +If the plugin returns C<NULL> or an invalid string (such as longer +than 4096 bytes), or if this callback is omitted, no description is +offered to the client. As this is not an error in the protocol, it is +not necessary to call C<nbdkit_error>. If the callback will not be +returning a compile-time constant string, you may find +C<nbdkit_strdup_intern> helpful for returning a value that avoids a +memory leak. + =head2 C<.can_write> int can_write (void *handle); diff --git a/docs/nbdkit-protocol.pod b/docs/nbdkit-protocol.pod index b923d367..2f46d735 100644 --- a/docs/nbdkit-protocol.pod +++ b/docs/nbdkit-protocol.pod @@ -200,6 +200,20 @@ request is no faster than a counterpart write would be, while normal zero requests still benefit from compressed network traffic regardless of the time taken. +=item C<NBD_INFO_NAME>, C<NBD_INFO_DESCRIPTION> + +Supported in nbdkit E<ge> 1.22.0. + +These protocol extensions allow a client to learn more information +about an export during C<NBD_OPT_GO>. The C<.default_export> callback +can inform a client of a canonical non-empty name in place of the +default export C<"">, and the C<.export_description> callback can give +a client more details about the export. + +=item C<NBD_INFO_BLOCK_SIZE> + +I<Not supported>. + =item Resize Extension I<Not supported>. diff --git a/plugins/eval/nbdkit-eval-plugin.pod b/plugins/eval/nbdkit-eval-plugin.pod index eb3c7f5a..22167ec5 100644 --- a/plugins/eval/nbdkit-eval-plugin.pod +++ b/plugins/eval/nbdkit-eval-plugin.pod @@ -100,6 +100,8 @@ features): =item B<dump_plugin=>SCRIPT +=item B<export_description=>SCRIPT + =item B<extents=>SCRIPT =item B<flush=>SCRIPT diff --git a/plugins/sh/nbdkit-sh-plugin.pod b/plugins/sh/nbdkit-sh-plugin.pod index 17a1f08c..41ceeb1b 100644 --- a/plugins/sh/nbdkit-sh-plugin.pod +++ b/plugins/sh/nbdkit-sh-plugin.pod @@ -350,6 +350,17 @@ handle will be C<""> (empty string). /path/to/script close <handle> +=item C<export_description> + + /path/to/script export_description <handle> + +The script should print a human-readable description of the disk image +on stdout. If the description ends with a newline character then the +newline is removed. + +This method is I<not> required; if it is absent, no export description +will be provided to the client. + =item C<get_size> /path/to/script get_size <handle> diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h index 63458d59..afe83e0b 100644 --- a/include/nbdkit-filter.h +++ b/include/nbdkit-filter.h @@ -80,6 +80,7 @@ struct nbdkit_next_ops { /* The rest of the next ops are the same as normal plugin operations. */ int64_t (*get_size) (nbdkit_backend *nxdata); + const char * (*export_description) (nbdkit_backend *nxdata); int (*can_write) (nbdkit_backend *nxdata); int (*can_flush) (nbdkit_backend *nxdata); @@ -195,6 +196,8 @@ struct nbdkit_filter { int64_t (*get_size) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, void *handle); + const char * (*export_description) (struct nbdkit_next_ops *next_ops, + nbdkit_backend *nxdata, void *handle); int (*can_write) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, void *handle); diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h index 539e3f6e..db010a5a 100644 --- a/include/nbdkit-plugin.h +++ b/include/nbdkit-plugin.h @@ -143,6 +143,7 @@ struct nbdkit_plugin { int (*list_exports) (int readonly, int is_tls, struct nbdkit_exports *exports); const char * (*default_export) (int readonly, int is_tls); + const char * (*export_description) (void *handle); }; NBDKIT_EXTERN_DECL (void, nbdkit_set_error, (int err)); diff --git a/server/internal.h b/server/internal.h index 348d185b..67bd5512 100644 --- a/server/internal.h +++ b/server/internal.h @@ -395,6 +395,7 @@ struct backend { int (*finalize) (struct backend *, void *handle); void (*close) (struct backend *, void *handle); + const char *(*export_description) (struct backend *, void *handle); int64_t (*get_size) (struct backend *, void *handle); int (*can_write) (struct backend *, void *handle); int (*can_flush) (struct backend *, void *handle); @@ -439,6 +440,8 @@ extern int backend_list_exports (struct backend *b, int readonly, __attribute__((__nonnull__ (1, 3))); extern const char *backend_default_export (struct backend *b, int readonly) __attribute__((__nonnull__ (1))); +extern const char *backend_export_description (struct backend *b) + __attribute__((__nonnull__ (1))); /* 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 427a0cec..3630163b 100644 --- a/server/backend.c +++ b/server/backend.c @@ -360,6 +360,28 @@ backend_reopen (struct backend *b, int readonly, const char *exportname) return 0; } +const char * +backend_export_description (struct backend *b) +{ + GET_CONN; + struct handle *h = get_handle (conn, b->i); + const char *s; + + controlpath_debug ("%s: export_description", b->name); + + assert (h->handle && (h->state & HANDLE_CONNECTED)); + /* Caching is not useful for this value. */ + s = b->export_description (b, h->handle); + + /* Ignore over-length strings. XXX Also ignore non-UTF8? */ + if (s && strnlen (s, NBD_MAX_STRING + 1) > NBD_MAX_STRING) { + controlpath_debug ("%s: export_description: ignoring invalid string", + b->name); + s = NULL; + } + return s; +} + int64_t backend_get_size (struct backend *b) { diff --git a/server/filters.c b/server/filters.c index dd4417be..54abf9a4 100644 --- a/server/filters.c +++ b/server/filters.c @@ -291,6 +291,7 @@ filter_close (struct backend *b, void *handle) static struct nbdkit_next_ops next_ops = { .reopen = backend_reopen, + .export_description = backend_export_description, .get_size = backend_get_size, .can_write = backend_can_write, .can_flush = backend_can_flush, @@ -334,6 +335,17 @@ filter_finalize (struct backend *b, void *handle) return 0; } +static const char * +filter_export_description (struct backend *b, void *handle) +{ + struct backend_filter *f = container_of (b, struct backend_filter, backend); + + if (f->filter.export_description) + return f->filter.export_description (&next_ops, b->next, handle); + else + return backend_export_description (b->next); +} + static int64_t filter_get_size (struct backend *b, void *handle) { @@ -571,6 +583,7 @@ static struct backend filter_functions = { .prepare = filter_prepare, .finalize = filter_finalize, .close = filter_close, + .export_description = filter_export_description, .get_size = filter_get_size, .can_write = filter_can_write, .can_flush = filter_can_flush, diff --git a/server/plugins.c b/server/plugins.c index 69835d32..010595c7 100644 --- a/server/plugins.c +++ b/server/plugins.c @@ -170,6 +170,7 @@ plugin_dump_fields (struct backend *b) HAS (open); HAS (close); + HAS (export_description); HAS (get_size); HAS (can_write); HAS (can_flush); @@ -369,6 +370,17 @@ plugin_close (struct backend *b, void *handle) conn->exportname = NULL; } +static const char * +plugin_export_description (struct backend *b, void *handle) +{ + struct backend_plugin *p = container_of (b, struct backend_plugin, backend); + + if (p->plugin.export_description) + return p->plugin.export_description (handle); + else + return NULL; +} + static int64_t plugin_get_size (struct backend *b, void *handle) { @@ -775,6 +787,7 @@ static struct backend plugin_functions = { .prepare = plugin_prepare, .finalize = plugin_finalize, .close = plugin_close, + .export_description = plugin_export_description, .get_size = plugin_get_size, .can_write = plugin_can_write, .can_flush = plugin_can_flush, diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c index 7fc97507..9c86639b 100644 --- a/server/protocol-handshake-newstyle.c +++ b/server/protocol-handshake-newstyle.c @@ -564,10 +564,10 @@ negotiate_handshake_newstyle_options (void) exportsize) == -1) return -1; - /* For now we send NBD_INFO_NAME if requested, and ignore all - * other info requests (including NBD_INFO_EXPORT if it was - * requested, because we replied already above). - * XXX NBD_INFO_DESCRIPTION is easy once we add .export_description. + /* For now we send NBD_INFO_NAME and NBD_INFO_DESCRIPTION if + * requested, and ignore all other info requests (including + * NBD_INFO_EXPORT if it was requested, because we replied + * already above). */ for (i = 0; i < nrinfos; ++i) { memcpy (&info, &data[4 + exportnamelen + 2 + i*2], 2); @@ -595,6 +595,23 @@ negotiate_handshake_newstyle_options (void) return -1; } break; + case NBD_INFO_DESCRIPTION: + { + const char *desc = backend_export_description (top); + + if (!desc) { + debug ("newstyle negotiation: %s: " + "NBD_INFO_DESCRIPTION: no description to send", + optname); + break; + } + if (send_newstyle_option_reply_info_str (option, + NBD_REP_INFO, + NBD_INFO_DESCRIPTION, + desc, -1) == -1) + return -1; + } + break; default: debug ("newstyle negotiation: %s: " "ignoring NBD_INFO_* request %u (%s)", diff --git a/plugins/sh/methods.h b/plugins/sh/methods.h index 3fd4ef42..42eb560c 100644 --- a/plugins/sh/methods.h +++ b/plugins/sh/methods.h @@ -49,6 +49,7 @@ extern int sh_list_exports (int readonly, int default_only, extern const char *sh_default_export (int readonly, int is_tls); extern void *sh_open (int readonly); extern void sh_close (void *handle); +extern const char *sh_export_description (void *handle); extern int64_t sh_get_size (void *handle); extern int sh_pread (void *handle, void *buf, uint32_t count, uint64_t offset, uint32_t flags); diff --git a/plugins/cc/cc.c b/plugins/cc/cc.c index 7d529b20..3251f312 100644 --- a/plugins/cc/cc.c +++ b/plugins/cc/cc.c @@ -377,6 +377,14 @@ cc_close (void *handle) subplugin.close (handle); } +static const char * +cc_export_description (void *handle) +{ + if (subplugin.export_description) + return subplugin.export_description (handle); + return NULL; +} + static int64_t cc_get_size (void *handle) { @@ -575,34 +583,35 @@ static struct nbdkit_plugin plugin = { /* And we must provide callbacks for everything else, which are * passed through to the subplugin. */ - .get_ready = cc_get_ready, - .after_fork = cc_after_fork, + .get_ready = cc_get_ready, + .after_fork = cc_after_fork, - .preconnect = cc_preconnect, - .list_exports = cc_list_exports, - .default_export = cc_default_export, - .open = cc_open, - .close = cc_close, + .preconnect = cc_preconnect, + .list_exports = cc_list_exports, + .default_export = cc_default_export, + .open = cc_open, + .close = cc_close, - .get_size = cc_get_size, - .can_write = cc_can_write, - .can_flush = cc_can_flush, - .is_rotational = cc_is_rotational, - .can_trim = cc_can_trim, - .can_zero = cc_can_zero, - .can_fast_zero = cc_can_fast_zero, - .can_extents = cc_can_extents, - .can_fua = cc_can_fua, - .can_multi_conn = cc_can_multi_conn, - .can_cache = cc_can_cache, + .export_description = cc_export_description, + .get_size = cc_get_size, + .can_write = cc_can_write, + .can_flush = cc_can_flush, + .is_rotational = cc_is_rotational, + .can_trim = cc_can_trim, + .can_zero = cc_can_zero, + .can_fast_zero = cc_can_fast_zero, + .can_extents = cc_can_extents, + .can_fua = cc_can_fua, + .can_multi_conn = cc_can_multi_conn, + .can_cache = cc_can_cache, - .pread = cc_pread, - .pwrite = cc_pwrite, - .flush = cc_flush, - .trim = cc_trim, - .zero = cc_zero, - .extents = cc_extents, - .cache = cc_cache, + .pread = cc_pread, + .pwrite = cc_pwrite, + .flush = cc_flush, + .trim = cc_trim, + .zero = cc_zero, + .extents = cc_extents, + .cache = cc_cache, .errno_is_preserved = 1, }; diff --git a/plugins/eval/eval.c b/plugins/eval/eval.c index a123734e..ae84bd40 100644 --- a/plugins/eval/eval.c +++ b/plugins/eval/eval.c @@ -70,6 +70,7 @@ static const char *known_methods[] = { "config_complete", "default_export", "dump_plugin", + "export_description", "extents", "flush", "get_ready", @@ -380,45 +381,46 @@ eval_config_complete (void) #define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL static struct nbdkit_plugin plugin = { - .name = "eval", - .version = PACKAGE_VERSION, - .load = eval_load, - .unload = eval_unload, + .name = "eval", + .version = PACKAGE_VERSION, + .load = eval_load, + .unload = eval_unload, - .dump_plugin = sh_dump_plugin, + .dump_plugin = sh_dump_plugin, - .config = eval_config, - .config_complete = eval_config_complete, - .config_help = eval_config_help, - .thread_model = sh_thread_model, - .get_ready = sh_get_ready, - .after_fork = sh_after_fork, + .config = eval_config, + .config_complete = eval_config_complete, + .config_help = eval_config_help, + .thread_model = sh_thread_model, + .get_ready = sh_get_ready, + .after_fork = sh_after_fork, - .preconnect = sh_preconnect, - .list_exports = sh_list_exports, - .default_export = sh_default_export, - .open = sh_open, - .close = sh_close, + .preconnect = sh_preconnect, + .list_exports = sh_list_exports, + .default_export = sh_default_export, + .open = sh_open, + .close = sh_close, - .get_size = sh_get_size, - .can_write = sh_can_write, - .can_flush = sh_can_flush, - .is_rotational = sh_is_rotational, - .can_trim = sh_can_trim, - .can_zero = sh_can_zero, - .can_extents = sh_can_extents, - .can_fua = sh_can_fua, - .can_multi_conn = sh_can_multi_conn, - .can_cache = sh_can_cache, - .can_fast_zero = sh_can_fast_zero, + .export_description = sh_export_description, + .get_size = sh_get_size, + .can_write = sh_can_write, + .can_flush = sh_can_flush, + .is_rotational = sh_is_rotational, + .can_trim = sh_can_trim, + .can_zero = sh_can_zero, + .can_extents = sh_can_extents, + .can_fua = sh_can_fua, + .can_multi_conn = sh_can_multi_conn, + .can_cache = sh_can_cache, + .can_fast_zero = sh_can_fast_zero, - .pread = sh_pread, - .pwrite = sh_pwrite, - .flush = sh_flush, - .trim = sh_trim, - .zero = sh_zero, - .extents = sh_extents, - .cache = sh_cache, + .pread = sh_pread, + .pwrite = sh_pwrite, + .flush = sh_flush, + .trim = sh_trim, + .zero = sh_zero, + .extents = sh_extents, + .cache = sh_cache, .errno_is_preserved = 1, }; diff --git a/plugins/sh/methods.c b/plugins/sh/methods.c index 12f498cd..dacb805d 100644 --- a/plugins/sh/methods.c +++ b/plugins/sh/methods.c @@ -456,6 +456,38 @@ sh_close (void *handle) } } +const char * +sh_export_description (void *handle) +{ + const char *method = "export_description"; + const char *script = get_script (method); + struct sh_handle *h = handle; + const char *args[] = { script, method, h->h, NULL }; + CLEANUP_FREE char *s = NULL; + size_t slen; + + switch (call_read (&s, &slen, args)) { + case OK: + if (slen > 0 && s[slen-1] == '\n') + s[slen-1] = '\0'; + return nbdkit_strdup_intern (s); + + case MISSING: + return NULL; + + case ERROR: + return NULL; + + case RET_FALSE: + nbdkit_error ("%s: %s method returned unexpected code (3/false)", + script, method); + errno = EIO; + return NULL; + + default: abort (); + } +} + int64_t sh_get_size (void *handle) { diff --git a/plugins/sh/sh.c b/plugins/sh/sh.c index 4fcc2a5a..db75d386 100644 --- a/plugins/sh/sh.c +++ b/plugins/sh/sh.c @@ -284,46 +284,47 @@ sh_config_complete (void) #define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL static struct nbdkit_plugin plugin = { - .name = "sh", - .version = PACKAGE_VERSION, - .load = sh_load, - .unload = sh_unload, + .name = "sh", + .version = PACKAGE_VERSION, + .load = sh_load, + .unload = sh_unload, - .dump_plugin = sh_dump_plugin, + .dump_plugin = sh_dump_plugin, - .config = sh_config, - .config_complete = sh_config_complete, - .config_help = sh_config_help, - .magic_config_key = "script", - .thread_model = sh_thread_model, - .get_ready = sh_get_ready, - .after_fork = sh_after_fork, + .config = sh_config, + .config_complete = sh_config_complete, + .config_help = sh_config_help, + .magic_config_key = "script", + .thread_model = sh_thread_model, + .get_ready = sh_get_ready, + .after_fork = sh_after_fork, - .preconnect = sh_preconnect, - .list_exports = sh_list_exports, - .default_export = sh_default_export, - .open = sh_open, - .close = sh_close, + .preconnect = sh_preconnect, + .list_exports = sh_list_exports, + .default_export = sh_default_export, + .open = sh_open, + .close = sh_close, - .get_size = sh_get_size, - .can_write = sh_can_write, - .can_flush = sh_can_flush, - .is_rotational = sh_is_rotational, - .can_trim = sh_can_trim, - .can_zero = sh_can_zero, - .can_extents = sh_can_extents, - .can_fua = sh_can_fua, - .can_multi_conn = sh_can_multi_conn, - .can_cache = sh_can_cache, - .can_fast_zero = sh_can_fast_zero, + .export_description = sh_export_description, + .get_size = sh_get_size, + .can_write = sh_can_write, + .can_flush = sh_can_flush, + .is_rotational = sh_is_rotational, + .can_trim = sh_can_trim, + .can_zero = sh_can_zero, + .can_extents = sh_can_extents, + .can_fua = sh_can_fua, + .can_multi_conn = sh_can_multi_conn, + .can_cache = sh_can_cache, + .can_fast_zero = sh_can_fast_zero, - .pread = sh_pread, - .pwrite = sh_pwrite, - .flush = sh_flush, - .trim = sh_trim, - .zero = sh_zero, - .extents = sh_extents, - .cache = sh_cache, + .pread = sh_pread, + .pwrite = sh_pwrite, + .flush = sh_flush, + .trim = sh_trim, + .zero = sh_zero, + .extents = sh_extents, + .cache = sh_cache, .errno_is_preserved = 1, }; diff --git a/filters/tls-fallback/tls-fallback.c b/filters/tls-fallback/tls-fallback.c index c1e549df..bec983be 100644 --- a/filters/tls-fallback/tls-fallback.c +++ b/filters/tls-fallback/tls-fallback.c @@ -119,6 +119,15 @@ tls_fallback_open (nbdkit_next_open *next, void *nxdata, int readonly, * can_zero, can_fast_zero, and can_fua). */ +static const char * +tls_fallback_export_description (struct nbdkit_next_ops *next_ops, + void *nxdata, void *handle) +{ + if (NOT_TLS) + return NULL; + return next_ops->export_description (nxdata); +} + static int64_t tls_fallback_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) @@ -195,22 +204,23 @@ tls_fallback_pread (struct nbdkit_next_ops *next_ops, void *nxdata, } static struct nbdkit_filter filter = { - .name = "tls-fallback", - .longname = "nbdkit tls-fallback filter", - .config = tls_fallback_config, - .config_help = tls_fallback_config_help, - .get_ready = tls_fallback_get_ready, - .list_exports = tls_fallback_list_exports, - .default_export = tls_fallback_default_export, - .open = tls_fallback_open, - .get_size = tls_fallback_get_size, - .can_write = tls_fallback_can_write, - .can_flush = tls_fallback_can_flush, - .is_rotational = tls_fallback_is_rotational, - .can_extents = tls_fallback_can_extents, - .can_multi_conn = tls_fallback_can_multi_conn, - .can_cache = tls_fallback_can_cache, - .pread = tls_fallback_pread, + .name = "tls-fallback", + .longname = "nbdkit tls-fallback filter", + .config = tls_fallback_config, + .config_help = tls_fallback_config_help, + .get_ready = tls_fallback_get_ready, + .list_exports = tls_fallback_list_exports, + .default_export = tls_fallback_default_export, + .open = tls_fallback_open, + .export_description = tls_fallback_export_description, + .get_size = tls_fallback_get_size, + .can_write = tls_fallback_can_write, + .can_flush = tls_fallback_can_flush, + .is_rotational = tls_fallback_is_rotational, + .can_extents = tls_fallback_can_extents, + .can_multi_conn = tls_fallback_can_multi_conn, + .can_cache = tls_fallback_can_cache, + .pread = tls_fallback_pread, }; NBDKIT_REGISTER_FILTER(filter) diff --git a/tests/test-export-info.sh b/tests/test-export-info.sh index 3b61979f..c7a48f6e 100755 --- a/tests/test-export-info.sh +++ b/tests/test-export-info.sh @@ -43,7 +43,6 @@ rm -f $files cleanup_fn rm -f $files # Create an nbdkit sh plugin for checking NBD_INFO replies to NBD_OPT_GO. -# XXX Update when .export_description is implemented in sh start_nbdkit -P export-info.pid -U $sock \ sh - <<'EOF' case "$1" in @@ -59,36 +58,34 @@ EOF nbdsh -c ' import os +def must_fail(f, *args, **kwds): + try: + f(*args, **kwds) + assert False + except nbd.Error: + pass + h.set_opt_mode(True) h.connect_unix(os.environ["sock"]) h.set_export_name("a") h.opt_info() -try: - h.get_canonical_export_name() - assert False -except nbd.Error as ex: - pass +must_fail(h.get_canonical_export_name) +must_fail(h.get_export_description) h.set_export_name("") h.opt_info() -try: - h.get_canonical_export_name() - assert False -except nbd.Error as ex: - pass +must_fail(h.get_canonical_export_name) +must_fail(h.get_export_description) h.opt_go() -try: - h.get_canonical_export_name() - assert False -except nbd.Error as ex: - pass +must_fail(h.get_canonical_export_name) +must_fail(h.get_export_description) h.shutdown() ' -# With client request, reflect the export name back +# With client request, reflect the export name and description back nbdsh -c ' import os @@ -99,13 +96,16 @@ h.connect_unix(os.environ["sock"]) h.set_export_name("a") h.opt_info() assert h.get_canonical_export_name() == "a" +assert h.get_export_description() == "a world" h.set_export_name("") h.opt_info() assert h.get_canonical_export_name() == "hello" +assert h.get_export_description() == "hello world" h.opt_go() assert h.get_canonical_export_name() == "hello" +assert h.get_export_description() == "hello world" h.shutdown() ' diff --git a/tests/test-tls-fallback.sh b/tests/test-tls-fallback.sh index 86a495bf..1972406a 100755 --- a/tests/test-tls-fallback.sh +++ b/tests/test-tls-fallback.sh @@ -93,6 +93,11 @@ h.connect_unix(os.environ["sock"]) assert h.opt_list(f) == 1 h.opt_info() assert h.get_canonical_export_name() == "" +try: + h.get_export_description() + assert False +except nbd.Error: + pass h.set_export_name("hello") h.opt_go() assert h.get_size() == 512 @@ -121,6 +126,7 @@ h.connect_unix(os.environ["sock"]) assert h.opt_list(f) == 2 h.opt_info() assert h.get_canonical_export_name() == "hello" +assert h.get_export_description() == "=hello=" h.opt_go() assert h.get_size() == 6 assert h.can_trim() diff --git a/TODO b/TODO index c7f2e3a6..c10b2e47 100644 --- a/TODO +++ b/TODO @@ -71,11 +71,6 @@ General ideas for improvements continue to keep their non-standard handshake while utilizing nbdkit to prototype new behaviors in serving the kernel. -* At present, we ignore NBD_INFO_DESCRIPTION requests from a client - during NBD_OPT_GO. This may be easiest if we add a new - .get_description callback, called after .open alongside .get_size - and .can_write. - * Background thread for filters. Some filters (readahead, cache and proposed scan filter - see below) could be more effective if they were able to defer work to a background thread. However it's not as -- 2.28.0

Add a new filter to make it easier to add exports to a plugin that does advertise them, to avoid advertising where a plugin's list might be an information leak, and to alter which export name is used in place of "". I would love to be able to have a strict mode enforcing that .open is called only with an export name that the plugin was willing to advertise, but for that, we'll need a way to call the plugin's .list_exports from within the context of .open (since we can't guarantee the guest will call NBD_OPT_LIST). So for now, strict mode requires redundant exportname= command line parameters. It may also be worth adding a way to pass exportname-file=/path/to/file, which then gets parsed the same was as the sh plugin (right now with NAMES, INTERLEAVED, or NAMES+DESCRIPTIONS), to avoid having to do so many export names directly on the command line. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- .../exportname/nbdkit-exportname-filter.pod | 154 ++++++++ filters/ext2/nbdkit-ext2-filter.pod | 5 + plugins/file/nbdkit-file-plugin.pod | 9 +- configure.ac | 2 + filters/exportname/Makefile.am | 67 ++++ tests/Makefile.am | 4 + filters/exportname/exportname.c | 342 ++++++++++++++++++ tests/test-exportname.sh | 193 ++++++++++ TODO | 9 + 9 files changed, 782 insertions(+), 3 deletions(-) create mode 100644 filters/exportname/nbdkit-exportname-filter.pod create mode 100644 filters/exportname/Makefile.am create mode 100644 filters/exportname/exportname.c create mode 100755 tests/test-exportname.sh diff --git a/filters/exportname/nbdkit-exportname-filter.pod b/filters/exportname/nbdkit-exportname-filter.pod new file mode 100644 index 00000000..4de56dee --- /dev/null +++ b/filters/exportname/nbdkit-exportname-filter.pod @@ -0,0 +1,154 @@ +=head1 NAME + +nbdkit-exportname-filter - adjust export names between client and plugin + +=head1 SYNOPSIS + + nbdkit --filter=exportname plugin [default-export=NAME] + [exportname-list=MODE] [exportname-strict=true] [exportname=NAME]... + [exportdesc=DESC] + +=head1 DESCRIPTION + +Some plugins (such as C<nbdkit-file-plugin(1)> and filters (such as +C<nbdkit-ext2-filter(1)> are able to serve different content based on +the export name requested by the client. The NBD protocol allows a +server to advertise the set of export names it is serving. However, +the list advertised (or absent) from the plugin may not always match +what you want an actual client to see. This filter can be used to +alter the advertised list, as well as configuring which export should +be treated as the default when the client requests the empty string +(C<"">) as an export name. + +=head1 PARAMETERS + +=over 4 + +=item B<default-export=>NAME + +When the client requests the default export name (C<"">), request the +export C<NAME> from the underlying plugin instead of relying on the +plugin's choice of default export. Setting NAME to the empty string +has the same effect as omitting this parameter. + +=item B<exportname-list=keep> + +=item B<exportname-list=error> + +=item B<exportname-list=empty> + +=item B<exportname-list=defaultonly> + +=item B<exportname-list=explicit> + +This parameter determines which exports are advertised to a guest that +requests a listing via C<NBD_OPT_LIST>. The default mode is C<keep> +to advertise whatever the underlying plugin reports. Mode C<error> +causes clients to see an error rather than an export list. Mode +C<empty> returns an empty list. Mode C<defaultonly> returns a list +that contains only the canonical name of the default export. Mode +C<explicit> returns only the exports set by C<exportname=>. Note that +the list of advertised exports need not reflect reality: an advertised +name may be rejected, or a client may connect to an export name that +was not advertised, but learned through other means. + +=item B<exportname-strict=false> + +=item B<exportname-strict=true> + +Normally, a client can pass whatever export name it wants, regardless +of whether that name is advertised. But setting this parameter to +true will cause the connection to fail if a client requests an export +name that was not included via an B<exportname=> parameter. At this +time, it is not possible to restrict a client to exports advertised by +the plugin without repeating that list via B<exportname>; this +technical limitation may be lifted in the future. + +=item B<exportname=>NAME + +This parameter adds C<NAME> to the list of advertised exports; it may +be set multiple times. + +=item B<exportdesc=keep> + +=item B<exportdesc=none> + +=item B<exportdesc=fixed:>STRING + +=item B<exportdesc=script:>SCRIPT + +The C<exportdesc> parameter controls what optional descriptions are +sent alongside an export name. If set to C<keep> (the default), +descriptions are determined by the plugin. If set to C<none>, +descriptions from the plugin are ignored (useful if you are worried +about a potential information leak). If set to C<fixed:STRING>, the +same fixed string description is offered for every export. If set to +C<script:SCRIPT>, this filter executes script with C<$name> set to the +export to be described, and uses the output of that command as the +description. + +=back + +=head1 EXAMPLES + +Suppose that the directory /path/to/dir contains permanent files named +file1, file2, and file3. The following commands show various ways to +alter the use of export names while serving that directory: + +Allow a client requesting C<""> to get the contents of file2, rather +than an error: + + nbdkit --filter=exportname file dir=/path/to/dir default-export=file2 + +Do not advertise any exports; a client must know in advance what +export names to try: + + nbdkit --filter=exportname file dir=/path/to/dir exportname-list=empty + +Allow clients to connect to file1 and file3, but not file2: + + nbdkit --filter=exportname file dir=/path/to/dir \ + exportname-list=explicit exportname-strict=true \ + exportname=file1 exportname=file3 + +Offer C<ls(3)> long descriptions alongside each export name: + + nbdkit --filter=exportname file dir=/path/to/dir \ + exportdesc=script:'ls -l /path/to/dir/"$name"' + +=head1 FILES + +=over 4 + +=item F<$filterdir/nbdkit-exportname-filter.so> + +The filter. + +Use C<nbdkit --dump-config> to find the location of C<$filterdir>. + +=back + +=head1 VERSION + +C<nbdkit-exportname-filter> first appeared in nbdkit 1.22. + +=head1 SEE ALSO + +L<nbdkit(1)>, +L<nbdkit-filter(3)>, +L<nbdkit-ext2-filter(1)>, +L<nbdkit-extentlist-filter(1)>, +L<nbdkit-fua-filter(1)>, +L<nbdkit-nocache-filter(1)>, +L<nbdkit-noparallel-filter(1)>, +L<nbdkit-nozero-filter(1)>, +L<nbdkit-file-plugin(1)>, +L<nbdkit-info-plugin(1)>. + +=head1 AUTHORS + +Eric Blake + +=head1 COPYRIGHT + +Copyright (C) 2020 Red Hat Inc. diff --git a/filters/ext2/nbdkit-ext2-filter.pod b/filters/ext2/nbdkit-ext2-filter.pod index 78d27f8f..1aef9c2e 100644 --- a/filters/ext2/nbdkit-ext2-filter.pod +++ b/filters/ext2/nbdkit-ext2-filter.pod @@ -65,6 +65,10 @@ exportname passed by the client. Note that this mode allows the client to deduce which files exist within the disk image, which may be a security risk in some situations. +At present, when using this mode, the server does not advertise any +particular exports; however, you may use +L<nbdkit-exportname-filter(1)> to perform that task. + =back =head1 FILES @@ -89,6 +93,7 @@ and removed in nbdkit 1.22. L<nbdkit(1)>, L<nbdkit-plugin(3)>, +L<nbdkit-exportname-filter(1)>, L<nbdkit-partition-filter(1)>, L<nbdkit-guestfs-plugin(1)>, L<http://e2fsprogs.sourceforge.net/>;, diff --git a/plugins/file/nbdkit-file-plugin.pod b/plugins/file/nbdkit-file-plugin.pod index fd56300c..22a6cbc2 100644 --- a/plugins/file/nbdkit-file-plugin.pod +++ b/plugins/file/nbdkit-file-plugin.pod @@ -82,10 +82,12 @@ directory named C<DIRNAME>, including those found by following symbolic links. Other special files in the directory (such as subdirectories, fifos, or Unix sockets) are ignored. When this mode is used, the file to be served is chosen by the export name passed by -the client. For now, the client must already know what files are -in the directory, although there are plans to expose them via +the client. For now, the client must already know what files are in +the directory, although there are plans to expose them via NBD_OPT_LIST; likewise, a client that requests the default export -(C<"">) will be rejected. For security, when using directory mode, +(C<"">) will be rejected. However, you can use +L<nbdkit-exportname-filter(1)> to adjust what export names the client +sees or uses as a default. For security, when using directory mode, this plugin will not accept export names containing slash (C</>). =back @@ -213,6 +215,7 @@ L<nbdkit-plugin(3)>, L<nbdkit-split-plugin(1)>, L<nbdkit-partitioning-plugin(1)>, L<nbdkit-tmpdisk-plugin(1)>, +L<nbdkit-exportname-filter(1)>, L<nbdkit-fua-filter(1)>, L<nbdkit-noextents-filter(1)>. diff --git a/configure.ac b/configure.ac index 56fc77fb..d950c5f0 100644 --- a/configure.ac +++ b/configure.ac @@ -104,6 +104,7 @@ filters="\ delay \ error \ exitlast \ + exportname \ ext2 \ extentlist \ fua \ @@ -1224,6 +1225,7 @@ AC_CONFIG_FILES([Makefile filters/delay/Makefile filters/error/Makefile filters/exitlast/Makefile + filters/exportname/Makefile filters/ext2/Makefile filters/extentlist/Makefile filters/fua/Makefile diff --git a/filters/exportname/Makefile.am b/filters/exportname/Makefile.am new file mode 100644 index 00000000..1bf54518 --- /dev/null +++ b/filters/exportname/Makefile.am @@ -0,0 +1,67 @@ +# 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. + +include $(top_srcdir)/common-rules.mk + +EXTRA_DIST = nbdkit-exportname-filter.pod + +filter_LTLIBRARIES = nbdkit-exportname-filter.la + +nbdkit_exportname_filter_la_SOURCES = \ + exportname.c \ + $(top_srcdir)/include/nbdkit-filter.h \ + $(NULL) + +nbdkit_exportname_filter_la_CPPFLAGS = \ + -I$(top_srcdir)/include \ + -I$(top_srcdir)/common/include \ + -I$(top_srcdir)/common/utils \ + $(NULL) +nbdkit_exportname_filter_la_CFLAGS = $(WARNINGS_CFLAGS) +nbdkit_exportname_filter_la_LDFLAGS = \ + -module -avoid-version -shared $(SHARED_LDFLAGS) \ + -Wl,--version-script=$(top_srcdir)/filters/filters.syms \ + $(NULL) +nbdkit_exportname_filter_la_LIBADD = \ + $(top_builddir)/common/utils/libutils.la \ + $(NULL) + +if HAVE_POD + +man_MANS = nbdkit-exportname-filter.1 +CLEANFILES += $(man_MANS) + +nbdkit-exportname-filter.1: nbdkit-exportname-filter.pod + $(PODWRAPPER) --section=1 --man $@ \ + --html $(top_builddir)/html/$@.html \ + $< + +endif HAVE_POD diff --git a/tests/Makefile.am b/tests/Makefile.am index d662daf6..6bb39de9 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1375,6 +1375,10 @@ EXTRA_DIST += \ TESTS += test-exitlast.sh EXTRA_DIST += test-exitlast.sh +# exportname filter test. +TESTS += test-exportname.sh +EXTRA_DIST += test-exportname.sh + # ext2 filter test. if HAVE_MKE2FS_WITH_D if HAVE_EXT2 diff --git a/filters/exportname/exportname.c b/filters/exportname/exportname.c new file mode 100644 index 00000000..164641ba --- /dev/null +++ b/filters/exportname/exportname.c @@ -0,0 +1,342 @@ +/* nbdkit + * Copyright (C) 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. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdbool.h> +#include <string.h> + +#include <nbdkit-filter.h> + +#include "cleanup.h" +#include "utils.h" + +static const char *default_export; +static enum { + LIST_KEEP, + LIST_ERROR, + LIST_EMPTY, + LIST_DEFAULT, + LIST_EXPLICIT, +} list; +static bool strict; +static enum { + DESC_KEEP, + DESC_NONE, + DESC_FIXED, + DESC_SCRIPT, +} desc_mode; +static const char *desc; +struct nbdkit_exports *exports; + +static void +exportname_load (void) +{ + exports = nbdkit_exports_new (); + if (!exports) { + nbdkit_error ("malloc: %m"); + exit (EXIT_FAILURE); + } +} + +static void +exportname_unload (void) +{ + nbdkit_exports_free (exports); +} + +/* Called for each key=value passed on the command line. */ +static int +exportname_config (nbdkit_next_config *next, void *nxdata, + const char *key, const char *value) +{ + int r; + + if (strcmp (key, "default-export") == 0 || + strcmp (key, "default_export") == 0) { + default_export = value; + return 0; + } + if (strcmp (key, "exportname-list") == 0 || + strcmp (key, "exportname_list") == 0) { + if (strcmp (value, "keep") == 0) + list = LIST_KEEP; + else if (strcmp (value, "error") == 0) + list = LIST_ERROR; + else if (strcmp (value, "empty") == 0) + list = LIST_EMPTY; + else if (strcmp (value, "defaultonly") == 0 || + strcmp (value, "default-only") == 0) + list = LIST_DEFAULT; + else if (strcmp (value, "explicit") == 0) + list = LIST_EXPLICIT; + else { + nbdkit_error ("unrecognized exportname-list mode: %s", value); + return -1; + } + return 0; + } + if (strcmp (key, "exportname-strict") == 0 || + strcmp (key, "exportname_strict") == 0) { + r = nbdkit_parse_bool (value); + if (r == -1) + return -1; + strict = r; + return 0; + } + if (strcmp (key, "exportname") == 0) + return nbdkit_add_export (exports, value, NULL); + if (strcmp (key, "exportdesc") == 0) { + if (strcmp (value, "keep") == 0) + desc_mode = DESC_KEEP; + else if (strcmp (value, "none") == 0) { + desc_mode = DESC_NONE; + desc = NULL; + } + else if (strncmp (value, "fixed:", 6) == 0) { + desc_mode = DESC_FIXED; + desc = value + 6; + } + else if (strncmp (value, "script:", 7) == 0) { + desc_mode = DESC_SCRIPT; + desc = value + 7; + } + else { + nbdkit_error ("unrecognized exportdesc mode: %s", value); + return -1; + } + return 0; + } + return next (nxdata, key, value); +} + +#define exportname_config_help \ + "default-export=<NAME> Canonical name for the \"\" default export.\n" \ + "exportname-list=<MODE> Which exports to advertise: keep (default), error,\n" \ + " empty, defaultonly, explicit.\n" \ + "exportname-strict=<BOOL> Limit clients to explicit exports (default false).\n" \ + "exportname=<NAME> Add an explicit export name, may be repeated.\n" \ + "exportdesc=<MODE> Set descriptions according to mode: keep (default),\n" \ + " none, fixed:STRING, script:SCRIPT.\n" \ + +static const char * +get_desc (const char *name, const char *def) +{ + FILE *fp; + CLEANUP_FREE char *cmd = NULL; + size_t cmdlen = 0; + char buf[4096]; /* Maximum NBD string; we truncate any longer response */ + size_t r; + + switch (desc_mode) { + case DESC_KEEP: + return def; + case DESC_NONE: + case DESC_FIXED: + return desc; + case DESC_SCRIPT: + break; + default: + abort (); + } + + /* Construct the command. */ + fp = open_memstream (&cmd, &cmdlen); + if (fp == NULL) { + nbdkit_debug ("open_memstream: %m"); + return NULL; + } + fprintf (fp, "export name; name="); + shell_quote (name, fp); + fprintf (fp, "\n%s\n", desc); + if (fclose (fp) == EOF) { + nbdkit_debug ("memstream failed: %m"); + return NULL; + } + nbdkit_debug ("%s", cmd); + fp = popen (cmd, "r"); + if (fp == NULL) { + nbdkit_debug ("popen: %m"); + return NULL; + } + + /* Now read the description */ + r = fread (buf, 1, sizeof buf, fp); + if (r == 0 && ferror (fp)) { + nbdkit_debug ("fread: %m"); + pclose (fp); + return NULL; + } + pclose (fp); + if (r && buf[r-1] == '\n') + r--; + return nbdkit_strndup_intern (buf, r); +} + +static int +exportname_list_exports (nbdkit_next_list_exports *next, void *nxdata, + int readonly, int is_tls, + struct nbdkit_exports *exps) +{ + size_t i; + struct nbdkit_exports *source; + CLEANUP_EXPORTS_FREE struct nbdkit_exports *exps2 = NULL; + + switch (list) { + case LIST_KEEP: + source = exps2 = nbdkit_exports_new (); + if (exps2 == NULL) + return -1; + if (next (nxdata, readonly, exps2) == -1) + return -1; + break; + case LIST_ERROR: + nbdkit_error ("export list restricted by policy"); + return -1; + case LIST_EMPTY: + return 0; + case LIST_DEFAULT: + return nbdkit_use_default_export (exps); + case LIST_EXPLICIT: + source = exports; + break; + default: + abort (); + } + + for (i = 0; i < nbdkit_exports_count (source); i++) { + struct nbdkit_export e = nbdkit_get_export (source, i); + + if (nbdkit_add_export (exps, e.name, + get_desc (e.name, e.description)) == -1) + return -1; + } + return 0; +} + +static const char * +exportname_default_export (nbdkit_next_default_export *next, void *nxdata, + int readonly, int is_tls) +{ + size_t i; + + /* If we are strict, do not allow connection unless "" was advertised. */ + if (strict) { + for (i = 0; i < nbdkit_exports_count (exports); i++) { + if (nbdkit_get_export (exports, i).name[0] == '\0') + return default_export ?: ""; + } + return NULL; + } + + if (default_export) + return default_export; + return next (nxdata, readonly); +} + +struct handle { + const char *name; +}; + +static void * +exportname_open (nbdkit_next_open *next, void *nxdata, + int readonly, const char *exportname, int is_tls) +{ + size_t i; + struct handle *h; + + if (strict) { + for (i = 0; i < nbdkit_exports_count (exports); i++) { + if (strcmp (nbdkit_get_export (exports, i).name, exportname) == 0) + break; + } + if (i == nbdkit_exports_count (exports)) { + nbdkit_error ("export '%s not found", exportname); + errno = ENOENT; + return NULL; + } + } + + h = malloc (sizeof *h); + if (h == NULL) { + nbdkit_error ("malloc: %m"); + return NULL; + } + + h->name = nbdkit_strdup_intern (exportname); + if (h->name == NULL) { + free (h); + return NULL; + } + if (next (nxdata, readonly, exportname) == -1) { + free (h); + return NULL; + } + + return h; +} + +static void +exportname_close (void *handle) +{ + free (handle); +} + +static const char * +exportname_export_description (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + struct handle *h = handle; + const char *def = NULL; + + if (desc_mode == DESC_KEEP) + def = next_ops->export_description (nxdata); + + return get_desc (h->name, def); +} + +static struct nbdkit_filter filter = { + .name = "exportname", + .longname = "nbdkit exportname filter", + .load = exportname_load, + .unload = exportname_unload, + .config = exportname_config, + .config_help = exportname_config_help, + .list_exports = exportname_list_exports, + .default_export = exportname_default_export, + .open = exportname_open, + .close = exportname_close, + .export_description = exportname_export_description, +}; + +NBDKIT_REGISTER_FILTER(filter) diff --git a/tests/test-exportname.sh b/tests/test-exportname.sh new file mode 100755 index 00000000..1def8b08 --- /dev/null +++ b/tests/test-exportname.sh @@ -0,0 +1,193 @@ +#!/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 + +requires_plugin sh +requires nbdinfo --version +requires nbdsh -c 'print(h.set_full_info)' +requires jq --version + +files="exportname.out exportname.sh" +rm -f $files +cleanup_fn rm -f $files + +query='[ [.exports[]] | sort_by(."export-name")[] | + [."export-name", .description, ."export-size"] ]' +fail=0 + +cat >exportname.sh <<\EOF +case $1 in + list_exports) + echo INTERLEAVED + echo a; echo x + echo b; echo y + echo c; echo z + ;; + default_export) echo a ;; + open) echo "$3" ;; + export_description | get_size) + case $2 in + a) echo 1 ;; + b) echo 2 ;; + c) echo 3 ;; + *) exit 1 ; + esac ;; + *) exit 2 ;; +esac +EOF +chmod +x exportname.sh + +# Establish a baseline +nbdkit -U - sh exportname.sh \ + --run 'nbdinfo --json --list "$uri"' > exportname.out +cat exportname.out +test "$(jq -c "$query" exportname.out)" = \ + '[["a","x",1],["b","y",2],["c","z",3]]' + +# Set the default export +nbdkit -U - --filter=exportname sh exportname.sh default-export= \ + --run 'nbdinfo --no-content --json "$uri"' > exportname.out +cat exportname.out +test "$(jq -c "$query" exportname.out)" = '[["","1",1]]' + +nbdkit -U - --filter=exportname sh exportname.sh default-export=b \ + --run 'nbdinfo --no-content --json "$uri"' > exportname.out +cat exportname.out +test "$(jq -c "$query" exportname.out)" = '[["b","2",2]]' + +# Test export list policies +nbdkit -U - --filter=exportname sh exportname.sh exportname-list=keep \ + --run 'nbdinfo --json --list "$uri"' > exportname.out +cat exportname.out +test "$(jq -c "$query" exportname.out)" = \ + '[["a","x",1],["b","y",2],["c","z",3]]' + +nbdkit -U - --filter=exportname sh exportname.sh exportname-list=error \ + --run 'nbdinfo --json --list "$uri"' > exportname.out && fail=1 || : +test ! -s exportname.out + +nbdkit -U - --filter=exportname sh exportname.sh exportname-list=empty \ + --run 'nbdinfo --json --list "$uri"' > exportname.out +cat exportname.out +test "$(jq -c "$query" exportname.out)" = '[]' + +nbdkit -U - --filter=exportname sh exportname.sh exportname-list=defaultonly \ + --run 'nbdinfo --json --list "$uri"' > exportname.out +cat exportname.out +got="$(jq -c "$query" exportname.out)" +# libnbd 1.4.0 and 1.4.1 differ on whether --list grabs description +test "$got" = '[["a",null,1]]' || test "$got" = '[["a","1",1]]' || fail=1 + +nbdkit -U - --filter=exportname sh exportname.sh default-export=b \ + exportname-list=defaultonly exportname=a exportname=b \ + --run 'nbdinfo --json --list "$uri"' > exportname.out +cat exportname.out +got="$(jq -c "$query" exportname.out)" +test "$got" = '[["b",null,2]]' || test "$got" = '[["b","2",2]]' || fail=1 + +nbdkit -U - --filter=exportname sh exportname.sh \ + exportname-list=explicit exportname=b exportname=a \ + --run 'nbdinfo --json --list "$uri"' > exportname.out +cat exportname.out +got="$(jq -c "$query" exportname.out)" +test "$got" = '[["a",null,1],["b",null,2]]' || + test "$got" = '[["a","1",1],["b","2",2]]' || fail=1 + +nbdkit -U - --filter=exportname sh exportname.sh exportname-list=explicit \ + --run 'nbdinfo --json --list "$uri"' > exportname.out +cat exportname.out +test "$(jq -c "$query" exportname.out)" = '[]' + +# Test description modes with lists +nbdkit -U - --filter=exportname sh exportname.sh exportdesc=keep \ + --run 'nbdinfo --json --list "$uri"' > exportname.out +cat exportname.out +test "$(jq -c "$query" exportname.out)" = \ + '[["a","x",1],["b","y",2],["c","z",3]]' + +nbdkit -U - --filter=exportname sh exportname.sh exportdesc=none \ + --run 'nbdinfo --json --list "$uri"' > exportname.out +cat exportname.out +test "$(jq -c "$query" exportname.out)" = \ + '[["a",null,1],["b",null,2],["c",null,3]]' + +nbdkit -U - --filter=exportname sh exportname.sh exportdesc=fixed:hi \ + --run 'nbdinfo --json --list "$uri"' > exportname.out +cat exportname.out +test "$(jq -c "$query" exportname.out)" = \ + '[["a","hi",1],["b","hi",2],["c","hi",3]]' + +nbdkit -U - --filter=exportname sh exportname.sh \ + exportdesc=script:'echo $name$name' \ + --run 'nbdinfo --json --list "$uri"' > exportname.out +cat exportname.out +test "$(jq -c "$query" exportname.out)" = \ + '[["a","aa",1],["b","bb",2],["c","cc",3]]' + +# Test description modes with connections +nbdkit -U - -e c --filter=exportname sh exportname.sh exportdesc=fixed:hi \ + --run 'nbdinfo --no-content --json "$uri"' > exportname.out +cat exportname.out +test "$(jq -c "$query" exportname.out)" = '[["c","hi",3]]' + +nbdkit -U - -e c --filter=exportname sh exportname.sh \ + exportdesc=script:'echo $name$name' \ + --run 'nbdinfo --no-content --json "$uri"' > exportname.out +cat exportname.out +test "$(jq -c "$query" exportname.out)" = '[["c","cc",3]]' + +# Test strict mode +nbdkit -U - --filter=exportname sh exportname.sh exportname-strict=true \ + --run 'nbdinfo --no-content --json "$uri"' > exportname.out && fail=1 +test ! -s exportname.out + +nbdkit -U - --filter=exportname sh exportname.sh exportname-strict=true \ + exportname=a exportname=b exportname=c \ + --run 'nbdinfo --no-content --json "$uri"' > exportname.out && fail=1 +test ! -s exportname.out + +nbdkit -U - --filter=exportname sh exportname.sh exportname-strict=true \ + exportname=a exportname=b exportname= default-export=a\ + --run 'nbdinfo --no-content --json "$uri"' > exportname.out +cat exportname.out +test "$(jq -c "$query" exportname.out)" = '[["a","1",1]]' + +nbdkit -U - -e a --filter=exportname sh exportname.sh exportname-strict=true \ + exportname=a exportname=b exportname=c \ + --run 'nbdinfo --no-content --json "$uri"' > exportname.out +cat exportname.out +test "$(jq -c "$query" exportname.out)" = '[["a","1",1]]' + +exit $fail diff --git a/TODO b/TODO index c10b2e47..82845b03 100644 --- a/TODO +++ b/TODO @@ -252,6 +252,15 @@ nbdkit-tls-fallback-filter: * Adjust handling of .list_exports to mask what the plugin advertises when tls is not yet active. +nbdkit-exportname-fitler: + +* find a way to call the plugin's .list_exports during .open, so that + we can enforce exportname-strict=true without command line redundancy + +* add a mode for passing in a file containing exportnames in the same + manner accepted by the sh/eval plugins, rather than one name (and no + description) per config parameter + Filters for security -------------------- -- 2.28.0

When using ext2file=exportname to pick the file to server from the client's export name, we do not want to leak the underlying plugin's export list. While touching this, take advantage of string lifetimes via nbdkit_strdup_intern and similar for less cleanup bookkeeping. Note that we don't actually implement a full .list_exports; doing that with NBD_OPT_LIST is likely prohibitive (it's likely the disk contains LOTS of files), and would require that we can do next_ops->pread prior to the client reaching our .open. For the former issue, it would be better if the NBD protocol adds a new option NBD_OPT_LIST_HIER that has replies that differentiate between containers and leaves, and only lists elements within a container supplied as an argument. For the latter, we already know we want to tweak filters to be able to open a plugin independently of a client's connection. And for now we punt, and leave the advertised list empty. It's also high time we had some testsuite coverage of ext2file=exportname. Although ext2.img doesn't technically depend on the new test, including the dependency ensures the file gets rebuilt in an incremental build. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- filters/ext2/nbdkit-ext2-filter.pod | 3 +- tests/Makefile.am | 16 +++++-- filters/ext2/ext2.c | 73 ++++++++++++++++++++--------- tests/test-ext2-exportname.sh | 73 +++++++++++++++++++++++++++++ 4 files changed, 138 insertions(+), 27 deletions(-) create mode 100755 tests/test-ext2-exportname.sh diff --git a/filters/ext2/nbdkit-ext2-filter.pod b/filters/ext2/nbdkit-ext2-filter.pod index 1aef9c2e..c15a2fcb 100644 --- a/filters/ext2/nbdkit-ext2-filter.pod +++ b/filters/ext2/nbdkit-ext2-filter.pod @@ -67,7 +67,8 @@ a security risk in some situations. At present, when using this mode, the server does not advertise any particular exports; however, you may use -L<nbdkit-exportname-filter(1)> to perform that task. +L<nbdkit-exportname-filter(1)> to perform that task. Similarly, the +underlying plugin must support the default export name, C<"">. =back diff --git a/tests/Makefile.am b/tests/Makefile.am index 6bb39de9..24764390 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1382,23 +1382,31 @@ EXTRA_DIST += test-exportname.sh # ext2 filter test. if HAVE_MKE2FS_WITH_D if HAVE_EXT2 -if HAVE_GUESTFISH -LIBGUESTFS_TESTS += test-ext2 +TESTS += test-ext2-exportname.sh +EXTRA_DIST += test-ext2-exportname.sh + check_DATA += ext2.img CLEANFILES += ext2.img -ext2.img: disk +ext2.img: disk test-ext2-exportname.sh rm -f $@ $@-t + echo /disks/disk.img > manifest guestfish \ sparse $@-t 2G : \ run : \ mkfs ext4 /dev/sda : \ mount /dev/sda / : \ mkdir /disks : \ - upload $< /disks/disk.img + upload $< /disks/disk.img : \ + upload manifest /manifest + rm manifest mv $@-t $@ +if HAVE_GUESTFISH + +LIBGUESTFS_TESTS += test-ext2 + test_ext2_SOURCES = test-ext2.c test.h test_ext2_CFLAGS = $(WARNINGS_CFLAGS) $(LIBGUESTFS_CFLAGS) test_ext2_LDADD = libtest.la $(LIBGUESTFS_LIBS) diff --git a/filters/ext2/ext2.c b/filters/ext2/ext2.c index 7e4c0161..549ef5ed 100644 --- a/filters/ext2/ext2.c +++ b/filters/ext2/ext2.c @@ -50,8 +50,10 @@ #include "cleanup.h" #include "io.h" -/* Filename parameter, or NULL to honor export name. */ -static char *file; +/* Filename parameter, or NULL to honor export name. Using the export + * name is opt-in (see ext2_config_complete). + */ +static const char *file; static void ext2_load (void) @@ -59,12 +61,6 @@ ext2_load (void) initialize_ext2_error_table (); } -static void -ext2_unload (void) -{ - free (file); -} - static int ext2_config (nbdkit_next_config *next, void *nxdata, const char *key, const char *value) @@ -74,11 +70,7 @@ ext2_config (nbdkit_next_config *next, void *nxdata, nbdkit_error ("ext2file parameter specified more than once"); return -1; } - file = strdup (value); - if (file == NULL) { - nbdkit_error ("strdup: %m"); - return -1; - } + file = value; return 0; } else @@ -94,10 +86,8 @@ ext2_config_complete (nbdkit_next_config_complete *next, void *nxdata) return -1; } - if (strcmp (file, "exportname") == 0) { - free (file); + if (strcmp (file, "exportname") == 0) file = NULL; - } else if (file[0] != '/') { nbdkit_error ("the file parameter must refer to an absolute path"); return -1; @@ -112,13 +102,54 @@ ext2_config_complete (nbdkit_next_config_complete *next, void *nxdata) /* The per-connection handle. */ struct handle { - char *exportname; /* Client export name. */ + const char *exportname; /* Client export name. */ ext2_filsys fs; /* Filesystem handle. */ ext2_ino_t ino; /* Inode of open file. */ ext2_file_t file; /* File handle. */ struct nbdkit_next next; /* "name" parameter to ext2fs_open. */ }; +/* Export list. */ +static int +ext2_list_exports (nbdkit_next_list_exports *next, void *nxdata, + int readonly, int is_tls, struct nbdkit_exports *exports) +{ + /* If we are honoring export names, the default export "" won't + * work, and we must not leak export names from the underlying + * plugin. Advertising all filenames within the ext2 image could be + * huge, and even if we wanted to, it would require that we could + * open the plugin prior to the client reaching our .open. So leave + * the list empty instead. + */ + if (!file) + return 0; + + /* If we are serving a specific ext2file, we don't care what export + * name the user passes, but the underlying plugin might; there's no + * harm in advertising that list. + */ + return next (nxdata, readonly, exports); +} + +/* Default export. */ +static const char * +ext2_default_export (nbdkit_next_default_export *next, void *nxdata, + int readonly, int is_tls) +{ + /* If we are honoring exports, "" will fail (even if we resolve to + * the inode of embedded "/", we can't serve directories), and we + * don't really have a sane default. XXX picking the largest + * embedded file might be in interesting knob to add. + */ + if (!file) + return NULL; + + /* Otherwise, we don't care about export name, so keeping things at + * "" is fine, regardless of the underlying plugin's default. + */ + return ""; +} + /* Create the per-connection handle. */ static void * ext2_open (nbdkit_next_open *next, void *nxdata, @@ -133,9 +164,8 @@ ext2_open (nbdkit_next_open *next, void *nxdata, } /* Save the client exportname in the handle. */ - h->exportname = strdup (exportname); + h->exportname = nbdkit_strdup_intern (exportname); if (h->exportname == NULL) { - nbdkit_error ("strdup: %m"); free (h); return NULL; } @@ -147,7 +177,6 @@ ext2_open (nbdkit_next_open *next, void *nxdata, /* Request write access to the underlying plugin, for journal replay. */ if (next (nxdata, 0, exportname) == -1) { - free (h->exportname); free (h); return NULL; } @@ -260,7 +289,6 @@ ext2_close (void *handle) ext2fs_file_close (h->file); ext2fs_close (h->fs); } - free (h->exportname); free (h); } @@ -431,11 +459,12 @@ static struct nbdkit_filter filter = { .name = "ext2", .longname = "nbdkit ext2 filter", .load = ext2_load, - .unload = ext2_unload, .config = ext2_config, .config_complete = ext2_config_complete, .config_help = ext2_config_help, .thread_model = ext2_thread_model, + .list_exports = ext2_list_exports, + .default_export = ext2_default_export, .open = ext2_open, .prepare = ext2_prepare, .close = ext2_close, diff --git a/tests/test-ext2-exportname.sh b/tests/test-ext2-exportname.sh new file mode 100755 index 00000000..c5a85f72 --- /dev/null +++ b/tests/test-ext2-exportname.sh @@ -0,0 +1,73 @@ +#!/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 + +requires_plugin file +requires nbdinfo --version +requires nbdsh -c 'print(h.set_full_info)' + +sock=`mktemp -u` +files="$sock ext2-exportname.pid ext2-exportname.out" +rm -f $files +cleanup_fn rm -f $files + +# Set up a long-running server responsive to the client's export name +start_nbdkit -P ext2-exportname.pid -U $sock --filter=ext2 \ + --filter=exportname file ext2.img ext2file=exportname \ + exportname-list=explicit exportname=hidden + +# Test that when serving by exportname, our description varies according +# to the client's request. +nbdinfo nbd+unix:///manifest\?socket=$sock > ext2-exportname.out +cat ext2-exportname.out +grep manifest ext2-exportname.out +grep 'content.*ASCII' ext2-exportname.out + +nbdinfo nbd+unix:///disks/disk.img\?socket=$sock > ext2-exportname.out +cat ext2-exportname.out +grep disk.img ext2-exportname.out +grep 'content.*MBR' ext2-exportname.out + +if nbdinfo nbd+unix://?socket=$sock > ext2-exportname.out; then + echo "unexpected success" + exit 1 +fi + +# Test that there is no export list advertised +nbdinfo --list --json nbd+unix://?socket=$sock > ext2-exportname.out +cat ext2-exportname.out +grep '"exports": \[]' ext2-exportname.out + +: -- 2.28.0

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 24764390..2139eb43 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -770,6 +770,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 \ @@ -777,6 +778,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..5d848c18 --- /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

Fairly straightforward. .list_exports uses the same idiom as .extents for returning an iterable of tuples, with additional support for a bare name rather than a name/desc tuple. .default_export and .export_description are rather easy clients of nbdkit_strdup_intern. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- plugins/python/nbdkit-python-plugin.pod | 25 ++++ tests/Makefile.am | 3 + plugins/python/python.c | 185 ++++++++++++++++++++---- tests/test-python-export-list.sh | 71 +++++++++ tests/python-export-list.py | 59 ++++++++ 5 files changed, 313 insertions(+), 30 deletions(-) create mode 100755 tests/test-python-export-list.sh create mode 100644 tests/python-export-list.py diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod index 79aed288..a3bd520c 100644 --- a/plugins/python/nbdkit-python-plugin.pod +++ b/plugins/python/nbdkit-python-plugin.pod @@ -178,6 +178,23 @@ See L</Threads> below. There are no arguments or return value. +=item C<list_exports> + +(Optional) + + def list_exports(readonly, is_tls): + # return an iterable object (eg. list) of + # (name, description) tuples or bare names: + return [ (name1, desc1), name2, (name3, desc3), ... ] + +=item C<default_export> + +(Optional) + + def default_export(readonly, is_tls): + # return a string + return "name" + =item C<open> (Required) @@ -199,6 +216,14 @@ After C<close> returns, the reference count of the handle is decremented in the C part, which usually means that the handle and its contents will be garbage collected. +=item C<export_description> + +(Optional) + + def export_description(h): + # return a string + return "description" + =item C<get_size> (Required) diff --git a/tests/Makefile.am b/tests/Makefile.am index c2d668c1..390df711 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1083,16 +1083,19 @@ TESTS += \ test-python.sh \ test-python-exception.sh \ test-python-export-name.sh \ + test-python-export-list.sh \ test-python-thread-model.sh \ test-shebang-python.sh \ $(NULL) EXTRA_DIST += \ python-exception.py \ python-export-name.py \ + python-export-list.py \ python-thread-model.py \ shebang.py \ test-python-exception.sh \ test-python-export-name.sh \ + test-python-export-list.sh \ test-python-plugin.py \ test-python-thread-model.sh \ test-python.sh \ diff --git a/plugins/python/python.c b/plugins/python/python.c index 27c5ede2..91cebc16 100644 --- a/plugins/python/python.c +++ b/plugins/python/python.c @@ -536,6 +536,99 @@ py_get_ready (void) return 0; } +static int +py_list_exports (int readonly, int is_tls, struct nbdkit_exports *exports) +{ + ACQUIRE_PYTHON_GIL_FOR_CURRENT_SCOPE; + PyObject *fn; + PyObject *r; + PyObject *iter, *t; + + if (!callback_defined ("list_exports", &fn)) + /* Do the same as the core server */ + return nbdkit_use_default_export (exports); + + PyErr_Clear (); + + r = PyObject_CallFunction (fn, "ii", readonly, is_tls); + Py_DECREF (fn); + if (check_python_failure ("list_exports") == -1) + return -1; + + iter = PyObject_GetIter (r); + if (iter == NULL) { + nbdkit_error ("list_exports method did not return " + "something which is iterable"); + Py_DECREF (r); + return -1; + } + + while ((t = PyIter_Next (iter)) != NULL) { + PyObject *py_name, *py_desc; + CLEANUP_FREE char *name = NULL; + CLEANUP_FREE char *desc = NULL; + + name = python_to_string (t); + if (!name) { + if (!PyTuple_Check (t) || PyTuple_Size (t) != 2) { + nbdkit_error ("list_exports method did not return an iterable of " + "2-tuples"); + Py_DECREF (iter); + Py_DECREF (r); + return -1; + } + py_name = PyTuple_GetItem (t, 0); + py_desc = PyTuple_GetItem (t, 1); + name = python_to_string (py_name); + desc = python_to_string (py_desc); + if (name == NULL || desc == NULL) { + nbdkit_error ("list_exports method did not return an iterable of " + "string 2-tuples"); + Py_DECREF (iter); + Py_DECREF (r); + return -1; + } + } + if (nbdkit_add_export (exports, name, desc) == -1) { + Py_DECREF (iter); + Py_DECREF (r); + return -1; + } + } + + Py_DECREF (iter); + Py_DECREF (r); + return 0; +} + +static const char * +py_default_export (int readonly, int is_tls) +{ + ACQUIRE_PYTHON_GIL_FOR_CURRENT_SCOPE; + PyObject *fn; + PyObject *r; + CLEANUP_FREE char *name = NULL; + + if (!callback_defined ("default_export", &fn)) + return ""; + + PyErr_Clear (); + + r = PyObject_CallFunction (fn, "ii", readonly, is_tls); + Py_DECREF (fn); + if (check_python_failure ("default_export") == -1) + return NULL; + + name = python_to_string (r); + Py_DECREF (r); + if (!name) { + nbdkit_error ("default_export method did not return a string"); + return NULL; + } + + return nbdkit_strdup_intern (name); +} + struct handle { int can_zero; PyObject *py_h; @@ -595,6 +688,35 @@ py_close (void *handle) free (h); } +static const char * +py_export_description (void *handle) +{ + ACQUIRE_PYTHON_GIL_FOR_CURRENT_SCOPE; + struct handle *h = handle; + PyObject *fn; + PyObject *r; + CLEANUP_FREE char *desc = NULL; + + if (!callback_defined ("export_description", &fn)) + return NULL; + + PyErr_Clear (); + + r = PyObject_CallFunctionObjArgs (fn, h->py_h, NULL); + Py_DECREF (fn); + if (check_python_failure ("export_description") == -1) + return NULL; + + desc = python_to_string (r); + Py_DECREF (r); + if (!desc) { + nbdkit_error ("export_description method did not return a string"); + return NULL; + } + + return nbdkit_strdup_intern (desc); +} + static int64_t py_get_size (void *handle) { @@ -1120,42 +1242,45 @@ py_extents (void *handle, uint32_t count, uint64_t offset, #define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL static struct nbdkit_plugin plugin = { - .name = "python", - .version = PACKAGE_VERSION, + .name = "python", + .version = PACKAGE_VERSION, - .load = py_load, - .unload = py_unload, - .dump_plugin = py_dump_plugin, + .load = py_load, + .unload = py_unload, + .dump_plugin = py_dump_plugin, - .config = py_config, - .config_complete = py_config_complete, - .config_help = py_config_help, + .config = py_config, + .config_complete = py_config_complete, + .config_help = py_config_help, - .thread_model = py_thread_model, - .get_ready = py_get_ready, + .thread_model = py_thread_model, + .get_ready = py_get_ready, + .list_exports = py_list_exports, + .default_export = py_default_export, - .open = py_open, - .close = py_close, + .open = py_open, + .close = py_close, - .get_size = py_get_size, - .is_rotational = py_is_rotational, - .can_multi_conn = py_can_multi_conn, - .can_write = py_can_write, - .can_flush = py_can_flush, - .can_trim = py_can_trim, - .can_zero = py_can_zero, - .can_fast_zero = py_can_fast_zero, - .can_fua = py_can_fua, - .can_cache = py_can_cache, - .can_extents = py_can_extents, + .export_description = py_export_description, + .get_size = py_get_size, + .is_rotational = py_is_rotational, + .can_multi_conn = py_can_multi_conn, + .can_write = py_can_write, + .can_flush = py_can_flush, + .can_trim = py_can_trim, + .can_zero = py_can_zero, + .can_fast_zero = py_can_fast_zero, + .can_fua = py_can_fua, + .can_cache = py_can_cache, + .can_extents = py_can_extents, - .pread = py_pread, - .pwrite = py_pwrite, - .flush = py_flush, - .trim = py_trim, - .zero = py_zero, - .cache = py_cache, - .extents = py_extents, + .pread = py_pread, + .pwrite = py_pwrite, + .flush = py_flush, + .trim = py_trim, + .zero = py_zero, + .cache = py_cache, + .extents = py_extents, }; NBDKIT_REGISTER_PLUGIN (plugin) diff --git a/tests/test-python-export-list.sh b/tests/test-python-export-list.sh new file mode 100755 index 00000000..f8569979 --- /dev/null +++ b/tests/test-python-export-list.sh @@ -0,0 +1,71 @@ +#!/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. + +source ./functions.sh +set -e +set -x + +if test ! -d "$SRCDIR"; then + echo "$0: could not locate python-export-list.py" + exit 1 +fi + +# Python has proven very difficult to valgrind, therefore it is disabled. +if [ "$NBDKIT_VALGRIND" = "1" ]; then + echo "$0: skipping Python test under valgrind." + exit 77 +fi + +requires nbdinfo --version +requires nbdsh -c 'print(h.set_full_info)' +requires jq --version + +pid=test-python-export-list.pid +sock=`mktemp -u` +out=test-python-export-list.out +files="$pid $sock $out" +rm -f $files +cleanup_fn rm -f $files + +start_nbdkit -P $pid -U $sock python $SRCDIR/python-export-list.py + +nbdinfo --list --json nbd+unix://\?socket=$sock > $out +cat $out +# libnbd 1.4.0 differs from 1.4.1 on whether --list grabs descriptions +result=$(jq -c '[.exports[] | [."export-name", .description]]' $out) +test "$result" = '[["hello","world"],["name only",null]]' || + test "$result" = '[["hello","world"],["name only","=name only="]]' + +nbdinfo --json nbd+unix://\?socket=$sock > $out +cat $out +diff -u <(jq -c '[.exports[] | [."export-name", .description]]' $out) \ + <(printf %s\\n '[["hello","=hello="]]') diff --git a/tests/python-export-list.py b/tests/python-export-list.py new file mode 100644 index 00000000..6feb6782 --- /dev/null +++ b/tests/python-export-list.py @@ -0,0 +1,59 @@ +# 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. + +# Create an nbdkit sh plugin which reflects the export name back to +# the caller in the virtual device data and size. + +import nbdkit + + +def list_exports(readonly, is_tls): + return [("hello", "world"), "name only"] + + +def default_export(readonly, is_tls): + return "hello" + + +def open(readonly): + return nbdkit.export_name() + + +def export_description(h): + return "=%s=" % h + + +def get_size(h): + return 0 + + +def pread(h, count, offset): + pass -- 2.28.0

On Mon, Sep 21, 2020 at 11:23:58AM -0500, Eric Blake wrote: It's optional, but if you don't care about a particular parameter you can use a single underscore, as in: let ocamlexample_default_export _ _ = "name1" I'm sure it's possible to get OCaml to warn about unused non-“_” parameters, but we have that warning turned off because it'd be quite annoying. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org