[Libguestfs] [nbdkit PATCH 4/9] server: Rework storage of per-backend handle

Eric Blake eblake at redhat.com
Fri Aug 30 03:08:24 UTC 2019


Right now, each connection maintains a bare list of void* handles
obtained from the backends, which is reallocated on-the-fly as
additional filters go through .open (and where a filter that skips
.open does not necessarily resize the array).  However, we know the
size of the array in advance (because we know how many filters were
loaded) and can avoid realloc churn; and we also want to move the code
towards being able to cache more than just the handle from the
filter/plugin.  So it makes sense to have each connection maintain an
array of structs that includes the handle and anything else associated
with a given backend/connection combination.

This patch refactors the allocation and tracking of handles, while
upcoming patches add some caching made possible by the new array of
structs.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 server/internal.h    | 15 ++++++++++----
 server/backend.c     |  7 +++++++
 server/connections.c | 47 ++++++++++++--------------------------------
 server/filters.c     |  2 +-
 server/plugins.c     |  4 ++--
 5 files changed, 34 insertions(+), 41 deletions(-)

diff --git a/server/internal.h b/server/internal.h
index 93ebeb78..9bf84022 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -148,6 +148,12 @@ typedef int (*connection_send_function) (struct connection *,
 typedef void (*connection_close_function) (struct connection *)
   __attribute__((__nonnull__ (1)));

+struct b_conn_handle {
+  void *handle;
+
+  // TODO add per-backend caching
+};
+
 struct connection {
   pthread_mutex_t request_lock;
   pthread_mutex_t read_lock;
@@ -158,7 +164,7 @@ struct connection {
   void *crypto_session;
   int nworkers;

-  void **handles;
+  struct b_conn_handle *handles;
   size_t nr_handles;

   uint32_t cflags;
@@ -185,9 +191,6 @@ struct connection {
 };

 extern int handle_single_connection (int sockin, int sockout);
-extern int connection_set_handle (struct connection *conn,
-                                  size_t i, void *handle)
-  __attribute__((__nonnull__ (1 /* not 3 */)));
 extern void *connection_get_handle (struct connection *conn, size_t i)
   __attribute__((__nonnull__ (1)));
 extern int connection_get_status (struct connection *conn)
@@ -317,6 +320,10 @@ extern void backend_set_name (struct backend *b, const char *name,
 extern void backend_unload (struct backend *b, void (*unload) (void))
   __attribute__((__nonnull__ (1)));

+extern void backend_set_handle (struct backend *b, struct connection *conn,
+                                void *handle)
+  __attribute__((__nonnull__ (1, 2 /* not 3 */)));
+
 extern int64_t backend_get_size (struct backend *b, struct connection *conn)
   __attribute__((__nonnull__ (1, 2)));
 extern int backend_can_write (struct backend *b, struct connection *conn)
diff --git a/server/backend.c b/server/backend.c
index ce3c5e2b..b2054aa2 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -122,6 +122,13 @@ backend_unload (struct backend *b, void (*unload) (void))
   free (b->name);
 }

+void
+backend_set_handle (struct backend *b, struct connection *conn, void *handle)
+{
+  assert (b->i < conn->nr_handles);
+  conn->handles[b->i].handle = handle;
+}
+
 int64_t
 backend_get_size (struct backend *b, struct connection *conn)
 {
diff --git a/server/connections.c b/server/connections.c
index 0184afea..1c501002 100644
--- a/server/connections.c
+++ b/server/connections.c
@@ -60,37 +60,11 @@ static int raw_send_other (struct connection *, const void *buf, size_t len,
                            int flags);
 static void raw_close (struct connection *);

-int
-connection_set_handle (struct connection *conn, size_t i, void *handle)
-{
-  size_t j;
-
-  if (i < conn->nr_handles)
-    conn->handles[i] = handle;
-  else {
-    j = conn->nr_handles;
-    conn->nr_handles = i+1;
-    conn->handles = realloc (conn->handles,
-                             conn->nr_handles * sizeof (void *));
-    if (conn->handles == NULL) {
-      perror ("realloc");
-      conn->nr_handles = 0;
-      return -1;
-    }
-    for (; j < i; ++j)
-      conn->handles[j] = NULL;
-    conn->handles[i] = handle;
-  }
-  return 0;
-}
-
 void *
 connection_get_handle (struct connection *conn, size_t i)
 {
-  if (i < conn->nr_handles)
-    return conn->handles[i];
-  else
-    return NULL;
+  assert (i < conn->nr_handles);
+  return conn->handles[i].handle;
 }

 int
@@ -292,6 +266,13 @@ new_connection (int sockin, int sockout, int nworkers)
     perror ("malloc");
     return NULL;
   }
+  conn->handles = calloc (backend->i + 1, sizeof *conn->handles);
+  if (conn->handles == NULL) {
+    perror ("malloc");
+    free (conn);
+    return NULL;
+  }
+  conn->nr_handles = backend->i + 1;

   conn->status = 1;
   conn->nworkers = nworkers;
@@ -383,12 +364,10 @@ free_connection (struct connection *conn)
    * thread will be in the process of unloading it.  The plugin.unload
    * callback should always be called.
    */
-  if (!quit) {
-    if (conn->nr_handles > 0 && conn->handles[0]) {
-      lock_request (conn);
-      backend->close (backend, conn);
-      unlock_request (conn);
-    }
+  if (!quit && connection_get_handle (conn, 0)) {
+    lock_request (conn);
+    backend->close (backend, conn);
+    unlock_request (conn);
   }

   if (conn->status_pipe[0] >= 0) {
diff --git a/server/filters.c b/server/filters.c
index acb44bda..614ce2f6 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -213,7 +213,7 @@ filter_open (struct backend *b, struct connection *conn, int readonly)
     handle = f->filter.open (next_open, &nxdata, readonly);
     if (handle == NULL)
       return -1;
-    connection_set_handle (conn, b->i, handle);
+    backend_set_handle (b, conn, handle);
     return 0;
   }
   else
diff --git a/server/plugins.c b/server/plugins.c
index da8931a3..d1654f8d 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -252,7 +252,7 @@ plugin_open (struct backend *b, struct connection *conn, int readonly)
   if (!handle)
     return -1;

-  connection_set_handle (conn, 0, handle);
+  backend_set_handle (b, conn, handle);
   return 0;
 }

@@ -284,7 +284,7 @@ plugin_close (struct backend *b, struct connection *conn)
   if (p->plugin.close)
     p->plugin.close (connection_get_handle (conn, 0));

-  connection_set_handle (conn, 0, NULL);
+  backend_set_handle (b, conn, NULL);
 }

 static int64_t
-- 
2.21.0




More information about the Libguestfs mailing list