Libguestfs - Jan 2018 - [PATCH nbdkit 0/3] Refactor plugin_* functions into a backend struct.

Somewhat invasive but mostly mechanical change to how plugins are
called.  This patch is in preparation for adding a second backend
subtype for filters.

Rich.

Mostly code motion.
---
 src/Makefile.am   |   1 +
 src/connections.c |  14 +++----
 src/internal.h    |  14 ++++---
 src/locks.c       | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/plugins.c     |  77 +++++-------------------------------
 5 files changed, 142 insertions(+), 79 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 12b9043..1f05eab 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -40,6 +40,7 @@ nbdkit_SOURCES = \
 	crypto.c \
 	errors.c \
 	internal.h \
+	locks.c \
 	main.c \
 	plugins.c \
 	protocol.h \
diff --git a/src/connections.c b/src/connections.c
index 111a810..74bb8e4 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -211,7 +211,7 @@ _handle_single_connection (int sockin, int sockout)
   int nworkers = threads ? threads : DEFAULT_PARALLEL_REQUESTS;
   pthread_t *workers = NULL;
 
-  if (!plugin_is_parallel() || nworkers == 1)
+  if (plugin_thread_model () < NBDKIT_THREAD_MODEL_PARALLEL || nworkers ==
1)
     nworkers = 0;
   conn = new_connection (sockin, sockout, nworkers);
   if (!conn)
@@ -287,9 +287,9 @@ handle_single_connection (int sockin, int sockout)
 {
   int r;
 
-  plugin_lock_connection ();
+  lock_connection ();
   r = _handle_single_connection (sockin, sockout);
-  plugin_unlock_connection ();
+  unlock_connection ();
 
   return r;
 }
@@ -740,12 +740,12 @@ negotiate_handshake (struct connection *conn)
 {
   int r;
 
-  plugin_lock_request (conn);
+  lock_request (conn);
   if (!newstyle)
     r = _negotiate_handshake_oldstyle (conn);
   else
     r = _negotiate_handshake_newstyle (conn);
-  plugin_unlock_request (conn);
+  unlock_request (conn);
 
   return r;
 }
@@ -1057,9 +1057,9 @@ recv_request_send_reply (struct connection *conn)
     error = ESHUTDOWN;
   }
   else {
-    plugin_lock_request (conn);
+    lock_request (conn);
     error = handle_request (conn, cmd, flags, offset, count, buf);
-    plugin_unlock_request (conn);
+    unlock_request (conn);
   }
 
   /* Send the reply packet. */
diff --git a/src/internal.h b/src/internal.h
index 73bc09e..068204b 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -144,17 +144,13 @@ extern int crypto_negotiate_tls (struct connection *conn,
int sockin, int sockou
 /* plugins.c */
 extern void plugin_register (const char *_filename, void *_dl, struct
nbdkit_plugin *(*plugin_init) (void));
 extern void plugin_cleanup (void);
+extern int plugin_thread_model (void);
 extern const char *plugin_name (void);
 extern void plugin_usage (void);
 extern const char *plugin_version (void);
 extern void plugin_dump_fields (void);
 extern void plugin_config (const char *key, const char *value);
 extern void plugin_config_complete (void);
-extern void plugin_lock_connection (void);
-extern void plugin_unlock_connection (void);
-extern void plugin_lock_request (struct connection *conn);
-extern void plugin_unlock_request (struct connection *conn);
-extern bool plugin_is_parallel (void);
 extern int plugin_errno_is_preserved (void);
 extern int plugin_open (struct connection *conn, int readonly);
 extern void plugin_close (struct connection *conn);
@@ -169,6 +165,14 @@ extern int plugin_flush (struct connection *conn);
 extern int plugin_trim (struct connection *conn, uint32_t count, uint64_t
offset);
 extern int plugin_zero (struct connection *conn, uint32_t count, uint64_t
offset, int may_trim);
 
+/* locks.c */
+extern void lock_connection (void);
+extern void unlock_connection (void);
+extern void lock_request (struct connection *conn);
+extern void unlock_request (struct connection *conn);
+extern void lock_unload (void);
+extern void unlock_unload (void);
+
 /* sockets.c */
 extern int *bind_unix_socket (size_t *);
 extern int *bind_tcpip_socket (size_t *);
diff --git a/src/locks.c b/src/locks.c
new file mode 100644
index 0000000..6021356
--- /dev/null
+++ b/src/locks.c
@@ -0,0 +1,115 @@
+/* nbdkit
+ * Copyright (C) 2013-2018 Red Hat Inc.
+ * All rights reserved.
+ *
+ * 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.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "internal.h"
+
+static pthread_mutex_t connection_lock = PTHREAD_MUTEX_INITIALIZER;
+static pthread_mutex_t all_requests_lock = PTHREAD_MUTEX_INITIALIZER;
+static pthread_rwlock_t unload_prevention_lock = PTHREAD_RWLOCK_INITIALIZER;
+
+void
+lock_connection (void)
+{
+  int thread_model = plugin_thread_model ();
+
+  if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) {
+    debug ("acquire connection lock");
+    pthread_mutex_lock (&connection_lock);
+  }
+}
+
+void
+unlock_connection (void)
+{
+  int thread_model = plugin_thread_model ();
+
+  if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) {
+    debug ("release connection lock");
+    pthread_mutex_unlock (&connection_lock);
+  }
+}
+
+void
+lock_request (struct connection *conn)
+{
+  int thread_model = plugin_thread_model ();
+
+  if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS) {
+    debug ("acquire global request lock");
+    pthread_mutex_lock (&all_requests_lock);
+  }
+
+  if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS) {
+    debug ("acquire per-connection request lock");
+    pthread_mutex_lock (connection_get_request_lock (conn));
+  }
+
+  debug ("acquire unload prevention lock");
+  pthread_rwlock_rdlock (&unload_prevention_lock);
+}
+
+void
+unlock_request (struct connection *conn)
+{
+  int thread_model = plugin_thread_model ();
+
+  debug ("release unload prevention lock");
+  pthread_rwlock_unlock (&unload_prevention_lock);
+
+  if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS) {
+    debug ("release per-connection request lock");
+    pthread_mutex_unlock (connection_get_request_lock (conn));
+  }
+
+  if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS) {
+    debug ("release global request lock");
+    pthread_mutex_unlock (&all_requests_lock);
+  }
+}
+
+void
+lock_unload (void)
+{
+  pthread_rwlock_wrlock (&unload_prevention_lock);
+}
+
+void
+unlock_unload (void)
+{
+  pthread_rwlock_unlock (&unload_prevention_lock);
+}
diff --git a/src/plugins.c b/src/plugins.c
index 9b5d2d5..b7ab43d 100644
--- a/src/plugins.c
+++ b/src/plugins.c
@@ -46,10 +46,6 @@
 #include "nbdkit-plugin.h"
 #include "internal.h"
 
-static pthread_mutex_t connection_lock = PTHREAD_MUTEX_INITIALIZER;
-static pthread_mutex_t all_requests_lock = PTHREAD_MUTEX_INITIALIZER;
-static pthread_rwlock_t unload_prevention_lock = PTHREAD_RWLOCK_INITIALIZER;
-
 /* Maximum read or write request that we will handle. */
 #define MAX_REQUEST_SIZE (64 * 1024 * 1024)
 
@@ -165,7 +161,7 @@ plugin_cleanup (void)
     /* Acquiring this lock prevents any plugin callbacks from running
      * simultaneously.
      */
-    pthread_rwlock_wrlock (&unload_prevention_lock);
+    lock_unload ();
 
     debug ("%s: unload", filename);
     if (plugin.unload)
@@ -176,10 +172,18 @@ plugin_cleanup (void)
     free (filename);
     filename = NULL;
 
-    pthread_rwlock_unlock (&unload_prevention_lock);
+    unlock_unload ();
   }
 }
 
