[Libguestfs] [nbdkit PATCH 4/4] server: Better documentation of .open ordering

[Eric Blake]

Because we pass a next() pointer to .open, the filter can choose
whether to call it first (inner-to-outer completion) or last
(outer-to-inner).  However, all our filters currently call it first,
and this logically lines up with the mirror ordering of outer-to-inner
used during .close.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 docs/nbdkit-filter.pod     | 17 +++++++++++++----
 server/filters.c           |  8 ++++++--
 tests/test-layers-filter.c |  5 +++--
 tests/test-layers.c        | 14 ++++++++------
 4 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod
index 00be376e..3c98d89a 100644
--- a/docs/nbdkit-filter.pod
+++ b/docs/nbdkit-filter.pod
@@ -310,14 +310,21 @@ read-only backend.  However, it is also acceptable to attempt write
 access to the plugin even if this filter is readonly, such as when a
 file system mounted read-only still requires write access to the
 underlying device in case a journal needs to be replayed for
-consistency as part of the mounting process.
+consistency as part of the mounting process.  The filter should
+generally call C<next> as its first step, to allocate from the plugin
+outwards, so that C<.close> running from the outer filter to the
+plugin will be in reverse.

 =head2 C<.close>

  void (*close) (void *handle);

 This is called when the client closes the connection.  It should clean
-up any per-connection resources used by the filter.
+up any per-connection resources used by the filter.  It is called
+beginning with the outermost filter and ending with the plugin (the
+opposite order of C<.open> if all filters call C<next> first),
+although this order technically does not matter since the callback
+cannot report failures or access the underlying plugin.

 =head2 C<.prepare>

@@ -348,8 +355,10 @@ Unlike other filter methods, prepare and finalize are not chained
 through the C<next_ops> structure.  Instead the core nbdkit server
 calls the prepare and finalize methods of all filters.  Prepare
 methods are called starting with the filter closest to the plugin and
-proceeding outwards.  Finalize methods are called in the reverse order
-of prepare methods.
+proceeding outwards (matching the order of C<.open> if all filters
+call C<next> before doing anything locally).  Finalize methods are
+called in the reverse order of prepare methods, with the outermost
+filter first (and matching the order of C<.close>).

 If there is an error, both callbacks should call C<nbdkit_error> with
 an error message and return C<-1>.
diff --git a/server/filters.c b/server/filters.c
index 78e32bc5..37e8b51e 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -205,6 +205,9 @@ filter_open (struct backend *b, struct connection *conn, int readonly)
   struct b_conn nxdata = { .b = b->next, .conn = conn };
   void *handle;

+  /* Most filters will call next_open first, resulting in
+   * inner-to-outer ordering.
+   */
   if (f->filter.open) {
     handle = f->filter.open (next_open, &nxdata, readonly);
     if (handle == NULL)
@@ -225,6 +228,7 @@ filter_close (struct backend *b, struct connection *conn)
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
   void *handle = connection_get_handle (conn, b->i);

+  /* outer-to-inner order, opposite .open */
   if (handle && f->filter.close)
     f->filter.close (handle);
   backend_close (b->next, conn);
@@ -409,7 +413,7 @@ filter_prepare (struct backend *b, struct connection *conn, int readonly)
   struct b_conn nxdata = { .b = b->next, .conn = conn };

   /* Call these in order starting from the filter closest to the
-   * plugin.
+   * plugin, similar to typical .open order.
    */
   if (backend_prepare (b->next, conn) == -1)
     return -1;
@@ -431,7 +435,7 @@ filter_finalize (struct backend *b, struct connection *conn)
   debug ("%s: finalize", b->name);

   /* Call these in reverse order to .prepare above, starting from the
-   * filter furthest away from the plugin.
+   * filter furthest away from the plugin, and matching .close order.
    */
   if (f->filter.finalize &&
       f->filter.finalize (&next_ops, &nxdata, handle) == -1)
diff --git a/tests/test-layers-filter.c b/tests/test-layers-filter.c
index cccfb654..d590cf95 100644
--- a/tests/test-layers-filter.c
+++ b/tests/test-layers-filter.c
@@ -79,11 +79,12 @@ test_layers_filter_open (nbdkit_next_open *next, void *nxdata, int readonly)
 {
   static int handle;

-  DEBUG_FUNCTION;
-
   if (next (nxdata, readonly) == -1)
     return NULL;

+  /* Debug after recursing, to show opposite order from .close */
+  DEBUG_FUNCTION;
+
   return &handle;
 }

diff --git a/tests/test-layers.c b/tests/test-layers.c
index 93b7770c..eac7f152 100644
--- a/tests/test-layers.c
+++ b/tests/test-layers.c
@@ -282,19 +282,20 @@ main (int argc, char *argv[])
      "test_layers_plugin_config_complete",
      NULL);

-  /* open methods called in order. */
+  /* open methods called in outer-to-inner order, but thanks to next
+   * pointer, complete in inner-to-outer order. */
   log_verify_seen_in_order
     ("testlayersfilter3: open readonly=0",
-     "filter3: test_layers_filter_open",
      "testlayersfilter2: open readonly=0",
-     "filter2: test_layers_filter_open",
      "testlayersfilter1: open readonly=0",
-     "filter1: test_layers_filter_open",
      "testlayersplugin: open readonly=0",
      "test_layers_plugin_open",
+     "filter1: test_layers_filter_open",
+     "filter2: test_layers_filter_open",
+     "filter3: test_layers_filter_open",
      NULL);

-  /* prepare methods called in order.
+  /* prepare methods called in inner-to-outer order.
    *
    * Note that prepare methods only exist for filters, and they must
    * be called from inner to outer (but finalize methods below are
@@ -595,7 +596,8 @@ main (int argc, char *argv[])
      "filter1: test_layers_filter_finalize",
      NULL);

-  /* close methods called in order */
+  /* close methods called outer-to-inner, which is reverse of completion
+   * of open */
   log_verify_seen_in_order
     ("filter3: test_layers_filter_close",
      "filter2: test_layers_filter_close",
-- 
2.21.0