filters: Use context rather than connection to track c_next

This is another API/ABI change to filters, but that doesn't matter.
Instead of having a 1:1 correspondence of automatic backend context to
the current connection (which means filters.c has to consult GET_CONN
on every action), switch things so that a given context tracks the
next context.  This requires another signature change to the internal
backend_open and backend_reopen functions, as well as a public change
to the type of the argument handed to filter .open calls.  A given
connection now only needs to track the top_context, and we no longer
need get_context()/set_context().  Internally, nbdkit_context and
nbdkit_next are the same type, but the public interface for filters
keeps them separate for type-safety reasons (assigning a backend
handle to the current filter's context is different than calling into
the backend via next_ops/nxdata as the backend's context).

A few filters were already using the typedef name 'nbdkit_backend *'
instead of 'void *'; while they would still compile as-is, updating
their type makes it look nicer.  A later patch will force type-safety
on all the remaining filters, but it's better to save that mechanical
churn separately from this ABI change.  Also, with this in place, an
upcoming patch can finally allow filters to open up a context
independently of the current client connection.

Author: ebblake

docs/nbdkit-filter.pod

@@ -132,18 +132,19 @@ C<nbdkit_next_after_fork>, C<nbdkit_next_preconnect>,
 C<nbdkit_next_list_exports>, C<nbdkit_next_default_export>,
 C<nbdkit_next_open>) and a structure called C<struct nbdkit_next_ops>.
 These abstract the next plugin or filter in the chain.  There is also
-an opaque pointer C<backend> or C<nxdata> which must be passed along
-when calling these functions.  The value of C<nxdata> passed to
-C<.prepare> has a stable lifetime that lasts to the corresponding
-C<.finalize>, with all intermediate functions (such as C<.pread>)
-receiving the same value for convenience.  Functions where C<nxdata>
-is not reused are C<.config>, C<.config_complete>, C<.get_ready>, and
-C<.after_fork>, which are called during initialization outside any
-connections, and C<.preconnect>, C<.list_exports>, C<.default_export>,
-and C<.open> which are called based on client connections but prior to
-the stable lifetime of C<.prepare>.  The value of C<backend> passed to
-C<.open> has a lifetime that lasts until the matching C<.close> for
-use by C<nbdkit_backend_reopen>.
+an opaque pointer C<backend>, C<context> or C<nxdata> which must be
+passed along when calling these functions.  The value of C<nxdata>
+passed to C<.prepare> has a stable lifetime that lasts to the
+corresponding C<.finalize>, with all intermediate functions (such as
+C<.pread>) receiving the same value for convenience.  Functions where
+C<nxdata> is not reused are C<.config>, C<.config_complete>,
+C<.get_ready>, and C<.after_fork>, which are called during
+initialization outside any connections, and C<.preconnect>,
+C<.list_exports>, C<.default_export>, and C<.open> which are called
+based on client connections but prior to the stable lifetime of
+C<.prepare>.  The value of C<context> passed to C<.open> has a
+lifetime that lasts until the matching C<.close> for use by
+C<nbdkit_backend_reopen>.
 
 =head2 Next config, open and close
 
@@ -429,7 +430,7 @@ C<next> because it cannot be altered.
 
 =head2 C<.open>
 