+int
+plugin_thread_model (void)
+{
+  assert (dl);
+
+  return plugin._thread_model;
+}
+
 const char *
 plugin_name (void)
 {
@@ -312,67 +316,6 @@ plugin_config_complete (void)
     exit (EXIT_FAILURE);
 }
 
-/* Handle the thread model. */
-void
-plugin_lock_connection (void)
-{
-  if (plugin._thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) {
-    debug ("%s: acquire connection lock", filename);
-    pthread_mutex_lock (&connection_lock);
-  }
-}
-
-void
-plugin_unlock_connection (void)
-{
-  if (plugin._thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) {
-    debug ("%s: release connection lock", filename);
-    pthread_mutex_unlock (&connection_lock);
-  }
-}
-
-void
-plugin_lock_request (struct connection *conn)
-{
-  if (plugin._thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS) {
-    debug ("acquire global request lock");
-    pthread_mutex_lock (&all_requests_lock);
-  }
-
-  if (plugin._thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS) {
-    debug ("acquire per-connection request lock");
-    pthread_mutex_lock (connection_get_request_lock (conn));
-  }
-
-  debug ("acquire unload prevention lock");
-  pthread_rwlock_rdlock (&unload_prevention_lock);
-}
-
-void
-plugin_unlock_request (struct connection *conn)
-{
-  debug ("release unload prevention lock");
-  pthread_rwlock_unlock (&unload_prevention_lock);
-
-  if (plugin._thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS) {
-    debug ("release per-connection request lock");
-    pthread_mutex_unlock (connection_get_request_lock (conn));
-  }
-
-  if (plugin._thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS) {
-    debug ("release global request lock");
-    pthread_mutex_unlock (&all_requests_lock);
-  }
-}
-
-bool
-plugin_is_parallel (void)
-{
-  assert (dl);
-
-  return plugin._thread_model >= NBDKIT_THREAD_MODEL_PARALLEL;
-}
-
 int
 plugin_errno_is_preserved (void)
 {
-- 
2.15.1

Introduce the concept of a backend.  Currently the only type of
backend is a plugin, and there can only be one of them.  Instead of
calling functions like ‘plugin_pwrite’ you call the backend method
‘backend->pwrite (backend, ...)’.

The change is largely mechanical.  I was able to remove ‘assert (dl)’
statements throughout since we can now prove they will never be
called.

Note this does not lift the restriction of one plugin per server, and
it can *never* do that because plugins can use global variables.
---
 src/connections.c |  40 +++--
 src/internal.h    |  49 ++---
 src/locks.c       |   8 +-
 src/main.c        |  31 ++--
 src/plugins.c     | 523 ++++++++++++++++++++++++++++++------------------------
 5 files changed, 360 insertions(+), 291 deletions(-)

diff --git a/src/connections.c b/src/connections.c
index 74bb8e4..921a5b2 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -211,16 +211,17 @@ _handle_single_connection (int sockin, int sockout)
   int nworkers = threads ? threads : DEFAULT_PARALLEL_REQUESTS;
   pthread_t *workers = NULL;
 
-  if (plugin_thread_model () < NBDKIT_THREAD_MODEL_PARALLEL || nworkers ==
1)
+  if (backend->thread_model (backend) < NBDKIT_THREAD_MODEL_PARALLEL ||
+      nworkers == 1)
     nworkers = 0;
   conn = new_connection (sockin, sockout, nworkers);
   if (!conn)
     goto done;
 
-  if (plugin_open (conn, readonly) == -1)
+  if (backend->open (backend, conn, readonly) == -1)
     goto done;
 
-  threadlocal_set_name (plugin_name ());
+  threadlocal_set_name (backend->name (backend));
 
   /* Handshake. */
   if (negotiate_handshake (conn) == -1)
@@ -251,7 +252,8 @@ _handle_single_connection (int sockin, int sockout)
         set_status (conn, -1);
         goto wait;
       }
