Libguestfs - Oct 2019 - Re: [PATCH NOT WORKING nbdkit v2 1/2] server: Add .ready_to_serve plugin method.

This is my second attempt at this.  The first version (also not
working) was here:
https://www.redhat.com/archives/libguestfs/2019-October/msg00062.html

In part 1/2 I introduce a new .ready_to_serve plugin method which is
called after forking and just before accepting any client connection.
The idea would be that plugins could start background threads here.
However this doesn't work well in practice because plugins which do
start any background threads seem to always get a segfault at
shutdown, with a race between __nptl_deallocate_tsd and some code
being unloaded (I'm not sure exactly what).

Part 2/2 restructures the VDDK plugin.  This time I fixed the message
passing so it doesn't deadlock as it did before.

I thought that this patch might have solved the other VDDK problem we
have around "Resolve Host" taking so long, but in fact it does not fix
that either.  (VDDK is still broken, news as 11).

Anyway I don't think there's anything worth saving here.  I'm mainly
posting it so we have a back up.

Rich.

This method can be used for plugins to get control after the server
has forked and changed user but before it accepts a connection.  This
is very late and the only real use for this is for a plugin to create
background threads for its own use.
---
 docs/nbdkit-filter.pod     | 20 +++++++++++++++-----
 docs/nbdkit-plugin.pod     | 27 ++++++++++++++++++++++++++-
 include/nbdkit-filter.h    |  2 ++
 include/nbdkit-plugin.h    |  2 ++
 server/filters.c           | 24 ++++++++++++++++++++++++
 server/internal.h          |  1 +
 server/main.c              |  3 +++
 server/plugins.c           | 16 ++++++++++++++++
 tests/test-layers-filter.c | 11 ++++++++++-
 tests/test-layers-plugin.c |  8 ++++++++
 tests/test-layers.c        | 12 ++++++++++++
 11 files changed, 119 insertions(+), 7 deletions(-)

diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod
index 07ede47..4f10494 100644
--- a/docs/nbdkit-filter.pod
+++ b/docs/nbdkit-filter.pod
@@ -126,8 +126,8 @@ which is required.
 
 =head1 NEXT PLUGIN
 
-F<nbdkit-filter.h> defines three function types
-(C<nbdkit_next_config>, C<nbdkit_next_config_complete>,
+F<nbdkit-filter.h> defines four function types
(C<nbdkit_next_config>,
+C<nbdkit_next_config_complete>, C<nbdkit_ready_to_serve>,
 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<nxdata> which must be passed along when calling
@@ -135,9 +135,10 @@ these functions.
 
 =head2 Next config, open and close
 
-The filter’s C<.config>, C<.config_complete> and C<.open>
methods may
-only call the next C<.config>, C<.config_complete> and
C<.open> method
-in the chain (optionally for C<.config>).
+The filter’s C<.config>, C<.config_complete>,
C<.ready_to_serve> and
+C<.open> methods may only call the next C<.config>,
+C<.config_complete>, C<.ready_to_serve> and C<.open> method
in the
+chain (optionally for C<.config>).
 
 The filter’s C<.close> method is called when an old connection closed,
 and this has no C<next> parameter because it cannot be
@@ -284,6 +285,15 @@ filter doesn't slow down other filters or plugins.
 If there is an error, C<.thread_model> should call C<nbdkit_error>
 with an error message and return C<-1>.
 
+=head2 C<.ready_to_serve>
+
+ int (*ready_to_serve) (nbdkit_next_ready_to_serve *next, void *nxdata);
+
+This intercepts the plugin C<.ready_to_serve> method.
+
+If there is an error, C<.ready_to_serve> should call
C<nbdkit_error>
+with an error message and return C<-1>.
+
 =head2 C<.open>
 
  void * (*open) (nbdkit_next_open *next, void *nxdata,
diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index e34ffd1..3ae1303 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -142,6 +142,13 @@ before any connections are accepted.  However, during
C<nbdkit
 C<.config_complete> (so a plugin which determines the results from a
 script must be prepared for a missing script).
 
+=item C<.ready_to_serve>
+
+This callback is called after nbdkit has forked into the background
+and changed user, and before it accepts any client connection.  If the
+plugin needs to create its own background threads then this is a good
+place to do that.
+
 =item C<.open>
 
 A new client has connected.
@@ -408,7 +415,8 @@ Any other bare parameters give errors.
 
 This optional callback is called after all the configuration has been
 passed to the plugin.  It is a good place to do checks, for example
-that the user has passed the required parameters to the plugin.
+that the user has passed the required parameters to the plugin.  It is
+also a good place to do general start-up work.
 
 If there is an error, C<.config_complete> should call
C<nbdkit_error>
 with an error message and return C<-1>.
@@ -437,6 +445,23 @@ are silently ignored.
 If there is an error, C<.thread_model> should call C<nbdkit_error>
 with an error message and return C<-1>.
 
+=head2 C<.ready_to_serve>
+
+ int ready_to_serve (void);
+
+This optional callback is called after nbdkit has forked into the
+background and changed user, and before it accepts any client
+connection.
+
+If the plugin needs to create its own background threads then this is
+a good place to do that.  However because nbdkit may have already
+forked into the background and so it is difficult to communicate
+errors back to the user, this is B<not> a good place to do general
+start-up work (use C<.config_complete> instead).
+
+If there is an error, C<.ready_to_serve> should call
C<nbdkit_error>
+with an error message and return C<-1>.
+
 =head2 C<.open>
 
  void *open (int readonly);
diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h
index c1930c1..7b3a64f 100644
--- a/include/nbdkit-filter.h
+++ b/include/nbdkit-filter.h
@@ -64,6 +64,7 @@ extern struct nbdkit_extent nbdkit_get_extent (const struct
nbdkit_extents *,
 typedef int nbdkit_next_config (void *nxdata,
                                 const char *key, const char *value);
 typedef int nbdkit_next_config_complete (void *nxdata);
+typedef int nbdkit_next_ready_to_serve (void *nxdata);
 typedef int nbdkit_next_open (void *nxdata,
                               int readonly);
 
@@ -129,6 +130,7 @@ struct nbdkit_filter {
   int (*config_complete) (nbdkit_next_config_complete *next, void *nxdata);
   const char *config_help;
   int (*thread_model) (void);
+  int (*ready_to_serve) (nbdkit_next_ready_to_serve *next, void *nxdata);
 
   void * (*open) (nbdkit_next_open *next, void *nxdata,
                   int readonly);
diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h
index 45ae705..746d9a1 100644
--- a/include/nbdkit-plugin.h
+++ b/include/nbdkit-plugin.h
@@ -134,6 +134,8 @@ struct nbdkit_plugin {
   int (*thread_model) (void);
 
   int (*can_fast_zero) (void *handle);
+
+  int (*ready_to_serve) (void);
 };
 
 extern void nbdkit_set_error (int err);
diff --git a/server/filters.c b/server/filters.c
index 1ac4a5f..ed00d11 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -190,6 +190,29 @@ plugin_magic_config_key (struct backend *b)
   return b->next->magic_config_key (b->next);
 }
 
+static int
+next_ready_to_serve (void *nxdata)
+{
+  struct backend *b = nxdata;
+  b->ready_to_serve (b);
+  return 0;
+}
+
+static void
+filter_ready_to_serve (struct backend *b)
+{
+  struct backend_filter *f = container_of (b, struct backend_filter, backend);
+
+  debug ("%s: ready_to_serve", b->name);
+
+  if (f->filter.ready_to_serve) {
+    if (f->filter.ready_to_serve (next_ready_to_serve, b->next) == -1)
+      exit (EXIT_FAILURE);
+  }
+  else
+    b->next->ready_to_serve (b->next);
+}
+
 static int
 next_open (void *nxdata, int readonly)
 {
@@ -668,6 +691,7 @@ static struct backend filter_functions = {
   .config = filter_config,
   .config_complete = filter_config_complete,
   .magic_config_key = plugin_magic_config_key,
+  .ready_to_serve = filter_ready_to_serve,
   .open = filter_open,
   .prepare = filter_prepare,
   .finalize = filter_finalize,
diff --git a/server/internal.h b/server/internal.h
index 167da59..4c9419d 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -312,6 +312,7 @@ struct backend {
   void (*config) (struct backend *, const char *key, const char *value);
   void (*config_complete) (struct backend *);
   const char *(*magic_config_key) (struct backend *);
+  void (*ready_to_serve) (struct backend *);
   void *(*open) (struct backend *, struct connection *conn, int readonly);
   int (*prepare) (struct backend *, struct connection *conn, void *handle,
                   int readonly);
diff --git a/server/main.c b/server/main.c
index 5623149..935d422 100644
--- a/server/main.c
+++ b/server/main.c
@@ -857,6 +857,7 @@ start_serving (void)
     for (i = 0; i < nr_socks; ++i)
       socks[i] = FIRST_SOCKET_ACTIVATION_FD + i;
     change_user ();
+    backend->ready_to_serve (backend);
     write_pidfile ();
     accept_incoming_connections (socks, nr_socks);
     free_listening_sockets (socks, nr_socks); /* also closes them */
@@ -866,6 +867,7 @@ start_serving (void)
   /* Handling a single connection on stdin/stdout. */
   if (listen_stdin) {
     change_user ();
+    backend->ready_to_serve (backend);
     write_pidfile ();
     threadlocal_new_server_thread ();
     if (handle_single_connection (0, 1) == -1)
@@ -882,6 +884,7 @@ start_serving (void)
   run_command ();
   change_user ();
   fork_into_background ();
+  backend->ready_to_serve (backend);
   write_pidfile ();
   accept_incoming_connections (socks, nr_socks);
   free_listening_sockets (socks, nr_socks);
diff --git a/server/plugins.c b/server/plugins.c
index 87daaf2..bb0269d 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -158,6 +158,7 @@ plugin_dump_fields (struct backend *b)
   HAS (config);
   HAS (config_complete);
   HAS (config_help);
+  HAS (ready_to_serve);
   HAS (open);
   HAS (close);
   HAS (get_size);
@@ -233,6 +234,20 @@ plugin_magic_config_key (struct backend *b)
   return p->plugin.magic_config_key;
 }
 
+static void
+plugin_ready_to_serve (struct backend *b)
+{
+  struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
+
+  debug ("%s: ready_to_serve", b->name);
+
+  if (!p->plugin.ready_to_serve)
+    return;
+
+  if (p->plugin.ready_to_serve () == -1)
+    exit (EXIT_FAILURE);
+}
+
 static void *
 plugin_open (struct backend *b, struct connection *conn, int readonly)
 {
@@ -656,6 +671,7 @@ static struct backend plugin_functions = {
   .config = plugin_config,
   .config_complete = plugin_config_complete,
   .magic_config_key = plugin_magic_config_key,
+  .ready_to_serve = plugin_ready_to_serve,
   .open = plugin_open,
   .prepare = plugin_prepare,
   .finalize = plugin_finalize,
diff --git a/tests/test-layers-filter.c b/tests/test-layers-filter.c
index d590cf9..315fff3 100644
--- a/tests/test-layers-filter.c
+++ b/tests/test-layers-filter.c
@@ -65,7 +65,7 @@ test_layers_filter_config (nbdkit_next_config *next, void
*nxdata,
 
 static int
 test_layers_filter_config_complete (nbdkit_next_config_complete *next,
-                                     void *nxdata)
+                                    void *nxdata)
 {
   DEBUG_FUNCTION;
   return next (nxdata);
@@ -74,6 +74,14 @@ test_layers_filter_config_complete
(nbdkit_next_config_complete *next,
 #define test_layers_filter_config_help          \
   "test_layers_" layer "_config_help"
 
+static int
+test_layers_filter_ready_to_serve (nbdkit_next_ready_to_serve *next,
+                                     void *nxdata)
+{
+  DEBUG_FUNCTION;
+  return next (nxdata);
+}
+
 static void *
 test_layers_filter_open (nbdkit_next_open *next, void *nxdata, int readonly)
 {
@@ -267,6 +275,7 @@ static struct nbdkit_filter filter = {
   .config            = test_layers_filter_config,
   .config_complete   = test_layers_filter_config_complete,
   .config_help       = test_layers_filter_config_help,
+  .ready_to_serve    = test_layers_filter_ready_to_serve,
   .open              = test_layers_filter_open,
   .close             = test_layers_filter_close,
   .prepare           = test_layers_filter_prepare,
diff --git a/tests/test-layers-plugin.c b/tests/test-layers-plugin.c
index e9ffd3b..62c0066 100644
--- a/tests/test-layers-plugin.c
+++ b/tests/test-layers-plugin.c
@@ -72,6 +72,13 @@ test_layers_plugin_config_complete (void)
 
 #define test_layers_plugin_config_help
"test_layers_plugin_config_help"
 
+static int
+test_layers_plugin_ready_to_serve (void)
+{
+  DEBUG_FUNCTION;
+  return 0;
+}
+
 static void *
 test_layers_plugin_open (int readonly)
 {
@@ -224,6 +231,7 @@ static struct nbdkit_plugin plugin = {
   .config            = test_layers_plugin_config,
   .config_complete   = test_layers_plugin_config_complete,
   .config_help       = test_layers_plugin_config_help,
+  .ready_to_serve    = test_layers_plugin_ready_to_serve,
   .open              = test_layers_plugin_open,
   .close             = test_layers_plugin_close,
   .get_size          = test_layers_plugin_get_size,
diff --git a/tests/test-layers.c b/tests/test-layers.c
index eac7f15..c12af47 100644
--- a/tests/test-layers.c
+++ b/tests/test-layers.c
@@ -282,6 +282,18 @@ main (int argc, char *argv[])
      "test_layers_plugin_config_complete",
      NULL);
 
+  /* ready_to_serve methods called in order. */
+  log_verify_seen_in_order
+    ("testlayersfilter3: ready_to_serve",
+     "filter3: test_layers_filter_ready_to_serve",
+     "testlayersfilter2: ready_to_serve",
+     "filter2: test_layers_filter_ready_to_serve",
+     "testlayersfilter1: ready_to_serve",
+     "filter1: test_layers_filter_ready_to_serve",
+     "testlayersplugin: ready_to_serve",
+     "test_layers_plugin_ready_to_serve",
+     NULL);
+
   /* open methods called in outer-to-inner order, but thanks to next
    * pointer, complete in inner-to-outer order. */
   log_verify_seen_in_order
-- 
2.23.0

This finally solves a long-standing problem that the plugin did not
follow the peculiar rules that VDDK imposes on multithreaded
programming (see "Multithreading Considerations" in the VDDK manual).

We add a background thread to the plugin which is responsible for
issuing the following VDDK calls:

  VixDiskLib_InitEx
  VixDiskLib_Exit
  VixDiskLib_ConnectEx
  VixDiskLib_Open
  VixDiskLib_Close
  VixDiskLib_Disconnect

(Other calls are still made directly by the plugin.)  This involves
some complicated synchronization but is otherwise uninteresting.

This allows us to relax the thread model from SERIALIZE_ALL_REQUESTS
to SERIALIZE_REQUESTS.  Note that VDDK does not allow parallel
requests to be made on one handle, so we cannot relax the model any
further.
---
 plugins/vddk/Makefile.am |   2 +
 plugins/vddk/octhread.c  | 399 +++++++++++++++++++++++++++++++++++++++
 plugins/vddk/vddk.c      | 190 ++++++-------------
 plugins/vddk/vddk.h      |  89 +++++++++
 tests/test-vddk-real.sh  |   2 +-
 5 files changed, 547 insertions(+), 135 deletions(-)

diff --git a/plugins/vddk/Makefile.am b/plugins/vddk/Makefile.am
index b806a7d..aff1288 100644
--- a/plugins/vddk/Makefile.am
+++ b/plugins/vddk/Makefile.am
@@ -42,6 +42,8 @@ plugin_LTLIBRARIES = nbdkit-vddk-plugin.la
 
 nbdkit_vddk_plugin_la_SOURCES = \
 	vddk.c \
+	octhread.c \
+	vddk.h \
 	vddk-structs.h \
 	vddk-stubs.h \
 	$(top_srcdir)/include/nbdkit-plugin.h \
diff --git a/plugins/vddk/octhread.c b/plugins/vddk/octhread.c
new file mode 100644
index 0000000..9da3fb2
--- /dev/null
+++ b/plugins/vddk/octhread.c
@@ -0,0 +1,399 @@
+/* nbdkit
+ * Copyright (C) 2013-2019 Red Hat Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of Red Hat nor the names of its contributors may be
+ * used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+ * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+/* This is the Open/Close Thread (octhread).
+ *
+ * Because of VDDK's complicated multi-threading requirements (see
+ * "Multithreading Considerations" in the VDDK manual) we have to
+ * issue various VDDK calls from a single thread.  In this file we
+ * create the extra thread which always runs while the VDDK plugin is
+ * loaded.
+ *
+ * The main VDDK plugin calls out to this thread synchronously
+ * whenever it needs to make one of the following calls:
+ *
+ *   VixDiskLib_InitEx
+ *   VixDiskLib_Exit
+ *   VixDiskLib_ConnectEx
+ *   VixDiskLib_Open
+ *   VixDiskLib_Close
+ *   VixDiskLib_Disconnect
+ *
+ * (Any other calls are made directly by the plugin.)
+ *
+ * To make one of the above calls, the plugin thread fills in struct
+ * request with the request type and signals on the request_cond.  The
+ * plugin thread then waits on the return_cond until that is signalled
+ * which indicates that the request was carried out (or there was an
+ * error).  The return value can be read out of the request struct.
+ * All this logic is wrapped in the octhread_do_* functions below.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <string.h>
+#include <errno.h>
+
+#include <pthread.h>
+
+#define NBDKIT_API_VERSION 2
+#include <nbdkit-plugin.h>
+
+#include "cleanup.h"
+
+#include "vddk.h"
+
+static pthread_t thread;
+
+/* Enable debugging of message passing: -D vddk.messagepassing=1 */
+int vddk_debug_messagepassing;
+
+/* Because the entire purpose of this is to serialize certain
+ * operations, the request struct can only be used by one calling
+ * thread.  The calling thread must hold request_lock during the
+ * entire request.
+ */
+static pthread_mutex_t request_lock = PTHREAD_MUTEX_INITIALIZER;
+static struct {
+  /* Which function to call. */
+  enum { NONE = 0,
+         INITEX = 1, EXIT, CONNECTEX, OPEN, CLOSE, DISCONNECT,
+         STOP } request_type;
+
+  /* Parameters. */
+  VixDiskLibConnectParams *params;
+  VixDiskLibConnection *connection_ret;
+  VixDiskLibConnection connection;
+  VixDiskLibHandle *handle_ret;
+  VixDiskLibHandle handle;
+  int readonly;
+
+  /* Return code. */
+  bool request_done;
+  VixError ret;
+} request;
+
+/* Request and return conditions. */
+static pthread_cond_t request_cond = PTHREAD_COND_INITIALIZER;
+static pthread_mutex_t request_cond_lock = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t return_cond = PTHREAD_COND_INITIALIZER;
+static pthread_mutex_t return_cond_lock = PTHREAD_MUTEX_INITIALIZER;
+
+static inline void
+wake_up_octhread (void)
+{
+  ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&request_cond_lock);
+  pthread_cond_signal (&request_cond);
+}
+
+static inline void
+wait_for_octhread (void)
+{
+  ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&return_cond_lock);
+  while (! request.request_done)
+    pthread_cond_wait (&return_cond, &return_cond_lock);
+  request.request_done = false;
+}
+
+static VixError
+do_request (void)
+{
+  if (vddk_debug_messagepassing)
+    nbdkit_debug ("waking up octhread with request %d",
request.request_type);
+  wake_up_octhread ();
+  if (vddk_debug_messagepassing)
+    nbdkit_debug ("waiting for octhread");
+  wait_for_octhread ();
+
+  return request.ret;
+}
+
+static void *octhread_run (void *);
+
+/*----------------------------------------------------------------------*/
+/* Plugins call these functions from the normal nbdkit thread context. */
+
+/* Start the octhread.  Called by vddk_ready_to_serve. */
+void
+start_octhread (void)
+{
+  int err;
+
+  err = pthread_create (&thread, NULL, octhread_run, NULL);
+  if (err != 0) {
+    errno = err;
+    nbdkit_error ("octhread: pthread_create: %m");
+    exit (EXIT_FAILURE);
+  }
+}
+
+/* Terminate the octhread.  Called from vddk_unload. */
+void
+stop_octhread (void)
+{
+  ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&request_lock);
+
+  request.request_type = STOP;
+  do_request ();
+
+  /* We must wait for the thread to exit to avoid races during
+   * shutdown, eg. if this plugin was unloaded while the thread was
+   * still exiting.
+   */
+  pthread_join (thread, NULL);
+}
+
+/* Request VixDiskLib_InitEx and wait for it to finish. */
+VixError
+octhread_do_VixDiskLib_InitEx (void)
+{
+  ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&request_lock);
+
+  request.request_type = INITEX;
+  return do_request ();
+}
+
+/* Request VixDiskLib_Exit and wait for it to finish. */
+VixError
+octhread_do_VixDiskLib_Exit (void)
+{
+  ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&request_lock);
+
+  request.request_type = EXIT;
+  return do_request ();
+}
+
+/* Request VixDiskLib_ConnectEx and wait for it to finish. */
+VixError
+octhread_do_VixDiskLib_ConnectEx (const VixDiskLibConnectParams *params,
+                                  int readonly,
+                                  VixDiskLibConnection *connection)
+{
+  ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&request_lock);
+
+  request.request_type = CONNECTEX;
+  request.params = (VixDiskLibConnectParams *) params;
+  request.readonly = readonly;
+  request.connection_ret = connection;
+  return do_request ();
+}
+
+/* Request VixDiskLib_Open and wait for it to finish. */
+VixError
+octhread_do_VixDiskLib_Open (const VixDiskLibConnection connection,
+                             int readonly,
+                             VixDiskLibHandle *handle)
+{
+  ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&request_lock);
+
+  request.request_type = OPEN;
+  request.connection = connection;
+  request.readonly = readonly;
+  request.handle_ret = handle;
+  return do_request ();
+}
+
+/* Request VixDiskLib_Close and wait for it to finish. */
+VixError
+octhread_do_VixDiskLib_Close (VixDiskLibHandle handle)
+{
+  ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&request_lock);
+
+  request.request_type = CLOSE;
+  request.handle = handle;
+  return do_request ();
+}
+
+/* Request VixDiskLib_Disconnect and wait for it to finish. */
+VixError
+octhread_do_VixDiskLib_Disconnect (VixDiskLibConnection connection)
+{
+  ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&request_lock);
+
+  request.request_type = DISCONNECT;
+  request.connection = connection;
+  return do_request ();
+}
+
+/*----------------------------------------------------------------------*/
+/* The octhread itself. */
+
+static void
+trim (char *str)
+{
+  size_t len = strlen (str);
+
+  if (len > 0 && str[len-1] == '\n')
+    str[len-1] = '\0';
+}
+
+/* Turn log messages from the library into nbdkit_debug. */
+static void
+debug_function (const char *fs, va_list args)
+{
+  CLEANUP_FREE char *str = NULL;
+
+  if (vasprintf (&str, fs, args) == -1) {
+    nbdkit_debug ("lost debug message: %s", fs);
+    return;
+  }
+
+  trim (str);
+
+  nbdkit_debug ("%s", str);
+}
+
+/* Turn error messages from the library into nbdkit_error. */
+static void
+error_function (const char *fs, va_list args)
+{
+  CLEANUP_FREE char *str = NULL;
+
+  if (vasprintf (&str, fs, args) == -1) {
+    nbdkit_error ("lost error message: %s", fs);
+    return;
+  }
+
+  trim (str);
+
+  nbdkit_error ("%s", str);
+}
+
+static void *
+octhread_run (void *arg)
+{
+  int err;
+  uint32_t flags;
+  bool stop = false;
+
+  nbdkit_debug ("octhread started");
+
+  while (!stop) {
+    /* Wait for an incoming request. */
+    {
+      ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&request_cond_lock);
+      while (request.request_type == NONE)
+        pthread_cond_wait (&request_cond, &request_cond_lock);
+    }
+
+    if (vddk_debug_messagepassing)
+      nbdkit_debug ("octhread: request %d", request.request_type);
+
+    switch (request.request_type) {
+    case INITEX:
+      DEBUG_CALL ("VixDiskLib_InitEx",
+                  "%d, %d, &debug_fn, &error_fn, &error_fn,
%s, %s",
+                  VDDK_MAJOR, VDDK_MINOR,
+                  libdir ? : VDDK_LIBDIR, config ? : "NULL");
+      err = VixDiskLib_InitEx (VDDK_MAJOR, VDDK_MINOR,
+                               &debug_function, /* log function */
+                               &error_function, /* warn function */
+                               &error_function, /* panic function */
+                               libdir ? : VDDK_LIBDIR, config);
+      break;
+
+    case EXIT:
+      DEBUG_CALL ("VixDiskLib_Exit", "");
+      VixDiskLib_Exit ();
+      err = VIX_OK;
+      break;
+
+    case CONNECTEX:
+      DEBUG_CALL ("VixDiskLib_ConnectEx",
+                  "params, %d, %s, %s, &connection",
+                  request.readonly,
+                  snapshot_moref ? : "NULL",
+                  transport_modes ? : "NULL");
+      err = VixDiskLib_ConnectEx (request.params,
+                                  request.readonly,
+                                  snapshot_moref,
+                                  transport_modes,
+                                  request.connection_ret);
+      break;
+
+    case OPEN:
+      flags = 0;
+      if (request.readonly)
+        flags |= VIXDISKLIB_FLAG_OPEN_READ_ONLY;
+      if (single_link)
+        flags |= VIXDISKLIB_FLAG_OPEN_SINGLE_LINK;
+      if (unbuffered)
+        flags |= VIXDISKLIB_FLAG_OPEN_UNBUFFERED;
+      DEBUG_CALL ("VixDiskLib_Open",
+                  "connection, %s, %d, &handle", filename,
flags);
+      err = VixDiskLib_Open (request.connection, filename, flags,
+                             request.handle_ret);
+      break;
+
+    case CLOSE:
+      DEBUG_CALL ("VixDiskLib_Close", "handle");
+      err = VixDiskLib_Close (request.handle);
+      break;
+
+    case DISCONNECT:
+      DEBUG_CALL ("VixDiskLib_Disconnect", "connection");
+      err = VixDiskLib_Disconnect (request.connection);
+      break;
+
+    case STOP:
+      stop = true;
+      err = VIX_OK;
+      break;
+
+    case NONE:
+    default:
+      abort ();
+    }
+
+    request.request_type = NONE;
+
+    /* Set up return field and signal to plugin thread that we have
+     * finished.
+     */
+    if (vddk_debug_messagepassing)
+      nbdkit_debug ("octhread: return %d", err);
+    request.ret = err;
+    request.request_done = true;
+
+    {
+      ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&return_cond_lock);
+      pthread_cond_signal (&return_cond);
+    }
+  } /* for (;;) */
+
+  nbdkit_debug ("octhread exiting");
+
+  pthread_exit (NULL);
+}
diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c
index 5d3764d..375fbfa 100644
--- a/plugins/vddk/vddk.c
+++ b/plugins/vddk/vddk.c
@@ -42,7 +42,6 @@
 #include <dlfcn.h>
 
 #define NBDKIT_API_VERSION 2
-
 #include <nbdkit-plugin.h>
 
 #include "cleanup.h"
@@ -50,7 +49,7 @@
 #include "minmax.h"
 #include "rounding.h"
 
-#include "vddk-structs.h"
+#include "vddk.h"
 
 /* Enable extra disk info debugging with: -D vddk.diskinfo=1 */
 int vddk_debug_diskinfo;
@@ -58,90 +57,34 @@ int vddk_debug_diskinfo;
 /* Enable debugging of extents code with: -D vddk.extents=1 */
 int vddk_debug_extents;
 
-/* For each VDDK API define a static global variable.  These globals
- * are initialized when the plugin is loaded (by vddk_load).
+/* For each VDDK API define a global variable.  These globals are
+ * initialized when the plugin is loaded (by vddk_load).
  */
-#define STUB(fn,ret,args) static ret (*fn) args
-#define OPTIONAL_STUB(fn,ret,args) static ret (*fn) args
+#define STUB(fn,ret,args) ret (*fn) args
+#define OPTIONAL_STUB(fn,ret,args) ret (*fn) args
 #include "vddk-stubs.h"
 #undef STUB
 #undef OPTIONAL_STUB
 
-/* Parameters passed to InitEx. */
-#define VDDK_MAJOR 5
-#define VDDK_MINOR 1
-
 static void *dl = NULL;                    /* dlopen handle */
-static int init_called = 0;                /* was InitEx called */
-
-static char *config = NULL;                /* config */
-static const char *cookie = NULL;          /* cookie */
-static const char *filename = NULL;        /* file */
-static char *libdir = NULL;                /* libdir */
-static uint16_t nfc_host_port = 0;         /* nfchostport */
-static char *password = NULL;              /* password */
-static uint16_t port = 0;                  /* port */
-static const char *server_name = NULL;     /* server */
-static bool single_link = false;           /* single-link */
-static const char *snapshot_moref = NULL;  /* snapshot */
-static const char *thumb_print = NULL;     /* thumbprint */
-static const char *transport_modes = NULL; /* transports */
-static bool unbuffered = false;            /* unbuffered */
-static const char *username = NULL;        /* user */
-static const char *vmx_spec = NULL;        /* vm */
-static bool is_remote = false;
-
-#define VDDK_ERROR(err, fs, ...)                                \
-  do {                                                          \
-    char *vddk_err_msg;                                         \
-    vddk_err_msg = VixDiskLib_GetErrorText ((err), NULL);       \
-    nbdkit_error (fs ": %s", ##__VA_ARGS__, vddk_err_msg);      \
-    VixDiskLib_FreeErrorText (vddk_err_msg);                    \
-  } while (0)
-
-#define DEBUG_CALL(fn, fs, ...)                                 \
-  nbdkit_debug ("VDDK call: %s (" fs ")", fn,
##__VA_ARGS__)
-
-static void
-trim (char *str)
-{
-  size_t len = strlen (str);
-
-  if (len > 0 && str[len-1] == '\n')
-    str[len-1] = '\0';
-}
-
-/* Turn log messages from the library into nbdkit_debug. */
-static void
-debug_function (const char *fs, va_list args)
-{
-  CLEANUP_FREE char *str = NULL;
-
-  if (vasprintf (&str, fs, args) == -1) {
-    nbdkit_debug ("lost debug message: %s", fs);
-    return;
-  }
-
-  trim (str);
-
-  nbdkit_debug ("%s", str);
-}
-
-/* Turn error messages from the library into nbdkit_error. */
-static void
-error_function (const char *fs, va_list args)
-{
-  CLEANUP_FREE char *str = NULL;
-
-  if (vasprintf (&str, fs, args) == -1) {
-    nbdkit_error ("lost error message: %s", fs);
-    return;
-  }
-
-  trim (str);
-
-  nbdkit_error ("%s", str);
-}
+static bool octhread_running = false;      /* octhread is running */
+
+char *config = NULL;                       /* config */
+const char *cookie = NULL;                 /* cookie */
+const char *filename = NULL;               /* file */
+char *libdir = NULL;                       /* libdir */
+uint16_t nfc_host_port = 0;                /* nfchostport */
+char *password = NULL;                     /* password */
+uint16_t port = 0;                         /* port */
+const char *server_name = NULL;            /* server */
+bool single_link = false;                  /* single-link */
+const char *snapshot_moref = NULL;         /* snapshot */
+const char *thumb_print = NULL;            /* thumbprint */
+const char *transport_modes = NULL;        /* transports */
+bool unbuffered = false;                   /* unbuffered */
+const char *username = NULL;               /* user */
+const char *vmx_spec = NULL;               /* vm */
+bool is_remote = false;                    /* is a remote connection? */
 
 /* Load and unload the plugin. */
 static void
@@ -194,9 +137,9 @@ vddk_load (void)
 static void
 vddk_unload (void)
 {
-  if (init_called) {
-    DEBUG_CALL ("VixDiskLib_Exit", "");
-    VixDiskLib_Exit ();
+  if (octhread_running) {
+    octhread_do_VixDiskLib_Exit ();
+    stop_octhread ();
   }
   if (dl)
     dlclose (dl);
@@ -292,8 +235,6 @@ vddk_config (const char *key, const char *value)
 static int
 vddk_config_complete (void)
 {
-  VixError err;
-
   if (filename == NULL) {
     nbdkit_error ("you must supply the file=<FILENAME> parameter
"
                   "after the plugin name on the command line");
@@ -330,29 +271,31 @@ vddk_config_complete (void)
 #undef missing
   }
 
+  return 0;
+}
+
+#define vddk_config_help \
+  "file=<FILENAME>     (required) The filename (eg. VMDK file) to
serve.\n" \
+  "Many optional parameters are supported, see
nbdkit-vddk-plugin(3)."
+
+static int
+vddk_ready_to_serve (void)
+{
+  VixError err;
+
+  start_octhread ();
+
   /* Initialize VDDK library. */
-  DEBUG_CALL ("VixDiskLib_InitEx",
-              "%d, %d, &debug_fn, &error_fn, &error_fn, %s,
%s",
-              VDDK_MAJOR, VDDK_MINOR,
-              libdir ? : VDDK_LIBDIR, config ? : "NULL");
-  err = VixDiskLib_InitEx (VDDK_MAJOR, VDDK_MINOR,
-                           &debug_function, /* log function */
-                           &error_function, /* warn function */
-                           &error_function, /* panic function */
-                           libdir ? : VDDK_LIBDIR, config);
+  err = octhread_do_VixDiskLib_InitEx ();
   if (err != VIX_OK) {
     VDDK_ERROR (err, "VixDiskLib_InitEx");
-    exit (EXIT_FAILURE);
+    return -1;
   }
-  init_called = 1;
 
+  octhread_running = true;
   return 0;
 }
 
-#define vddk_config_help \
-  "file=<FILENAME>     (required) The filename (eg. VMDK file) to
serve.\n" \
-  "Many optional parameters are supported, see
nbdkit-vddk-plugin(3)."
-
 static void
 vddk_dump_plugin (void)
 {
@@ -376,11 +319,11 @@ vddk_dump_plugin (void)
 #endif
 }
 
-/* XXX To really do threading correctly in accordance with the VDDK
- * documentation, we must do all open/close calls from a single
- * thread.  This is a huge pain.
+/* See "Multithreading Considerations" in the VDDK manual.  VDDK does
+ * not allow parallel requests on the same handle, so we cannot use
+ * the parallel thread model.
  */
-#define THREAD_MODEL NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS
+#define THREAD_MODEL NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS
 
 /* The per-connection handle. */
 struct vddk_handle {
@@ -424,7 +367,6 @@ vddk_open (int readonly)
 {
   struct vddk_handle *h;
   VixError err;
-  uint32_t flags;
 
   h = malloc (sizeof *h);
   if (h == NULL) {
@@ -462,33 +404,15 @@ vddk_open (int readonly)
    * Advanced Transport modes, but I could not make it work with
    * either ESXi or vCenter servers.
    */
-
-  DEBUG_CALL ("VixDiskLib_ConnectEx",
-              "h->params, %d, %s, %s, &connection",
-              readonly,
-              snapshot_moref ? : "NULL",
-              transport_modes ? : "NULL");
-  err = VixDiskLib_ConnectEx (h->params,
-                              readonly,
-                              snapshot_moref,
-                              transport_modes,
-                              &h->connection);
+  err = octhread_do_VixDiskLib_ConnectEx (h->params, readonly,
+                                          &h->connection);
   if (err != VIX_OK) {
     VDDK_ERROR (err, "VixDiskLib_ConnectEx");
     goto err1;
   }
 
-  flags = 0;
-  if (readonly)
-    flags |= VIXDISKLIB_FLAG_OPEN_READ_ONLY;
-  if (single_link)
-    flags |= VIXDISKLIB_FLAG_OPEN_SINGLE_LINK;
-  if (unbuffered)
-    flags |= VIXDISKLIB_FLAG_OPEN_UNBUFFERED;
-
-  DEBUG_CALL ("VixDiskLib_Open",
-              "connection, %s, %d, &handle", filename, flags);
-  err = VixDiskLib_Open (h->connection, filename, flags, &h->handle);
+  err = octhread_do_VixDiskLib_Open (h->connection, readonly,
+                                     &h->handle);
   if (err != VIX_OK) {
     VDDK_ERROR (err, "VixDiskLib_Open: %s", filename);
     goto err2;
@@ -500,8 +424,7 @@ vddk_open (int readonly)
   return h;
 
  err2:
-  DEBUG_CALL ("VixDiskLib_Disconnect", "connection");
-  VixDiskLib_Disconnect (h->connection);
+  octhread_do_VixDiskLib_Disconnect (h->connection);
  err1:
   free_connect_params (h->params);
  err0:
@@ -515,10 +438,8 @@ vddk_close (void *handle)
 {
   struct vddk_handle *h = handle;
 
-  DEBUG_CALL ("VixDiskLib_Close", "handle");
-  VixDiskLib_Close (h->handle);
-  DEBUG_CALL ("VixDiskLib_Disconnect", "connection");
-  VixDiskLib_Disconnect (h->connection);
+  octhread_do_VixDiskLib_Close (h->handle);
+  octhread_do_VixDiskLib_Disconnect (h->connection);
   free_connect_params (h->params);
   free (h);
 }
@@ -833,10 +754,11 @@ static struct nbdkit_plugin plugin = {
   .version           = PACKAGE_VERSION,
   .load              = vddk_load,
   .unload            = vddk_unload,
+  .dump_plugin       = vddk_dump_plugin,
   .config            = vddk_config,
   .config_complete   = vddk_config_complete,
   .config_help       = vddk_config_help,
-  .dump_plugin       = vddk_dump_plugin,
+  .ready_to_serve    = vddk_ready_to_serve,
   .open              = vddk_open,
   .close             = vddk_close,
   .get_size          = vddk_get_size,
diff --git a/plugins/vddk/vddk.h b/plugins/vddk/vddk.h
new file mode 100644
index 0000000..7d012eb
--- /dev/null
+++ b/plugins/vddk/vddk.h
@@ -0,0 +1,89 @@
+/* nbdkit
+ * Copyright (C) 2013-2019 Red Hat Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of Red Hat nor the names of its contributors may be
+ * used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+ * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#ifndef NBDKIT_VDDK_H
+#define NBDKIT_VDDK_H
+
+#include "vddk-structs.h"
+
+/* For each VDDK API define an extern global variable.  These globals
+ * are initialized when the plugin is loaded (by vddk_load).
+ */
+#define STUB(fn,ret,args) extern ret (*fn) args
+#define OPTIONAL_STUB(fn,ret,args) extern ret (*fn) args
+#include "vddk-stubs.h"
+#undef STUB
+#undef OPTIONAL_STUB
+
+extern char *config;
+extern const char *cookie;
+extern const char *filename;
+extern char *libdir;
+extern uint16_t nfc_host_port;
+extern char *password;
+extern uint16_t port;
+extern const char *server_name;
+extern bool single_link;
+extern const char *snapshot_moref;
+extern const char *thumb_print;
+extern const char *transport_modes;
+extern bool unbuffered;
+extern const char *username;
+extern const char *vmx_spec;
+extern bool is_remote;
+
+#define VDDK_ERROR(err, fs, ...)                                \
+  do {                                                          \
+    char *vddk_err_msg;                                         \
+    vddk_err_msg = VixDiskLib_GetErrorText ((err), NULL);       \
+    nbdkit_error (fs ": %s", ##__VA_ARGS__, vddk_err_msg);      \
+    VixDiskLib_FreeErrorText (vddk_err_msg);                    \
+  } while (0)
+
+#define DEBUG_CALL(fn, fs, ...)                                 \
+  nbdkit_debug ("VDDK call: %s (" fs ")", fn,
##__VA_ARGS__)
+
+/* Parameters passed to InitEx. */
+#define VDDK_MAJOR 5
+#define VDDK_MINOR 1
+
+/* Functions in octhread.c */
+extern void start_octhread (void);
+extern void stop_octhread (void);
+extern VixError octhread_do_VixDiskLib_InitEx (void);
+extern VixError octhread_do_VixDiskLib_Exit (void);
+extern VixError octhread_do_VixDiskLib_ConnectEx (const VixDiskLibConnectParams
*params, int readonly, VixDiskLibConnection *connection);
+extern VixError octhread_do_VixDiskLib_Open (const VixDiskLibConnection
connection, int readonly, VixDiskLibHandle *handle);
+extern VixError octhread_do_VixDiskLib_Close (VixDiskLibHandle handle);
+extern VixError octhread_do_VixDiskLib_Disconnect (VixDiskLibConnection
connection);
+
+#endif /* NBDKIT_VDDK_H */
diff --git a/tests/test-vddk-real.sh b/tests/test-vddk-real.sh
index 60f634c..6b31808 100755
--- a/tests/test-vddk-real.sh
+++ b/tests/test-vddk-real.sh
@@ -57,7 +57,7 @@ qemu-img create -f vmdk test-vddk-real.vmdk 100M
 export old_ld_library_path="$LD_LIBRARY_PATH"
 export LD_LIBRARY_PATH="$vddkdir/$lib:$LD_LIBRARY_PATH"
 
-nbdkit -f -v -U - \
+nbdkit -f -v -U - -D vddk.messagepassing=1 \
        --filter=readahead \
        vddk libdir="$vddkdir" file=test-vddk-real.vmdk \
        --run '
-- 
2.23.0

On 10/11/19 4:42 AM, Richard W.M. Jones wrote:
> This method can be used for plugins to get control after the server
> has forked and changed user but before it accepts a connection.  This
> is very late and the only real use for this is for a plugin to create
> background threads for its own use.
> ---
> +++ b/docs/nbdkit-filter.pod
>   
> +=head2 C<.ready_to_serve>
> +
> + int (*ready_to_serve) (nbdkit_next_ready_to_serve *next, void *nxdata);
> +
> +This intercepts the plugin C<.ready_to_serve> method.
> +
> +If there is an error, C<.ready_to_serve> should call
C<nbdkit_error>
> +with an error message and return C<-1>.
Do we need to require the filter to be involved in the recursion?

With .open and .config, we do, because there are parameters to the 
function, where the filter may change the parameter handed to the plugin 
(for example, changing read-write to read-only or altering a config 
key).  But for other functions where there is no parameter, we let 
nbdkit control the recursion (.prepare, .finalize, .close).  There is no 
other parameter here to be changed, so having nbdkit run the recursion 
may make more sense.

Should there be a counterpart function for cleanup?  We have:

.load/.unload - called once per nbdkit process, too early for forks
[here's where .ready_to_serve fits]
.open/.close - called once per connection, too early for next_ops
.close/.finalize - called once per connection, full access

The new .ready_to_serve is called once per nbdkit process, but at a 
point where it knows all .loads are complete and it is safe to fork.  I 
guess .unload can also do a pthread_join() on any thread created during 
.ready_to_serve.  We needed .finalize separate from .close because the 
filter may still need to manipulate the plugin, but there is no 
manipulation of the plugin needed during .load or .ready_to_serve, so 
having a trio of related functions rather than two nested pairs of 
functions still works.
> +++ b/docs/nbdkit-plugin.pod
> @@ -142,6 +142,13 @@ before any connections are accepted.  However, during
C<nbdkit
>   C<.config_complete> (so a plugin which determines the results from
a
>   script must be prepared for a missing script).
>   
> +=item C<.ready_to_serve>
> +
> +This callback is called after nbdkit has forked into the background
> +and changed user, and before it accepts any client connection.  If the
> +plugin needs to create its own background threads then this is a good
> +place to do that.
> +
Should there be any caveat about the threads needing to be joined no 
later than .unload, so that there is no risk of the helper threads 
segfaulting after the .so is unloaded?
>   =item C<.open>
>   
>   A new client has connected.
> @@ -408,7 +415,8 @@ Any other bare parameters give errors.
>   
>   This optional callback is called after all the configuration has been
>   passed to the plugin.  It is a good place to do checks, for example
> -that the user has passed the required parameters to the plugin.
> +that the user has passed the required parameters to the plugin.  It is
> +also a good place to do general start-up work.
Should this emphasize that .load must not leave other threads running on 
its completion, because it is invoked pre-fork()?
>   
>   If there is an error, C<.config_complete> should call
C<nbdkit_error>
>   with an error message and return C<-1>.
> @@ -437,6 +445,23 @@ are silently ignored.
>   If there is an error, C<.thread_model> should call
C<nbdkit_error>
>   with an error message and return C<-1>.
>   
> +=head2 C<.ready_to_serve>
> +
> + int ready_to_serve (void);
> +
> +This optional callback is called after nbdkit has forked into the
> +background and changed user, and before it accepts any client
> +connection.
> +
> +If the plugin needs to create its own background threads then this is
> +a good place to do that.  However because nbdkit may have already
> +forked into the background and so it is difficult to communicate
> +errors back to the user, this is B<not> a good place to do general
> +start-up work (use C<.config_complete> instead).
> +
> +If there is an error, C<.ready_to_serve> should call
C<nbdkit_error>
> +with an error message and return C<-1>.
> +
>   =head2 C<.open>
>   
>    void *open (int readonly);
> diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h
> index c1930c1..7b3a64f 100644
> +++ b/server/filters.c
> @@ -190,6 +190,29 @@ plugin_magic_config_key (struct backend *b)
>     return b->next->magic_config_key (b->next);
>   }
>   
> +static int
> +next_ready_to_serve (void *nxdata)
> +{
> +  struct backend *b = nxdata;
> +  b->ready_to_serve (b);
> +  return 0;
> +}
> +
> +static void
> +filter_ready_to_serve (struct backend *b)
> +{
> +  struct backend_filter *f = container_of (b, struct backend_filter,
backend);
> +
> +  debug ("%s: ready_to_serve", b->name);
> +
> +  if (f->filter.ready_to_serve) {
> +    if (f->filter.ready_to_serve (next_ready_to_serve, b->next) ==
-1)
> +      exit (EXIT_FAILURE);
> +  }
> +  else
> +    b->next->ready_to_serve (b->next);
Should we add a backend_ready_to_serve() helper function?  Do we want 
nbdkit to run the recursion itself, instead of requiring the filters to 
call next_ready_to_serve?

> +++ b/tests/test-layers-filter.c
> @@ -65,7 +65,7 @@ test_layers_filter_config (nbdkit_next_config *next, void
*nxdata,
>   
>   static int
>   test_layers_filter_config_complete (nbdkit_next_config_complete *next,
> -                                     void *nxdata)
> +                                    void *nxdata)
>   {
>     DEBUG_FUNCTION;
>     return next (nxdata);
> @@ -74,6 +74,14 @@ test_layers_filter_config_complete
(nbdkit_next_config_complete *next,
>   #define test_layers_filter_config_help          \
>     "test_layers_" layer "_config_help"
>   
> +static int
> +test_layers_filter_ready_to_serve (nbdkit_next_ready_to_serve *next,
> +                                     void *nxdata)
Indentation looks off.

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