[Libguestfs] [RFC nbdkit PATCH] protocol: Alter .list_exports, add .default_export
Eric Blake
eblake at redhat.com
Mon Aug 24 12:52:33 UTC 2020
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
More information about the Libguestfs
mailing list