- void * (*open) (nbdkit_next_open *next, nbdkit_backend *backend,
+ void * (*open) (nbdkit_next_open *next, nbdkit_context *context,
                  int readonly, const char *exportname, int is_tls);
 
 This is called when a new client connection is opened and can be used
@@ -471,7 +472,7 @@ outer filter to the plugin will be in reverse.  Skipping a call to
 C<next> is acceptable if the filter will not access C<next_ops>
 during any of the remaining callbacks reached on the same connection.
 
-The value of C<backend> in this call has a lifetime that lasts until
+The value of C<context> in this call has a lifetime that lasts until
 the counterpart C<.close>, and it is this value (and not the distinct
 C<nxdata> of C<.pread> and friends) that must be passed as the first
 parameter to C<nbdkit_backend_reopen> by a filter attempting to retry
@@ -481,14 +482,14 @@ a connection into the backend.
 
 Filters have access to a function for reopening the backend:
 
-  int nbdkit_backend_reopen (nbdkit_backend *backend, int readonly,
+  int nbdkit_backend_reopen (nbdkit_context *context, int readonly,
                              const char *exportname, void **nxdata);
 
 This function is used by L<nbdkit-retry-filter(1)> to close and reopen
 the underlying plugin, with possible changes to the C<readonly> and
 C<exportname> parameters in relation to the original opening.  It
 should be used with caution because it is difficult to use safely.
-The C<backend> parameter to this function should be the C<backend>
+The C<context> parameter to this function should be the C<context>
 parameter originally passed in to C<.open>; while the C<nxdata>
 pointer should be the address of C<nxdata> from any function with a
 C<next_ops> parameter (such as C<.pread>) that wants to call into the

filters/checkwrite/checkwrite.c

@@ -44,7 +44,7 @@
 #include "minmax.h"
 
 static void *
-checkwrite_open (nbdkit_next_open *next, nbdkit_backend *nxdata,
+checkwrite_open (nbdkit_next_open *next, nbdkit_context *nxdata,
                  int readonly, const char *exportname, int is_tls)
 {
   /* Ignore readonly flag passed in, open the plugin readonly. */

filters/exitlast/exitlast.c

@@ -50,7 +50,7 @@
 static _Atomic unsigned connections;
 
 static void *
-exitlast_open (nbdkit_next_open *next, nbdkit_backend *nxdata,
+exitlast_open (nbdkit_next_open *next, nbdkit_context *nxdata,
                int readonly, const char *exportname, int is_tls)
 {
   if (next (nxdata, readonly, exportname) == -1)

filters/exitwhen/exitwhen.c

@@ -482,7 +482,7 @@ exitwhen_preconnect (nbdkit_next_preconnect *next, void *nxdata, int readonly)
 }
 
 static void *
-exitwhen_open (nbdkit_next_open *next, nbdkit_backend *nxdata,
+exitwhen_open (nbdkit_next_open *next, nbdkit_context *nxdata,
                int readonly, const char *exportname, int is_tls)
 {
   if (next (nxdata, readonly, exportname) == -1)

filters/gzip/gzip.c

@@ -75,7 +75,7 @@ gzip_thread_model (void)
 }
 
 static void *
-gzip_open (nbdkit_next_open *next, nbdkit_backend *nxdata,
+gzip_open (nbdkit_next_open *next, nbdkit_context *nxdata,
            int readonly, const char *exportname, int is_tls)
 {
   /* Always pass readonly=1 to the underlying plugin. */

filters/limit/limit.c

@@ -90,7 +90,7 @@ limit_preconnect (nbdkit_next_preconnect *next, nbdkit_backend *nxdata,
 }
 
 static void *
-limit_open (nbdkit_next_open *next, nbdkit_backend *nxdata,
+limit_open (nbdkit_next_open *next, nbdkit_context *nxdata,
             int readonly, const char *exportname, int is_tls)
 {
   if (next (nxdata, readonly, exportname) == -1)

filters/retry/retry.c

@@ -109,13 +109,13 @@ retry_config (nbdkit_next_config *next, nbdkit_backend *nxdata,
 struct retry_handle {
   int readonly;                 /* Save original readonly setting. */
   char *exportname;             /* Client exportname. */
-  nbdkit_backend *backend;      /* Backend learned during .open. */
+  nbdkit_context *context;      /* Context learned during .open. */
   unsigned reopens;
   bool open;
 };
 
 static void *
-retry_open (nbdkit_next_open *next, nbdkit_backend *nxdata,
+retry_open (nbdkit_next_open *next, nbdkit_context *nxdata,
             int readonly, const char *exportname, int is_tls)
 {
   struct retry_handle *h;
@@ -131,7 +131,7 @@ retry_open (nbdkit_next_open *next, nbdkit_backend *nxdata,
 
   h->readonly = readonly;
   h->exportname = strdup (exportname);
-  h->backend = nxdata;
+  h->context = nxdata;
   if (h->exportname == NULL) {
     nbdkit_error ("strdup: %m");
     free (h);
@@ -209,7 +209,7 @@ do_retry (struct retry_handle *h, struct retry_data *data,
 
   /* Reopen the connection. */
   h->reopens++;
-  if (nbdkit_backend_reopen (h->backend, h->readonly || force_readonly,
+  if (nbdkit_backend_reopen (h->context, h->readonly || force_readonly,
                              h->exportname, next) == -1) {
     /* If the reopen fails we treat it the same way as a command
      * failing.

filters/tar/tar.c

@@ -112,7 +112,7 @@ struct handle {
 };
 
 static void *
-tar_open (nbdkit_next_open *next, nbdkit_backend *nxdata,
+tar_open (nbdkit_next_open *next, nbdkit_context *nxdata,
           int readonly, const char *exportname, int is_tls)
 {
   struct handle *h;

include/nbdkit-filter.h

@@ -54,12 +54,15 @@ extern "C" {
  * the next filter or plugin.
  */
 typedef struct backend nbdkit_backend;
+typedef struct context nbdkit_context;
 typedef struct context nbdkit_next;
 #elif defined NBDKIT_RETRY_FILTER /* Hack to expose reopen to retry filter */
 typedef struct nbdkit_backend nbdkit_backend;
+typedef struct nbdkit_context nbdkit_context;
 typedef struct nbdkit_next_ops nbdkit_next;
 #else
 typedef void nbdkit_backend;
+typedef void nbdkit_context;
 typedef void nbdkit_next;
 #endif
 
@@ -74,7 +77,7 @@ typedef int nbdkit_next_list_exports (nbdkit_backend *nxdata, int readonly,
                                       struct nbdkit_exports *exports);
 typedef const char *nbdkit_next_default_export (nbdkit_backend *nxdata,
                                                 int readonly);
-typedef int nbdkit_next_open (nbdkit_backend *backend,
+typedef int nbdkit_next_open (nbdkit_context *context,
                               int readonly, const char *exportname);
 
 struct nbdkit_next_ops {
@@ -155,7 +158,7 @@ NBDKIT_EXTERN_DECL (const struct nbdkit_export, nbdkit_get_export,
  * Used by the retry filter.
  */
 NBDKIT_EXTERN_DECL (int, nbdkit_backend_reopen,
-                    (nbdkit_backend *backend, int readonly,
+                    (nbdkit_context *context, int readonly,
                      const char *exportname, nbdkit_next **nxdata));
 #endif
 
@@ -199,7 +202,7 @@ struct nbdkit_filter {
                                   nbdkit_backend *nxdata,
                                   int readonly, int is_tls);
 
-  void * (*open) (nbdkit_next_open *next, nbdkit_backend *backend,
+  void * (*open) (nbdkit_next_open *next, nbdkit_context *context,
                   int readonly, const char *exportname, int is_tls);
   void (*close) (void *handle);
 

server/backend.c

@@ -163,13 +163,12 @@ backend_list_exports (struct backend *b, int readonly,
                       struct nbdkit_exports *exports)
 {
   GET_CONN;
-  struct context *c = get_context (conn, b);
   size_t count;
 
   controlpath_debug ("%s: list_exports readonly=%d tls=%d",
                      b->name, readonly, conn->using_tls);
 
-  assert (c == NULL);
+  assert (conn->top_context == NULL);
 
   if (b->list_exports (b, readonly, conn->using_tls, exports) == -1 ||
       exports_resolve_default (exports, b, readonly) == -1) {
@@ -186,14 +185,13 @@ const char *
 backend_default_export (struct backend *b, int readonly)
 {
   GET_CONN;
-  struct context *c = get_context (conn, b);
   const char *s;
 
   controlpath_debug ("%s: default_export readonly=%d tls=%d",
                      b->name, readonly, conn->using_tls);
 
   if (conn->default_exportname[b->i] == NULL) {
-    assert (c == NULL);
+    assert (conn->top_context == NULL);
     s = b->default_export (b, readonly, conn->using_tls);
     /* Ignore over-length strings. XXX Also ignore non-UTF8? */
     if (s && strnlen (s, NBD_MAX_STRING + 1) > NBD_MAX_STRING) {
@@ -247,10 +245,10 @@ backend_open (struct backend *b, int readonly, const char *exportname)
   controlpath_debug ("%s: open readonly=%d exportname=\"%s\" tls=%d",
                      b->name, readonly, exportname, conn->using_tls);
 
-  assert (conn->contexts[b->i] == NULL);
   c->next = next_ops;
   c->handle = NULL;
   c->b = b;
+  c->c_next = NULL;
   c->state = 0;
   c->exportsize = -1;
   c->can_write = readonly ? 0 : -1;
@@ -277,15 +275,12 @@ backend_open (struct backend *b, int readonly, const char *exportname)
   /* Most filters will call next_open first, resulting in
    * inner-to-outer ordering.
    */
-  c->handle = b->open (b, readonly, exportname, conn->using_tls);
+  c->handle = b->open (c, readonly, exportname, conn->using_tls);
   controlpath_debug ("%s: open returned handle %p", b->name, c->handle);
 
   if (c->handle == NULL) {
-    if (b->i) { /* Do not strand backend if this layer failed */
-      struct context *c2 = get_context (conn, b->next);
-      if (c2 != NULL)
-        backend_close (c2);
-    }
+    if (b->i && c->c_next != NULL)
+      backend_close (c->c_next);
     free (c);
     return NULL;
   }
@@ -297,7 +292,6 @@ backend_open (struct backend *b, int readonly, const char *exportname)
 int
 backend_prepare (struct context *c)
 {
-  GET_CONN;
   struct backend *b = c->b;
 
   assert (c->handle);
@@ -307,11 +301,8 @@ backend_prepare (struct context *c)
    * plugin, similar to typical .open order.  But remember that
    * a filter may skip opening its backend.
    */
-  if (b->i) {
-    struct context *c2 = get_context (conn, b->next);
-    if (c2 != NULL && backend_prepare (c2) == -1)
-      return -1;
-  }
+  if (b->i && c->c_next != NULL && backend_prepare (c->c_next) == -1)
+    return -1;
 
   controlpath_debug ("%s: prepare readonly=%d", b->name, c->can_write == 0);
 
@@ -324,7 +315,6 @@ backend_prepare (struct context *c)
 int
 backend_finalize (struct context *c)
 {
-  GET_CONN;
   struct backend *b = c->b;
 
   /* Call these in reverse order to .prepare above, starting from the
@@ -344,32 +334,25 @@ backend_finalize (struct context *c)
     }
   }
 
-  if (b->i) {
-    struct context *c2 = get_context (conn, b->next);
-    if (c2 != NULL)
-      return backend_finalize (c2);
-  }
+  if (b->i && c->c_next != NULL)
+    return backend_finalize (c->c_next);
   return 0;
 }
 
 void
 backend_close (struct context *c)
 {
-  GET_CONN;
   struct backend *b = c->b;
+  struct context *c_next = c->c_next;
 
   /* outer-to-inner order, opposite .open */
   assert (c->handle);
   assert (c->state & HANDLE_OPEN);
   controlpath_debug ("%s: close", b->name);
   b->close (c);
   free (c);
-  set_context (conn, b, NULL);
-  if (b->i) {
-    struct context *c2 = get_context (conn, b->next);
-    if (c2 != NULL)
-      backend_close (c2);
-  }
+  if (c_next != NULL)
+    backend_close (c_next);
 }
 
 bool
@@ -383,27 +366,26 @@ backend_valid_range (struct context *c, uint64_t offset, uint32_t count)
 /* Core functionality of nbdkit_backend_reopen for retry filter */
 
 int
-backend_reopen (struct backend *b, int readonly, const char *exportname)
+backend_reopen (struct context *c, int readonly, const char *exportname)
 {
-  GET_CONN;
-  struct context *c;
+  struct backend *b = c->b;
 
   controlpath_debug ("%s: reopen readonly=%d exportname=\"%s\"",
-                     b->name, readonly, exportname);
+                     b->next->name, readonly, exportname);
 
-  c = get_context (conn, b);
-  if (c) {
-    if (backend_finalize (c) == -1)
+  if (c->c_next) {
+    if (backend_finalize (c->c_next) == -1)
       return -1;
-    backend_close (c);
+    backend_close (c->c_next);
+    c->c_next = NULL;
   }
-  c = backend_open (b, readonly, exportname);
-  if (c == NULL)
+  c->c_next = backend_open (b->next, readonly, exportname);
+  if (c->c_next == NULL)
     return -1;
-  set_context (conn, b, c);
-  if (backend_prepare (c) == -1) {
-    backend_finalize (c);
-    backend_close (c);
+  if (backend_prepare (c->c_next) == -1) {
+    backend_finalize (c->c_next);
+    backend_close (c->c_next);
+    c->c_next = NULL;
     return -1;
   }
   return 0;