[Libguestfs] [nbdkit PATCH v3 05/16] server: Update signature of backend_open
Eric Blake
eblake at redhat.com
Fri Mar 5 23:31:09 UTC 2021
Instead of returning an int on success, return a pointer to everything
needed to call into the next layer via next_ops. The public interface
in nbdkit-filter.h still returns int, so we need a next_open() wrapper
added back in filters.h. Similarly, an upcoming patch wants to update
backend_* functions to directly take a struct context* instead of the
current approach of a struct backend* coupled with querying TLS for
the current connection. This will be easier by making struct context
track the struct backend that it was opened under. Struct connection
still maintains a list of contexts, but now instead of directly
populating direct members of an array during backend_open, we now
populate pointers in the callers of backend_open; this will let us
progress towards filters opening a context independently from the
current client connection.
---
server/internal.h | 15 +++++--
server/backend.c | 65 +++++++++++++++-------------
server/connections.c | 7 +--
server/filters.c | 16 ++++++-
server/protocol-handshake-newstyle.c | 8 ++--
server/protocol-handshake.c | 7 ++-
6 files changed, 73 insertions(+), 45 deletions(-)
diff --git a/server/internal.h b/server/internal.h
index 5e382fe0..1add5f36 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -208,6 +208,7 @@ enum {
struct context {
void *handle; /* Plugin or filter handle. */
+ struct backend *b; /* Backend that provided handle. */
unsigned char state; /* Bitmask of HANDLE_* values */
@@ -254,7 +255,7 @@ struct connection {
void *crypto_session;
int nworkers;
- struct context *contexts; /* One per plugin and filter. */
+ struct context **contexts; /* One per plugin and filter. */
char **default_exportname; /* One per plugin and filter. */
uint32_t cflags;
@@ -277,7 +278,13 @@ struct connection {
static inline struct context *
get_context (struct connection *conn, int i)
{
- return &conn->contexts[i];
+ return conn->contexts[i];
+}
+
+static inline void
+set_context (struct connection *conn, int i, struct context *context)
+{
+ conn->contexts[i] = context;
}
extern void handle_single_connection (int sockin, int sockout);
@@ -443,8 +450,8 @@ extern const char *backend_export_description (struct backend *b)
* freed on return of this function, so backends must save the
* exportname if they need to refer to it later.
*/
-extern int backend_open (struct backend *b,
- int readonly, const char *exportname)
+extern struct context *backend_open (struct backend *b,
+ int readonly, const char *exportname)
__attribute__((__nonnull__ (1, 3)));
extern int backend_prepare (struct backend *b)
__attribute__((__nonnull__ (1)));
diff --git a/server/backend.c b/server/backend.c
index 0a67e8be..cebd55ab 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -169,8 +169,7 @@ backend_list_exports (struct backend *b, int readonly,
controlpath_debug ("%s: list_exports readonly=%d tls=%d",
b->name, readonly, conn->using_tls);
- assert (c->handle == NULL);
- assert ((c->state & HANDLE_OPEN) == 0);
+ assert (c == NULL);
if (b->list_exports (b, readonly, conn->using_tls, exports) == -1 ||
exports_resolve_default (exports, b, readonly) == -1) {
@@ -194,8 +193,7 @@ backend_default_export (struct backend *b, int readonly)
b->name, readonly, conn->using_tls);
if (conn->default_exportname[b->i] == NULL) {
- assert (c->handle == NULL);
- assert ((c->state & HANDLE_OPEN) == 0);
+ assert (c == NULL);
s = b->default_export (b, readonly, conn->using_tls);
/* Ignore over-length strings. XXX Also ignore non-UTF8? */
if (s && strnlen (s, NBD_MAX_STRING + 1) > NBD_MAX_STRING) {
@@ -213,18 +211,22 @@ backend_default_export (struct backend *b, int readonly)
return conn->default_exportname[b->i];
}
-int
+struct context *
backend_open (struct backend *b, int readonly, const char *exportname)
{
GET_CONN;
- struct context *c = get_context (conn, b->i);
+ struct context *c = malloc (sizeof *c);
+
+ if (c == NULL) {
+ nbdkit_error ("malloc: %m");
+ return NULL;
+ }
controlpath_debug ("%s: open readonly=%d exportname=\"%s\" tls=%d",
b->name, readonly, exportname, conn->using_tls);
- assert (c->handle == NULL);
- assert ((c->state & HANDLE_OPEN) == 0);
- assert (c->can_write == -1);
+ assert (conn->contexts[b->i] == NULL);
+ reset_context (c);
if (readonly)
c->can_write = 0;
@@ -233,7 +235,8 @@ backend_open (struct backend *b, int readonly, const char *exportname)
exportname = backend_default_export (b, readonly);
if (exportname == NULL) {
nbdkit_error ("default export (\"\") not permitted");
- return -1;
+ free (c);
+ return NULL;
}
}
@@ -246,11 +249,13 @@ backend_open (struct backend *b, int readonly, const char *exportname)
if (c->handle == NULL) {
if (b->i) /* Do not strand backend if this layer failed */
backend_close (b->next);
- return -1;
+ free (c);
+ return NULL;
}
c->state |= HANDLE_OPEN;
- return 0;
+ c->b = b;
+ return c;
}
int
@@ -266,7 +271,7 @@ backend_prepare (struct backend *b)
* plugin, similar to typical .open order. But remember that
* a filter may skip opening its backend.
*/
- if (b->i && get_context (conn, b->i-1)->handle != NULL &&
+ if (b->i && get_context (conn, b->i-1) != NULL &&
backend_prepare (b->next) == -1)
return -1;
@@ -301,7 +306,7 @@ backend_finalize (struct backend *b)
}
}
- if (b->i)
+ if (b->i && get_context (conn, b->i-1))
return backend_finalize (b->next);
return 0;
}
@@ -313,16 +318,13 @@ backend_close (struct backend *b)
struct context *c = get_context (conn, b->i);
/* outer-to-inner order, opposite .open */
-
- if (c->handle) {
- assert (c->state & HANDLE_OPEN);
- controlpath_debug ("%s: close", b->name);
- b->close (b, c->handle);
- }
- else
- assert (! (c->state & HANDLE_OPEN));
- reset_context (c);
- if (b->i)
+ assert (c->handle);
+ assert (c->state & HANDLE_OPEN);
+ controlpath_debug ("%s: close", b->name);
+ b->close (b, c->handle);
+ free (c);
+ set_context (conn, b->i, NULL);
+ if (b->i && get_context (conn, b->i-1))
backend_close (b->next);
}
@@ -342,16 +344,21 @@ backend_valid_range (struct backend *b, uint64_t offset, uint32_t count)
int
backend_reopen (struct backend *b, int readonly, const char *exportname)
{
+ GET_CONN;
+ struct context *h;
+
controlpath_debug ("%s: reopen readonly=%d exportname=\"%s\"",
b->name, readonly, exportname);
- if (backend_finalize (b) == -1)
- return -1;
- backend_close (b);
- if (backend_open (b, readonly, exportname) == -1) {
+ if (get_context (conn, b->i)) {
+ if (backend_finalize (b) == -1)
+ return -1;
backend_close (b);
- return -1;
}
+ h = backend_open (b, readonly, exportname);
+ if (h == NULL)
+ return -1;
+ set_context (conn, b->i, h);
if (backend_prepare (b) == -1) {
backend_finalize (b);
backend_close (b);
diff --git a/server/connections.c b/server/connections.c
index 87db2d80..e3fdc649 100644
--- a/server/connections.c
+++ b/server/connections.c
@@ -240,7 +240,6 @@ new_connection (int sockin, int sockout, int nworkers)
struct connection *conn;
int opt;
socklen_t optlen = sizeof opt;
- struct backend *b;
conn = calloc (1, sizeof *conn);
if (conn == NULL) {
@@ -259,9 +258,6 @@ new_connection (int sockin, int sockout, int nworkers)
perror ("malloc");
goto error1;
}
- for_each_backend (b)
- reset_context (get_context (conn, b->i));
-
conn->default_exportname = calloc (top->i + 1,
sizeof *conn->default_exportname);
if (conn->default_exportname == NULL) {
@@ -364,7 +360,8 @@ free_connection (struct connection *conn)
*/
if (!quit) {
lock_request ();
- backend_close (top);
+ if (get_context (conn, top->i))
+ backend_close (top);
unlock_request ();
}
diff --git a/server/filters.c b/server/filters.c
index 31be9742..dde36306 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -260,6 +260,18 @@ filter_default_export (struct backend *b, int readonly, int is_tls)
return backend_default_export (b->next, readonly);
}
+static int
+next_open (struct backend *b, int readonly, const char *exportname)
+{
+ GET_CONN;
+ struct context *c = backend_open (b, readonly, exportname);
+
+ if (c == NULL)
+ return -1;
+ set_context (conn, b->i, c);
+ return 0;
+}
+
static void *
filter_open (struct backend *b, int readonly, const char *exportname,
int is_tls)
@@ -271,9 +283,9 @@ filter_open (struct backend *b, int readonly, const char *exportname,
* inner-to-outer ordering.
*/
if (f->filter.open)
- handle = f->filter.open (backend_open, b->next, readonly, exportname,
+ handle = f->filter.open (next_open, b->next, readonly, exportname,
is_tls);
- else if (backend_open (b->next, readonly, exportname) == -1)
+ else if (next_open (b->next, readonly, exportname) == -1)
handle = NULL;
else
handle = NBDKIT_HANDLE_NOT_NEEDED;
diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c
index fb4a93bc..015e7f9b 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -567,9 +567,11 @@ negotiate_handshake_newstyle_options (void)
*/
if (finish_newstyle_options (&exportsize,
&data[4], exportnamelen) == -1) {
- if (backend_finalize (top) == -1)
- return -1;
- backend_close (top);
+ if (get_context (conn, top->i)) {
+ if (backend_finalize (top) == -1)
+ return -1;
+ backend_close (top);
+ }
if (send_newstyle_option_reply (option, NBD_REP_ERR_UNKNOWN) == -1)
return -1;
continue;
diff --git a/server/protocol-handshake.c b/server/protocol-handshake.c
index 80233713..82e7e647 100644
--- a/server/protocol-handshake.c
+++ b/server/protocol-handshake.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2013-2020 Red Hat Inc.
+ * Copyright (C) 2013-2021 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -79,9 +79,12 @@ protocol_common_open (uint64_t *exportsize, uint16_t *flags,
int64_t size;
uint16_t eflags = NBD_FLAG_HAS_FLAGS;
int fl;
+ struct context *c;
- if (backend_open (top, read_only, exportname) == -1)
+ c = backend_open (top, read_only, exportname);
+ if (c == NULL)
return -1;
+ set_context (conn, top->i, c);
/* Prepare (for filters), called just after open. */
if (backend_prepare (top) == -1)
--
2.30.1
More information about the Libguestfs
mailing list