-      if (asprintf (&worker->name, "%s.%d", plugin_name (),
nworkers) < 0) {
+      if (asprintf (&worker->name,
+                    "%s.%d", backend->name (backend), nworkers)
< 0) {
         perror ("asprintf");
         set_status (conn, -1);
         free (worker);
@@ -340,7 +342,7 @@ free_connection (struct connection *conn)
    */
   if (!quit) {
     if (conn->handle)
-      plugin_close (conn);
+      backend->close (backend, conn);
   }
 
   free (conn);
@@ -352,7 +354,7 @@ compute_eflags (struct connection *conn, uint16_t *flags)
   uint16_t eflags = NBD_FLAG_HAS_FLAGS;
   int fl;
 
-  fl = plugin_can_write (conn);
+  fl = backend->can_write (backend, conn);
   if (fl == -1)
     return -1;
   if (readonly || !fl) {
@@ -363,7 +365,7 @@ compute_eflags (struct connection *conn, uint16_t *flags)
     eflags |= NBD_FLAG_SEND_WRITE_ZEROES;
   }
 
-  fl = plugin_can_flush (conn);
+  fl = backend->can_flush (backend, conn);
   if (fl == -1)
     return -1;
   if (fl) {
@@ -371,7 +373,7 @@ compute_eflags (struct connection *conn, uint16_t *flags)
     conn->can_flush = 1;
   }
 
-  fl = plugin_is_rotational (conn);
+  fl = backend->is_rotational (backend, conn);
   if (fl == -1)
     return -1;
   if (fl) {
@@ -379,7 +381,7 @@ compute_eflags (struct connection *conn, uint16_t *flags)
     conn->is_rotational = 1;
   }
 
-  fl = plugin_can_trim (conn);
+  fl = backend->can_trim (backend, conn);
   if (fl == -1)
     return -1;
   if (fl) {
@@ -407,7 +409,7 @@ _negotiate_handshake_oldstyle (struct connection *conn)
     return -1;
   }
 
-  r = plugin_get_size (conn);
+  r = backend->get_size (backend, conn);
   if (r == -1)
     return -1;
   if (r < 0) {
@@ -703,7 +705,7 @@ _negotiate_handshake_newstyle (struct connection *conn)
     return -1;
 
   /* Finish the newstyle handshake. */
-  r = plugin_get_size (conn);
+  r = backend->get_size (backend, conn);
   if (r == -1)
     return -1;
   if (r < 0) {
@@ -848,7 +850,7 @@ get_error (struct connection *conn)
 {
   int ret = threadlocal_get_error ();
 
-  if (!ret && plugin_errno_is_preserved ())
+  if (!ret && backend->errno_is_preserved (backend))
     ret = errno;
   return ret ? ret : EIO;
 }
@@ -881,28 +883,28 @@ handle_request (struct connection *conn,
 
   switch (cmd) {
   case NBD_CMD_READ:
-    if (plugin_pread (conn, buf, count, offset) == -1)
+    if (backend->pread (backend, conn, buf, count, offset) == -1)
       return get_error (conn);
     break;
 
   case NBD_CMD_WRITE:
-    if (plugin_pwrite (conn, buf, count, offset) == -1)
+    if (backend->pwrite (backend, conn, buf, count, offset) == -1)
       return get_error (conn);
     break;
 
   case NBD_CMD_FLUSH:
-    if (plugin_flush (conn) == -1)
+    if (backend->flush (backend, conn) == -1)
       return get_error (conn);
     break;
 
   case NBD_CMD_TRIM:
-    if (plugin_trim (conn, count, offset) == -1)
+    if (backend->trim (backend, conn, count, offset) == -1)
       return get_error (conn);
     break;
 
   case NBD_CMD_WRITE_ZEROES:
-    if (plugin_zero (conn, count, offset,
-                     !(flags & NBD_CMD_FLAG_NO_HOLE)) == -1)
+    if (backend->zero (backend, conn, count, offset,
+                       !(flags & NBD_CMD_FLAG_NO_HOLE)) == -1)
       return get_error (conn);
     break;
 
@@ -910,7 +912,7 @@ handle_request (struct connection *conn,
     abort ();
   }
 
-  if (flush_after_command && plugin_flush (conn) == -1)
+  if (flush_after_command && backend->flush (backend, conn) == -1)
     return get_error (conn);
 
   return 0;
diff --git a/src/internal.h b/src/internal.h
index 068204b..d0c60f2 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -108,6 +108,8 @@ extern int threads;
 extern volatile int quit;
 extern int quit_fd;
 
+struct backend *backend;
+
 /* cleanup.c */
 extern void cleanup_free (void *ptr);
 #define CLEANUP_FREE __attribute__((cleanup (cleanup_free)))
@@ -142,28 +144,31 @@ extern int crypto_negotiate_tls (struct connection *conn,
int sockin, int sockou
 #define debug nbdkit_debug
 
 /* plugins.c */
-extern void plugin_register (const char *_filename, void *_dl, struct
nbdkit_plugin *(*plugin_init) (void));
-extern void plugin_cleanup (void);
-extern int plugin_thread_model (void);
-extern const char *plugin_name (void);
-extern void plugin_usage (void);
-extern const char *plugin_version (void);
-extern void plugin_dump_fields (void);
-extern void plugin_config (const char *key, const char *value);
-extern void plugin_config_complete (void);
-extern int plugin_errno_is_preserved (void);
-extern int plugin_open (struct connection *conn, int readonly);
-extern void plugin_close (struct connection *conn);
-extern int64_t plugin_get_size (struct connection *conn);
-extern int plugin_can_write (struct connection *conn);
-extern int plugin_can_flush (struct connection *conn);
-extern int plugin_is_rotational (struct connection *conn);
-extern int plugin_can_trim (struct connection *conn);
-extern int plugin_pread (struct connection *conn, void *buf, uint32_t count,
uint64_t offset);
-extern int plugin_pwrite (struct connection *conn, void *buf, uint32_t count,
uint64_t offset);
-extern int plugin_flush (struct connection *conn);
-extern int plugin_trim (struct connection *conn, uint32_t count, uint64_t
offset);
-extern int plugin_zero (struct connection *conn, uint32_t count, uint64_t
offset, int may_trim);
+struct backend {
+  void (*free) (struct backend *);
+  int (*thread_model) (struct backend *);
+  const char *(*name) (struct backend *);
+  void (*usage) (struct backend *);
+  const char *(*version) (struct backend *);
+  void (*dump_fields) (struct backend *);
+  void (*config) (struct backend *, const char *key, const char *value);
+  void (*config_complete) (struct backend *);
+  int (*errno_is_preserved) (struct backend *);
+  int (*open) (struct backend *, struct connection *conn, int readonly);
+  void (*close) (struct backend *, struct connection *conn);
+  int64_t (*get_size) (struct backend *, struct connection *conn);
+  int (*can_write) (struct backend *, struct connection *conn);
+  int (*can_flush) (struct backend *, struct connection *conn);
+  int (*is_rotational) (struct backend *, struct connection *conn);
+  int (*can_trim) (struct backend *, struct connection *conn);
+  int (*pread) (struct backend *, struct connection *conn, void *buf, uint32_t
count, uint64_t offset);
+  int (*pwrite) (struct backend *, struct connection *conn, void *buf, uint32_t
count, uint64_t offset);
+  int (*flush) (struct backend *, struct connection *conn);
+  int (*trim) (struct backend *, struct connection *conn, uint32_t count,
uint64_t offset);
+  int (*zero) (struct backend *, struct connection *conn, uint32_t count,
uint64_t offset, int may_trim);
+};
+
+extern struct backend *plugin_register (const char *_filename, void *_dl,
struct nbdkit_plugin *(*plugin_init) (void));
 
 /* locks.c */
 extern void lock_connection (void);
diff --git a/src/locks.c b/src/locks.c
index 6021356..62b2dd0 100644
--- a/src/locks.c
+++ b/src/locks.c
@@ -45,7 +45,7 @@ static pthread_rwlock_t unload_prevention_lock =
PTHREAD_RWLOCK_INITIALIZER;
 void
 lock_connection (void)
 {
-  int thread_model = plugin_thread_model ();
+  int thread_model = backend->thread_model (backend);
 
   if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) {
     debug ("acquire connection lock");
@@ -56,7 +56,7 @@ lock_connection (void)
 void
 unlock_connection (void)
 {
-  int thread_model = plugin_thread_model ();
+  int thread_model = backend->thread_model (backend);
 
   if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) {
     debug ("release connection lock");
@@ -67,7 +67,7 @@ unlock_connection (void)
 void
 lock_request (struct connection *conn)
 {
-  int thread_model = plugin_thread_model ();
+  int thread_model = backend->thread_model (backend);
 
   if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS) {
     debug ("acquire global request lock");
@@ -86,7 +86,7 @@ lock_request (struct connection *conn)
 void
 unlock_request (struct connection *conn)
 {
-  int thread_model = plugin_thread_model ();
+  int thread_model = backend->thread_model (backend);
 
   debug ("release unload prevention lock");
   pthread_rwlock_unlock (&unload_prevention_lock);
diff --git a/src/main.c b/src/main.c
index 4eca859..b3e6bad 100644
--- a/src/main.c
+++ b/src/main.c
@@ -64,7 +64,7 @@
 
 static int is_short_name (const char *);
 static char *make_random_fifo (void);
-static void open_plugin_so (const char *filename, int short_name);
+static struct backend *open_plugin_so (const char *filename, int short_name);
 static void start_serving (void);
 static void set_up_signals (void);
 static void run_command (void);
@@ -103,6 +103,9 @@ volatile int quit;
 int quit_fd;
 static int write_quit_fd;
 
+/* The currently loaded plugin. */
+struct backend *backend;
+
 static char *random_fifo_dir = NULL;
 static char *random_fifo = NULL;
 
@@ -493,12 +496,12 @@ main (int argc, char *argv[])
     }
   }
 
-  open_plugin_so (filename, short_name);
+  backend = open_plugin_so (filename, short_name);
 
   if (help) {
     usage ();
     printf ("\n%s:\n\n", filename);
-    plugin_usage ();
+    backend->usage (backend);
     exit (EXIT_SUCCESS);
   }
 
@@ -506,8 +509,8 @@ main (int argc, char *argv[])
     const char *v;
 
     display_version ();
-    printf ("%s", plugin_name ());
-    if ((v = plugin_version ()) != NULL)
+    printf ("%s", backend->name (backend));
+    if ((v = backend->version (backend)) != NULL)
       printf (" %s", v);
     printf ("\n");
     exit (EXIT_SUCCESS);
@@ -518,7 +521,7 @@ main (int argc, char *argv[])
    * we assume it is 'script=...'.
    */
   if (optind < argc && (p = strchr (argv[optind], '=')) ==
NULL) {
-    plugin_config ("script", argv[optind]);
+    backend->config (backend, "script", argv[optind]);
     ++optind;
   }
 
@@ -528,14 +531,14 @@ main (int argc, char *argv[])
    * script=... parameter (and do not wait for config_complete).
    */
   if (dump_plugin) {
-    plugin_dump_fields ();
+    backend->dump_fields (backend);
     exit (EXIT_SUCCESS);
   }
 
   while (optind < argc) {
     if ((p = strchr (argv[optind], '=')) != NULL) {
       *p = '\0';
-      plugin_config (argv[optind], p+1);
+      backend->config (backend, argv[optind], p+1);
       ++optind;
     }
     else {
@@ -546,11 +549,12 @@ main (int argc, char *argv[])
     }
   }
 
-  plugin_config_complete ();
+  backend->config_complete (backend);
 
   start_serving ();
 
-  plugin_cleanup ();
+  backend->free (backend);
+  backend = NULL;
 
   free (unixsocket);
   free (pidfile);
@@ -609,9 +613,10 @@ make_random_fifo (void)
   return unixsocket;
 }
 
-static void
+static struct backend *
 open_plugin_so (const char *name, int short_name)
 {
+  struct backend *ret;
   char *filename = (char *) name;
   int free_filename = 0;
   void *dl;
@@ -647,10 +652,12 @@ open_plugin_so (const char *name, int short_name)
   }
 
   /* Register the plugin. */
-  plugin_register (filename, dl, plugin_init);
+  ret = plugin_register (filename, dl, plugin_init);
 
   if (free_filename)
     free (filename);
+
+  return ret;
 }
 
 static void
diff --git a/src/plugins.c b/src/plugins.c
index b7ab43d..e81a3d3 100644
--- a/src/plugins.c
+++ b/src/plugins.c
@@ -49,192 +49,102 @@
 /* Maximum read or write request that we will handle. */
 #define MAX_REQUEST_SIZE (64 * 1024 * 1024)
 
-/* Currently the server can only load one plugin (see TODO).  Hence we
- * can just use globals to store these.
+/* We extend the generic backend struct with extra fields relating
+ * to this plugin.
  */
-static char *filename;
-static void *dl;
-static struct nbdkit_plugin plugin;
-
-void
-plugin_register (const char *_filename,
-                 void *_dl, struct nbdkit_plugin *(*plugin_init) (void))
+struct backend_plugin {
+  struct backend backend;
+  char *filename;
+  void *dl;
+  struct nbdkit_plugin plugin;
+};
+
+static void
+plugin_free (struct backend *b)
 {
-  const struct nbdkit_plugin *_plugin;
-  size_t i, len, size;
-
-  filename = strdup (_filename);
-  if (filename == NULL) {
-    perror ("strdup");
-    exit (EXIT_FAILURE);
-  }
-  dl = _dl;
-
-  debug ("registering %s", filename);
-
-  /* Call the initialization function which returns the address of the
-   * plugin's own 'struct nbdkit_plugin'.
-   */
-  _plugin = plugin_init ();
-  if (!_plugin) {
-    fprintf (stderr, "%s: %s: plugin registration function failed\n",
-             program_name, filename);
-    exit (EXIT_FAILURE);
-  }
-
-  /* Check for incompatible future versions. */
-  if (_plugin->_api_version != 1) {
-    fprintf (stderr, "%s: %s: plugin is incompatible with this version of
nbdkit (_api_version = %d)\n",
-             program_name, filename, _plugin->_api_version);
-    exit (EXIT_FAILURE);
-  }
-
-  /* Since the plugin might be much older than the current version of
-   * nbdkit, only copy up to the self-declared _struct_size of the
-   * plugin and zero out the rest.  If the plugin is much newer then
-   * we'll only call the "old" fields.
-   */
-  size = sizeof plugin;         /* our struct */
-  memset (&plugin, 0, size);
-  if (size > _plugin->_struct_size)
-    size = _plugin->_struct_size;
-  memcpy (&plugin, _plugin, size);
-
-  /* Check for the minimum fields which must exist in the
-   * plugin struct.
-   */
-  if (plugin.name == NULL) {
-    fprintf (stderr, "%s: %s: plugin must have a .name field\n",
-             program_name, filename);
-    exit (EXIT_FAILURE);
-  }
-  if (plugin.open == NULL) {
-    fprintf (stderr, "%s: %s: plugin must have a .open callback\n",
-             program_name, filename);
-    exit (EXIT_FAILURE);
-  }
-  if (plugin.get_size == NULL) {
-    fprintf (stderr, "%s: %s: plugin must have a .get_size
callback\n",
-             program_name, filename);
-    exit (EXIT_FAILURE);
-  }
-  if (plugin.pread == NULL) {
-    fprintf (stderr, "%s: %s: plugin must have a .pread callback\n",
-             program_name, filename);
-    exit (EXIT_FAILURE);
-  }
+  struct backend_plugin *p = (struct backend_plugin *) b;
 
-  len = strlen (plugin.name);
-  if (len == 0) {
-    fprintf (stderr, "%s: %s: plugin.name field must not be empty\n",
-             program_name, filename);
-    exit (EXIT_FAILURE);
-  }
-  for (i = 0; i < len; ++i) {
-    if (!((plugin.name[i] >= '0' && plugin.name[i] <=
'9') ||
-          (plugin.name[i] >= 'a' && plugin.name[i] <=
'z') ||
-          (plugin.name[i] >= 'A' && plugin.name[i] <=
'Z'))) {
-      fprintf (stderr, "%s: %s: plugin.name ('%s') field must
contain only ASCII alphanumeric characters\n",
-               program_name, filename, plugin.name);
-      exit (EXIT_FAILURE);
-    }
-  }
-  /* Copy the module's name into local storage, so that plugin.name
-   * survives past unload. */
-  if (!(plugin.name = strdup (plugin.name))) {
-    perror ("strdup");
-    exit (EXIT_FAILURE);
-  }
-
-  debug ("registered %s (name %s)", filename, plugin.name);
-
-  /* Call the on-load callback if it exists. */
-  debug ("%s: load", filename);
-  if (plugin.load)
-    plugin.load ();
-}
-
-void
-plugin_cleanup (void)
-{
-  if (dl) {
+  if (p->dl) {
     /* Acquiring this lock prevents any plugin callbacks from running
      * simultaneously.
      */
     lock_unload ();
 
-    debug ("%s: unload", filename);
-    if (plugin.unload)
-      plugin.unload ();
+    debug ("%s: unload", p->filename);
+    if (p->plugin.unload)
+      p->plugin.unload ();
 
-    dlclose (dl);
-    dl = NULL;
-    free (filename);
-    filename = NULL;
+    dlclose (p->dl);
+    free (p->filename);
 
     unlock_unload ();
   }
+
+  free (p);
 }
 
-int
-plugin_thread_model (void)
+static int
+plugin_thread_model (struct backend *b)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
 
-  return plugin._thread_model;
+  return p->plugin._thread_model;
 }
 
-const char *
-plugin_name (void)
+static const char *
+plugin_name (struct backend *b)
 {
-  return plugin.name;
+  struct backend_plugin *p = (struct backend_plugin *) b;
+
+  return p->plugin.name;
 }
 
-void
-plugin_usage (void)
+static void
+plugin_usage (struct backend *b)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
 
-  printf ("%s", plugin.name);
-  if (plugin.longname)
-    printf (" (%s)", plugin.longname);
+  printf ("%s", p->plugin.name);
+  if (p->plugin.longname)
+    printf (" (%s)", p->plugin.longname);
   printf ("\n");
-  if (plugin.description) {
+  if (p->plugin.description) {
     printf ("\n");
-    printf ("%s\n", plugin.description);
+    printf ("%s\n", p->plugin.description);
   }
-  if (plugin.config_help) {
+  if (p->plugin.config_help) {
     printf ("\n");
-    printf ("%s\n", plugin.config_help);
+    printf ("%s\n", p->plugin.config_help);
   }
 }
 
-const char *
-plugin_version (void)
+static const char *
+plugin_version (struct backend *b)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
 
-  return plugin.version;
+  return p->plugin.version;
 }
 
 /* This implements the --dump-plugin option. */
-void
-plugin_dump_fields (void)
+static void
+plugin_dump_fields (struct backend *b)
 {
+  struct backend_plugin *p = (struct backend_plugin *) b;
   char *path;
 
-  path = nbdkit_absolute_path (filename);
+  path = nbdkit_absolute_path (p->filename);
   printf ("path=%s\n", path);
   free (path);
 
-  printf ("name=%s\n", plugin.name);
-  if (plugin.version)
-    printf ("version=%s\n", plugin.version);
+  printf ("name=%s\n", p->plugin.name);
+  if (p->plugin.version)
+    printf ("version=%s\n", p->plugin.version);
 
-  printf ("api_version=%d\n", plugin._api_version);
-  printf ("struct_size=%" PRIu64 "\n",
plugin._struct_size);
+  printf ("api_version=%d\n", p->plugin._api_version);
+  printf ("struct_size=%" PRIu64 "\n",
p->plugin._struct_size);
   printf ("thread_model=");
-  switch (plugin._thread_model) {
+  switch (p->plugin._thread_model) {
   case NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS:
     printf ("serialize_connections");
     break;
@@ -248,13 +158,13 @@ plugin_dump_fields (void)
     printf ("parallel");
     break;
   default:
-    printf ("%d # unknown thread model!", plugin._thread_model);
+    printf ("%d # unknown thread model!",
p->plugin._thread_model);
     break;
   }
   printf ("\n");
-  printf ("errno_is_preserved=%d\n", plugin.errno_is_preserved);
+  printf ("errno_is_preserved=%d\n",
p->plugin.errno_is_preserved);
 
-#define HAS(field) if (plugin.field) printf ("has_%s=1\n", #field)
+#define HAS(field) if (p->plugin.field) printf ("has_%s=1\n",
#field)
   HAS (longname);
   HAS (description);
   HAS (load);
@@ -278,64 +188,64 @@ plugin_dump_fields (void)
 #undef HAS
 
   /* Custom fields. */
-  if (plugin.dump_plugin)
-    plugin.dump_plugin ();
+  if (p->plugin.dump_plugin)
+    p->plugin.dump_plugin ();
 }
 
-void
-plugin_config (const char *key, const char *value)
+static void
+plugin_config (struct backend *b, const char *key, const char *value)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
 
   debug ("%s: config key=%s, value=%s",
-         filename, key, value);
+         p->filename, key, value);
 
-  if (plugin.config == NULL) {
+  if (p->plugin.config == NULL) {
     fprintf (stderr, "%s: %s: this plugin does not need command line
configuration\n"
              "Try using: %s --help %s\n",
-             program_name, filename,
-             program_name, filename);
+             program_name, p->filename,
+             program_name, p->filename);
     exit (EXIT_FAILURE);
   }
 
-  if (plugin.config (key, value) == -1)
+  if (p->plugin.config (key, value) == -1)
     exit (EXIT_FAILURE);
 }
 
-void
-plugin_config_complete (void)
+static void
+plugin_config_complete (struct backend *b)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
 
-  debug ("%s: config_complete", filename);
+  debug ("%s: config_complete", p->filename);
 
-  if (!plugin.config_complete)
+  if (!p->plugin.config_complete)
     return;
 
-  if (plugin.config_complete () == -1)
+  if (p->plugin.config_complete () == -1)
     exit (EXIT_FAILURE);
 }
 
-int
-plugin_errno_is_preserved (void)
+static int
+plugin_errno_is_preserved (struct backend *b)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
 
-  return plugin.errno_is_preserved;
+  return p->plugin.errno_is_preserved;
 }
 
-int
-plugin_open (struct connection *conn, int readonly)
+static int
+plugin_open (struct backend *b, struct connection *conn, int readonly)
 {
+  struct backend_plugin *p = (struct backend_plugin *) b;
   void *handle;
 
-  assert (dl);
   assert (connection_get_handle (conn) == NULL);
-  assert (plugin.open != NULL);
+  assert (p->plugin.open != NULL);
 
-  debug ("%s: open readonly=%d", filename, readonly);
+  debug ("%s: open readonly=%d", p->filename, readonly);
 
-  handle = plugin.open (readonly);
+  handle = p->plugin.open (readonly);
   if (!handle)
     return -1;
 
@@ -343,179 +253,192 @@ plugin_open (struct connection *conn, int readonly)
   return 0;
 }
 
-void
-plugin_close (struct connection *conn)
+static void
+plugin_close (struct backend *b, struct connection *conn)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
+
   assert (connection_get_handle (conn));
 
   debug ("close");
 
-  if (plugin.close)
-    plugin.close (connection_get_handle (conn));
+  if (p->plugin.close)
+    p->plugin.close (connection_get_handle (conn));
 
   connection_set_handle (conn, NULL);
 }
 
-int64_t
-plugin_get_size (struct connection *conn)
+static int64_t
+plugin_get_size (struct backend *b, struct connection *conn)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
+
   assert (connection_get_handle (conn));
-  assert (plugin.get_size != NULL);
+  assert (p->plugin.get_size != NULL);
 
   debug ("get_size");
 
-  return plugin.get_size (connection_get_handle (conn));
+  return p->plugin.get_size (connection_get_handle (conn));
 }
 
-int
-plugin_can_write (struct connection *conn)
+static int
+plugin_can_write (struct backend *b, struct connection *conn)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
+
   assert (connection_get_handle (conn));
 
   debug ("can_write");
 
-  if (plugin.can_write)
-    return plugin.can_write (connection_get_handle (conn));
+  if (p->plugin.can_write)
+    return p->plugin.can_write (connection_get_handle (conn));
   else
-    return plugin.pwrite != NULL;
+    return p->plugin.pwrite != NULL;
 }
 
-int
-plugin_can_flush (struct connection *conn)
+static int
+plugin_can_flush (struct backend *b, struct connection *conn)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
+
   assert (connection_get_handle (conn));
 
   debug ("can_flush");
 
-  if (plugin.can_flush)
-    return plugin.can_flush (connection_get_handle (conn));
+  if (p->plugin.can_flush)
+    return p->plugin.can_flush (connection_get_handle (conn));
   else
-    return plugin.flush != NULL;
+    return p->plugin.flush != NULL;
 }
 
