search for: get_conn

...out); extern int connection_get_status (void); extern int connection_set_status (int value); diff --git a/server/backend.c b/server/backend.c index 616c24d8..9669ada1 100644 --- a/server/backend.c +++ b/server/backend.c @@ -154,7 +154,7 @@ int backend_open (struct backend *b, int readonly) { GET_CONN; - struct b_conn_handle *h = &conn->handles[b->i]; + struct handle *h = get_handle (conn, b->i); controlpath_debug ("%s: open readonly=%d", b->name, readonly); @@ -178,7 +178,7 @@ backend_open (struct backend *b, int readonly) h->state |= HANDLE_OPEN; i...

...internal.h index a1fa7309..1e7b4cf0 100644 --- a/server/internal.h +++ b/server/internal.h @@ -493,6 +493,7 @@ extern int threadlocal_get_error (void); extern void *threadlocal_buffer (size_t size); extern void threadlocal_set_conn (struct connection *conn); extern struct connection *threadlocal_get_conn (void); +#define GET_CONN (threadlocal_get_conn ()) /* Declare program_name. */ #if HAVE_DECL_PROGRAM_INVOCATION_SHORT_NAME == 1 diff --git a/server/public.c b/server/public.c index 418945fe..8fa7e21b 100644 --- a/server/public.c +++ b/server/public.c @@ -533,7 +533,7 @@ nbdkit_nanosleep (unsig...

...f0 100644 > --- a/server/internal.h > +++ b/server/internal.h > @@ -493,6 +493,7 @@ extern int threadlocal_get_error (void); > extern void *threadlocal_buffer (size_t size); > extern void threadlocal_set_conn (struct connection *conn); > extern struct connection *threadlocal_get_conn (void); > +#define GET_CONN (threadlocal_get_conn ()) Do we want any checking, such as whether this is non-NULL? For example, patch 3 has: -typedef void (*connection_close_function) (struct connection *) __attribute__((__nonnull__ (1))); +typedef void (*connection_close_function) (void);...

v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00081.html v2 replaces struct connection *conn = GET_CONN; with GET_CONN; which sets conn implicitly and asserts that it is non-NULL. If we actually want to test if conn is non-NULL or behave differently, then you must use threadlocal_get_conn() instead, and some existing uses do that. Rich.

The third patch is a large but mechanical change which gets rid of passing around struct connection * entirely within the server, preferring instead to reference the connection through thread-local storage. I hope this is a gateway to simplifying other parts of the code. Rich.

...a7ec8 100644 --- a/server/backend.c +++ b/server/backend.c @@ -151,8 +151,9 @@ backend_unload (struct backend *b, void (*unload) (void)) } int -backend_open (struct backend *b, struct connection *conn, int readonly) +backend_open (struct backend *b, int readonly) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; controlpath_debug ("%s: open readonly=%d", b->name, readonly); @@ -166,12 +167,12 @@ backend_open (struct backend *b, struct connection *conn, int readonly) /* Most filters will call next_open first, resulting in...

It's confusing to use the same terminology for a single backend as for the linked list of backends. In particular it's often not clear if we're calling the next backend or the whole chain of backends. --- server/internal.h | 14 ++++++++++-- server/connections.c | 20 ++++++++--------- server/locks.c | 2 +- server/main.c

.../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 &gt...

...f573a1aa0d880b12052f6e1e. --- server/locks.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/locks.c b/server/locks.c index 5d54d311..9269c75e 100644 --- a/server/locks.c +++ b/server/locks.c @@ -93,7 +93,7 @@ unlock_connection (void) void lock_request (void) { - GET_CONN; + struct connection *conn = threadlocal_get_conn (); if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS && pthread_mutex_lock (&all_requests_lock)) @@ -110,7 +110,7 @@ lock_request (void) void unlock_request () { - GET_CONN; + struct connection *conn =...

...+81,13 @@ send_newstyle_option_reply (uint32_t option, uint32_t reply) /* Reply to NBD_OPT_LIST with the plugin's list of export names. */ static int -send_newstyle_option_reply_exportnames (uint32_t option) +send_newstyle_option_reply_exportnames (uint32_t option, size_t *nr_options) { GET_CONN; struct nbd_fixed_new_option_reply fixed_new_option_reply; - size_t i; + size_t i, list_len; CLEANUP_EXPORTS_FREE struct nbdkit_exports *exps = NULL; + int r; exps = nbdkit_exports_new (); if (exps == NULL) @@ -91,7 +95,8 @@ send_newstyle_option_reply_exportnames (uint32_t option)...

...end.c index cf87e35a..d39fdeaf 100644 --- a/server/backend.c +++ b/server/backend.c @@ -152,12 +152,13 @@ backend_unload (struct backend *b, void (*unload) (void)) } int -backend_open (struct backend *b, int readonly) +backend_open (struct backend *b, int readonly, const char *exportname) { GET_CONN; struct handle *h = get_handle (conn, b->i); - controlpath_debug ("%s: open readonly=%d", b->name, readonly); + controlpath_debug ("%s: open readonly=%d exportname=\"%s\"", + b->name, readonly, exportname); assert (h->handle...

...a TCP connection, > >so you can just store it there, unless I'm misunderstanding something. > > Plugins cannot access the TCP connection except through public APIs ;) Sure, plugins can't, but there's some code in the server which you've written which could be doing: GET_CONN; const char *new_name; ... new_name = plugin->default_export (...); if (conn->default_exportname != NULL) free (conn->default_exportname); conn->default_exportname = strdup (new_name); > I've got the patch working (it passes 'make check-valgrind'), so >...

...ld work. > +++ 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 &gt...

...c b/server/locks.c > index ef6726d8..d187b422 100644 > --- a/server/locks.c > +++ b/server/locks.c > @@ -91,8 +91,12 @@ unlock_connection (void) > } > > void > -lock_request (struct connection *conn) > +lock_request (void) > { > + struct connection *conn = GET_CONN; > + > + assert (conn != NULL); > + Here's a site where we could exploit the macro guaranteeing a non-null result. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

...can just store it there, unless I'm misunderstanding something. >> >> Plugins cannot access the TCP connection except through public APIs ;) > > Sure, plugins can't, but there's some code in the server which you've > written which could be doing: > > GET_CONN; > const char *new_name; > ... > new_name = plugin->default_export (...); > if (conn->default_exportname != NULL) > free (conn->default_exportname); > conn->default_exportname = strdup (new_name); So you're suggesting that the signature for .de...

...@@ -281,8 +282,13 @@ static int plugin_list_exports (struct backend *b, int readonly, int default_only, struct nbdkit_exports *exports) { - /* XXX No plugin support yet, so for now just advertise "" */ - return nbdkit_add_export (exports, "", NULL); + GET_CONN; + struct backend_plugin *p = container_of (b, struct backend_plugin, backend); + + if (!p->plugin.list_exports) + return nbdkit_add_export (exports, "", NULL); + + return p->plugin.list_exports (readonly, default_only, exports); } static void * -- 2.28.0

...ame, count); - /* Best effort caching of default export name */ - if (!h->default_exportname && count) - h->default_exportname = strdup (nbdkit_get_export (exports, 0).name); } return r; } +const char * +backend_default_export (struct backend *b, int readonly) +{ + GET_CONN; + struct handle *h = get_handle (conn, b->i); + const char *s; + + controlpath_debug ("%s: default_export readonly=%d tls=%d", + b->name, readonly, conn->using_tls); + + if (h->default_exportname == NULL) { + assert (h->handle == NULL); + asse...

This takes Rich's API proposal and starts fleshing it out with enough meat that I was able to test 'nbdkit eval' advertising multiple exports with descriptions paired with 'qemu-nbd --list'. Eric Blake (3): server: Add exports list functions server: Prepare to use export list from plugin sh, eval: Add .list_exports support Richard W.M. Jones (1): server: Implement

...2_t reply, return 0; } +/* Can be used for NBD_INFO_NAME and NBD_INFO_DESCRIPTION. */ +static int +send_newstyle_option_reply_info_str (uint32_t option, uint32_t reply, + uint16_t info, const char *str, + size_t len) +{ + GET_CONN; + struct nbd_fixed_new_option_reply fixed_new_option_reply; + struct nbd_fixed_new_option_reply_info_name_or_desc name; + + if (len == -1) + len = strlen (str); + assert (len <= NBD_MAX_STRING); + + fixed_new_option_reply.magic = htobe64 (NBD_REP_MAGIC); + fixed_new_option_reply.option...

...t backend *, void *handle); void (*close) (struct backend *, void *handle); diff --git a/server/backend.c b/server/backend.c index 75ca53be..8f4fed9d 100644 --- a/server/backend.c +++ b/server/backend.c @@ -186,8 +186,8 @@ backend_open (struct backend *b, int readonly, const char *exportname) GET_CONN; struct handle *h = get_handle (conn, b->i); - controlpath_debug ("%s: open readonly=%d exportname=\"%s\"", - b->name, readonly, exportname); + controlpath_debug ("%s: open readonly=%d exportname=\"%s\" tls=%d", +...