Libguestfs - Feb 2020 - Re: [PATCH nbdkit 3/3] server: filters: Remove struct b_h.

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                        | 32 ++++++++++++++--------------
 server/plugins.c                     |  2 +-
 server/protocol-handshake-newstyle.c |  8 +++----
 server/protocol-handshake.c          | 28 ++++++++++++------------
 server/protocol.c                    | 18 ++++++++--------
 8 files changed, 67 insertions(+), 57 deletions(-)

diff --git a/server/internal.h b/server/internal.h
index 45ac60ad..c3622671 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -118,8 +118,18 @@ extern char *unixsocket;
 extern const char *user, *group;
 extern bool verbose;
 
-extern struct backend *backend;
-#define for_each_backend(b) for (b = backend; b != NULL; b = b->next)
+/* Linked list of backends.  Each backend struct is followed by either
+ * a filter or plugin struct.  "top" points to the first one.  They
+ * are linked through the backend->next field.
+ *
+ *         ┌──────────┐    ┌──────────┐    ┌──────────┐
+ * top ───▶│ backend  │───▶│ backend  │───▶│ backend  │
+ *         │ b->i = 2 │    │ b->i = 1 │    │ b->i = 0 │
+ *         │ filter   │    │ filter   │    │ plugin   │
+ *         └──────────┘    └──────────┘    └──────────┘
+ */
+extern struct backend *top;
+#define for_each_backend(b) for (b = top; b != NULL; b = b->next)
 
 /* quit.c */
 extern volatile int quit;
diff --git a/server/connections.c b/server/connections.c
index a2049325..7e9584b3 100644
--- a/server/connections.c
+++ b/server/connections.c
@@ -146,23 +146,23 @@ handle_single_connection (int sockin, int sockout)
 
   lock_connection ();
 
-  if (backend->thread_model (backend) < NBDKIT_THREAD_MODEL_PARALLEL ||
+  if (top->thread_model (top) < NBDKIT_THREAD_MODEL_PARALLEL ||
       nworkers == 1)
     nworkers = 0;
   conn = new_connection (sockin, sockout, nworkers);
   if (!conn)
     goto done;
 
-  /* NB: because of an asynchronous exit backend can be set to NULL at
+  /* NB: because of an asynchronous exit top can be set to NULL at
    * just about any time.
    */
-  if (backend)
-    plugin_name = backend->plugin_name (backend);
+  if (top)
+    plugin_name = top->plugin_name (top);
   else
     plugin_name = "(unknown)";
   threadlocal_set_name (plugin_name);
 
-  if (backend && backend->preconnect (backend, read_only) == -1)
+  if (top && top->preconnect (top, read_only) == -1)
     goto done;
 
   /* NBD handshake.
@@ -225,7 +225,7 @@ handle_single_connection (int sockin, int sockout)
 
   /* Finalize (for filters), called just before close. */
   lock_request ();
-  r = backend_finalize (backend);
+  r = backend_finalize (top);
   unlock_request ();
   if (r == -1)
     goto done;
@@ -251,12 +251,12 @@ new_connection (int sockin, int sockout, int nworkers)
 
   conn->status_pipe[0] = conn->status_pipe[1] = -1;
 
-  conn->handles = calloc (backend->i + 1, sizeof *conn->handles);
+  conn->handles = calloc (top->i + 1, sizeof *conn->handles);
   if (conn->handles == NULL) {
     perror ("malloc");
     goto error;
   }
-  conn->nr_handles = backend->i + 1;
+  conn->nr_handles = top->i + 1;
   for_each_backend (b)
     reset_b_conn_handle (&conn->handles[b->i]);
 
@@ -277,7 +277,7 @@ new_connection (int sockin, int sockout, int nworkers)
      * we aren't accepting until the plugin is not running, making
      * non-atomicity okay.
      */
-    assert (backend->thread_model (backend) <+    assert
(top->thread_model (top) <            
NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS);
     lock_request (NULL);
     if (pipe (conn->status_pipe)) {
@@ -354,7 +354,7 @@ free_connection (struct connection *conn)
    */
   if (!quit) {
     lock_request ();
-    backend_close (backend);
+    backend_close (top);
     unlock_request ();
   }
 
diff --git a/server/locks.c b/server/locks.c
index f005710d..6211648d 100644
--- a/server/locks.c
+++ b/server/locks.c
@@ -69,7 +69,7 @@ name_of_thread_model (int model)
 void
 lock_init_thread_model (void)
 {
-  thread_model = backend->thread_model (backend);
+  thread_model = top->thread_model (top);
   debug ("using thread model: %s", name_of_thread_model
(thread_model));
 }
 
diff --git a/server/main.c b/server/main.c
index 550a8714..dbeca624 100644
--- a/server/main.c
+++ b/server/main.c
@@ -102,8 +102,8 @@ bool verbose;                   /* -v */
 bool vsock;                     /* --vsock */
 unsigned int socket_activation  /* $LISTEN_FDS and $LISTEN_PID set */;
 
-/* The currently loaded plugin. */
-struct backend *backend;
+/* The linked list of zero or more filters, and one plugin. */
+struct backend *top;
 
 static char *random_fifo_dir = NULL;
 static char *random_fifo = NULL;
@@ -555,10 +555,10 @@ main (int argc, char *argv[])
 
   /* Open the plugin (first) and then wrap the plugin with the
    * filters.  The filters are wrapped in reverse order that they
-   * appear on the command line so that in the end ‘backend’ points to
+   * appear on the command line so that in the end ‘top’ points to
    * the first filter on the command line.
    */
-  backend = open_plugin_so (0, filename, short_name);
+  top = open_plugin_so (0, filename, short_name);
   i = 1;
   while (filter_filenames) {
     struct filter_filename *t = filter_filenames;
@@ -566,7 +566,7 @@ main (int argc, char *argv[])
     filename = t->filename;
     short_name = is_short_name (filename);
 
-    backend = open_filter_so (backend, i++, filename, short_name);
+    top = open_filter_so (top, i++, filename, short_name);
 
     filter_filenames = t->next;
     free (t);
@@ -586,7 +586,7 @@ main (int argc, char *argv[])
       printf ("\n");
       b->usage (b);
     }
-    backend->free (backend);
+    top->free (top);
     exit (EXIT_SUCCESS);
   }
 
@@ -601,7 +601,7 @@ main (int argc, char *argv[])
         printf (" %s", v);
       printf ("\n");
     }
-    backend->free (backend);
+    top->free (top);
     exit (EXIT_SUCCESS);
   }
 
