It's been several weeks since I posted v2 (I got distracted by improving libnbd to better test things, which in turn surfaced some major memory leak problems in nbdsh that are now fixed). Many of the patches are minor rebases from v2, with the biggest changes being fallout from: - patch 2: rename nbdkit_add_default_export to nbdkit_use_default_export - overall: this missed 1.22, so update appropriate documentation - libnbd's 'nbdinfo --list' differs in behavior between 1.4.0 and 1.4.1 regarding descriptions, so fix the tests to work for both versions - rebased on top of rewriting test-layers to use libnbd - rebased on top of putting nbdkit_*_intern() in place already I'm probably at the point where it is just worth committing this series, and dealing with any further changes as followup patches. Key: [----] : patches are identical [####] : number of functional differences between upstream/downstream patch [down] : patch is downstream-only The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively 001/14:[0059] [FC] 'api: Add .default_export' 002/14:[down] 'api: Add nbdkit_use_default_export' 003/14:[0059] [FC] 'server: Respond to NBD_INFO_NAME request' 004/14:[0021] [FC] 'sh, eval: Implement .default_export' 005/14:[0036] [FC] 'api: Alter .list_exports' 006/14:[0032] [FC] 'api: Add .export_description' 007/14:[0015] [FC] 'exportname: New filter' 008/14:[0049] [FC] 'filters: Add .export_description wrappers' 009/14:[0002] [FC] 'ext2: Supply .list_exports and .default_export' 010/14:[----] [--] 'nbd: Implement .default_export, .export_description' 011/14:[0010] [FC] 'nbd: Add dynamic-export=true option' 012/14:[0002] [FC] 'nbd: Implement .list_exports' 013/14:[0019] [FC] 'python: Implement .list_exports and friends' 014/14:[0017] [FC] 'ocaml: Implement .list_exports and friends' Eric Blake (14): api: Add .default_export api: Add nbdkit_use_default_export server: Respond to NBD_INFO_NAME request sh, eval: Implement .default_export api: Alter .list_exports api: Add .export_description exportname: New filter filters: Add .export_description wrappers ext2: Supply .list_exports and .default_export nbd: Implement .default_export, .export_description nbd: Add dynamic-export=true option nbd: Implement .list_exports python: Implement .list_exports and friends ocaml: Implement .list_exports and friends docs/nbdkit-filter.pod | 65 +++- docs/nbdkit-plugin.pod | 112 +++++- docs/nbdkit-protocol.pod | 14 + .../exportname/nbdkit-exportname-filter.pod | 154 ++++++++ filters/ext2/nbdkit-ext2-filter.pod | 6 + filters/log/nbdkit-log-filter.pod | 2 +- plugins/eval/nbdkit-eval-plugin.pod | 6 + plugins/file/nbdkit-file-plugin.pod | 9 +- plugins/nbd/nbdkit-nbd-plugin.pod | 30 +- plugins/python/nbdkit-python-plugin.pod | 25 ++ plugins/sh/nbdkit-sh-plugin.pod | 39 +- include/nbdkit-common.h | 2 + include/nbdkit-filter.h | 13 +- include/nbdkit-plugin.h | 4 +- configure.ac | 2 + filters/exportname/Makefile.am | 67 ++++ tests/Makefile.am | 31 +- server/internal.h | 15 +- server/backend.c | 93 +++-- server/exports.c | 32 +- server/filters.c | 31 +- server/nbdkit.syms | 1 + server/plugins.c | 33 +- server/protocol-handshake-newstyle.c | 88 ++++- server/test-public.c | 9 +- plugins/ocaml/NBDKit.mli | 9 + plugins/sh/methods.h | 2 + plugins/ocaml/NBDKit.ml | 17 + plugins/ocaml/example.ml | 33 +- plugins/cc/cc.c | 73 ++-- plugins/eval/eval.c | 70 ++-- plugins/nbd/nbd.c | 250 +++++++++++-- plugins/ocaml/ocaml.c | 92 ++++- plugins/ondemand/ondemand.c | 11 +- plugins/python/python.c | 185 ++++++++-- plugins/sh/methods.c | 84 ++++- plugins/sh/sh.c | 70 ++-- plugins/sh/example.sh | 2 +- filters/exportname/exportname.c | 342 ++++++++++++++++++ filters/ext2/ext2.c | 122 +++++-- filters/gzip/gzip.c | 37 +- filters/log/log.c | 8 +- filters/partition/partition.c | 56 ++- filters/tar/tar.c | 46 ++- filters/tls-fallback/tls-fallback.c | 60 ++- filters/xz/xz.c | 40 +- tests/test-eval-exports.sh | 41 ++- tests/test-export-info.sh | 111 ++++++ tests/test-exportname.sh | 193 ++++++++++ tests/test-ext2-exportname.sh | 73 ++++ tests/test-nbd-dynamic-content.sh | 75 ++++ tests/test-nbd-dynamic-list.sh | 162 +++++++++ tests/test-python-export-list.sh | 71 ++++ tests/test-tls-fallback.sh | 53 ++- tests/python-export-list.py | 59 +++ tests/test-layers-filter.c | 13 +- tests/test-layers-plugin.c | 8 + tests/test-layers.c | 12 + TODO | 20 +- 59 files changed, 2950 insertions(+), 433 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-export-info.sh create mode 100755 tests/test-exportname.sh create mode 100755 tests/test-ext2-exportname.sh create mode 100755 tests/test-nbd-dynamic-content.sh create mode 100755 tests/test-nbd-dynamic-list.sh create mode 100755 tests/test-python-export-list.sh create mode 100644 tests/python-export-list.py -- 2.28.0
Eric Blake
2020-Sep-21 16:23 UTC
[Libguestfs] [nbdkit PATCH v3 01/14] api: Add .default_export
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
Eric Blake
2020-Sep-21 16:23 UTC
[Libguestfs] [nbdkit PATCH v3 02/14] api: Add nbdkit_use_default_export
Rather than defaulting .list_exports to blindly returning "", it is nicer to have it reflect the result of .default_export. Meanwhile, language bindings will have a C callback for both .list_exports and .default_export, even if the underlying script does not, which means any logic in plugins.c for calling .default_export when .list_export is missing would have to be duplicated in each language binding. Better is to make it easy for languages, by wrapping things in a new helper function; this also lets us take maximum advantage of caching done in backend.c (since language bindings can't access our internals, they would have to duplicate caching if we did not add this helper function). Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/nbdkit-plugin.pod | 40 +++++++++++++++++++++++++++++----------- include/nbdkit-common.h | 2 ++ server/internal.h | 4 ++++ server/backend.c | 15 ++++++++------- server/exports.c | 24 ++++++++++++++++++++++++ server/nbdkit.syms | 1 + server/plugins.c | 2 +- server/test-public.c | 9 ++++++++- plugins/sh/methods.c | 2 +- 9 files changed, 78 insertions(+), 21 deletions(-) diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 180c9daa..5962b1b8 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -675,10 +675,11 @@ error message and return C<-1>. This optional callback is called if the client tries to list the exports served by the plugin (using C<NBD_OPT_LIST>). If the plugin -does not supply this callback then a single export called C<""> is -returned. The NBD protocol defines C<""> as the default export, so -this is suitable for plugins which ignore the export name and always -serve the same content. See also L</EXPORT NAME> below. +does not supply this callback then the result of C<.default_export> is +advertised as the lone export. The NBD protocol defines C<""> as the +default export, so this is suitable for plugins which ignore the +export name and always serve the same content. See also L</EXPORT +NAME> below. 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 @@ -694,9 +695,8 @@ C<"">). The plugin can ignore this flag and return all exports if it wants. The C<exports> parameter is an opaque object for collecting the list -of exports. Call C<nbdkit_add_export> to add a single export to the -list. If the plugin has a concept of a default export (usually but -not always called C<"">) then it should return that first in the list. +of exports. Call C<nbdkit_add_export> as needed to add specific +exports to the list. int nbdkit_add_export (struct nbdkit_export *exports, const char *name, const char *description); @@ -711,9 +711,25 @@ function. C<nbdkit_add_export> returns C<0> on success or C<-1> on failure; on failure C<nbdkit_error> has already been called, with C<errno> set to a suitable value. -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>. +There are also situations where a plugin may wish to duplicate the +nbdkit default behavior of supplying an export list containing only +the result of C<.default_export> when C<.list_exports> is missing; +this is most common in a language binding where it is not known at +compile time whether the language script will be providing an +implementation for C<.list_exports>, and is done by calling +C<nbdkit_use_default_export>. + + int nbdkit_use_default_export (struct nbdkit_export *exports); + +C<nbdkit_use_default_export> returns C<0> on success or C<-1> on +failure; on failure C<nbdkit_error> has already been called, with +C<errno> set to a suitable value. + +The plugin may also leave the export list empty, by not calling either +helper. Once the plugin is happy with the list contents, 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> @@ -724,7 +740,9 @@ 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. +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. 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/include/nbdkit-common.h b/include/nbdkit-common.h index b3e83293..eb4416df 100644 --- a/include/nbdkit-common.h +++ b/include/nbdkit-common.h @@ -151,6 +151,8 @@ struct nbdkit_exports; NBDKIT_EXTERN_DECL (int, nbdkit_add_export, (struct nbdkit_exports *, const char *name, const char *description)); +NBDKIT_EXTERN_DECL (int, nbdkit_use_default_export, + (struct nbdkit_exports *)); /* A static non-NULL pointer which can be used when you don't need a * per-connection handle. diff --git a/server/internal.h b/server/internal.h index dfb4da31..e0422574 100644 --- a/server/internal.h +++ b/server/internal.h @@ -564,4 +564,8 @@ extern struct connection *threadlocal_get_conn (void); struct connection *conn = threadlocal_get_conn (); \ assert (conn != NULL) +/* exports.c */ +extern int exports_resolve_default (struct nbdkit_exports *exports, + struct backend *b, int readonly); + #endif /* NBDKIT_INTERNAL_H */ diff --git a/server/backend.c b/server/backend.c index 6e82629c..74d934fc 100644 --- a/server/backend.c +++ b/server/backend.c @@ -164,7 +164,7 @@ backend_list_exports (struct backend *b, int readonly, int default_only, { GET_CONN; struct handle *h = get_handle (conn, b->i); - int r; + size_t count; assert (!default_only); /* XXX Switch to is_tls... */ controlpath_debug ("%s: list_exports readonly=%d default_only=%d", @@ -173,14 +173,15 @@ backend_list_exports (struct backend *b, int readonly, int default_only, assert (h->handle == NULL); assert ((h->state & HANDLE_OPEN) == 0); - r = b->list_exports (b, readonly, default_only, exports); - if (r == -1) + if (b->list_exports (b, readonly, default_only, exports) == -1 || + exports_resolve_default (exports, b, readonly) == -1) { controlpath_debug ("%s: list_exports failed", b->name); - else { - size_t count = nbdkit_exports_count (exports); - controlpath_debug ("%s: list_exports returned %zu names", b->name, count); + return -1; } - return r; + + count = nbdkit_exports_count (exports); + controlpath_debug ("%s: list_exports returned %zu names", b->name, count); + return 0; } const char * diff --git a/server/exports.c b/server/exports.c index 8d3faec4..dc804390 100644 --- a/server/exports.c +++ b/server/exports.c @@ -52,6 +52,7 @@ DEFINE_VECTOR_TYPE(exports, struct nbdkit_export); struct nbdkit_exports { exports exports; + bool use_default; }; struct nbdkit_exports * @@ -65,6 +66,7 @@ nbdkit_exports_new (void) return NULL; } r->exports = (exports) empty_vector; + r->use_default = false; return r; } @@ -141,3 +143,25 @@ nbdkit_add_export (struct nbdkit_exports *exps, return 0; } + +int +nbdkit_use_default_export (struct nbdkit_exports *exps) +{ + exps->use_default = true; + return 0; +} + +int +exports_resolve_default (struct nbdkit_exports *exps, struct backend *b, + int readonly) +{ + const char *def = NULL; + + if (exps->use_default) { + def = backend_default_export (b, readonly); + exps->use_default = false; + } + if (def) + return nbdkit_add_export (exps, def, NULL); + return 0; +} diff --git a/server/nbdkit.syms b/server/nbdkit.syms index 281a1459..1eb18bb0 100644 --- a/server/nbdkit.syms +++ b/server/nbdkit.syms @@ -76,6 +76,7 @@ nbdkit_stdio_safe; nbdkit_strdup_intern; nbdkit_strndup_intern; + nbdkit_use_default_export; nbdkit_vdebug; nbdkit_verror; nbdkit_vprintf_intern; diff --git a/server/plugins.c b/server/plugins.c index 4cd93efc..15082acb 100644 --- a/server/plugins.c +++ b/server/plugins.c @@ -292,7 +292,7 @@ plugin_list_exports (struct backend *b, int readonly, int default_only, struct backend_plugin *p = container_of (b, struct backend_plugin, backend); if (!p->plugin.list_exports) - return nbdkit_add_export (exports, "", NULL); + return nbdkit_use_default_export (exports); return p->plugin.list_exports (readonly, default_only, exports); } diff --git a/server/test-public.c b/server/test-public.c index ff0d2b28..141e9eb5 100644 --- a/server/test-public.c +++ b/server/test-public.c @@ -75,7 +75,14 @@ threadlocal_get_conn (void) abort (); } -int connection_get_status (void) +int +connection_get_status (void) +{ + abort (); +} + +const char * +backend_default_export (struct backend *b, int readonly) { abort (); } diff --git a/plugins/sh/methods.c b/plugins/sh/methods.c index e08baaf5..23e4f084 100644 --- a/plugins/sh/methods.c +++ b/plugins/sh/methods.c @@ -315,7 +315,7 @@ sh_list_exports (int readonly, int default_only, return parse_exports (script, s, slen, exports); case MISSING: - return nbdkit_add_export (exports, "", NULL); + return nbdkit_use_default_export (exports); case ERROR: return -1; -- 2.28.0
Eric Blake
2020-Sep-21 16:23 UTC
[Libguestfs] [nbdkit PATCH v3 03/14] server: Respond to NBD_INFO_NAME request
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@redhat.com> --- 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
Eric Blake
2020-Sep-21 16:23 UTC
[Libguestfs] [nbdkit PATCH v3 04/14] sh, eval: Implement .default_export
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@redhat.com> --- 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
Eric Blake
2020-Sep-21 16:23 UTC
[Libguestfs] [nbdkit PATCH v3 05/14] api: Alter .list_exports
I'm about to add an 'exportname' filter, and in the process, I noticed a few shortcomings in our API. The existing 'tls-fallback' filter already mentioned that we have situations where we want to expose a different list before and after TLS negotiation on a tls=1 setup, but .list_exports is executed before .open has set nbdkit_is_tls(). And since a previous patch split out the .default_export functionality, it is easiest to just repurpose the existing parameter to a new name. However, it does adjust the signature for filters calling into the next layer, as filters don't change whether TLS has been attempted. This also fixes the cc plugin to expose .list_exports. Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/nbdkit-filter.pod | 10 ++++++-- docs/nbdkit-plugin.pod | 17 ++++++------- filters/log/nbdkit-log-filter.pod | 2 +- plugins/sh/nbdkit-sh-plugin.pod | 8 ++----- include/nbdkit-filter.h | 3 +-- include/nbdkit-plugin.h | 2 +- server/internal.h | 5 ++-- server/backend.c | 9 ++++--- server/filters.c | 6 ++--- server/plugins.c | 4 ++-- server/protocol-handshake-newstyle.c | 2 +- plugins/cc/cc.c | 9 +++++++ plugins/sh/methods.c | 5 ++-- filters/log/log.c | 8 +++---- filters/tls-fallback/tls-fallback.c | 12 ++++++++-- tests/test-tls-fallback.sh | 36 ++++++++++++++++++++++++---- tests/test-layers-filter.c | 4 ++-- TODO | 6 ----- 18 files changed, 93 insertions(+), 55 deletions(-) diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod index cd6d8fef..916d62dd 100644 --- a/docs/nbdkit-filter.pod +++ b/docs/nbdkit-filter.pod @@ -342,12 +342,18 @@ an error message and return C<-1>. =head2 C<.list_exports> int (*list_exports) (nbdkit_next_list_exports *next, void *nxdata, - int readonly, int default_only, + int readonly, int is_tls, struct nbdkit_exports *exports); This intercepts the plugin C<.list_exports> method and can be used to filter which exports are advertised. +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. + It is possible for filters to transform the exports list received back from the layer below. Without error checking it would look like this: @@ -359,7 +365,7 @@ from the layer below. Without error checking it would look like this: char *name, *desc; exports2 = nbdkit_exports_new (); - next_list_exports (nxdata, readonly, default_only, exports); + next_list_exports (nxdata, readonly, exports); for (i = 0; i < nbdkit_exports_count (exports2); ++i) { e = nbdkit_get_export (exports2, i); name = adjust (e.name); diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 30fc0ecf..f1476032 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -670,7 +670,7 @@ error message and return C<-1>. =head2 C<.list_exports> - int list_exports (int readonly, int default_only, + int list_exports (int readonly, int is_tls, struct nbdkit_exports *exports); This optional callback is called if the client tries to list the @@ -688,11 +688,11 @@ 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. -If the C<default_only> flag is set then the client is querying for the -name of the default export, and the plugin may optimize by adding only -a single export to the returned list (the default export name, usually -C<"">). The plugin can ignore this flag and return all exports if it -wants. +The C<is_tls> flag informs the plugin whether this listing was +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 any exports listed when C<is_tls> is C<false> do not leak +unintended information. The C<exports> parameter is an opaque object for collecting the list of exports. Call C<nbdkit_add_export> as needed to add specific @@ -794,8 +794,9 @@ server requires TLS authentication, then that has occurred as well. But if the server is set up to have optional TLS authentication, you may check C<nbdkit_is_tls> to learn whether the client has completed TLS authentication. When running the server in a mode that permits -but not requires TLS, be careful that you do not allow unauthenticated -clients to cause a denial of service against authentication. +but does not require TLS, be careful that you do not allow +unauthenticated clients to cause a denial of service against +authentication. If there is an error, C<.open> should call C<nbdkit_error> with an error message and return C<NULL>. diff --git a/filters/log/nbdkit-log-filter.pod b/filters/log/nbdkit-log-filter.pod index 721fc118..40bce76d 100644 --- a/filters/log/nbdkit-log-filter.pod +++ b/filters/log/nbdkit-log-filter.pod @@ -72,7 +72,7 @@ on the connection). An example logging session of a client that requests an export list before performing a single successful read is: - 2020-08-06 02:07:23.080415 ListExports id=1 readonly=0 default_only=0 ... + 2020-08-06 02:07:23.080415 ListExports id=1 readonly=0 tls=0 ... 2020-08-06 02:07:23.080502 ...ListExports id=1 exports=[""] return=0 2020-08-06 02:07:23.080712 connection=1 Connect export='' tls=0 size=0x400 write=1 flush=1 rotational=0 trim=1 zero=2 fua=2 extents=1 cache=2 fast_zero=1 2020-08-06 02:07:23.080907 connection=1 Read id=1 offset=0x0 count=0x200 ... diff --git a/plugins/sh/nbdkit-sh-plugin.pod b/plugins/sh/nbdkit-sh-plugin.pod index f3a6147c..17a1f08c 100644 --- a/plugins/sh/nbdkit-sh-plugin.pod +++ b/plugins/sh/nbdkit-sh-plugin.pod @@ -268,13 +268,9 @@ with status C<1>; unrecognized output is ignored. =item C<list_exports> - /path/to/script list_exports <readonly> <default_only> + /path/to/script list_exports <readonly> <tls> -The C<readonly> parameter will be C<true> or C<false>. The -C<default_only> parameter will be C<true> if the caller is only -interested in the canonical name of the default export, or C<false> to -get a full list of export names; the script may safely ignore this -parameter and always provide a full list if desired. +The C<readonly> and C<tls> parameters will be C<true> or C<false>. The first line of output informs nbdkit how to parse the rest of the output, the remaining lines then supply the inputs of the C diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h index 2c5b36be..63458d59 100644 --- a/include/nbdkit-filter.h +++ b/include/nbdkit-filter.h @@ -66,7 +66,6 @@ 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 ignored, struct nbdkit_exports *exports); typedef const char *nbdkit_next_default_export (nbdkit_backend *nxdata, int readonly); @@ -179,7 +178,7 @@ struct nbdkit_filter { int (*preconnect) (nbdkit_next_preconnect *next, nbdkit_backend *nxdata, int readonly); int (*list_exports) (nbdkit_next_list_exports *next, nbdkit_backend *nxdata, - int readonly, int default_only, + int readonly, int is_tls, struct nbdkit_exports *exports); const char * (*default_export) (nbdkit_next_default_export *next, nbdkit_backend *nxdata, diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h index 28e83757..539e3f6e 100644 --- a/include/nbdkit-plugin.h +++ b/include/nbdkit-plugin.h @@ -140,7 +140,7 @@ struct nbdkit_plugin { int (*get_ready) (void); int (*after_fork) (void); - int (*list_exports) (int readonly, int default_only, + int (*list_exports) (int readonly, int is_tls, struct nbdkit_exports *exports); const char * (*default_export) (int readonly, int is_tls); }; diff --git a/server/internal.h b/server/internal.h index e0422574..348d185b 100644 --- a/server/internal.h +++ b/server/internal.h @@ -386,7 +386,7 @@ 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 ignored, + int (*list_exports) (struct backend *, int readonly, int is_tls, struct nbdkit_exports *exports); const char *(*default_export) (struct backend *, int readonly, int is_tls); void *(*open) (struct backend *, int readonly, const char *exportname, @@ -435,9 +435,8 @@ extern void backend_unload (struct backend *b, void (*unload) (void)) __attribute__((__nonnull__ (1))); extern int backend_list_exports (struct backend *b, int readonly, - int ignored, struct nbdkit_exports *exports) - __attribute__((__nonnull__ (1, 4))); + __attribute__((__nonnull__ (1, 3))); 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 diff --git a/server/backend.c b/server/backend.c index 74d934fc..427a0cec 100644 --- a/server/backend.c +++ b/server/backend.c @@ -159,21 +159,20 @@ backend_unload (struct backend *b, void (*unload) (void)) } int -backend_list_exports (struct backend *b, int readonly, int default_only, +backend_list_exports (struct backend *b, int readonly, struct nbdkit_exports *exports) { GET_CONN; struct handle *h = get_handle (conn, b->i); size_t count; - assert (!default_only); /* XXX Switch to is_tls... */ - controlpath_debug ("%s: list_exports readonly=%d default_only=%d", - b->name, readonly, default_only); + controlpath_debug ("%s: list_exports readonly=%d tls=%d", + b->name, readonly, conn->using_tls); assert (h->handle == NULL); assert ((h->state & HANDLE_OPEN) == 0); - if (b->list_exports (b, readonly, default_only, exports) == -1 || + if (b->list_exports (b, readonly, conn->using_tls, exports) == -1 || exports_resolve_default (exports, b, readonly) == -1) { controlpath_debug ("%s: list_exports failed", b->name); return -1; diff --git a/server/filters.c b/server/filters.c index bb22be76..dd4417be 100644 --- a/server/filters.c +++ b/server/filters.c @@ -238,15 +238,15 @@ plugin_magic_config_key (struct backend *b) } static int -filter_list_exports (struct backend *b, int readonly, int default_only, +filter_list_exports (struct backend *b, int readonly, int is_tls, struct nbdkit_exports *exports) { struct backend_filter *f = container_of (b, struct backend_filter, backend); if (f->filter.list_exports) return f->filter.list_exports (backend_list_exports, b->next, - readonly, default_only, exports); - return backend_list_exports (b->next, readonly, default_only, exports); + readonly, is_tls, exports); + return backend_list_exports (b->next, readonly, exports); } static const char * diff --git a/server/plugins.c b/server/plugins.c index 15082acb..69835d32 100644 --- a/server/plugins.c +++ b/server/plugins.c @@ -286,7 +286,7 @@ plugin_preconnect (struct backend *b, int readonly) } static int -plugin_list_exports (struct backend *b, int readonly, int default_only, +plugin_list_exports (struct backend *b, int readonly, int is_tls, struct nbdkit_exports *exports) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); @@ -294,7 +294,7 @@ plugin_list_exports (struct backend *b, int readonly, int default_only, if (!p->plugin.list_exports) return nbdkit_use_default_export (exports); - return p->plugin.list_exports (readonly, default_only, exports); + return p->plugin.list_exports (readonly, is_tls, exports); } static const char * diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c index dd4ca601..7fc97507 100644 --- a/server/protocol-handshake-newstyle.c +++ b/server/protocol-handshake-newstyle.c @@ -88,7 +88,7 @@ send_newstyle_option_reply_exportnames (uint32_t option) 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) + if (backend_list_exports (top, read_only, exps) == -1) return send_newstyle_option_reply (option, NBD_REP_ERR_PLATFORM); for (i = 0; i < nbdkit_exports_count (exps); i++) { diff --git a/plugins/cc/cc.c b/plugins/cc/cc.c index 1d688056..7d529b20 100644 --- a/plugins/cc/cc.c +++ b/plugins/cc/cc.c @@ -348,6 +348,14 @@ cc_preconnect (int readonly) return 0; } +static int +cc_list_exports (int readonly, int is_tls, struct nbdkit_exports *exports) +{ + if (subplugin.list_exports) + return subplugin.list_exports (readonly, is_tls, exports); + return nbdkit_use_default_export (exports); +} + static const char * cc_default_export (int readonly, int is_tls) { @@ -571,6 +579,7 @@ static struct nbdkit_plugin plugin = { .after_fork = cc_after_fork, .preconnect = cc_preconnect, + .list_exports = cc_list_exports, .default_export = cc_default_export, .open = cc_open, .close = cc_close, diff --git a/plugins/sh/methods.c b/plugins/sh/methods.c index 15e0d053..12f498cd 100644 --- a/plugins/sh/methods.c +++ b/plugins/sh/methods.c @@ -302,13 +302,12 @@ parse_exports (const char *script, } int -sh_list_exports (int readonly, int default_only, - struct nbdkit_exports *exports) +sh_list_exports (int readonly, int is_tls, struct nbdkit_exports *exports) { const char *method = "list_exports"; const char *script = get_script (method); const char *args[] = { script, method, readonly ? "true" : "false", - default_only ? "true" : "false", NULL }; + is_tls ? "true" : "false", NULL }; CLEANUP_FREE char *s = NULL; size_t slen; diff --git a/filters/log/log.c b/filters/log/log.c index 0935c16a..67a3d904 100644 --- a/filters/log/log.c +++ b/filters/log/log.c @@ -248,16 +248,16 @@ output_return (struct handle *h, const char *act, uint64_t id, int r, int *err) /* List exports. */ static int log_list_exports (nbdkit_next_list_exports *next, void *nxdata, - int readonly, int default_only, + int readonly, int is_tls, struct nbdkit_exports *exports) { static uint64_t id; int r; int err; - output (NULL, "ListExports", ++id, "readonly=%d default_only=%d ...", - readonly, default_only); - r = next (nxdata, readonly, default_only, exports); + output (NULL, "ListExports", ++id, "readonly=%d tls=%d ...", + readonly, is_tls); + r = next (nxdata, readonly, exports); if (r == -1) { err = errno; output_return (NULL, "...ListExports", id, r, &err); diff --git a/filters/tls-fallback/tls-fallback.c b/filters/tls-fallback/tls-fallback.c index 0fcc2bcf..c1e549df 100644 --- a/filters/tls-fallback/tls-fallback.c +++ b/filters/tls-fallback/tls-fallback.c @@ -74,7 +74,15 @@ tls_fallback_get_ready (nbdkit_next_get_ready *next, void *nxdata, return next (nxdata); } -/* TODO: list_exports needs is_tls parameter */ +static int +tls_fallback_list_exports (nbdkit_next_list_exports *next, void *nxdata, + int readonly, int is_tls, + struct nbdkit_exports *exports) +{ + if (!is_tls) + return nbdkit_add_export (exports, "", NULL); + return next (nxdata, readonly, exports); +} static const char * tls_fallback_default_export (nbdkit_next_default_export *next, void *nxdata, @@ -192,7 +200,7 @@ static struct nbdkit_filter filter = { .config = tls_fallback_config, .config_help = tls_fallback_config_help, .get_ready = tls_fallback_get_ready, - /* XXX .list_exports needs is_tls parameter */ + .list_exports = tls_fallback_list_exports, .default_export = tls_fallback_default_export, .open = tls_fallback_open, .get_size = tls_fallback_get_size, diff --git a/tests/test-tls-fallback.sh b/tests/test-tls-fallback.sh index f3c4b14b..86a495bf 100755 --- a/tests/test-tls-fallback.sh +++ b/tests/test-tls-fallback.sh @@ -35,7 +35,7 @@ set -e set -x requires_plugin sh -requires nbdsh -c 'exit(not h.supports_tls())' +requires nbdsh -c 'print(h.set_full_info)' -c 'exit(not h.supports_tls())' # Does the nbdkit binary support TLS? if ! nbdkit --dump-config | grep -sq tls=yes; then @@ -66,9 +66,12 @@ check () { fi } case $1 in - list_exports) echo NAMES; echo hello; echo world ;; + list_exports) check "$3"; echo INTERLEAVED + echo hello; echo world + echo world; echo tour ;; default_export) check "$3"; echo hello ;; open) check "$4"; echo $3 ;; + export_description) echo "=$2=" ;; 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 ;; @@ -79,8 +82,19 @@ EOF # Plaintext client sees only dummy volume nbdsh -c ' import os -h.set_export_name("hello") + +def f(name, desc): + assert name == "" + assert desc == "" + +h.set_opt_mode(True) +h.set_full_info(True) h.connect_unix(os.environ["sock"]) +assert h.opt_list(f) == 1 +h.opt_info() +assert h.get_canonical_export_name() == "" +h.set_export_name("hello") +h.opt_go() assert h.get_size() == 512 assert not h.can_trim() assert h.pread(5, 0) == b"dummy" @@ -89,11 +103,25 @@ assert h.pread(5, 0) == b"dummy" # Encrypted client sees desired volumes nbdsh -c ' import os -h.set_export_name("hello") + +def f(name, desc): + if name == "hello": + assert desc == "world" + elif name == "world": + assert desc == "tour" + else: + assert False + +h.set_opt_mode(True) +h.set_full_info(True) h.set_tls(nbd.TLS_REQUIRE) h.set_tls_psk_file("keys.psk") h.set_tls_username("qemu") h.connect_unix(os.environ["sock"]) +assert h.opt_list(f) == 2 +h.opt_info() +assert h.get_canonical_export_name() == "hello" +h.opt_go() assert h.get_size() == 6 assert h.can_trim() assert h.pread(5, 0) == b"hello" diff --git a/tests/test-layers-filter.c b/tests/test-layers-filter.c index 2d14a37e..fcd13830 100644 --- a/tests/test-layers-filter.c +++ b/tests/test-layers-filter.c @@ -113,11 +113,11 @@ test_layers_filter_preconnect (nbdkit_next_preconnect *next, static int test_layers_filter_list_exports (nbdkit_next_list_exports *next, void *nxdata, - int readonly, int default_only, + int readonly, int is_tls, struct nbdkit_exports *exports) { DEBUG_FUNCTION; - return next (nxdata, readonly, default_only, exports); + return next (nxdata, readonly, exports); } static const char * diff --git a/TODO b/TODO index f6b8c004..c7f2e3a6 100644 --- a/TODO +++ b/TODO @@ -71,12 +71,6 @@ General ideas for improvements continue to keep their non-standard handshake while utilizing nbdkit to prototype new behaviors in serving the kernel. -* The current signature of .list_exports is awkward; it overloads the - list response to NBD_OPT_LIST with the resolution of the default - export name in .open, and it is missing a tls parameter. It is - probably worth splitting out a new .default_export callback for the - latter purpose, as well as fixing the signature for the former. - * 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 -- 2.28.0
Eric Blake
2020-Sep-21 16:23 UTC
[Libguestfs] [nbdkit PATCH v3 06/14] api: Add .export_description
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
Eric Blake
2020-Sep-21 16:23 UTC
[Libguestfs] [nbdkit PATCH v3 07/14] exportname: New filter
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@redhat.com> --- .../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
Eric Blake
2020-Sep-21 16:23 UTC
[Libguestfs] [nbdkit PATCH v3 08/14] filters: Add .export_description wrappers
When extracting an obvious subset of a larger container (ext2, gzip, partition, tar, xz), it's fairly easy to add a nice updated description for what the client is seeing, but only if the original plugin offered a description itself (there's no need to volunteer something if the plugin did not think it worth doing). Signed-off-by: Eric Blake <eblake@redhat.com> --- filters/ext2/ext2.c | 51 ++++++++++++++++++++----------- filters/gzip/gzip.c | 37 +++++++++++++++-------- filters/partition/partition.c | 56 ++++++++++++++++++++++++----------- filters/tar/tar.c | 46 ++++++++++++++++++---------- filters/xz/xz.c | 40 +++++++++++++++++-------- 5 files changed, 155 insertions(+), 75 deletions(-) diff --git a/filters/ext2/ext2.c b/filters/ext2/ext2.c index 75ac2c4c..7e4c0161 100644 --- a/filters/ext2/ext2.c +++ b/filters/ext2/ext2.c @@ -294,6 +294,22 @@ static int ext2_thread_model (void) return NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS; } +/* Description. */ +static const char * +ext2_export_description (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + struct handle *h = handle; + const char *fname = file ?: h->exportname; + const char *slash = fname[0] == '/' ? "" : "/"; + const char *base = next_ops->export_description (nxdata); + + if (!base) + return NULL; + return nbdkit_printf_intern ("embedded '%s%s' from within ext2 image: %s", + slash, fname, base); +} + /* Get the disk size. */ static int64_t ext2_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) @@ -412,23 +428,24 @@ ext2_flush (struct nbdkit_next_ops *next_ops, void *nxdata, */ 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, - .open = ext2_open, - .prepare = ext2_prepare, - .close = ext2_close, - .can_fua = ext2_can_fua, - .can_cache = ext2_can_cache, - .get_size = ext2_get_size, - .pread = ext2_pread, - .pwrite = ext2_pwrite, - .flush = ext2_flush, + .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, + .open = ext2_open, + .prepare = ext2_prepare, + .close = ext2_close, + .can_fua = ext2_can_fua, + .can_cache = ext2_can_cache, + .export_description = ext2_export_description, + .get_size = ext2_get_size, + .pread = ext2_pread, + .pwrite = ext2_pwrite, + .flush = ext2_flush, }; NBDKIT_REGISTER_FILTER(filter) diff --git a/filters/gzip/gzip.c b/filters/gzip/gzip.c index 929260bf..20b533b9 100644 --- a/filters/gzip/gzip.c +++ b/filters/gzip/gzip.c @@ -287,6 +287,18 @@ gzip_can_cache (struct nbdkit_next_ops *next_ops, void *nxdata, return NBDKIT_CACHE_EMULATE; } +/* Description. */ +static const char * +gzip_export_description (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + const char *base = next_ops->export_description (nxdata); + + if (!base) + return NULL; + return nbdkit_printf_intern ("expansion of gzip-compressed image: %s", base); +} + /* Get the file size. */ static int64_t gzip_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, @@ -339,18 +351,19 @@ gzip_pread (struct nbdkit_next_ops *next_ops, void *nxdata, } static struct nbdkit_filter filter = { - .name = "gzip", - .longname = "nbdkit gzip filter", - .unload = gzip_unload, - .thread_model = gzip_thread_model, - .open = gzip_open, - .prepare = gzip_prepare, - .can_write = gzip_can_write, - .can_extents = gzip_can_extents, - .can_cache = gzip_can_cache, - .prepare = gzip_prepare, - .get_size = gzip_get_size, - .pread = gzip_pread, + .name = "gzip", + .longname = "nbdkit gzip filter", + .unload = gzip_unload, + .thread_model = gzip_thread_model, + .open = gzip_open, + .prepare = gzip_prepare, + .can_write = gzip_can_write, + .can_extents = gzip_can_extents, + .can_cache = gzip_can_cache, + .prepare = gzip_prepare, + .export_description = gzip_export_description, + .get_size = gzip_get_size, + .pread = gzip_pread, }; NBDKIT_REGISTER_FILTER(filter) diff --git a/filters/partition/partition.c b/filters/partition/partition.c index 849f0cc7..d897b374 100644 --- a/filters/partition/partition.c +++ b/filters/partition/partition.c @@ -37,6 +37,7 @@ #include <stdint.h> #include <string.h> #include <inttypes.h> +#include <assert.h> #include <nbdkit-filter.h> @@ -83,6 +84,7 @@ partition_config_complete (nbdkit_next_config_complete *next, void *nxdata) struct handle { int64_t offset; int64_t range; + const char *type; }; /* Open a connection. */ @@ -139,13 +141,17 @@ partition_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, /* Is it GPT? */ if (size >= 2 * 34 * SECTOR_SIZE && - memcmp (&lba01[SECTOR_SIZE], "EFI PART", 8) == 0) + memcmp (&lba01[SECTOR_SIZE], "EFI PART", 8) == 0) { r = find_gpt_partition (next_ops, nxdata, size, &lba01[SECTOR_SIZE], &h->offset, &h->range); + h->type = "GPT"; + } /* Is it MBR? */ - else if (lba01[0x1fe] == 0x55 && lba01[0x1ff] == 0xAA) + else if (lba01[0x1fe] == 0x55 && lba01[0x1ff] == 0xAA) { r = find_mbr_partition (next_ops, nxdata, size, lba01, &h->offset, &h->range); + h->type = "MBR"; + } else { nbdkit_error ("disk does not contain MBR or GPT partition table signature"); r = -1; @@ -168,6 +174,21 @@ partition_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, return 0; } +/* Description. */ +static const char * +partition_export_description (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + struct handle *h = handle; + const char *base = next_ops->export_description (nxdata); + + assert (h->type); + if (!base) + return NULL; + return nbdkit_printf_intern ("partition %d of %s disk: %s", partnum, h->type, + base); +} + /* Get the file size. */ static int64_t partition_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, @@ -266,21 +287,22 @@ partition_cache (struct nbdkit_next_ops *next_ops, void *nxdata, } static struct nbdkit_filter filter = { - .name = "partition", - .longname = "nbdkit partition filter", - .config = partition_config, - .config_complete = partition_config_complete, - .config_help = partition_config_help, - .open = partition_open, - .prepare = partition_prepare, - .close = partition_close, - .get_size = partition_get_size, - .pread = partition_pread, - .pwrite = partition_pwrite, - .trim = partition_trim, - .zero = partition_zero, - .extents = partition_extents, - .cache = partition_cache, + .name = "partition", + .longname = "nbdkit partition filter", + .config = partition_config, + .config_complete = partition_config_complete, + .config_help = partition_config_help, + .open = partition_open, + .prepare = partition_prepare, + .close = partition_close, + .export_description = partition_export_description, + .get_size = partition_get_size, + .pread = partition_pread, + .pwrite = partition_pwrite, + .trim = partition_trim, + .zero = partition_zero, + .extents = partition_extents, + .cache = partition_cache, }; NBDKIT_REGISTER_FILTER(filter) diff --git a/filters/tar/tar.c b/filters/tar/tar.c index cb42b918..fa4b61dc 100644 --- a/filters/tar/tar.c +++ b/filters/tar/tar.c @@ -295,6 +295,19 @@ tar_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, return 0; } +/* Description. */ +static const char * +tar_export_description (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + const char *base = next_ops->export_description (nxdata); + + if (!base) + return NULL; + return nbdkit_printf_intern ("embedded %s from within tar file: %s", + entry, base); +} + /* Get the file size. */ static int64_t tar_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, @@ -395,22 +408,23 @@ tar_cache (struct nbdkit_next_ops *next_ops, void *nxdata, } static struct nbdkit_filter filter = { - .name = "tar", - .longname = "nbdkit tar filter", - .config = tar_config, - .config_complete = tar_config_complete, - .config_help = tar_config_help, - .thread_model = tar_thread_model, - .open = tar_open, - .close = tar_close, - .prepare = tar_prepare, - .get_size = tar_get_size, - .pread = tar_pread, - .pwrite = tar_pwrite, - .trim = tar_trim, - .zero = tar_zero, - .extents = tar_extents, - .cache = tar_cache, + .name = "tar", + .longname = "nbdkit tar filter", + .config = tar_config, + .config_complete = tar_config_complete, + .config_help = tar_config_help, + .thread_model = tar_thread_model, + .open = tar_open, + .close = tar_close, + .prepare = tar_prepare, + .export_description = tar_export_description, + .get_size = tar_get_size, + .pread = tar_pread, + .pwrite = tar_pwrite, + .trim = tar_trim, + .zero = tar_zero, + .extents = tar_extents, + .cache = tar_cache, }; NBDKIT_REGISTER_FILTER(filter) diff --git a/filters/xz/xz.c b/filters/xz/xz.c index 26cfa959..72801536 100644 --- a/filters/xz/xz.c +++ b/filters/xz/xz.c @@ -47,6 +47,7 @@ #include "xzfile.h" #include "blkcache.h" +#include "cleanup.h" static uint64_t maxblock = 512 * 1024 * 1024; static uint32_t maxdepth = 8; @@ -156,6 +157,18 @@ xz_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, return 0; } +/* Description. */ +static const char * +xz_export_description (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + const char *base = next_ops->export_description (nxdata); + + if (!base) + return NULL; + return nbdkit_printf_intern ("expansion of xz-compressed image: %s", base); +} + /* Get the file size. */ static int64_t xz_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) @@ -245,19 +258,20 @@ static int xz_thread_model (void) } static struct nbdkit_filter filter = { - .name = "xz", - .longname = "nbdkit XZ filter", - .config = xz_config, - .config_help = xz_config_help, - .thread_model = xz_thread_model, - .open = xz_open, - .close = xz_close, - .prepare = xz_prepare, - .get_size = xz_get_size, - .can_write = xz_can_write, - .can_extents = xz_can_extents, - .can_cache = xz_can_cache, - .pread = xz_pread, + .name = "xz", + .longname = "nbdkit XZ filter", + .config = xz_config, + .config_help = xz_config_help, + .thread_model = xz_thread_model, + .open = xz_open, + .close = xz_close, + .prepare = xz_prepare, + .export_description = xz_export_description, + .get_size = xz_get_size, + .can_write = xz_can_write, + .can_extents = xz_can_extents, + .can_cache = xz_can_cache, + .pread = xz_pread, }; NBDKIT_REGISTER_FILTER(filter) -- 2.28.0
Eric Blake
2020-Sep-21 16:23 UTC
[Libguestfs] [nbdkit PATCH v3 09/14] ext2: Supply .list_exports and .default_export
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@redhat.com> --- 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
Eric Blake
2020-Sep-21 16:23 UTC
[Libguestfs] [nbdkit PATCH v3 10/14] nbd: Implement .default_export, .export_description
As long as we can only connect to one export name of the server, we don't need .list_exports, and our default export name was hard-coded on the command line. With new libnbd 1.4 functionality, we can also report any description that the server handed us. (Of course, limiting ourselves to one export name is boring, so the next patch will take further advantage of libnbd 1.4 to lift that restriction.) Signed-off-by: Eric Blake <eblake@redhat.com> --- plugins/nbd/nbd.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index 34194950..e8ce4124 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -558,6 +558,10 @@ nbdplug_open_handle (int readonly) goto errnbd; if (nbd_add_meta_context (h->nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1) goto errnbd; +#if LIBNBD_HAVE_NBD_SET_FULL_INFO + if (nbd_set_full_info (h->nbd, 1) == -1) + goto errnbd; +#endif if (nbd_set_tls (h->nbd, tls) == -1) goto errnbd; if (tls_certificates && @@ -619,6 +623,13 @@ nbdplug_open_handle (int readonly) return NULL; } +/* Canonical name of default export. */ +static const char * +nbdplug_default_export (int readonly, int is_tls) +{ + return export; +} + /* Create the per-connection handle. */ static void * nbdplug_open (int readonly) @@ -652,6 +663,19 @@ nbdplug_close (void *handle) nbdplug_close_handle (h); } +/* Description. */ +static const char * +nbdplug_export_description (void *handle) +{ +#if LIBNBD_HAVE_NBD_GET_EXPORT_DESCRIPTION + struct handle *h = handle; + CLEANUP_FREE char *desc = nbd_get_export_description (h->nbd); + if (desc) + return nbdkit_strdup_intern (desc); +#endif + return NULL; +} + /* Get the file size. */ static int64_t nbdplug_get_size (void *handle) @@ -947,8 +971,10 @@ static struct nbdkit_plugin plugin = { .magic_config_key = "uri", .after_fork = nbdplug_after_fork, .dump_plugin = nbdplug_dump_plugin, + .default_export = nbdplug_default_export, .open = nbdplug_open, .close = nbdplug_close, + .export_description = nbdplug_export_description, .get_size = nbdplug_get_size, .can_write = nbdplug_can_write, .can_flush = nbdplug_can_flush, -- 2.28.0
Eric Blake
2020-Sep-21 16:23 UTC
[Libguestfs] [nbdkit PATCH v3 11/14] nbd: Add dynamic-export=true option
Now that nbdkit gives us the ability to know what export name the client wants, we can pass that name on to the server. This only works for non-shared connections (in a shared connection, we only connect to the server once, but we can't differentiate content without different connections); and when uri is specified, it requires libnbd 1.4 (as we need opt_mode to override the export name from the uri with the export name from the client). libnbd 1.4 also lets us report the canonical name of the "" export according to the server. With this patch, it will merely change our advertisement from [""] to ["name"], but the next patch will further add .list_exports so that a client can see the same advertisement that the server sent. Signed-off-by: Eric Blake <eblake@redhat.com> --- 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
Eric Blake
2020-Sep-21 16:23 UTC
[Libguestfs] [nbdkit PATCH v3 12/14] nbd: Implement .list_exports
With new-enough libnbd and our new dynamic-export mode, we can grab the export list from the server for replay to the client. Signed-off-by: Eric Blake <eblake@redhat.com> --- plugins/nbd/nbdkit-nbd-plugin.pod | 7 ++ tests/Makefile.am | 2 + plugins/nbd/nbd.c | 53 ++++++++++ tests/test-nbd-dynamic-content.sh | 2 +- tests/test-nbd-dynamic-list.sh | 162 ++++++++++++++++++++++++++++++ 5 files changed, 225 insertions(+), 1 deletion(-) create mode 100755 tests/test-nbd-dynamic-list.sh diff --git a/plugins/nbd/nbdkit-nbd-plugin.pod b/plugins/nbd/nbdkit-nbd-plugin.pod index 55f6fff0..5820ada8 100644 --- a/plugins/nbd/nbdkit-nbd-plugin.pod +++ b/plugins/nbd/nbdkit-nbd-plugin.pod @@ -168,6 +168,13 @@ 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>. +If libnbd is new enough, dynamic export mode is able to advertise the +same exports as listed by the server; C<nbdkit --dump-plugin nbd> will +contain C<libnbd_dynamic_list=1> if this is the case. Regardless of +what this plugin lists, it is also possible to use +L<nbdkit-exportname-filter(1)> to adjust what export names the client +sees or uses as a default. + =item B<tls=off> =item B<tls=on> diff --git a/tests/Makefile.am b/tests/Makefile.am index 2139eb43..c2d668c1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -771,6 +771,7 @@ if HAVE_LIBNBD LIBGUESTFS_TESTS += test-nbd TESTS += \ test-nbd-dynamic-content.sh \ + test-nbd-dynamic-list.sh \ test-nbd-extents.sh \ test-nbd-qcow2.sh \ test-nbd-tls.sh \ @@ -779,6 +780,7 @@ TESTS += \ $(NULL) EXTRA_DIST += \ test-nbd-dynamic-content.sh \ + test-nbd-dynamic-list.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 7389b6d9..488eadb2 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -407,6 +407,11 @@ nbdplug_dump_plugin (void) printf ("libnbd_tls=%d\n", nbd_supports_tls (nbd)); printf ("libnbd_uri=%d\n", nbd_supports_uri (nbd)); printf ("libnbd_vsock=%d\n", USE_VSOCK); +#if LIBNBD_HAVE_NBD_OPT_LIST + printf ("libnbd_dynamic_list=1\n"); +#else + printf ("libnbd_dynamic_list=0\n"); +#endif nbd_close (nbd); } @@ -685,6 +690,53 @@ nbdplug_open_handle (int readonly, const char *client_export) return NULL; } +#if LIBNBD_HAVE_NBD_OPT_LIST +static int +collect_one (void *opaque, const char *name, const char *desc) +{ + struct nbdkit_exports *exports = opaque; + + if (nbdkit_add_export (exports, name, desc) == -1) + nbdkit_debug ("Unable to share export %s: %s", name, nbd_get_error ()); + return 0; +} +#endif /* LIBNBD_HAVE_NBD_OPT_LIST */ + +/* Export list. */ +static int +nbdplug_list_exports (int readonly, int is_tls, struct nbdkit_exports *exports) +{ +#if LIBNBD_HAVE_NBD_OPT_LIST + if (dynamic_export) { + struct nbd_handle *nbd = nbd_create (); + int r = -1; + + if (!nbd) + goto out; + if (nbd_set_opt_mode (nbd, 1) == -1) + goto out; + if (nbdplug_connect (nbd) == -1) + goto out; + if (nbd_opt_list (nbd, (nbd_list_callback) { .callback = collect_one, + .user_data = exports }) == -1) + goto out; + r = 0; + out: + if (r == -1) + nbdkit_error ("Unable to get list: %s", nbd_get_error ()); + if (nbd) { + if (nbd_aio_is_negotiating (nbd)) + nbd_opt_abort (nbd); + else if (nbd_aio_is_ready (nbd)) + nbd_shutdown (nbd, 0); + nbd_close (nbd); + } + return r; + } +#endif + return nbdkit_use_default_export (exports); +} + /* Canonical name of default export. */ static const char * nbdplug_default_export (int readonly, int is_tls) @@ -1068,6 +1120,7 @@ static struct nbdkit_plugin plugin = { .magic_config_key = "uri", .after_fork = nbdplug_after_fork, .dump_plugin = nbdplug_dump_plugin, + .list_exports = nbdplug_list_exports, .default_export = nbdplug_default_export, .open = nbdplug_open, .close = nbdplug_close, diff --git a/tests/test-nbd-dynamic-content.sh b/tests/test-nbd-dynamic-content.sh index 5d848c18..9e0e152c 100755 --- a/tests/test-nbd-dynamic-content.sh +++ b/tests/test-nbd-dynamic-content.sh @@ -35,7 +35,7 @@ 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 +# content. requires_plugin info requires nbdsh --version diff --git a/tests/test-nbd-dynamic-list.sh b/tests/test-nbd-dynamic-list.sh new file mode 100755 index 00000000..419ec9bb --- /dev/null +++ b/tests/test-nbd-dynamic-list.sh @@ -0,0 +1,162 @@ +#!/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 newer libnbd, showing that dynamic mode affects +# export listing. +requires_plugin sh +requires nbdinfo --version + +# Does the nbd plugin support dynamic lists? +if ! nbdkit --dump-plugin nbd | grep -sq libnbd_dynamic_list=1; then + echo "$0: nbd plugin built without dynamic export list support" + exit 77 +fi + +base=test-nbd-dynamic-list +sock1=`mktemp -u` +sock2=`mktemp -u` +pid1="$base.pid1" +pid2="$base.pid2" +files="$sock1 $sock2 $pid1 $pid2 $base.list $base.out1 $base.out2" +rm -f $files +cleanup_fn rm -f $files + +fail=0 + +# Start a long-running server with .list_exports and .default_export +# set to varying contents +start_nbdkit -P $pid1 -U $sock1 eval get_size='echo "$2"|wc -c' \ + open='echo "$3"' list_exports="cat '$PWD/$base.list'" \ + default_export="cat '$PWD/$base.list'" + +# Long-running nbd bridge, which should pass export list through +start_nbdkit -P $pid2 -U $sock2 nbd socket=$sock1 dynamic-export=true + +# check_success_one EXPORT +# - nbdinfo of EXPORT on both servers should succeed, with matching output +check_success_one () +{ + nbdinfo --no-content "nbd+unix:///$1?socket=$sock1" > $base.out1 + nbdinfo --no-content "nbd+unix:///$1?socket=$sock2" > $base.out2 + cat $base.out2 + diff -u $base.out1 $base.out2 +} + +# check_success_list +# - nbdinfo --list on both servers should succeed, with matching output +check_success_list () +{ + nbdinfo --list --json nbd+unix://\?socket=$sock1 > $base.out1 + nbdinfo --list --json nbd+unix://\?socket=$sock2 > $base.out2 + cat $base.out2 + diff -u $base.out1 $base.out2 +} + +# check_success EXPORT... - both sub-tests, on all EXPORTs +check_success() +{ + for exp; do + check_success_one "$exp" + done + check_success_list +} + +# check_fail_one EXPORT +# - nbdinfo of EXPORT on both servers should fail +check_fail_one () +{ + if nbdinfo --no-content "nbd+unix:///$1?socket=$sock1" > $base.out1; then + fail=1 + fi + if nbdinfo --no-content "nbd+unix:///$1?socket=$sock2" > $base.out2; then + fail=1 + fi +} + +# check_fail_list +# - nbdinfo --list on both servers should fail +check_fail_list () +{ + if nbdinfo --list --json nbd+unix://\?socket=$sock1 > $base.out1; then + fail=1 + fi + if nbdinfo --list --json nbd+unix://\?socket=$sock2 > $base.out2; then + fail=1 + fi +} + +# With no file, list_exports and the default export fail, +# but other exports work +check_fail_one "" +check_success_one name +check_fail_list + +# With an empty list, there are 0 exports, and any export works +touch $base.list +check_success "" name + +# An explicit advertisement of the default export, any export works +echo > $base.list +check_success "" name + +# A non-empty default name +echo name > $base.list +check_success "" name + +# Multiple exports, with descriptions +cat > $base.list <<EOF +INTERLEAVED +name1 +desc1 +name2 +desc2 +EOF +echo name > $base.list +check_success "" name1 + +# Longest possible name and description +long=$(printf %04096d 1) +echo NAMES+DESCRIPTIONS > $base.list +echo $long >> $base.list +echo $long >> $base.list +check_success "" $long + +# An invalid name prevents list, but we can still connect +echo 2$long >> $base.list +check_success_one "" +check_fail_list + +exit $fail -- 2.28.0
Eric Blake
2020-Sep-21 16:23 UTC
[Libguestfs] [nbdkit PATCH v3 13/14] python: Implement .list_exports and friends
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@redhat.com> --- 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
Eric Blake
2020-Sep-21 16:23 UTC
[Libguestfs] [nbdkit PATCH v3 14/14] ocaml: Implement .list_exports and friends
Fairly straightforward. A user can supply { "name"; None } or { "name"; Some "description" } as desired. Signed-off-by: Eric Blake <eblake@redhat.com> --- plugins/ocaml/NBDKit.mli | 9 ++++ plugins/ocaml/NBDKit.ml | 17 ++++++++ plugins/ocaml/example.ml | 33 +++++++++----- plugins/ocaml/ocaml.c | 92 +++++++++++++++++++++++++++++++++++++++- 4 files changed, 140 insertions(+), 11 deletions(-) diff --git a/plugins/ocaml/NBDKit.mli b/plugins/ocaml/NBDKit.mli index 3ebbf18f..aaec519c 100644 --- a/plugins/ocaml/NBDKit.mli +++ b/plugins/ocaml/NBDKit.mli @@ -51,6 +51,12 @@ type extent = { } (** The type of the extent list returned by [.extents]. *) +type export = { + name : string; + description : string option; +} +(** The type of the export list returned by [.list_exports]. *) + type thread_model | THREAD_MODEL_SERIALIZE_CONNECTIONS | THREAD_MODEL_SERIALIZE_ALL_REQUESTS @@ -78,10 +84,13 @@ type 'a plugin = { after_fork : (unit -> unit) option; preconnect : (bool -> unit) option; + list_exports : (bool -> bool -> export list) option; + default_export : (bool -> bool -> string) option; open_connection : (bool -> 'a) option; (* required *) close : ('a -> unit) option; get_size : ('a -> int64) option; (* required *) + export_description : ('a -> string) option; can_cache : ('a -> cache_flag) option; can_extents : ('a -> bool) option; diff --git a/plugins/ocaml/NBDKit.ml b/plugins/ocaml/NBDKit.ml index 9ce3bf3e..1823fc71 100644 --- a/plugins/ocaml/NBDKit.ml +++ b/plugins/ocaml/NBDKit.ml @@ -53,6 +53,11 @@ type extent = { is_zero : bool; } +type export = { + name : string; + description : string option; +} + type 'a plugin = { name : string; longname : string; @@ -73,10 +78,13 @@ type 'a plugin = { after_fork : (unit -> unit) option; preconnect : (bool -> unit) option; + list_exports : (bool -> bool -> export list) option; + default_export : (bool -> bool -> string) option; open_connection : (bool -> 'a) option; close : ('a -> unit) option; get_size : ('a -> int64) option; + export_description : ('a -> string) option; can_cache : ('a -> cache_flag) option; can_extents : ('a -> bool) option; @@ -118,10 +126,13 @@ let default_callbacks = { after_fork = None; preconnect = None; + list_exports = None; + default_export = None; open_connection = None; close = None; get_size = None; + export_description = None; can_cache = None; can_extents = None; @@ -162,10 +173,13 @@ external set_get_ready : (unit -> unit) -> unit = "ocaml_nbdkit_set_get_ready" external set_after_fork : (unit -> unit) -> unit = "ocaml_nbdkit_set_after_fork" external set_preconnect : (bool -> unit) -> unit = "ocaml_nbdkit_set_preconnect" +external set_list_exports : (bool -> bool -> export list) -> unit = "ocaml_nbdkit_set_list_exports" +external set_default_export : (bool -> bool -> string) -> unit = "ocaml_nbdkit_set_default_export" external set_open : (bool -> 'a) -> unit = "ocaml_nbdkit_set_open" external set_close : ('a -> unit) -> unit = "ocaml_nbdkit_set_close" external set_get_size : ('a -> int64) -> unit = "ocaml_nbdkit_set_get_size" +external set_export_description : ('a -> string) -> unit = "ocaml_nbdkit_set_export_description" external set_can_cache : ('a -> cache_flag) -> unit = "ocaml_nbdkit_set_can_cache" external set_can_extents : ('a -> bool) -> unit = "ocaml_nbdkit_set_can_extents" @@ -225,10 +239,13 @@ let register_plugin plugin may set_after_fork plugin.after_fork; may set_preconnect plugin.preconnect; + may set_list_exports plugin.list_exports; + may set_default_export plugin.default_export; may set_open plugin.open_connection; may set_close plugin.close; may set_get_size plugin.get_size; + may set_export_description plugin.export_description; may set_can_cache plugin.can_cache; may set_can_extents plugin.can_extents; diff --git a/plugins/ocaml/example.ml b/plugins/ocaml/example.ml index 448de8c4..de493bf2 100644 --- a/plugins/ocaml/example.ml +++ b/plugins/ocaml/example.ml @@ -41,6 +41,13 @@ let ocamlexample_config key value | _ -> failwith (Printf.sprintf "unknown parameter: %s" key) +let ocamlexample_list_exports ro tls : NBDKit.export list + [ { name = "name1"; description = Some "desc1" }; + { name = "name2"; description = None } ] + +let ocamlexample_default_export ro tls + "name1" + (* Any type (even unit) can be used as a per-connection handle. * This is just an example. The same value that you return from * your [open_connection] function is passed back as the first @@ -58,6 +65,9 @@ let ocamlexample_open readonly incr id; { h_id = !id } +let ocamlexample_export_description h + "some description" + let ocamlexample_get_size h Int64.of_int (Bytes.length !disk) @@ -80,20 +90,23 @@ let plugin = { (* name, open_connection, get_size and pread are required, * everything else is optional. *) - NBDKit.name = "ocamlexample"; - version = "1.0"; + NBDKit.name = "ocamlexample"; + version = "1.0"; - load = Some ocamlexample_load; - unload = Some ocamlexample_unload; + load = Some ocamlexample_load; + unload = Some ocamlexample_unload; - config = Some ocamlexample_config; + config = Some ocamlexample_config; - open_connection = Some ocamlexample_open; - get_size = Some ocamlexample_get_size; - pread = Some ocamlexample_pread; - pwrite = Some ocamlexample_pwrite; + list_exports = Some ocamlexample_list_exports; + default_export = Some ocamlexample_default_export; + open_connection = Some ocamlexample_open; + export_description = Some ocamlexample_export_description; + get_size = Some ocamlexample_get_size; + pread = Some ocamlexample_pread; + pwrite = Some ocamlexample_pwrite; - thread_model = Some ocamlexample_thread_model; + thread_model = Some ocamlexample_thread_model; } let () = NBDKit.register_plugin plugin diff --git a/plugins/ocaml/ocaml.c b/plugins/ocaml/ocaml.c index d8b61108..8fb73755 100644 --- a/plugins/ocaml/ocaml.c +++ b/plugins/ocaml/ocaml.c @@ -123,10 +123,13 @@ static value get_ready_fn; static value after_fork_fn; static value preconnect_fn; +static value list_exports_fn; +static value default_export_fn; static value open_fn; static value close_fn; static value get_size_fn; +static value export_description_fn; static value can_cache_fn; static value can_extents_fn; @@ -311,6 +314,65 @@ preconnect_wrapper (int readonly) CAMLreturnT (int, 0); } +static int +list_exports_wrapper (int readonly, int is_tls, struct nbdkit_exports *exports) +{ + CAMLparam0 (); + CAMLlocal2 (rv, v); + + caml_leave_blocking_section (); + + rv = caml_callback2_exn (list_exports_fn, Val_bool (readonly), + Val_bool (is_tls)); + if (Is_exception_result (rv)) { + nbdkit_error ("%s", caml_format_exception (Extract_exception (rv))); + caml_enter_blocking_section (); + CAMLreturnT (int, -1); + } + + /* Convert exports list into calls to nbdkit_add_export. */ + while (rv != Val_emptylist) { + const char *name, *desc = NULL; + + v = Field (rv, 0); /* export struct */ + name = String_val (Field (v, 0)); + if (Is_block (Field (v, 1))) + desc = String_val (Field (Field (v, 1), 0)); + if (nbdkit_add_export (exports, name, desc) == -1) { + caml_enter_blocking_section (); + CAMLreturnT (int, -1); + } + + rv = Field (rv, 1); + } + + caml_enter_blocking_section (); + CAMLreturnT (int, 0); +} + +static const char * +default_export_wrapper (int readonly, int is_tls) +{ + const char *name; + CAMLparam0 (); + CAMLlocal1 (rv); + + caml_leave_blocking_section (); + + rv = caml_callback2_exn (default_export_fn, Val_bool (readonly), + Val_bool (is_tls)); + if (Is_exception_result (rv)) { + nbdkit_error ("%s", caml_format_exception (Extract_exception (rv))); + caml_enter_blocking_section (); + CAMLreturnT (const char *, NULL); + } + + name = nbdkit_strdup_intern (String_val (rv)); + + caml_enter_blocking_section (); + CAMLreturnT (const char *, name); +} + static void * open_wrapper (int readonly) { @@ -358,6 +420,28 @@ close_wrapper (void *h) CAMLreturn0; } +static const char * +export_description_wrapper (void *h) +{ + const char *desc; + CAMLparam0 (); + CAMLlocal1 (rv); + + caml_leave_blocking_section (); + + rv = caml_callback_exn (export_description_fn, *(value *) h); + if (Is_exception_result (rv)) { + nbdkit_error ("%s", caml_format_exception (Extract_exception (rv))); + caml_enter_blocking_section (); + CAMLreturnT (const char *, NULL); + } + + desc = nbdkit_strdup_intern (String_val (rv)); + + caml_enter_blocking_section (); + CAMLreturnT (const char *, desc); +} + static int64_t get_size_wrapper (void *h) { @@ -747,7 +831,7 @@ extents_wrapper (void *h, uint32_t count, uint64_t offset, uint32_t flags, } /* Convert extents list into calls to nbdkit_add_extent. */ - while (rv != Val_int (0)) { + while (rv != Val_emptylist) { uint64_t length; uint32_t type = 0; @@ -856,10 +940,13 @@ SET(get_ready) SET(after_fork) SET(preconnect) +SET(list_exports) +SET(default_export) SET(open) SET(close) SET(get_size) +SET(export_description) SET(can_write) SET(can_flush) @@ -900,10 +987,13 @@ remove_roots (void) REMOVE (after_fork); REMOVE (preconnect); + REMOVE (list_exports); + REMOVE (default_export); REMOVE (open); REMOVE (close); REMOVE (get_size); + REMOVE (export_description); REMOVE (can_cache); REMOVE (can_extents); -- 2.28.0
Richard W.M. Jones
2020-Sep-22 08:00 UTC
Re: [Libguestfs] [nbdkit PATCH v3 00/14] exportname filter
On Mon, Sep 21, 2020 at 11:23:44AM -0500, Eric Blake wrote:> It's been several weeks since I posted v2 (I got distracted by > improving libnbd to better test things, which in turn surfaced some > major memory leak problems in nbdsh that are now fixed). Many of the > patches are minor rebases from v2, with the biggest changes being > fallout from: > > - patch 2: rename nbdkit_add_default_export to nbdkit_use_default_export > - overall: this missed 1.22, so update appropriate documentation > - libnbd's 'nbdinfo --list' differs in behavior between 1.4.0 and 1.4.1 > regarding descriptions, so fix the tests to work for both versionsI guess this refers to patch 6 which doesn't use nbdinfo but uses the Python API instead. Using the Python API is a sensible solution. But I wanted to add that there's no real reason to try for compatibility with nbdinfo 1.4.0 since that has been superseded everywhere that matters by 1.4.1 ... even in RHEL AV 8.3.0 where we will ship something called "libnbd-1.4.0-X.el8" which is really version 1.4.1.> - rebased on top of rewriting test-layers to use libnbd > - rebased on top of putting nbdkit_*_intern() in place already > > I'm probably at the point where it is just worth committing this > series, and dealing with any further changes as followup patches.I agree with this, but I'll take a quick look this morning to see if there are any problems I can spot. You can go ahead and push it now if you want. Thanks, Rich.> Key: > [----] : patches are identical > [####] : number of functional differences between upstream/downstream patch > [down] : patch is downstream-only > The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively > > 001/14:[0059] [FC] 'api: Add .default_export' > 002/14:[down] 'api: Add nbdkit_use_default_export' > 003/14:[0059] [FC] 'server: Respond to NBD_INFO_NAME request' > 004/14:[0021] [FC] 'sh, eval: Implement .default_export' > 005/14:[0036] [FC] 'api: Alter .list_exports' > 006/14:[0032] [FC] 'api: Add .export_description' > 007/14:[0015] [FC] 'exportname: New filter' > 008/14:[0049] [FC] 'filters: Add .export_description wrappers' > 009/14:[0002] [FC] 'ext2: Supply .list_exports and .default_export' > 010/14:[----] [--] 'nbd: Implement .default_export, .export_description' > 011/14:[0010] [FC] 'nbd: Add dynamic-export=true option' > 012/14:[0002] [FC] 'nbd: Implement .list_exports' > 013/14:[0019] [FC] 'python: Implement .list_exports and friends' > 014/14:[0017] [FC] 'ocaml: Implement .list_exports and friends' > > Eric Blake (14): > api: Add .default_export > api: Add nbdkit_use_default_export > server: Respond to NBD_INFO_NAME request > sh, eval: Implement .default_export > api: Alter .list_exports > api: Add .export_description > exportname: New filter > filters: Add .export_description wrappers > ext2: Supply .list_exports and .default_export > nbd: Implement .default_export, .export_description > nbd: Add dynamic-export=true option > nbd: Implement .list_exports > python: Implement .list_exports and friends > ocaml: Implement .list_exports and friends > > docs/nbdkit-filter.pod | 65 +++- > docs/nbdkit-plugin.pod | 112 +++++- > docs/nbdkit-protocol.pod | 14 + > .../exportname/nbdkit-exportname-filter.pod | 154 ++++++++ > filters/ext2/nbdkit-ext2-filter.pod | 6 + > filters/log/nbdkit-log-filter.pod | 2 +- > plugins/eval/nbdkit-eval-plugin.pod | 6 + > plugins/file/nbdkit-file-plugin.pod | 9 +- > plugins/nbd/nbdkit-nbd-plugin.pod | 30 +- > plugins/python/nbdkit-python-plugin.pod | 25 ++ > plugins/sh/nbdkit-sh-plugin.pod | 39 +- > include/nbdkit-common.h | 2 + > include/nbdkit-filter.h | 13 +- > include/nbdkit-plugin.h | 4 +- > configure.ac | 2 + > filters/exportname/Makefile.am | 67 ++++ > tests/Makefile.am | 31 +- > server/internal.h | 15 +- > server/backend.c | 93 +++-- > server/exports.c | 32 +- > server/filters.c | 31 +- > server/nbdkit.syms | 1 + > server/plugins.c | 33 +- > server/protocol-handshake-newstyle.c | 88 ++++- > server/test-public.c | 9 +- > plugins/ocaml/NBDKit.mli | 9 + > plugins/sh/methods.h | 2 + > plugins/ocaml/NBDKit.ml | 17 + > plugins/ocaml/example.ml | 33 +- > plugins/cc/cc.c | 73 ++-- > plugins/eval/eval.c | 70 ++-- > plugins/nbd/nbd.c | 250 +++++++++++-- > plugins/ocaml/ocaml.c | 92 ++++- > plugins/ondemand/ondemand.c | 11 +- > plugins/python/python.c | 185 ++++++++-- > plugins/sh/methods.c | 84 ++++- > plugins/sh/sh.c | 70 ++-- > plugins/sh/example.sh | 2 +- > filters/exportname/exportname.c | 342 ++++++++++++++++++ > filters/ext2/ext2.c | 122 +++++-- > filters/gzip/gzip.c | 37 +- > filters/log/log.c | 8 +- > filters/partition/partition.c | 56 ++- > filters/tar/tar.c | 46 ++- > filters/tls-fallback/tls-fallback.c | 60 ++- > filters/xz/xz.c | 40 +- > tests/test-eval-exports.sh | 41 ++- > tests/test-export-info.sh | 111 ++++++ > tests/test-exportname.sh | 193 ++++++++++ > tests/test-ext2-exportname.sh | 73 ++++ > tests/test-nbd-dynamic-content.sh | 75 ++++ > tests/test-nbd-dynamic-list.sh | 162 +++++++++ > tests/test-python-export-list.sh | 71 ++++ > tests/test-tls-fallback.sh | 53 ++- > tests/python-export-list.py | 59 +++ > tests/test-layers-filter.c | 13 +- > tests/test-layers-plugin.c | 8 + > tests/test-layers.c | 12 + > TODO | 20 +- > 59 files changed, 2950 insertions(+), 433 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-export-info.sh > create mode 100755 tests/test-exportname.sh > create mode 100755 tests/test-ext2-exportname.sh > create mode 100755 tests/test-nbd-dynamic-content.sh > create mode 100755 tests/test-nbd-dynamic-list.sh > create mode 100755 tests/test-python-export-list.sh > create mode 100644 tests/python-export-list.py > > -- > 2.28.0 > > _______________________________________________ > Libguestfs mailing list > Libguestfs@redhat.com > https://www.redhat.com/mailman/listinfo/libguestfs-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Richard W.M. Jones
2020-Sep-22 10:07 UTC
Re: [Libguestfs] [nbdkit PATCH v3 03/14] server: Respond to NBD_INFO_NAME request
On Mon, Sep 21, 2020 at 11:23:47AM -0500, Eric Blake wrote:> 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"the the" Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Richard W.M. Jones
2020-Sep-22 10:34 UTC
Re: [Libguestfs] [nbdkit PATCH v3 06/14] api: Add .export_description
On Mon, Sep 21, 2020 at 11:23:50AM -0500, Eric Blake wrote:> +=item C<NBD_INFO_NAME>, C<NBD_INFO_DESCRIPTION> > + > +Supported in nbdkit E<ge> 1.22.0.Should be something like 1.24 or 1.23.x ? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Richard W.M. Jones
2020-Sep-22 11:04 UTC
Re: [Libguestfs] [nbdkit PATCH v3 14/14] ocaml: Implement .list_exports and friends
On Mon, Sep 21, 2020 at 11:23:58AM -0500, Eric Blake wrote:> diff --git a/plugins/ocaml/example.ml b/plugins/ocaml/example.ml > index 448de8c4..de493bf2 100644 > --- a/plugins/ocaml/example.ml > +++ b/plugins/ocaml/example.ml > @@ -41,6 +41,13 @@ let ocamlexample_config key value > | _ -> > failwith (Printf.sprintf "unknown parameter: %s" key) > > +let ocamlexample_list_exports ro tls : NBDKit.export list > + [ { name = "name1"; description = Some "desc1" }; > + { name = "name2"; description = None } ] > + > +let ocamlexample_default_export ro tls > + "name1"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
Reasonably Related Threads
- [nbdkit PATCH v2 0/8] exportname filter
- [nbdkit PATCH 0/5] Implement .default_export, nbdkit_string_intern
- [RFC nbdkit PATCH] protocol: Alter .list_exports, add .default_export
- [nbdkit PATCH 0/2] ext2 export list tweaks
- [nbdkit PATCH 0/2] More language bindings for .list_exports