[Libguestfs] Extending nbdkit to support listing exports

[Richard W.M. Jones]

On Tue, Jul 21, 2020 at 09:04:04AM -0500, Eric Blake wrote:
> On 7/21/20 7:46 AM, Richard W.M. Jones wrote:
> >(b) Listing exports with NBD_OPT_LIST
> >-------------------------------------
> >
> >I believe we need a new .list_exports() callback for filters and
> >plugins to handle this case.  I'm not completely clear on the API, but
> >it could be something as simple as:
> >
> >   const char **list_exports (void);
> >
> >returning a NULL-terminated list of strings.
> 
> But what is the lifetime on those strings?  Can the list change over
> time (that is, if I'm running 'nbdkit file dir=base', does it rerun
> opendir()/stat()/closedir() to populate a fresh list for each
> client, or does it only populate the list once, regardless of later
> underlying changes in the directory hierarchy it is wrapping)?
> Returning const char ** means the plugin has to manage the memory
> (the server will not touch anything), so whether the plugin computes
> the list up-front (compute the list during .load, free it during
> .unload) or per-client (compute the list during .preconnect or
> .list_exports, free during .close) should be reasonable.

Yeah this wasn't very well specified.  As a general principle we
should do whatever is easiest for the plugin to implement, even if
that means extra complexity / copying in the server.

> Thus, I'm assuming the sequence would be:
> 
> client connects
>   .preconnect
> if client calls NBD_OPT_LIST
>   .list_exports
> if client calls NBD_OPT_INFO or NBD_OPT_GO
>   .open
>   .get_size
>   .can_FOO...
> client disconnects
>   .close
> 
> that is, .preconnect always corresponds to the initial client
> connection before NBD_OPT_ handling, and .open corresponds to the
> point when the client requests a specific export, but .close can
> gracefully clean up whatever per-client results it generated for any
> client that actually asked for a listing.  It also gives us the
> flexibility that a single client can both obtain a listing and
> choose which export to then request, rather than having to do two
> separate connections (one for the listing, the second specifying a
> specific export).

We may need to consider this case for libnbd too.  Unfortunately it's
rather difficult to implement.

> I was wondering if we should instead have the core server handle
> memory, similar to how we handle .exports (that is, the server

[I think you meant "extents"]

> allocates a container, passes it to the plugin, and the plugin calls
> nbdkit_export_list_add("...") as many times as it wants, where the
> server now frees the list instead of throwing the burden on the
> plugin).  Offhand, I'm thinking either approach could be made to
> work, so it becomes a decision on which is easier to maintain (both
> from the perspective of the core server code, and for an arbitrary
> plugin writer).

Yes this sounds better.  It was probably the reason why extents are
implemented this way.

> >Is it conceivable we will need additional fields in future?
> 
> For now, only one field - a description string.  That's all the more
> that NBD_REP_SERVER supports, but 'qemu-nbd -D' is an example of
> setting it.

Ah I didn't realise there existed a server which supported this.  We
might need to consider implementing this for libnbd too.

> If my idea of adding NBD_OPT_INFO_HIER and NBD_REP_SERVER_DIR goes
> anywhere, that may be an opportunity to pass other information as
> well.
> 
> >
> >Plugins which do not implement this would be assumed to return a
> >single export "" (or perhaps the -e parameter??)
> 
> Both sound fine.  In turn, what happens if -e is in use when a
> plugin does have .list_exports?  Do we just ignore -e in that case?

The -e option was a bit misguided and has never done anything
important.  I believe it would be a good move to deprecate this option
right away.

By commenting it out I can see the only things it does at the moment are:

 - Act as the default reply in our current implementation of
   NBD_OPT_LIST.  We would replace this implementation under the
   current proposal anyway.

 - Passed through as $exportname to the --run script, where it is
   effectively useless.  Export names are per-connection, not
   per-server, and the true export name for plugins which support them
   is not and cannot ever be passed to this script.

 - Be incompatible with the -o option for no particularly strong
   reason, but nominally because the oldstyle protocol doesn't support
   export names.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/