I'm about to add an 'exportname' filter, and in the process, I noticed a few shortcomings in our API. Time to fix those before the 1.22 release locks our API in stone. First, .list_exports needs to know if it is pre- or post-TLS, as that may affect which names are exported. Next, overloading .list_exports to do both NBD_OPT_LIST and mapping "" to a canonical name is proving to be awkward; the canonical mapping is only needed during an NBD_INFO_NAME response to NBD_OPT_GO, and making .open try to grab the entire .list_exports list just to use only the first entry (even if the plugin optimized based on the bool to only provide one entry) is awkward, compared to just having a dedicated function. Finally, as long as we are going to support NBD_INFO_NAME, we can also support NBD_INFO_DESCRIPTION; but while we map "" to a canonical name prior to calling .open, getting the description makes more sense after the connection is established, alongside .get_size. --- I obviously need to finish the code to go with this, but here's where I would like to see the API before we finalize the 1.22 release. docs/nbdkit-filter.pod | 41 +++++++++++++++++++---- docs/nbdkit-plugin.pod | 74 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 98 insertions(+), 17 deletions(-) diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod index bf01db14..f15bdd96 100644 --- a/docs/nbdkit-filter.pod +++ b/docs/nbdkit-filter.pod @@ -339,12 +339,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: @@ -355,8 +361,8 @@ 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); - next_list_exports (nxdata, readonly, default_only, exports); + exports2 = nbdkit_exports_new (); + 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); @@ -376,11 +382,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 @@ -408,6 +412,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, @@ -527,6 +545,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 5c641e83..ecb6bba8 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 @@ -664,15 +670,16 @@ 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 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 reply is determined by the +result of C<.default_export>, which in turn defaults to advertising +the name C<"">. The NBD protocol defines C<""> as the default export, +so this default 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 @@ -681,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 not requires 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> to add a single export to the @@ -709,6 +716,37 @@ 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<"">. If the plugin does not supply this callback, +the connection continues with the empty name; if the plugin returns +a 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. 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 +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 +not requires TLS, be careful that a default export name does not leak +unintended information. + +If the plugin returns C<NULL>, 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); @@ -769,6 +807,22 @@ 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 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>. + =head2 C<.can_write> int can_write (void *handle); -- 2.28.0