@@ -615,16 +615,16 @@ main (int argc, char *argv[])
    * first parameter is bare it is prefixed with the key "script",
and
    * any other bare parameters are errors.
    */
-  magic_config_key = backend->magic_config_key (backend);
+  magic_config_key = top->magic_config_key (top);
   for (i = 0; optind < argc; ++i, ++optind) {
     p = strchr (argv[optind], '=');
     if (p && is_config_key (argv[optind], p - argv[optind])) { /*
key=value */
       *p = '\0';
-      backend->config (backend, argv[optind], p+1);
+      top->config (top, argv[optind], p+1);
     }
     else if (magic_config_key == NULL) {
       if (i == 0)               /* magic script parameter */
-        backend->config (backend, "script", argv[optind]);
+        top->config (top, "script", argv[optind]);
       else {
         fprintf (stderr,
                  "%s: expecting key=value on the command line but got:
%s\n",
@@ -633,7 +633,7 @@ main (int argc, char *argv[])
       }
     }
     else {                      /* magic config key */
-      backend->config (backend, magic_config_key, argv[optind]);
+      top->config (top, magic_config_key, argv[optind]);
     }
   }
 
@@ -643,12 +643,12 @@ main (int argc, char *argv[])
    * parameters.
    */
   if (dump_plugin) {
-    backend->dump_fields (backend);
-    backend->free (backend);
+    top->dump_fields (top);
+    top->free (top);
     exit (EXIT_SUCCESS);
   }
 
-  backend->config_complete (backend);
+  top->config_complete (top);
 
   /* Select the correct thread model based on config. */
   lock_init_thread_model ();
@@ -660,8 +660,8 @@ main (int argc, char *argv[])
 
   start_serving ();
 
-  backend->free (backend);
-  backend = NULL;
+  top->free (top);
+  top = NULL;
 
   free (unixsocket);
   free (pidfile);
diff --git a/server/plugins.c b/server/plugins.c
index 9595269c..16b4099b 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -144,7 +144,7 @@ plugin_dump_fields (struct backend *b)
   printf ("max_thread_model=%s\n",
           name_of_thread_model (p->plugin._thread_model));
   printf ("thread_model=%s\n",
-          name_of_thread_model (backend->thread_model (backend)));
+          name_of_thread_model (top->thread_model (top)));
   printf ("errno_is_preserved=%d\n",
!!p->plugin.errno_is_preserved);
   if (p->plugin.magic_config_key)
     printf ("magic_config_key=%s\n", p->plugin.magic_config_key);
diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
index 41b2a6e4..946060ac 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -478,9 +478,9 @@ negotiate_handshake_newstyle_options (void)
          * disconnecting.
          */
         if (finish_newstyle_options (&exportsize) == -1) {
-          if (backend_finalize (backend) == -1)
+          if (backend_finalize (top) == -1)
             return -1;
-          backend_close (backend);
+          backend_close (top);
           if (send_newstyle_option_reply (option, NBD_REP_ERR_UNKNOWN) == -1)
             return -1;
           continue;
@@ -518,9 +518,9 @@ negotiate_handshake_newstyle_options (void)
         return -1;
 
       if (option == NBD_OPT_INFO) {
-        if (backend_finalize (backend) == -1)
+        if (backend_finalize (top) == -1)
           return -1;
-        backend_close (backend);
+        backend_close (top);
       }
 
       break;
diff --git a/server/protocol-handshake.c b/server/protocol-handshake.c
index a32fcde0..70ea4933 100644
--- a/server/protocol-handshake.c
+++ b/server/protocol-handshake.c
@@ -79,14 +79,14 @@ protocol_common_open (uint64_t *exportsize, uint16_t *flags)
   uint16_t eflags = NBD_FLAG_HAS_FLAGS;
   int fl;
 
-  if (backend_open (backend, read_only) == -1)
+  if (backend_open (top, read_only) == -1)
     return -1;
 
   /* Prepare (for filters), called just after open. */
-  if (backend_prepare (backend) == -1)
+  if (backend_prepare (top) == -1)
     return -1;
 
-  size = backend_get_size (backend);
+  size = backend_get_size (top);
   if (size == -1)
     return -1;
   if (size < 0) {
@@ -98,57 +98,57 @@ protocol_common_open (uint64_t *exportsize, uint16_t *flags)
   /* Check all flags even if they won't be advertised, to prime the
    * cache and make later request validation easier.
    */
-  fl = backend_can_write (backend);
+  fl = backend_can_write (top);
   if (fl == -1)
     return -1;
   if (!fl)
     eflags |= NBD_FLAG_READ_ONLY;
 
-  fl = backend_can_zero (backend);
+  fl = backend_can_zero (top);
   if (fl == -1)
     return -1;
   if (fl)
     eflags |= NBD_FLAG_SEND_WRITE_ZEROES;
 
-  fl = backend_can_fast_zero (backend);
+  fl = backend_can_fast_zero (top);
   if (fl == -1)
     return -1;
   if (fl)
     eflags |= NBD_FLAG_SEND_FAST_ZERO;
 
-  fl = backend_can_trim (backend);
+  fl = backend_can_trim (top);
   if (fl == -1)
     return -1;
   if (fl)
     eflags |= NBD_FLAG_SEND_TRIM;
 
-  fl = backend_can_fua (backend);
+  fl = backend_can_fua (top);
   if (fl == -1)
     return -1;
   if (fl)
     eflags |= NBD_FLAG_SEND_FUA;
 
-  fl = backend_can_flush (backend);
+  fl = backend_can_flush (top);
   if (fl == -1)
     return -1;
   if (fl)
     eflags |= NBD_FLAG_SEND_FLUSH;
 
-  fl = backend_is_rotational (backend);
+  fl = backend_is_rotational (top);
   if (fl == -1)
     return -1;
   if (fl)
     eflags |= NBD_FLAG_ROTATIONAL;
 
   /* multi-conn is useless if parallel connections are not allowed. */
-  fl = backend_can_multi_conn (backend);
+  fl = backend_can_multi_conn (top);
   if (fl == -1)
     return -1;
-  if (fl && (backend->thread_model (backend) >
+  if (fl && (top->thread_model (top) >
              NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS))
     eflags |= NBD_FLAG_CAN_MULTI_CONN;
 
-  fl = backend_can_cache (backend);
+  fl = backend_can_cache (top);
   if (fl == -1)
     return -1;
   if (fl)
@@ -159,7 +159,7 @@ protocol_common_open (uint64_t *exportsize, uint16_t *flags)
    * not have to worry about errors, and makes test-layers easier to
    * write.
    */
-  fl = backend_can_extents (backend);
+  fl = backend_can_extents (top);
   if (fl == -1)
     return -1;
 
diff --git a/server/protocol.c b/server/protocol.c
index d41ad569..b56d16bd 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -72,7 +72,7 @@ validate_request (uint16_t cmd, uint16_t flags, uint64_t
offset, uint32_t count,
   case NBD_CMD_TRIM:
   case NBD_CMD_WRITE_ZEROES:
   case NBD_CMD_BLOCK_STATUS:
-    if (!backend_valid_range (backend, offset, count)) {
+    if (!backend_valid_range (top, offset, count)) {
       /* XXX Allow writes to extend the disk? */
       nbdkit_error ("invalid request: %s: offset and count are out of
range: "
                     "offset=%" PRIu64 " count=%" PRIu32,
@@ -238,31 +238,31 @@ handle_request (uint16_t cmd, uint16_t flags, uint64_t
offset, uint32_t count,
 
   switch (cmd) {
   case NBD_CMD_READ:
-    if (backend_pread (backend, buf, count, offset, 0, &err) == -1)
+    if (backend_pread (top, buf, count, offset, 0, &err) == -1)
       return err;
     break;
 
   case NBD_CMD_WRITE:
     if (flags & NBD_CMD_FLAG_FUA)
       f |= NBDKIT_FLAG_FUA;
-    if (backend_pwrite (backend, buf, count, offset, f, &err) == -1)
+    if (backend_pwrite (top, buf, count, offset, f, &err) == -1)
       return err;
     break;
 
   case NBD_CMD_FLUSH:
-    if (backend_flush (backend, 0, &err) == -1)
+    if (backend_flush (top, 0, &err) == -1)
       return err;
     break;
 
   case NBD_CMD_TRIM:
     if (flags & NBD_CMD_FLAG_FUA)
       f |= NBDKIT_FLAG_FUA;
-    if (backend_trim (backend, count, offset, f, &err) == -1)
+    if (backend_trim (top, count, offset, f, &err) == -1)
       return err;
     break;
 
   case NBD_CMD_CACHE:
-    if (backend_cache (backend, count, offset, 0, &err) == -1)
+    if (backend_cache (top, count, offset, 0, &err) == -1)
       return err;
     break;
 
@@ -273,14 +273,14 @@ handle_request (uint16_t cmd, uint16_t flags, uint64_t
offset, uint32_t count,
       f |= NBDKIT_FLAG_FUA;
     if (flags & NBD_CMD_FLAG_FAST_ZERO)
       f |= NBDKIT_FLAG_FAST_ZERO;
-    if (backend_zero (backend, count, offset, f, &err) == -1)
+    if (backend_zero (top, count, offset, f, &err) == -1)
       return err;
     break;
 
   case NBD_CMD_BLOCK_STATUS:
     if (flags & NBD_CMD_FLAG_REQ_ONE)
       f |= NBDKIT_FLAG_REQ_ONE;
-    if (backend_extents (backend, count, offset, f,
+    if (backend_extents (top, count, offset, f,
                          extents, &err) == -1)
       return err;
     break;
@@ -683,7 +683,7 @@ protocol_recv_request_send_reply (void)
 
     /* Allocate the extents list for block status only. */
     if (cmd == NBD_CMD_BLOCK_STATUS) {
-      extents = nbdkit_extents_new (offset, backend_get_size (backend));
+      extents = nbdkit_extents_new (offset, backend_get_size (top));
       if (extents == NULL) {
         error = ENOMEM;
         goto send_reply;
-- 
2.25.0

Also add an inline accessor function (get_handle).

This also removes the unused function connection_get_handle.
---
 server/internal.h    | 24 ++++++++++++++++-----
 server/backend.c     | 50 ++++++++++++++++++++++----------------------
 server/connections.c | 11 +---------
 3 files changed, 45 insertions(+), 40 deletions(-)

diff --git a/server/internal.h b/server/internal.h
index c3622671..9d314bf8 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -170,16 +170,24 @@ typedef int (*connection_send_function) (const void *buf,
size_t len,
   __attribute__((__nonnull__ (1)));
 typedef void (*connection_close_function) (void);
 
+/* struct handle stores data per connection and backend.  Primarily
+ * this is the filter or plugin handle, but other state is also stored
+ * here.
+ *
+ * Use get_handle (conn, 0) to return the struct handle for the
+ * plugin, and get_handle (conn, b->i) to return the struct handle for
+ * the i'th backend (if b->i >= 1 then for a filter).
+ */
 enum {
   HANDLE_OPEN = 1,      /* Set if .open passed, so .close is needed */
   HANDLE_CONNECTED = 2, /* Set if .prepare passed, so .finalize is needed */
   HANDLE_FAILED = 4,    /* Set if .finalize failed */
 };
 
-struct b_conn_handle {
-  void *handle;
+struct handle {
+  void *handle;         /* Plugin or filter handle. */
 
-  unsigned char state; /* Bitmask of HANDLE_* values */
+  unsigned char state;  /* Bitmask of HANDLE_* values */
 
   uint64_t exportsize;
   int can_write;
@@ -195,7 +203,7 @@ struct b_conn_handle {
 };
 
 static inline void
-reset_b_conn_handle (struct b_conn_handle *h)
+reset_handle (struct handle *h)
 {
   h->handle = NULL;
   h->state = 0;
@@ -222,7 +230,7 @@ struct connection {
   void *crypto_session;
   int nworkers;
 
-  struct b_conn_handle *handles;
+  struct handle *handles;       /* One per plugin and filter. */
   size_t nr_handles;
 
   char exportname[NBD_MAX_STRING + 1];
@@ -239,6 +247,12 @@ struct connection {
   connection_close_function close;
 };
 
+static inline struct handle *
+get_handle (struct connection *conn, int i)
+{
+  return &conn->handles[i];
+}
+
 extern void handle_single_connection (int sockin, int sockout);
 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;
   if (b->i) /* A filter must not succeed unless its backend did also */
-    assert (conn->handles[b->i - 1].handle);
+    assert (get_handle (conn, b->i-1)->handle != NULL);
   return 0;
 }
 
@@ -186,7 +186,7 @@ int
 backend_prepare (struct backend *b)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
 
   assert (h->handle);
   assert ((h->state & (HANDLE_OPEN | HANDLE_CONNECTED)) == HANDLE_OPEN);
@@ -209,7 +209,7 @@ int
 backend_finalize (struct backend *b)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
 
   /* Call these in reverse order to .prepare above, starting from the
    * filter furthest away from the plugin, and matching .close order.
@@ -238,7 +238,7 @@ void
 backend_close (struct backend *b)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
 
   /* outer-to-inner order, opposite .open */
   controlpath_debug ("%s: close", b->name);
@@ -249,7 +249,7 @@ backend_close (struct backend *b)
   }
   else
     assert (! (h->state & HANDLE_OPEN));
-  reset_b_conn_handle (h);
+  reset_handle (h);
   if (b->i)
     backend_close (b->next);
 }
@@ -258,7 +258,7 @@ bool
 backend_valid_range (struct backend *b, uint64_t offset, uint32_t count)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
 
   assert (h->exportsize <= INT64_MAX); /* Guaranteed by negotiation phase
*/
   return count > 0 && offset <= h->exportsize &&
@@ -291,7 +291,7 @@ int64_t
 backend_get_size (struct backend *b)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
 
   controlpath_debug ("%s: get_size", b->name);
 
@@ -305,7 +305,7 @@ int
 backend_can_write (struct backend *b)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
 
   controlpath_debug ("%s: can_write", b->name);
 
@@ -319,7 +319,7 @@ int
 backend_can_flush (struct backend *b)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
 
   controlpath_debug ("%s: can_flush", b->name);
 
@@ -333,7 +333,7 @@ int
 backend_is_rotational (struct backend *b)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
 
   controlpath_debug ("%s: is_rotational", b->name);
 
@@ -347,7 +347,7 @@ int
 backend_can_trim (struct backend *b)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
   int r;
 
   controlpath_debug ("%s: can_trim", b->name);
@@ -368,7 +368,7 @@ int
 backend_can_zero (struct backend *b)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
   int r;
 
   controlpath_debug ("%s: can_zero", b->name);
@@ -389,7 +389,7 @@ int
 backend_can_fast_zero (struct backend *b)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
   int r;
 
   controlpath_debug ("%s: can_fast_zero", b->name);
@@ -410,7 +410,7 @@ int
 backend_can_extents (struct backend *b)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
 
   controlpath_debug ("%s: can_extents", b->name);
 
@@ -424,7 +424,7 @@ int
 backend_can_fua (struct backend *b)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
   int r;
 
   controlpath_debug ("%s: can_fua", b->name);
@@ -445,7 +445,7 @@ int
 backend_can_multi_conn (struct backend *b)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
 
   assert (h->handle && (h->state & HANDLE_CONNECTED));
   controlpath_debug ("%s: can_multi_conn", b->name);
@@ -459,7 +459,7 @@ int
 backend_can_cache (struct backend *b)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
 
   controlpath_debug ("%s: can_cache", b->name);
 
@@ -475,7 +475,7 @@ backend_pread (struct backend *b,
                uint32_t flags, int *err)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
   int r;
 
   assert (h->handle && (h->state & HANDLE_CONNECTED));
@@ -496,7 +496,7 @@ backend_pwrite (struct backend *b,
                 uint32_t flags, int *err)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
   bool fua = !!(flags & NBDKIT_FLAG_FUA);
   int r;
 
@@ -520,7 +520,7 @@ backend_flush (struct backend *b,
                uint32_t flags, int *err)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
   int r;
 
   assert (h->handle && (h->state & HANDLE_CONNECTED));
@@ -540,7 +540,7 @@ backend_trim (struct backend *b,
               int *err)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
   bool fua = !!(flags & NBDKIT_FLAG_FUA);
   int r;
 
@@ -566,7 +566,7 @@ backend_zero (struct backend *b,
               int *err)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
   bool fua = !!(flags & NBDKIT_FLAG_FUA);
   bool fast = !!(flags & NBDKIT_FLAG_FAST_ZERO);
   int r;
@@ -601,7 +601,7 @@ backend_extents (struct backend *b,
                  struct nbdkit_extents *extents, int *err)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
   int r;
 
   assert (h->handle && (h->state & HANDLE_CONNECTED));
@@ -632,7 +632,7 @@ backend_cache (struct backend *b,
                uint32_t flags, int *err)
 {
   GET_CONN;
-  struct b_conn_handle *h = &conn->handles[b->i];
+  struct handle *h = get_handle (conn, b->i);
   int r;
 
   assert (h->handle && (h->state & HANDLE_CONNECTED));
diff --git a/server/connections.c b/server/connections.c
index 7e9584b3..d4cdf994 100644
--- a/server/connections.c
+++ b/server/connections.c
@@ -58,15 +58,6 @@ static int raw_send_socket (const void *buf, size_t len, int
flags);
 static int raw_send_other (const void *buf, size_t len, int flags);
 static void raw_close (void);
 
-void *
-connection_get_handle (size_t i)
-{
-  GET_CONN;
-
-  assert (i < conn->nr_handles);
-  return conn->handles[i].handle;
-}
-
 int
 connection_get_status (void)
 {
@@ -258,7 +249,7 @@ new_connection (int sockin, int sockout, int nworkers)
   }
   conn->nr_handles = top->i + 1;
   for_each_backend (b)
-    reset_b_conn_handle (&conn->handles[b->i]);
+    reset_handle (get_handle (conn, b->i));
 
   conn->status = 1;
   conn->nworkers = nworkers;
-- 
2.25.0

This was previously used as ‘nxdata’ and stored a tuple of ’b->next’
and the real filter handle.  However after recent changes we don't
need it.  We can use ‘b->next’ as nxdata, and the handle is passed to
us by the calling functions.

Inspired by Eric Blakes observations in this email:
https://www.redhat.com/archives/libguestfs/2020-February/msg00092.html
---
 server/filters.c | 217 ++++++++++++++++-------------------------------
 1 file changed, 73 insertions(+), 144 deletions(-)

diff --git a/server/filters.c b/server/filters.c
index c916217c..92b0ceb3 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -49,16 +49,6 @@ struct backend_filter {
   struct nbdkit_filter filter;
 };
 
-/* Literally a backend and the filter's handle.
- *
- * This is the implementation of our handle in .open, and serves as
- * a stable ‘void *nxdata’ in the filter API.
- */
-struct b_h {
-  struct backend *b;
-  void *handle;
-};
-
 /* Note this frees the whole chain. */
 static void
 filter_free (struct backend *b)
@@ -186,20 +176,19 @@ filter_config_complete (struct backend *b)
 static int
 next_preconnect (void *nxdata, int readonly)
 {
-  struct b_h *b_h = nxdata;
-  return b_h->b->preconnect (b_h->b, readonly);
+  struct backend *b_next = nxdata;
+  return b_next->preconnect (b_next, readonly);
 }
 
 static int
 filter_preconnect (struct backend *b, int readonly)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h nxdata = { .b = b->next };
 
   debug ("%s: preconnect", b->name);
 
   if (f->filter.preconnect)
-    return f->filter.preconnect (next_preconnect, &nxdata, readonly);
+    return f->filter.preconnect (next_preconnect, b->next, readonly);
   else
     return b->next->preconnect (b->next, readonly);
 }
@@ -216,201 +205,181 @@ plugin_magic_config_key (struct backend *b)
 static int
 next_open (void *nxdata, int readonly)
 {
-  struct b_h *b_h = nxdata;
+  struct backend *b_next = nxdata;
 
-  return backend_open (b_h->b, readonly);
+  return backend_open (b_next, readonly);
 }
 
 static void *
 filter_open (struct backend *b, int readonly)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = malloc (sizeof *nxdata);
-
-  if (!nxdata) {
-    nbdkit_error ("malloc: %m");
-    return NULL;
-  }
-
-  nxdata->b = b->next;
-  nxdata->handle = NULL;
+  void *handle;
 
   /* Most filters will call next_open first, resulting in
    * inner-to-outer ordering.
    */
   if (f->filter.open)
-    nxdata->handle = f->filter.open (next_open, nxdata, readonly);
+    handle = f->filter.open (next_open, b->next, readonly);
   else if (backend_open (b->next, readonly) == -1)
-    nxdata->handle = NULL;
+    handle = NULL;
   else
-    nxdata->handle = NBDKIT_HANDLE_NOT_NEEDED;
-  if (nxdata->handle == NULL) {
-    free (nxdata);
-    return NULL;
-  }
-  return nxdata;
+    handle = NBDKIT_HANDLE_NOT_NEEDED;
+  return handle;
 }
 
 static void
 filter_close (struct backend *b, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  if (handle && f->filter.close) {
-    assert (nxdata->b == b->next);
-    f->filter.close (nxdata->handle);
-    free (nxdata);
-  }
+  if (handle && f->filter.close)
+    f->filter.close (handle);
 }
 
 /* The next_functions structure contains pointers to backend
- * functions.  However because these functions are all expecting a
- * backend and a handle, we cannot call them directly, but must
- * write some next_* functions that unpack the two parameters from a
- * single ‘void *nxdata’ struct pointer (‘b_h’).
+ * functions.  These are only needed for type safety (nxdata is void
+ * pointer, backend_* functions expect a struct backend * parameter).
+ * nxdata is a pointer to the next backend in the linked list.
  */
 
 static int
 next_reopen (void *nxdata, int readonly)
 {
-  struct b_h *b_h = nxdata;
-  return backend_reopen (b_h->b, readonly);
+  struct backend *b_next = nxdata;
+  return backend_reopen (b_next, readonly);
 }
 
 static int64_t
 next_get_size (void *nxdata)
 {
-  struct b_h *b_h = nxdata;
-  return backend_get_size (b_h->b);
+  struct backend *b_next = nxdata;
+  return backend_get_size (b_next);
 }
 
 static int
 next_can_write (void *nxdata)
 {
-  struct b_h *b_h = nxdata;
-  return backend_can_write (b_h->b);
+  struct backend *b_next = nxdata;
+  return backend_can_write (b_next);
 }
 
 static int
 next_can_flush (void *nxdata)
 {
-  struct b_h *b_h = nxdata;
-  return backend_can_flush (b_h->b);
+  struct backend *b_next = nxdata;
+  return backend_can_flush (b_next);
 }
 
 static int
 next_is_rotational (void *nxdata)
 {
-  struct b_h *b_h = nxdata;
-  return backend_is_rotational (b_h->b);
+  struct backend *b_next = nxdata;
+  return backend_is_rotational (b_next);
 }
 
 static int
 next_can_trim (void *nxdata)
 {
-  struct b_h *b_h = nxdata;
-  return backend_can_trim (b_h->b);
+  struct backend *b_next = nxdata;
+  return backend_can_trim (b_next);
 }
 
 static int
 next_can_zero (void *nxdata)
 {
-  struct b_h *b_h = nxdata;
-  return backend_can_zero (b_h->b);
+  struct backend *b_next = nxdata;
+  return backend_can_zero (b_next);
 }
 
 static int
 next_can_fast_zero (void *nxdata)
 {
-  struct b_h *b_h = nxdata;
-  return backend_can_fast_zero (b_h->b);
+  struct backend *b_next = nxdata;
+  return backend_can_fast_zero (b_next);
 }
 
 static int
 next_can_extents (void *nxdata)
 {
-  struct b_h *b_h = nxdata;
-  return backend_can_extents (b_h->b);
+  struct backend *b_next = nxdata;
+  return backend_can_extents (b_next);
 }
 
 static int
 next_can_fua (void *nxdata)
 {
-  struct b_h *b_h = nxdata;
-  return backend_can_fua (b_h->b);
+  struct backend *b_next = nxdata;
+  return backend_can_fua (b_next);
 }
 
 static int
 next_can_multi_conn (void *nxdata)
 {
-  struct b_h *b_h = nxdata;
-  return backend_can_multi_conn (b_h->b);
+  struct backend *b_next = nxdata;
+  return backend_can_multi_conn (b_next);
 }
 
 static int
 next_can_cache (void *nxdata)
 {
-  struct b_h *b_h = nxdata;
-  return backend_can_cache (b_h->b);
+  struct backend *b_next = nxdata;
+  return backend_can_cache (b_next);
 }
 
 static int
 next_pread (void *nxdata, void *buf, uint32_t count, uint64_t offset,
             uint32_t flags, int *err)
 {
-  struct b_h *b_h = nxdata;
-  return backend_pread (b_h->b, buf, count, offset, flags,
-                        err);
+  struct backend *b_next = nxdata;
+  return backend_pread (b_next, buf, count, offset, flags, err);
 }
 
 static int
 next_pwrite (void *nxdata, const void *buf, uint32_t count, uint64_t offset,
              uint32_t flags, int *err)
 {
-  struct b_h *b_h = nxdata;
-  return backend_pwrite (b_h->b, buf, count, offset, flags,
-                         err);
+  struct backend *b_next = nxdata;
+  return backend_pwrite (b_next, buf, count, offset, flags, err);
 }
 
 static int
 next_flush (void *nxdata, uint32_t flags, int *err)
 {
-  struct b_h *b_h = nxdata;
-  return backend_flush (b_h->b, flags, err);
+  struct backend *b_next = nxdata;
+  return backend_flush (b_next, flags, err);
 }
 
 static int
 next_trim (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags,
            int *err)
 {
-  struct b_h *b_h = nxdata;
-  return backend_trim (b_h->b, count, offset, flags, err);
+  struct backend *b_next = nxdata;
+  return backend_trim (b_next, count, offset, flags, err);
 }
 
 static int
 next_zero (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags,
            int *err)
 {
-  struct b_h *b_h = nxdata;
-  return backend_zero (b_h->b, count, offset, flags, err);
+  struct backend *b_next = nxdata;
+  return backend_zero (b_next, count, offset, flags, err);
 }
 
 static int
 next_extents (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags,
               struct nbdkit_extents *extents, int *err)
 {
-  struct b_h *b_h = nxdata;
-  return backend_extents (b_h->b, count, offset, flags,
-                          extents, err);
+  struct backend *b_next = nxdata;
+  return backend_extents (b_next, count, offset, flags, extents, err);
 }
 
 static int
 next_cache (void *nxdata, uint32_t count, uint64_t offset,
             uint32_t flags, int *err)
 {
-  struct b_h *b_h = nxdata;
-  return backend_cache (b_h->b, count, offset, flags, err);
+  struct backend *b_next = nxdata;
+  return backend_cache (b_next, count, offset, flags, err);
 }
 
 static struct nbdkit_next_ops next_ops = {
@@ -439,11 +408,9 @@ static int
 filter_prepare (struct backend *b, void *handle, int readonly)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.prepare &&
-      f->filter.prepare (&next_ops, nxdata, nxdata->handle, readonly)
== -1)
+      f->filter.prepare (&next_ops, b->next, handle, readonly) == -1)
     return -1;
 
   return 0;
@@ -453,11 +420,9 @@ static int
 filter_finalize (struct backend *b, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.finalize &&
-      f->filter.finalize (&next_ops, nxdata, nxdata->handle) == -1)
+      f->filter.finalize (&next_ops, b->next, handle) == -1)
     return -1;
   return 0;
 }
@@ -466,11 +431,9 @@ static int64_t
 filter_get_size (struct backend *b, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.get_size)
-    return f->filter.get_size (&next_ops, nxdata, nxdata->handle);
+    return f->filter.get_size (&next_ops, b->next, handle);
   else
     return backend_get_size (b->next);
 }
@@ -479,11 +442,9 @@ static int
 filter_can_write (struct backend *b, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.can_write)
-    return f->filter.can_write (&next_ops, nxdata, nxdata->handle);
+    return f->filter.can_write (&next_ops, b->next, handle);
   else
     return backend_can_write (b->next);
 }
@@ -492,11 +453,9 @@ static int
 filter_can_flush (struct backend *b, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.can_flush)
-    return f->filter.can_flush (&next_ops, nxdata, nxdata->handle);
+    return f->filter.can_flush (&next_ops, b->next, handle);
   else
     return backend_can_flush (b->next);
 }
@@ -505,11 +464,9 @@ static int
 filter_is_rotational (struct backend *b, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.is_rotational)
-    return f->filter.is_rotational (&next_ops, nxdata,
nxdata->handle);
+    return f->filter.is_rotational (&next_ops, b->next, handle);
   else
     return backend_is_rotational (b->next);
 }
@@ -518,11 +475,9 @@ static int
 filter_can_trim (struct backend *b, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.can_trim)
-    return f->filter.can_trim (&next_ops, nxdata, nxdata->handle);
+    return f->filter.can_trim (&next_ops, b->next, handle);
   else
     return backend_can_trim (b->next);
 }
@@ -531,11 +486,9 @@ static int
 filter_can_zero (struct backend *b, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.can_zero)
-    return f->filter.can_zero (&next_ops, nxdata, nxdata->handle);
+    return f->filter.can_zero (&next_ops, b->next, handle);
   else
     return backend_can_zero (b->next);
 }
@@ -544,11 +497,9 @@ static int
 filter_can_fast_zero (struct backend *b, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.can_fast_zero)
-    return f->filter.can_fast_zero (&next_ops, nxdata,
nxdata->handle);
+    return f->filter.can_fast_zero (&next_ops, b->next, handle);
   else
     return backend_can_fast_zero (b->next);
 }
@@ -557,11 +508,9 @@ static int
 filter_can_extents (struct backend *b, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.can_extents)
-    return f->filter.can_extents (&next_ops, nxdata, nxdata->handle);
+    return f->filter.can_extents (&next_ops, b->next, handle);
   else
     return backend_can_extents (b->next);
 }
@@ -570,11 +519,9 @@ static int
 filter_can_fua (struct backend *b, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.can_fua)
-    return f->filter.can_fua (&next_ops, nxdata, nxdata->handle);
+    return f->filter.can_fua (&next_ops, b->next, handle);
   else
     return backend_can_fua (b->next);
 }
@@ -583,11 +530,9 @@ static int
 filter_can_multi_conn (struct backend *b, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.can_multi_conn)
-    return f->filter.can_multi_conn (&next_ops, nxdata,
nxdata->handle);
+    return f->filter.can_multi_conn (&next_ops, b->next, handle);
   else
     return backend_can_multi_conn (b->next);
 }
@@ -596,11 +541,9 @@ static int
 filter_can_cache (struct backend *b, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.can_cache)
-    return f->filter.can_cache (&next_ops, nxdata, nxdata->handle);
+    return f->filter.can_cache (&next_ops, b->next, handle);
   else
     return backend_can_cache (b->next);
 }
@@ -611,11 +554,9 @@ filter_pread (struct backend *b, void *handle,
               uint32_t flags, int *err)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.pread)
-    return f->filter.pread (&next_ops, nxdata, nxdata->handle,
+    return f->filter.pread (&next_ops, b->next, handle,
                             buf, count, offset, flags, err);
   else
     return backend_pread (b->next, buf, count, offset, flags, err);
@@ -627,11 +568,9 @@ filter_pwrite (struct backend *b, void *handle,
                uint32_t flags, int *err)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.pwrite)
-    return f->filter.pwrite (&next_ops, nxdata, nxdata->handle,
+    return f->filter.pwrite (&next_ops, b->next, handle,
                              buf, count, offset, flags, err);
   else
     return backend_pwrite (b->next, buf, count, offset, flags, err);
@@ -642,11 +581,9 @@ filter_flush (struct backend *b, void *handle,
               uint32_t flags, int *err)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.flush)
-    return f->filter.flush (&next_ops, nxdata, nxdata->handle, flags,
err);
+    return f->filter.flush (&next_ops, b->next, handle, flags, err);
   else
     return backend_flush (b->next, flags, err);
 }
