9:39 a.m.
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/