From 25068fffea6ec1295d8aa1e62135ec41eb44d74d Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 4 Oct 2019 11:52:54 -0500 Subject: [PATCH] server: Move prepare/finalize/close recursion to backend.c 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 --- server/backend.c | 54 +++++++++++++++++++++++++++++++++++------------------- server/filters.c | 35 ++++++++--------------------------- server/internal.h | 5 +---- server/plugins.c | 10 ++-------- 4 files changed, 46 insertions(+), 58 deletions(-) diff --git a/server/backend.c b/server/backend.c index 641af5ff..d13cc107 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,18 +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); -} - -void -backend_set_handle (struct backend *b, struct connection *conn, void *handle) -{ - assert (b->i < conn->nr_handles); - assert (conn->handles[b->i].handle == NULL); - conn->handles[b->i].handle = handle; + if (b->i) + backend_close (b->next, conn); } bool 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/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/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.11.4.GIT