Re: [Libguestfs] [nbdkit PATCH v2 2/8] api: Add nbdkit_add_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(a)redhat.com&gt; --- docs/nbdkit-plugin.pod | 21 +++++++++++++-------- 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 +- tests/test-layers.c | 12 ++++++++++++ 10 files changed, 74 insertions(+), 18 deletions(-) diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 1f208b27..50cf991d 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,12 +695,13 @@ 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, or C<nbdkit_add_default_export> to add a single +entry based on the results of C<.default_export>. int nbdkit_add_export (struct nbdkit_export *exports, const char *name, const char *description); + int nbdkit_add_default_export (struct nbdkit_export *exports); The C<name> must be a non-NULL, UTF-8 string between 0 and 4096 bytes in length. Export names must be unique. C<description> is an @@ -725,7 +727,10 @@ default export C<"">, where the plugin provides a UTF-8 string between connection continues with the empty name; if the plugin returns a valid string, nbdkit returns that name to clients that support it (see the C<NBD_INFO_NAME> response to C<NBD_OPT_GO>), and behaves as if the -client had passed that string instead of an empty name. +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. 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 c377e18d..5e8c0b6f 100644 --- a/include/nbdkit-common.h +++ b/include/nbdkit-common.h @@ -130,6 +130,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_add_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 8c8448e6..e2a68513 100644 --- a/server/internal.h +++ b/server/internal.h @@ -543,4 +543,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 00e65e3c..3e0a1d24 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..3259fbdb 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_add_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 a67669b7..212e36aa 100644 --- a/server/nbdkit.syms +++ b/server/nbdkit.syms @@ -39,6 +39,7 @@ # The functions we want plugins and filters to call. global: nbdkit_absolute_path; + nbdkit_add_default_export; nbdkit_add_export; nbdkit_add_extent; nbdkit_debug; diff --git a/server/plugins.c b/server/plugins.c index 8172759f..da28b2f1 100644 --- a/server/plugins.c +++ b/server/plugins.c @@ -289,7 +289,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_add_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 0149dd9b..ee71f2fc 100644 --- a/server/test-public.c +++ b/server/test-public.c @@ -66,7 +66,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 8e2e4256..2192ae25 100644 --- a/plugins/sh/methods.c +++ b/plugins/sh/methods.c @@ -316,7 +316,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_add_default_export (exports); case ERROR: return -1; diff --git a/tests/test-layers.c b/tests/test-layers.c index db0a2da0..fa7730e6 100644 --- a/tests/test-layers.c +++ b/tests/test-layers.c @@ -331,6 +331,18 @@ main (int argc, char *argv[]) * than open-coding a naive client. */ + /* .default_export methods 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 _______________________________________________ Libguestfs mailing list Libguestfs(a)redhat.com https://www.redhat.com/mailman/listinfo/libguestfs