Eric Blake
2020-Jul-30 02:22 UTC
Re: [Libguestfs] [PATCH nbdkit v2] PROPOSED: server: Implement list_exports.
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
Richard W.M. Jones
2020-Jul-30 07:41 UTC
Re: [Libguestfs] [PATCH nbdkit v2] PROPOSED: server: Implement list_exports.
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/a510aae84a7a8d9cd9ead824fbfac8e0000e85b5/TODO#L71 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
Richard W.M. Jones
2020-Jul-30 09:50 UTC
Re: [Libguestfs] [PATCH nbdkit v2] PROPOSED: server: Implement list_exports.
[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
Richard W.M. Jones
2020-Jul-31 07:24 UTC
Re: [Libguestfs] [PATCH nbdkit v2] PROPOSED: server: Implement list_exports.
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
Reasonably Related Threads
- Re: [PATCH nbdkit v2] PROPOSED: server: Implement list_exports.
- [nbdkit PATCH v3 06/14] api: Add .export_description
- [PATCH nbdkit v2] PROPOSED: server: Implement list_exports.
- [nbdkit PATCH] filters: Remove most next_* wrappers
- [nbdkit PATCH 1/5] api: Add .default_export