[Libguestfs] [nbdkit PATCH 4/5] server: Move prepare/finalize/close recursion to backend.c

Eric Blake eblake at redhat.com
Mon Oct 7 14:50:15 UTC 2019


Drop backend_set_handle, and instead use the return value of open to
control that.  While open recursion still has to go through filters
(because a filter may change the readonly parameter), the recursion
for prepare/finalize/close fits better in backend.c.  This will also
help an upcoming patch better handle prepare/finalize during reopen.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 server/internal.h |  5 +----
 server/backend.c  | 46 +++++++++++++++++++++++++++++++++++-----------
 server/filters.c  | 35 ++++++++---------------------------
 server/plugins.c  | 10 ++--------
 4 files changed, 46 insertions(+), 50 deletions(-)

diff --git a/server/internal.h b/server/internal.h
index d4b57419..eb0e30c1 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -303,7 +303,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 *);
-  int (*open) (struct backend *, struct connection *conn, int readonly);
+  void *(*open) (struct backend *, struct connection *conn, int readonly);
   int (*prepare) (struct backend *, struct connection *conn, void *handle,
                   int readonly);
   int (*finalize) (struct backend *, struct connection *conn, void *handle);
@@ -361,9 +361,6 @@ extern int backend_finalize (struct backend *b, struct connection *conn)
   __attribute__((__nonnull__ (1, 2)));
 extern void backend_close (struct backend *b, struct connection *conn)
   __attribute__((__nonnull__ (1, 2)));
-extern void backend_set_handle (struct backend *b, struct connection *conn,
-                                void *handle)
-  __attribute__((__nonnull__ (1, 2, 3)));
 extern bool backend_valid_range (struct backend *b, struct connection *conn,
                                  uint64_t offset, uint32_t count)
   __attribute__((__nonnull__ (1, 2)));
diff --git a/server/backend.c b/server/backend.c
index 641af5ff..702c9b96 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -172,7 +172,6 @@ int
 backend_open (struct backend *b, struct connection *conn, int readonly)
 {
   struct b_conn_handle *h = &conn->handles[b->i];
-  int r;

   debug ("%s: open readonly=%d", b->name, readonly);

@@ -180,18 +179,22 @@ backend_open (struct backend *b, struct connection *conn, int readonly)
   assert (h->can_write == -1);
   if (readonly)
     h->can_write = 0;
-  r = b->open (b, conn, readonly);
-  if (r == 0) {
-    assert (h->handle != NULL);
-    if (b->i) /* A filter must not succeed unless its backend did also */
-      assert (conn->handles[b->i - 1].handle);
-  }
-  else {
-    assert (h->handle == NULL);
+
+  /* Most filters will call next_open first, resulting in
+   * inner-to-outer ordering.
+   */
+  h->handle = b->open (b, conn, readonly);
+  debug ("%s: open returned handle %p", b->name, h->handle);
+
+  if (h->handle == NULL) {
     if (b->i) /* Do not strand backend if this layer failed */
       backend_close (b->next, conn);
+    return -1;
   }
-  return r;
+
+  if (b->i) /* A filter must not succeed unless its backend did also */
+    assert (conn->handles[b->i - 1].handle);
+  return 0;
 }

 int
@@ -199,6 +202,14 @@ backend_prepare (struct backend *b, struct connection *conn)
 {
   struct b_conn_handle *h = &conn->handles[b->i];

+  assert (h->handle);
+
+  /* Call these in order starting from the filter closest to the
+   * plugin, similar to typical .open order.
+   */
+  if (b->i && backend_prepare (b->next, conn) == -1)
+    return -1;
+
   debug ("%s: prepare readonly=%d", b->name, h->can_write == 0);

   return b->prepare (b, conn, h->handle, h->can_write == 0);
@@ -209,9 +220,19 @@ backend_finalize (struct backend *b, struct connection *conn)
 {
   struct b_conn_handle *h = &conn->handles[b->i];

+  /* Call these in reverse order to .prepare above, starting from the
+   * filter furthest away from the plugin, and matching .close order.
+   */
+
+  assert (h->handle);
   debug ("%s: finalize", b->name);

-  return b->finalize (b, conn, h->handle);
+  if (b->finalize (b, conn, h->handle) == -1)
+    return -1;
+
+  if (b->i)
+    return backend_finalize (b->next, conn);
+  return 0;
 }

 void
@@ -219,10 +240,13 @@ backend_close (struct backend *b, struct connection *conn)
 {
   struct b_conn_handle *h = &conn->handles[b->i];

+  /* outer-to-inner order, opposite .open */
   debug ("%s: close", b->name);

   b->close (b, conn, h->handle);
   reset_b_conn_handle (h);
+  if (b->i)
+    backend_close (b->next, conn);
 }

 void
diff --git a/server/filters.c b/server/filters.c
index 0635f0bb..1ac4a5f4 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -198,28 +198,21 @@ next_open (void *nxdata, int readonly)
   return backend_open (b_conn->b, b_conn->conn, readonly);
 }

-static int
+static void *
 filter_open (struct backend *b, struct connection *conn, int readonly)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
   struct b_conn nxdata = { .b = b->next, .conn = conn };
-  void *handle;

   /* Most filters will call next_open first, resulting in
    * inner-to-outer ordering.
    */
-  if (f->filter.open) {
-    handle = f->filter.open (next_open, &nxdata, readonly);
-    if (handle == NULL)
-      return -1;
-    backend_set_handle (b, conn, handle);
-  }
-  else {
-    if (backend_open (b->next, conn, readonly) == -1)
-      return -1;
-    backend_set_handle (b, conn, NBDKIT_HANDLE_NOT_NEEDED);
-  }
-  return 0;
+  if (f->filter.open)
+    return f->filter.open (next_open, &nxdata, readonly);
+  else if (backend_open (b->next, conn, readonly) == -1)
+    return NULL;
+  else
+    return NBDKIT_HANDLE_NOT_NEEDED;
 }

 static void
