7:42 a.m.
Updated proposal, taking into account the default export. Instead of
adding a second call, I made a couple of changes to list_exports:
(1) If the plugin has a concept of a default export, it should add it
as the first element in the exports list.
(2) There is a new default_only flag which tells the plugin that the
client is trying to request the name of the default export, so the
plugin may return only a single element. However it's fine for
plugins to ignore this flag.
What about plugins that don't have any concept of a default export?
This makes the default export be whatever happens to be first in the
list. Not sure if this is a bad thing or not.
Rich.
7:42 a.m.
See also:
https://www.redhat.com/archives/libguestfs/2020-July/msg00090.html
---
docs/nbdkit-plugin.pod | 57 +++++++++++++++++++++++++++++++++++++---
docs/nbdkit-protocol.pod | 7 +++--
2 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index f8e9962a..7ce89e9e 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -152,6 +152,9 @@ the plugin:
│ preconnect │ │
└──────┬─────┘ │
┌──────┴─────┐ │
+ │list_exports│ │
+ └──────┬─────┘ │
+ ┌──────┴─────┐ │
│ open │ │
└──────┬─────┘ │
┌──────┴─────┐ NBD option │
@@ -160,10 +163,10 @@ the plugin:
┌──────┴─────┐ ┌──────┴─────┐ client #2
│ get_size │ │ preconnect │
└──────┬─────┘ └──────┬─────┘
- ┌──────┴─────┐ data ┌──────┴─────┐
- │ pread │ serving │ open │
- └──────┬─────┘↺ └──────┬─────┘
- ┌──────┴─────┐ ...
+ ┌──────┴─────┐ data
+ │ pread │ serving
+ └──────┬─────┘↺ ...
+ ┌──────┴─────┐
│ pwrite │
└──────┬─────┘↺ ┌──────┴─────┐
┌──────┴─────┐ │ close │
@@ -236,6 +239,12 @@ L<getpid(2)>.
Called when a TCP connection has been made to the server. This
happens early, before NBD or TLS negotiation.
+=item C<.list_exports>
+
+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<.open>
A new client has connected and finished the NBD handshake. TLS
@@ -652,6 +661,46 @@ Returning C<0> will allow the connection to continue. If there
is an
error or you want to deny the connection, call C<nbdkit_error> with an
error message and return C<-1>.
+=head2 C<.list_exports>
+
+ int list_exports (int readonly, int default_only,
+ 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.
+
+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.
+
+ int nbdkit_add_export (struct nbdkit_export *exports,
+ const char *name, const char *description);
+
+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
+optional description of the export which some clients can display but
+which is otherwise unused (if you don't want a description, you can
+pass this parameter as C<NULL>). The string(s) are copied into the
+exports list so you may free them immediately after calling this
+function.
+
+If the C<default_only> flag then the client is querying for the name
+of the default export, and the plugin may add 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<readonly> flag informs the plugin that the server was started
+with the I<-r> flag on the command line.
+
+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<.open>
void *open (int readonly);
diff --git a/docs/nbdkit-protocol.pod b/docs/nbdkit-protocol.pod
index e0e4042b..39b9481a 100644
--- a/docs/nbdkit-protocol.pod
+++ b/docs/nbdkit-protocol.pod
@@ -99,12 +99,15 @@ Supported in nbdkit E<ge> 1.1.12, and the default in nbdkit
E<ge> 1.3.
=item export names
Partially supported in nbdkit E<ge> 1.1.12. Support for plugins to
-read the client export name added in nbdkit E<ge> 1.15.2.
+read the client export name added in nbdkit E<ge> 1.15.2. Support for
+C<NBD_OPT_LIST> was added in nbdkit E<ge> 1.21.20.
Versions of nbdkit before 1.16 could advertise a single export name to
clients, specified through the now deprecated I<-e> option. In nbdkit
1.15.2, plugins could read the client requested export name using
-C<nbdkit_export_name()> and serve different content.
+C<nbdkit_export_name()> and serve different content. In nbdkit
+1.21.20, plugins could implement C<.list_exports> to answer
+C<NBD_OPT_LIST> queries.
=item C<NBD_FLAG_NO_ZEROES>
--
2.27.0
9:22 p.m.
On 7/22/20 7:42 AM, Richard W.M. Jones wrote:
See also:
https://www.redhat.com/archives/libguestfs/2020-July/msg00090.html
---
docs/nbdkit-plugin.pod | 57 +++++++++++++++++++++++++++++++++++++---
docs/nbdkit-protocol.pod | 7 +++--
2 files changed, 58 insertions(+), 6 deletions(-)
I'm playing with code for an initial implementation of this, and quickly
ran into a design issue. For plugins, your proposal is straightforward.
But for filters, we have several cases:
1. Filter doesn't care about export name, so it passes through
.list_exports to the plugin and advertises the plugin's name; then it
further passes through the client's export name to .open()
2. Filter is providing its own export names, so it skips the plugin's
.list_exports altogether, and eventually calls .open("") to get the
plugin's default export. But where does the filter get the information
on what exports exist? For example, the tar filter would want to use
next_ops->pread to determine what files are contained within the tar
served by the underlying plugin. Except that .list_exports is called
prior to .open, so we have no plugin handle or next_ops to use for pread.
3. The filter wants to provide a cartesian product of export names (for
each export offered by the plugin, the filter then determines a sublist
of items from that plugin content), which implies multiple .plugin opens
(in this mode, we probably DO want to pursue my proposal in an earlier
thread about enhancing the NBD protocol to add an NBD_OPT_LIST_HIER and
NBD_REP_SERVER_DIR to allow hierarchical traversal rather than flat
listing of every possible export at once).
For 1), the interface for the filter is like any other interface -
provide a hook for filters to call, along with a next_ops to defer into
the plugin. But for 2) I'm thinking we need some way to make it easy to
write a filter that shares a single underlying connection to a plugin
among multiple connections to the plugin; and for 3), that extends even
further into more than one plugin .open for a single filter .list_exports.
For comparison, I'm envisioning something like the current nbd plugin
having a --shared mode; by default (--shared=false), we get:
client1 -> nbdkit nbd .open() -> server connection 1
client2 -> nbdkit nbd .open() -> server connection 2
but with --shared=true, we get:
client1 -\
nbdkit nbd .after_fork() -> single server connection
client2
-/
We need something similar for a filter to be able to request a single
shared .open of the underlying plugin (shared by all connections into
the filter), and which has a lifetime visible between the filter's
.after_fork and .unload regardless of how many .open/.prepare/.close
client connections come and go, rather than having to repeat a
next_ops->open for each filter .open. Kind of the opposite of the
reopen filter (which uses multiple calls into next_ops->open from a
single .open to the filter).
While filters like tar and ext2 will probably benefit from doing
explicit sharing of a single underlying plugin, it would also allow us
to create a new --filter=shared that can be thrown in front of any
plugin (rendering the existing 'nbdkit nbd --shared' obsolete in favor
of 'nbdkit nbd --filter=shared').
Basically, I'm thinking we need the following filter-only functions:
int nbdkit_plugin_open(int readonly, struct nbdkit_next_ops **next_ops,
void **nxdata);
which a filter can call at any point between .after_fork and .unload to
open a distinct connection into the plugin independent of any filter
clients, and which returns by reference a next_ops/nxdata pair that the
filter can then use in place of waiting for a per-connection
next_ops/nxdata passed through .prepare/.can_write/.pread/etc.
void nbdkit_plugin_close (struct nbdkit_next_ops *next_ops,
void *nxdata);
which a filter later calls to close its own connection into the
underlying plugin. Any plugin that uses these new functions would
probably implement a .open that does NOT call next_open(nxdata), because
it instead reuses the shared handle that it opened globally.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
2:41 a.m.
On Wed, Jul 29, 2020 at 09:22:31PM -0500, Eric Blake wrote:
On 7/22/20 7:42 AM, Richard W.M. Jones wrote:
>See also:
>https://www.redhat.com/archives/libguestfs/2020-July/msg00090.html
>---
> docs/nbdkit-plugin.pod | 57 +++++++++++++++++++++++++++++++++++++---
> docs/nbdkit-protocol.pod | 7 +++--
> 2 files changed, 58 insertions(+), 6 deletions(-)
I'm playing with code for an initial implementation of this, and
quickly ran into a design issue. For plugins, your proposal is
straightforward. But for filters, we have several cases:
1. Filter doesn't care about export name, so it passes through
.list_exports to the plugin and advertises the plugin's name; then
it further passes through the client's export name to .open()
2. Filter is providing its own export names, so it skips the
plugin's .list_exports altogether, and eventually calls .open("") to
get the plugin's default export. But where does the filter get the
information on what exports exist? For example, the tar filter
would want to use next_ops->pread to determine what files are
contained within the tar served by the underlying plugin. Except
that .list_exports is called prior to .open, so we have no plugin
handle or next_ops to use for pread.
I envisaged it would need to get this as a filter parameter from the
command line, IOW from the user. (Or I suppose if not supplied, then
assume "").
3. The filter wants to provide a cartesian product of export names
(for each export offered by the plugin, the filter then determines a
sublist of items from that plugin content), which implies multiple
.plugin opens (in this mode, we probably DO want to pursue my
proposal in an earlier thread about enhancing the NBD protocol to
add an NBD_OPT_LIST_HIER and NBD_REP_SERVER_DIR to allow
hierarchical traversal rather than flat listing of every possible
export at once).
This one sounds complicated ...
For 1), the interface for the filter is like any other interface -
provide a hook for filters to call, along with a next_ops to defer
into the plugin. But for 2) I'm thinking we need some way to make
it easy to write a filter that shares a single underlying connection
to a plugin among multiple connections to the plugin; and for 3),
that extends even further into more than one plugin .open for a
single filter .list_exports.
Could use the same mechanism as filter background threads?
This is currently unsolved but see:
https://github.com/libguestfs/nbdkit/blob/a510aae84a7a8d9cd9ead824fbfac8e...
Those are my thoughts so far, will have to come back to this
email to give a more detailed response later.
Rich.
For comparison, I'm envisioning something like the current nbd
plugin having a --shared mode; by default (--shared=false), we get:
client1 -> nbdkit nbd .open() -> server connection 1
client2 -> nbdkit nbd .open() -> server connection 2
but with --shared=true, we get:
client1 -\
> nbdkit nbd .after_fork() -> single server connection
client2 -/
We need something similar for a filter to be able to request a
single shared .open of the underlying plugin (shared by all
connections into the filter), and which has a lifetime visible
between the filter's .after_fork and .unload regardless of how many
.open/.prepare/.close client connections come and go, rather than
having to repeat a next_ops->open for each filter .open. Kind of
the opposite of the reopen filter (which uses multiple calls into
next_ops->open from a single .open to the filter).
While filters like tar and ext2 will probably benefit from doing
explicit sharing of a single underlying plugin, it would also allow
us to create a new --filter=shared that can be thrown in front of
any plugin (rendering the existing 'nbdkit nbd --shared' obsolete in
favor of 'nbdkit nbd --filter=shared').
Basically, I'm thinking we need the following filter-only functions:
int nbdkit_plugin_open(int readonly, struct nbdkit_next_ops **next_ops,
void **nxdata);
which a filter can call at any point between .after_fork and .unload
to open a distinct connection into the plugin independent of any
filter clients, and which returns by reference a next_ops/nxdata
pair that the filter can then use in place of waiting for a
per-connection next_ops/nxdata passed through
.prepare/.can_write/.pread/etc.
void nbdkit_plugin_close (struct nbdkit_next_ops *next_ops,
void *nxdata);
which a filter later calls to close its own connection into the
underlying plugin. Any plugin that uses these new functions would
probably implement a .open that does NOT call next_open(nxdata),
because it instead reuses the shared handle that it opened globally.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
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
4:50 a.m.
[continuing ...]
On Wed, Jul 29, 2020 at 09:22:31PM -0500, Eric Blake wrote:
We need something similar for a filter to be able to request a
single shared .open of the underlying plugin (shared by all
connections into the filter), and which has a lifetime visible
between the filter's .after_fork and .unload regardless of how many
.open/.prepare/.close client connections come and go, rather than
having to repeat a next_ops->open for each filter .open. Kind of
the opposite of the reopen filter (which uses multiple calls into
next_ops->open from a single .open to the filter).
I'm not sure why only "single" & "shared". Why not allow
filters to
open plugin connections on their own as a general mechanism? This
would solve the filter background threads problem, and the list
exports problem too.
This seems to solve the problem without needing to share one plugin,
unless I'm missing some bigger point.
While filters like tar and ext2 will probably benefit from doing
explicit sharing of a single underlying plugin, it would also allow
us to create a new --filter=shared that can be thrown in front of
any plugin (rendering the existing 'nbdkit nbd --shared' obsolete in
favor of 'nbdkit nbd --filter=shared').
Basically, I'm thinking we need the following filter-only functions:
int nbdkit_plugin_open(int readonly, struct nbdkit_next_ops **next_ops,
void **nxdata);
which a filter can call at any point between .after_fork and .unload
to open a distinct connection into the plugin independent of any
filter clients, and which returns by reference a next_ops/nxdata
pair that the filter can then use in place of waiting for a
per-connection next_ops/nxdata passed through
.prepare/.can_write/.pread/etc.
void nbdkit_plugin_close (struct nbdkit_next_ops *next_ops,
void *nxdata);
which a filter later calls to close its own connection into the
underlying plugin. Any plugin that uses these new functions would
probably implement a .open that does NOT call next_open(nxdata),
because it instead reuses the shared handle that it opened globally.
Agreed, except that filters can call this as often as they like.
Not sure this actually solves the list_exports problem however since
presumably list_exports still happens before open so it woudn't appear
in the next_ops struct?
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
2:24 a.m.
On Thu, Jul 30, 2020 at 10:50:13AM +0100, Richard W.M. Jones wrote:
Not sure this actually solves the list_exports problem however since
presumably list_exports still happens before open so it woudn't appear
in the next_ops struct?
Actually I don't think this matters. Plugins ought to be able to
handle .list_exports at any point. They have no "context" to know if
it happened before .open, because there is no handle. (And if it was
a problem, we could just say that plugins must be able to handle
.list_exports at any time).
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
8:08 a.m.
On 7/22/20 7:42 AM, Richard W.M. Jones wrote:
Updated proposal, taking into account the default export. Instead
of
adding a second call, I made a couple of changes to list_exports:
(1) If the plugin has a concept of a default export, it should add it
as the first element in the exports list.
(2) There is a new default_only flag which tells the plugin that the
client is trying to request the name of the default export, so the
plugin may return only a single element. However it's fine for
plugins to ignore this flag.
Works for me. Also, a plugin can implement the callback and set 0
exports, which is different from omitting the callback which always
provides "" as one export. Returning 0 exports may make it harder for
clients to actually make a meaningful connection, but that situation can
indeed make sense (such as the file plugin set to serve all files in a
directory by exportname, but the directory is empty).
What about plugins that don't have any concept of a default export?
This makes the default export be whatever happens to be first in the
list. Not sure if this is a bad thing or not.
I'm not seeing it as too bad of a problem. A client that does not call
NBD_OPT_LIST will never know the difference, and the plugin can still
choose whether to accept or reject "" as a valid export name.
So far, the patch is just about public interface. Implementation-wise,
I see no problem with providing either of two implementations, with the
user none the wiser other than observing memory usage and timing:
idea 1:
struct nbdkit_exports is merely a vector of {name,description} pairs.
During .list_exports, nbdkit_add_export merely strdup's its input onto
the growing list stored in memory, and then after .list_exports returns,
that list is used by the various clients:
- implementing NBD_OPT_LIST traverses the list calling NBD_REP_SERVER
for each list element, then ends with NBD_REP_ACK
- implementing NBD_OPT_INFO("") scrapes the first element out of the
list, and ignores the rest
nbdkit then has to free the list after use.
idea 2:
struct nbdkit_exports wraps a function pointer. nbdkit_add_export then
triggers a call to that underlying function.
- implementing NBD_OPT_LIST sets a function pointer that directly
replies with NBD_REP_SERVER each time it is called (works since
handshake phase is synchronous - there is no other traffic in parallel
on that connection to worry about). When .list_exports returns, all that
is left is sending NBD_REP_ACK
- implementing NBD_OPT_INFO("") sets an initial function pointer that
grabs info on the first call, then alters to a second function pointer
that is a no-op for remaining calls
since there was no copying of strings, there is no further cleanup needed
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
1389
days inactive
1398
days old
6 comments
2 participants
participants (2)
-
Eric Blake -
Richard W.M. Jones