Re: [Libguestfs] [PATCH nbdkit] server: Pass the export name through filter .open calls.

On 7/21/20 3:47 PM, Richard W.M. Jones wrote: >+++ b/server/plugins.c >@@ -278,12 +278,30 @@ plugin_preconnect (struct backend *b, int readonly) > } > static void * >-plugin_open (struct backend *b, int readonly) >+plugin_open (struct backend *b, int readonly, const char *exportname) > { >+ GET_CONN; > struct backend_plugin *p = container_of (b, struct backend_plugin, backend); > assert (p->plugin.open != NULL); >+ /* Save the exportname since the lifetime of the string passed in >+ * here is likely to be brief. In addition this provides a place >+ * for nbdkit_export_name to retrieve it if called from the plugin. >+ * >+ * In API V3 we propose to pass the exportname as an extra parameter >+ * to the (new) plugin.open and deprecate nbdkit_export_name for V3 >+ * users. Even then we will still need to save it in the handle >+ * because of the lifetime issue. >+ */ >+ if (conn->exportname == NULL) { Can't we assert(!conn->exportname) at this point? After all, we only ever call .open at most once per connection.

>+++ b/server/protocol-handshake-newstyle.c >@@ -200,11 +200,29 @@ conn_recv_full (void *buf, size_t len, const char *fmt, ...) > * in that function, and must not cause any wire traffic. > */ > static int >-finish_newstyle_options (uint64_t *exportsize) >+finish_newstyle_options (uint64_t *exportsize, >+ const char *exportname_in, uint32_t exportnamelen) > { > GET_CONN; >- if (protocol_common_open (exportsize, &conn->eflags) == -1) >+ /* Since the exportname string passed here comes directly out of the >+ * NBD protocol make a temporary copy of the exportname into a >+ * \0-terminated buffer. >+ */ >+ CLEANUP_FREE char *exportname = strndup (exportname_in, exportnamelen); >+ if (exportname == NULL) { >+ nbdkit_error ("strndup: %m"); >+ return -1; >+ } >+ >+ if (conn->exportname_from_set_meta_context && >+ strcmp (conn->exportname_from_set_meta_context, exportname) != 0) { >+ debug ("newstyle negotiation: NBD_OPT_SET_META_CONTEXT export name \"%s\" ≠ final client exportname \"%s\", so discarding the previous context", Long line, and I don't know if we use UTF-8 in other debug messages or should stick to straight ascii. Compilation may have a glitch if compiled under a different locale than the end binary runs in, but these days, it's uncommon to find someone running in a single-byte locale instead of UTF-8.

>@@ -148,7 +166,7 @@ ext2_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, > struct ext2_inode inode; > int64_t r; > CLEANUP_FREE char *name = NULL; >- const char *fname = file ?: nbdkit_export_name (); >+ const char *fname = file ?: h->exportname; Hmm - we already pass the same 'readonly' state to filter's .prepare, but not to backend_prepare(), which has to reconstruct it. Would it be easier to also change the signature of backend_prepare() to take both the original readonly and exportname passed to backend_open(), rather than making the filter have to save it off in the filter? It looks like protocol-handshake.c is the only caller, and still has everything in scope at the time.