@@ -657,11 +594,9 @@ filter_trim (struct backend *b, void *handle,
              uint32_t flags, int *err)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.trim)
-    return f->filter.trim (&next_ops, nxdata, nxdata->handle, count,
offset,
+    return f->filter.trim (&next_ops, b->next, handle, count, offset,
                            flags, err);
   else
     return backend_trim (b->next, count, offset, flags, err);
@@ -672,11 +607,9 @@ filter_zero (struct backend *b, void *handle,
              uint32_t count, uint64_t offset, uint32_t flags, int *err)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.zero)
-    return f->filter.zero (&next_ops, nxdata, nxdata->handle,
+    return f->filter.zero (&next_ops, b->next, handle,
                            count, offset, flags, err);
   else
     return backend_zero (b->next, count, offset, flags, err);
@@ -688,11 +621,9 @@ filter_extents (struct backend *b, void *handle,
                 struct nbdkit_extents *extents, int *err)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.extents)
-    return f->filter.extents (&next_ops, nxdata, nxdata->handle,
+    return f->filter.extents (&next_ops, b->next, handle,
                               count, offset, flags,
                               extents, err);
   else
@@ -706,11 +637,9 @@ filter_cache (struct backend *b, void *handle,
               uint32_t flags, int *err)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
-  struct b_h *nxdata = handle;
 
-  assert (nxdata->b == b->next);
   if (f->filter.cache)
-    return f->filter.cache (&next_ops, nxdata, nxdata->handle,
+    return f->filter.cache (&next_ops, b->next, handle,
                             count, offset, flags, err);
   else
     return backend_cache (b->next, count, offset, flags, err);
-- 
2.25.0

On 2/12/20 7:40 AM, Richard W.M. Jones wrote:
> 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.
Yes, that has caught me, too.  I like the rename.

ACK

> +++ b/server/internal.h
> @@ -118,8 +118,18 @@ extern char *unixsocket;
>   extern const char *user, *group;
>   extern bool verbose;
>   
> -extern struct backend *backend;
> -#define for_each_backend(b) for (b = backend; b != NULL; b = b->next)
> +/* Linked list of backends.  Each backend struct is followed by either
> + * a filter or plugin struct.  "top" points to the first one. 
They
> + * are linked through the backend->next field.
> + *
> + *         ┌──────────┐    ┌──────────┐    ┌──────────┐
> + * top ───▶│ backend  │───▶│ backend  │───▶│ backend  │
> + *         │ b->i = 2 │    │ b->i = 1 │    │ b->i = 0 │
> + *         │ filter   │    │ filter   │    │ plugin   │
> + *         └──────────┘    └──────────┘    └──────────┘
> + */
And the graphic helps, too.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On 2/12/20 7:40 AM, Richard W.M. Jones wrote:
> Also add an inline accessor function (get_handle).
> 
> This also removes the unused function connection_get_handle.
> ---
>   server/internal.h    | 24 ++++++++++++++++-----
>   server/backend.c     | 50 ++++++++++++++++++++++----------------------
>   server/connections.c | 11 +---------
>   3 files changed, 45 insertions(+), 40 deletions(-)
ACK.

I'm not sure when I removed the last use of connection_get_handle, or 
why I didn't clean it up back then.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On 2/12/20 7:40 AM, Richard W.M. Jones wrote:
> This was previously used as ‘nxdata’ and stored a tuple of ’b->next’
> and the real filter handle.  However after recent changes we don't
> need it.  We can use ‘b->next’ as nxdata, and the handle is passed to
> us by the calling functions.
> 
> Inspired by Eric Blakes observations in this email:
Blake's
> https://www.redhat.com/archives/libguestfs/2020-February/msg00092.html
> ---
>   server/filters.c | 217 ++++++++++++++++-------------------------------
>   1 file changed, 73 insertions(+), 144 deletions(-)
> 
> @@ -216,201 +205,181 @@ plugin_magic_config_key (struct backend *b)
>   static int
>   next_open (void *nxdata, int readonly)
>   {
> -  struct b_h *b_h = nxdata;
> +  struct backend *b_next = nxdata;
>   
> -  return backend_open (b_h->b, readonly);
> +  return backend_open (b_next, readonly);
>   }
With this change, 'next_open' and '(int (*)(void *,
int))backend_open'
now have identical semantics.  I'm trying to see if there are further 
changes we could make that would alleviate the need for function casts. 
I don't know if it is worth changing nbdkit-filter.h to use 'struct 
backend *' instead of 'void *' (while leaving struct backend an 
incomplete type in the public header) - it would be ABI compatible, and 
although it would require recompilation, we already state that filter 
recompilation is par for the course (since only plugins promise API 
compatibility).

But one step at a time; your patch is fine as-is.
ACK

>   /* The next_functions structure contains pointers to backend
> - * functions.  However because these functions are all expecting a
> - * backend and a handle, we cannot call them directly, but must
> - * write some next_* functions that unpack the two parameters from a
> - * single ‘void *nxdata’ struct pointer (‘b_h’).
> + * functions.  These are only needed for type safety (nxdata is void
> + * pointer, backend_* functions expect a struct backend * parameter).
> + * nxdata is a pointer to the next backend in the linked list.
>    */
>   
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org