@@ -227,10 +220,8 @@ filter_close (struct backend *b, struct connection *conn, void *handle)
 {
   struct backend_filter *f = container_of (b, struct backend_filter, backend);

-  /* outer-to-inner order, opposite .open */
   if (handle && f->filter.close)
     f->filter.close (handle);
-  backend_close (b->next, conn);
 }

 /* The next_functions structure contains pointers to backend
@@ -411,12 +402,6 @@ filter_prepare (struct backend *b, struct connection *conn, void *handle,
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

-  /* Call these in order starting from the filter closest to the
-   * plugin, similar to typical .open order.
-   */
-  if (backend_prepare (b->next, conn) == -1)
-    return -1;
-
   if (f->filter.prepare &&
       f->filter.prepare (&next_ops, &nxdata, handle, readonly) == -1)
     return -1;
@@ -430,14 +415,10 @@ filter_finalize (struct backend *b, struct connection *conn, void *handle)
   struct backend_filter *f = container_of (b, struct backend_filter, backend);
   struct b_conn nxdata = { .b = b->next, .conn = conn };

-  /* Call these in reverse order to .prepare above, starting from the
-   * filter furthest away from the plugin, and matching .close order.
-   */
   if (f->filter.finalize &&
       f->filter.finalize (&next_ops, &nxdata, handle) == -1)
     return -1;
-
-  return backend_finalize (b->next, conn);
+  return 0;
 }

 static int64_t
diff --git a/server/plugins.c b/server/plugins.c
index f309ac22..87daaf26 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -233,20 +233,14 @@ plugin_magic_config_key (struct backend *b)
   return p->plugin.magic_config_key;
 }

-static int
+static void *
 plugin_open (struct backend *b, struct connection *conn, int readonly)
 {
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
-  void *handle;

   assert (p->plugin.open != NULL);

-  handle = p->plugin.open (readonly);
-  if (!handle)
-    return -1;
-
-  backend_set_handle (b, conn, handle);
-  return 0;
+  return p->plugin.open (readonly);
 }

 /* We don't expose .prepare and .finalize to plugins since they aren't
-- 
2.21.0




More information about the Libguestfs mailing list