-int
-plugin_is_rotational (struct connection *conn)
+static int
+plugin_is_rotational (struct backend *b, struct connection *conn)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
+
   assert (connection_get_handle (conn));
 
   debug ("is_rotational");
 
-  if (plugin.is_rotational)
-    return plugin.is_rotational (connection_get_handle (conn));
+  if (p->plugin.is_rotational)
+    return p->plugin.is_rotational (connection_get_handle (conn));
   else
     return 0; /* assume false */
 }
 
-int
-plugin_can_trim (struct connection *conn)
+static int
+plugin_can_trim (struct backend *b, struct connection *conn)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
+
   assert (connection_get_handle (conn));
 
   debug ("can_trim");
 
-  if (plugin.can_trim)
-    return plugin.can_trim (connection_get_handle (conn));
+  if (p->plugin.can_trim)
+    return p->plugin.can_trim (connection_get_handle (conn));
   else
-    return plugin.trim != NULL;
+    return p->plugin.trim != NULL;
 }
 
-int
-plugin_pread (struct connection *conn,
+static int
+plugin_pread (struct backend *b, struct connection *conn,
               void *buf, uint32_t count, uint64_t offset)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
+
   assert (connection_get_handle (conn));
-  assert (plugin.pread != NULL);
+  assert (p->plugin.pread != NULL);
 
   debug ("pread count=%" PRIu32 " offset=%" PRIu64, count,
offset);
 
-  return plugin.pread (connection_get_handle (conn), buf, count, offset);
+  return p->plugin.pread (connection_get_handle (conn), buf, count, offset);
 }
 
