[Libguestfs] [RFC nbdkit PATCH] protocol: Alter .list_exports, add .default_export
Richard W.M. Jones
rjones at redhat.com
Mon Aug 24 16:16:16 UTC 2020
On Mon, Aug 24, 2020 at 07:52:33AM -0500, Eric Blake wrote:
> 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.
Yes, looks like a good change with a reasonable motive behind it.
Rich.
> 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
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at 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
More information about the Libguestfs
mailing list