On 8/24/20 7:52 AM, Eric Blake wrote:
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.
+++ b/docs/nbdkit-plugin.pod
+=head2 C<.default_export>
+
+ const char *default_export (int readonly, int is_tls);
Oh fun. For some plugins (like ondemand), this is trivial: return a
compile-time constant string. But for others (like sh and eval),
there's a lifetime issue: this callback is used _before_ .open, ergo
there is no handle struct that it can be associated with. What's more,
this is called _after_ .preconnect, which means it is logical to expect
that the default export name might change over time (consider a plugin
that advertises the largest file in a directory as its default, but
where the directory can change _which_ file is largest between when the
first client connects and when the second client connects). And the
string returned by the sh script is in malloc'd memory (by it's very
nature of coming from the user script, rather than being a compile-time
constant). Without a handle to store this string in, we would have a
memory leak: there is no way to associate this inside the handle's
struct so that .close can reclaim it, but storing it globally is not
thread-safe to parallel client connections. So I'm thinking I need to
add a helper function:
const char *nbdkit_string_intern (const char *str);
Return a pointer to a copy of str, where nbdkit owns the lifetime of the
copy (allowing the caller to not have to worry about persisting str
indefinitely). If called when there is no client connection (such as
during .load), the copy will remain valid through .unload; if called in
the context of a client connection (any callback from .preconnect
through .close), the copy will remain valid through .close.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization:
qemu.org |
libvirt.org