-int
-plugin_pwrite (struct connection *conn,
+static int
+plugin_pwrite (struct backend *b, struct connection *conn,
                void *buf, uint32_t count, uint64_t offset)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
+
   assert (connection_get_handle (conn));
 
   debug ("pwrite count=%" PRIu32 " offset=%" PRIu64, count,
offset);
 
-  if (plugin.pwrite != NULL)
-    return plugin.pwrite (connection_get_handle (conn), buf, count, offset);
+  if (p->plugin.pwrite != NULL)
+    return p->plugin.pwrite (connection_get_handle (conn), buf, count,
offset);
   else {
     errno = EROFS;
     return -1;
   }
 }
 
-int
-plugin_flush (struct connection *conn)
+static int
+plugin_flush (struct backend *b, struct connection *conn)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
+
   assert (connection_get_handle (conn));
 
   debug ("flush");
 
-  if (plugin.flush != NULL)
-    return plugin.flush (connection_get_handle (conn));
+  if (p->plugin.flush != NULL)
+    return p->plugin.flush (connection_get_handle (conn));
   else {
     errno = EINVAL;
     return -1;
   }
 }
 
-int
-plugin_trim (struct connection *conn, uint32_t count, uint64_t offset)
+static int
+plugin_trim (struct backend *b, struct connection *conn,
+             uint32_t count, uint64_t offset)
 {
-  assert (dl);
+  struct backend_plugin *p = (struct backend_plugin *) b;
+
   assert (connection_get_handle (conn));
 
   debug ("trim count=%" PRIu32 " offset=%" PRIu64, count,
offset);
 
-  if (plugin.trim != NULL)
-    return plugin.trim (connection_get_handle (conn), count, offset);
+  if (p->plugin.trim != NULL)
+    return p->plugin.trim (connection_get_handle (conn), count, offset);
   else {
     errno = EINVAL;
     return -1;
   }
 }
 
-int
-plugin_zero (struct connection *conn,
+static int
+plugin_zero (struct backend *b, struct connection *conn,
              uint32_t count, uint64_t offset, int may_trim)
 {
-  assert (dl);
-  assert (connection_get_handle (conn));
+  struct backend_plugin *p = (struct backend_plugin *) b;
   char *buf;
   uint32_t limit;
   int result;
   int err = 0;
 
+  assert (connection_get_handle (conn));
+
   debug ("zero count=%" PRIu32 " offset=%" PRIu64 "
may_trim=%d",
          count, offset, may_trim);
 
   if (!count)
     return 0;
-  if (plugin.zero) {
+  if (p->plugin.zero) {
     errno = 0;
-    result = plugin.zero (connection_get_handle (conn), count, offset,
may_trim);
+    result = p->plugin.zero (connection_get_handle (conn),
+                             count, offset, may_trim);
     if (result == -1) {
       err = threadlocal_get_error ();
-      if (!err && plugin_errno_is_preserved ())
+      if (!err && plugin_errno_is_preserved (b))
         err = errno;
     }
     if (result == 0 || err != EOPNOTSUPP)
       return result;
   }
 
-  assert (plugin.pwrite);
+  assert (p->plugin.pwrite);
   threadlocal_set_error (0);
   limit = count < MAX_REQUEST_SIZE ? count : MAX_REQUEST_SIZE;
   buf = calloc (limit, 1);
@@ -525,7 +448,8 @@ plugin_zero (struct connection *conn,
   }
 
   while (count) {
-    result = plugin.pwrite (connection_get_handle (conn), buf, limit, offset);
+    result = p->plugin.pwrite (connection_get_handle (conn),
+                               buf, limit, offset);
     if (result < 0)
       break;
     count -= limit;
@@ -538,3 +462,134 @@ plugin_zero (struct connection *conn,
   errno = err;
   return result;
 }
+
+static struct backend plugin_functions = {
+  .free = plugin_free,
+  .thread_model = plugin_thread_model,
+  .name = plugin_name,
+  .usage = plugin_usage,
+  .version = plugin_version,
+  .dump_fields = plugin_dump_fields,
+  .config = plugin_config,
+  .config_complete = plugin_config_complete,
+  .errno_is_preserved = plugin_errno_is_preserved,
+  .open = plugin_open,
+  .close = plugin_close,
+  .get_size = plugin_get_size,
+  .can_write = plugin_can_write,
+  .can_flush = plugin_can_flush,
+  .is_rotational = plugin_is_rotational,
+  .can_trim = plugin_can_trim,
+  .pread = plugin_pread,
+  .pwrite = plugin_pwrite,
+  .flush = plugin_flush,
+  .trim = plugin_trim,
+  .zero = plugin_zero,
+};
+
+/* Register and load a plugin. */
+struct backend *
+plugin_register (const char *_filename,
+                 void *_dl, struct nbdkit_plugin *(*plugin_init) (void))
+{
+  struct backend_plugin *p;
+  const struct nbdkit_plugin *_plugin;
+  size_t i, len, size;
+
+  p = malloc (sizeof *p);
+  if (p == NULL) {
+  out_of_memory:
+    perror ("strdup");
+    exit (EXIT_FAILURE);
+  }
+
+  p->backend = plugin_functions;
+  p->filename = strdup (_filename);
+  if (p->filename == NULL) goto out_of_memory;
+  p->dl = _dl;
+
+  debug ("registering %s", p->filename);
+
+  /* Call the initialization function which returns the address of the
+   * plugin's own 'struct nbdkit_plugin'.
+   */
+  _plugin = plugin_init ();
+  if (!_plugin) {
+    fprintf (stderr, "%s: %s: plugin registration function failed\n",
+             program_name, p->filename);
+    exit (EXIT_FAILURE);
+  }
+
+  /* Check for incompatible future versions. */
+  if (_plugin->_api_version != 1) {
+    fprintf (stderr, "%s: %s: plugin is incompatible with this version of
nbdkit (_api_version = %d)\n",
+             program_name, p->filename, _plugin->_api_version);
+    exit (EXIT_FAILURE);
+  }
+
+  /* Since the plugin might be much older than the current version of
+   * nbdkit, only copy up to the self-declared _struct_size of the
+   * plugin and zero out the rest.  If the plugin is much newer then
+   * we'll only call the "old" fields.
+   */
+  size = sizeof p->plugin;      /* our struct */
+  memset (&p->plugin, 0, size);
+  if (size > _plugin->_struct_size)
+    size = _plugin->_struct_size;
+  memcpy (&p->plugin, _plugin, size);
+
+  /* Check for the minimum fields which must exist in the
+   * plugin struct.
+   */
+  if (p->plugin.name == NULL) {
+    fprintf (stderr, "%s: %s: plugin must have a .name field\n",
+             program_name, p->filename);
+    exit (EXIT_FAILURE);
+  }
+  if (p->plugin.open == NULL) {
+    fprintf (stderr, "%s: %s: plugin must have a .open callback\n",
+             program_name, p->filename);
+    exit (EXIT_FAILURE);
+  }
+  if (p->plugin.get_size == NULL) {
+    fprintf (stderr, "%s: %s: plugin must have a .get_size
callback\n",
+             program_name, p->filename);
+    exit (EXIT_FAILURE);
+  }
+  if (p->plugin.pread == NULL) {
+    fprintf (stderr, "%s: %s: plugin must have a .pread callback\n",
+             program_name, p->filename);
+    exit (EXIT_FAILURE);
+  }
+
+  len = strlen (p->plugin.name);
+  if (len == 0) {
+    fprintf (stderr, "%s: %s: plugin.name field must not be empty\n",
+             program_name, p->filename);
+    exit (EXIT_FAILURE);
+  }
+  for (i = 0; i < len; ++i) {
+    if (!((p->plugin.name[i] >= '0' &&
p->plugin.name[i] <= '9') ||
+          (p->plugin.name[i] >= 'a' &&
p->plugin.name[i] <= 'z') ||
+          (p->plugin.name[i] >= 'A' &&
p->plugin.name[i] <= 'Z'))) {
+      fprintf (stderr, "%s: %s: plugin.name ('%s') field must
contain only ASCII alphanumeric characters\n",
+               program_name, p->filename, p->plugin.name);
+      exit (EXIT_FAILURE);
+    }
+  }
+  /* Copy the module's name into local storage, so that plugin.name
+   * survives past unload. */
+  if (!(p->plugin.name = strdup (p->plugin.name))) {
+    perror ("strdup");
+    exit (EXIT_FAILURE);
+  }
+
+  debug ("registered %s (name %s)", p->filename,
p->plugin.name);
+
+  /* Call the on-load callback if it exists. */
+  debug ("%s: load", p->filename);
+  if (p->plugin.load)
+    p->plugin.load ();
+
+  return (struct backend *) p;
+}
-- 
2.15.1

---
 src/internal.h | 2 +-
 src/plugins.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/internal.h b/src/internal.h
index d0c60f2..2617337 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -162,7 +162,7 @@ struct backend {
   int (*is_rotational) (struct backend *, struct connection *conn);
   int (*can_trim) (struct backend *, struct connection *conn);
   int (*pread) (struct backend *, struct connection *conn, void *buf, uint32_t
count, uint64_t offset);
-  int (*pwrite) (struct backend *, struct connection *conn, void *buf, uint32_t
count, uint64_t offset);
+  int (*pwrite) (struct backend *, struct connection *conn, const void *buf,
uint32_t count, uint64_t offset);
   int (*flush) (struct backend *, struct connection *conn);
   int (*trim) (struct backend *, struct connection *conn, uint32_t count,
uint64_t offset);
   int (*zero) (struct backend *, struct connection *conn, uint32_t count,
uint64_t offset, int may_trim);
diff --git a/src/plugins.c b/src/plugins.c
index e81a3d3..dfa9bc5 100644
--- a/src/plugins.c
+++ b/src/plugins.c
@@ -357,7 +357,7 @@ plugin_pread (struct backend *b, struct connection *conn,
 
 static int
 plugin_pwrite (struct backend *b, struct connection *conn,
-               void *buf, uint32_t count, uint64_t offset)
+               const void *buf, uint32_t count, uint64_t offset)
 {
   struct backend_plugin *p = (struct backend_plugin *) b;
 
-- 
2.15.1

On 01/16/2018 08:11 AM, Richard W.M. Jones wrote:
> Mostly code motion.
> ---
>  src/Makefile.am   |   1 +
>  src/connections.c |  14 +++----
>  src/internal.h    |  14 ++++---
>  src/locks.c       | 115
++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/plugins.c     |  77 +++++-------------------------------
>  5 files changed, 142 insertions(+), 79 deletions(-)
> 
> +++ b/src/locks.c
> +void
> +lock_connection (void)
> +{
> +  int thread_model = plugin_thread_model ();
> +
> +  if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) {
> +++ b/src/plugins.c
> +int
> +plugin_thread_model (void)
> +{
> +  assert (dl);
> +
> +  return plugin._thread_model;
> +}
The new code asserts dl prior to locking the connection;
> +
>  const char *
>  plugin_name (void)
>  {
> @@ -312,67 +316,6 @@ plugin_config_complete (void)
>      exit (EXIT_FAILURE);
>  }
>  
> -/* Handle the thread model. */
> -void
> -plugin_lock_connection (void)
> -{
> -  if (plugin._thread_model <=
NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) {
> -    debug ("%s: acquire connection lock", filename);
> -    pthread_mutex_lock (&connection_lock);
> -  }
> -}
but the old code did not (and just blindly assumed that _thread_model is
always valid).  Can that new assertion ever fire during racy unload
sequences?

Otherwise, looks right to me.

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

On 01/16/2018 08:11 AM, Richard W.M. Jones wrote:
> Introduce the concept of a backend.  Currently the only type of
> backend is a plugin, and there can only be one of them.  Instead of
> calling functions like ‘plugin_pwrite’ you call the backend method
> ‘backend->pwrite (backend, ...)’.
> 
> The change is largely mechanical.  I was able to remove ‘assert (dl)’
> statements throughout since we can now prove they will never be
> called.
> 
> Note this does not lift the restriction of one plugin per server, and
> it can *never* do that because plugins can use global variables.
> ---
>  src/connections.c |  40 +++--
>  src/internal.h    |  49 ++---
>  src/locks.c       |   8 +-
>  src/main.c        |  31 ++--
>  src/plugins.c     | 523
++++++++++++++++++++++++++++++------------------------
>  5 files changed, 360 insertions(+), 291 deletions(-)
> 
> +++ b/src/internal.h
> @@ -108,6 +108,8 @@ extern int threads;
>  extern volatile int quit;
>  extern int quit_fd;
>  
> +struct backend *backend;
Missing an 'extern', which means...

> +++ b/src/main.c
> @@ -64,7 +64,7 @@
>  
>  static int is_short_name (const char *);
>  static char *make_random_fifo (void);
> -static void open_plugin_so (const char *filename, int short_name);
> +static struct backend *open_plugin_so (const char *filename, int
short_name);
>  static void start_serving (void);
>  static void set_up_signals (void);
>  static void run_command (void);
> @@ -103,6 +103,9 @@ volatile int quit;
>  int quit_fd;
>  static int write_quit_fd;
>  
> +/* The currently loaded plugin. */
> +struct backend *backend;
...you are declaring more than one instance, and the linker may either
balk or do unexpected things.
> +++ b/src/plugins.c
> @@ -49,192 +49,102 @@
>  /* Maximum read or write request that we will handle. */
>  #define MAX_REQUEST_SIZE (64 * 1024 * 1024)
>  
> -/* Currently the server can only load one plugin (see TODO).  Hence we
> - * can just use globals to store these.
> +/* We extend the generic backend struct with extra fields relating
> + * to this plugin.
>   */
> -static char *filename;
> -static void *dl;
> -static struct nbdkit_plugin plugin;
> -
> -void
> -plugin_register (const char *_filename,
> -                 void *_dl, struct nbdkit_plugin *(*plugin_init) (void))
> +struct backend_plugin {
> +  struct backend backend;
> +  char *filename;
> +  void *dl;
> +  struct nbdkit_plugin plugin;
> +};
So we still have a global variable 'backend', and I agree with your
assessment that since plugins can use global variables we can't safely
load the same plugin multiple times in one server.  But this refactoring
does mean more state is now passed as explicit context rather than at a
distance through globals, so I'm liking it.
> +
> +static void
> +plugin_free (struct backend *b)
>  {
> +  struct backend_plugin *p = (struct backend_plugin *) b;
qemu has a macro container_of() which makes casts like this be a bit
more declarative and utilize a bit more type-safety from the compiler
(it also lets you build derived type wrappers and upcast back to the
parent base class without requiring the base class to be the first
member of the derived class, although that's a side-effect we don't need
to exploit here):

#define container_of(ptr, type, member) ({                      \
        const typeof(((type *) 0)->member) *__mptr = (ptr);     \
        (type *) ((char *) __mptr - offsetof(type, member));})

which would be used here as:

struct backend_plugin *p = container_of (b, backend_plugin, backend);


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

On 01/16/2018 08:11 AM, Richard W.M. Jones wrote:
> ---
>  src/internal.h | 2 +-
>  src/plugins.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
ACK

Worth squashing in when you create the backend code?

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