[PATCH nbdkit 0/3] server: Remove explicit connection parameter.

older
[PATCH v2] lib: add support for...

Richard W.M. Jones

11 Feb 2020 11 Feb '20

5:15 p.m.

The third patch is a large but mechanical change which gets rid of passing around struct connection * entirely within the server, preferring instead to reference the connection through thread-local storage. I hope this is a gateway to simplifying other parts of the code. Rich.

Show replies by date

Richard W.M. Jones

11 Feb 11 Feb

5:15 p.m.

New subject: [PATCH nbdkit 1/3] server: Add GET_CONN macro, alias for threadlocal_get_conn ().

Since we're going to be calling this function a lot, add a short alias for it. --- server/internal.h | 1 + server/public.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/server/internal.h b/server/internal.h index a1fa7309..1e7b4cf0 100644 --- a/server/internal.h +++ b/server/internal.h @@ -493,6 +493,7 @@ extern int threadlocal_get_error (void); extern void *threadlocal_buffer (size_t size); extern void threadlocal_set_conn (struct connection *conn); extern struct connection *threadlocal_get_conn (void); +#define GET_CONN (threadlocal_get_conn ()) /* Declare program_name. */ #if HAVE_DECL_PROGRAM_INVOCATION_SHORT_NAME == 1 diff --git a/server/public.c b/server/public.c index 418945fe..8fa7e21b 100644 --- a/server/public.c +++ b/server/public.c @@ -533,7 +533,7 @@ nbdkit_nanosleep (unsigned sec, unsigned nsec) * NBD_CMD_DISC or a problem with the connection * - the input socket detects POLLRDHUP/POLLHUP/POLLERR */ - struct connection *conn = threadlocal_get_conn (); + struct connection *conn = GET_CONN; struct pollfd fds[] = { [0].fd = quit_fd, [0].events = POLLIN, @@ -595,7 +595,7 @@ nbdkit_nanosleep (unsigned sec, unsigned nsec) const char * nbdkit_export_name (void) { - struct connection *conn = threadlocal_get_conn (); + struct connection *conn = GET_CONN; if (!conn) { nbdkit_error ("no connection in this thread"); @@ -608,7 +608,7 @@ nbdkit_export_name (void) int nbdkit_peer_name (struct sockaddr *addr, socklen_t *addrlen) { - struct connection *conn = threadlocal_get_conn (); + struct connection *conn = GET_CONN; int s; if (!conn) { -- 2.25.0

Eric Blake

5:25 p.m.

New subject: [PATCH nbdkit 1/3] server: Add GET_CONN macro, alias for threadlocal_get_conn ().

On 2/11/20 11:15 AM, Richard W.M. Jones wrote:

...

Since we're going to be calling this function a lot, add a short alias for it. --- server/internal.h | 1 + server/public.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/server/internal.h b/server/internal.h index a1fa7309..1e7b4cf0 100644 --- a/server/internal.h +++ b/server/internal.h @@ -493,6 +493,7 @@ extern int threadlocal_get_error (void); extern void *threadlocal_buffer (size_t size); extern void threadlocal_set_conn (struct connection *conn); extern struct connection *threadlocal_get_conn (void); +#define GET_CONN (threadlocal_get_conn ())

Do we want any checking, such as whether this is non-NULL? For example, patch 3 has: -typedef void (*connection_close_function) (struct connection *) __attribute__((__nonnull__ (1))); +typedef void (*connection_close_function) (void); which loses the compile-time checking that we have a non-NULL connection parameter; so replacing a parameter passed through a nonnull attribute with a macro that guarantees a nonnull result might be in our favor, where we can leave explicit calls to threadlocal_get_conn() rather than macro usage for the few callers that expect to handle the corner cases where a threadlocal connection might not yet be set. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

Richard W.M. Jones

5:29 p.m.

New subject: [PATCH nbdkit 1/3] server: Add GET_CONN macro, alias for threadlocal_get_conn ().

On Tue, Feb 11, 2020 at 11:25:49AM -0600, Eric Blake wrote:

...

On 2/11/20 11:15 AM, Richard W.M. Jones wrote:

...
Since we're going to be calling this function a lot, add a short alias for it. --- server/internal.h | 1 + server/public.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/server/internal.h b/server/internal.h index a1fa7309..1e7b4cf0 100644 --- a/server/internal.h +++ b/server/internal.h @@ -493,6 +493,7 @@ extern int threadlocal_get_error (void); extern void *threadlocal_buffer (size_t size); extern void threadlocal_set_conn (struct connection *conn); extern struct connection *threadlocal_get_conn (void); +#define GET_CONN (threadlocal_get_conn ())

Do we want any checking, such as whether this is non-NULL?

For example, patch 3 has:

-typedef void (*connection_close_function) (struct connection *) __attribute__((__nonnull__ (1))); +typedef void (*connection_close_function) (void);

which loses the compile-time checking that we have a non-NULL connection parameter; so replacing a parameter passed through a nonnull attribute with a macro that guarantees a nonnull result might be in our favor, where we can leave explicit calls to threadlocal_get_conn() rather than macro usage for the few callers that expect to handle the corner cases where a threadlocal connection might not yet be set.

Yes let's do this. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html

Richard W.M. Jones

5:15 p.m.

New subject: [PATCH nbdkit 2/3] server: connections: Don't free TLS storage of conn until end of function.

We want to do this as late as possible. In particular, backend_close may call plugin.close which may (although it's unlikely) call one of the functions which relies on the implicit connection pointer fetched through thread-local storage (TLS) which would then fail. --- server/connections.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/connections.c b/server/connections.c index 66520fcb..9978afb4 100644 --- a/server/connections.c +++ b/server/connections.c @@ -331,7 +331,6 @@ free_connection (struct connection *conn) if (!conn) return; - threadlocal_set_conn (NULL); conn->close (conn); if (listen_stdin) { int fd; @@ -368,6 +367,7 @@ free_connection (struct connection *conn) free (conn->handles); free (conn); + threadlocal_set_conn (NULL); } /* Write buffer to conn->sockout with send() and either succeed completely -- 2.25.0

Richard W.M. Jones

5:15 p.m.

New subject: [PATCH nbdkit 3/3] server: Remove explicit connection parameter, use TLS instead.

Since commit 86fdb48c6a5362d66865493d9d2172166f99722e we have stored the connection object in thread-local storage. In this very large, but mostly mechanical change we stop passing the connection pointer around everywhere, and instead use the value stored in thread-local storage. This assumes a 1-1 mapping between the connection and the current thread which is true in *most* places. Occasionally we still have the explicit connection pointer, especially just before we launch a thread or call threadlocal_set_conn. --- server/internal.h | 191 +++++++++---------- server/backend.c | 160 +++++++++------- server/connections.c | 68 ++++--- server/crypto.c | 14 +- server/filters.c | 270 +++++++++++++-------------- server/locks.c | 12 +- server/plugins.c | 64 +++---- server/protocol-handshake-newstyle.c | 172 +++++++++-------- server/protocol-handshake-oldstyle.c | 7 +- server/protocol-handshake.c | 40 ++-- server/protocol.c | 127 ++++++------- server/public.c | 2 +- server/test-public.c | 2 +- 13 files changed, 574 insertions(+), 555 deletions(-) diff --git a/server/internal.h b/server/internal.h index 1e7b4cf0..aed5a7da 100644 --- a/server/internal.h +++ b/server/internal.h @@ -154,15 +154,12 @@ enum { SEND_MORE = 1, /* Hint to use MSG_MORE/corking to group send()s */ }; -typedef int (*connection_recv_function) (struct connection *, - void *buf, size_t len) - __attribute__((__nonnull__ (1, 2))); -typedef int (*connection_send_function) (struct connection *, - const void *buf, size_t len, +typedef int (*connection_recv_function) (void *buf, size_t len) + __attribute__((__nonnull__ (1))); +typedef int (*connection_send_function) (const void *buf, size_t len, int flags) - __attribute__((__nonnull__ (1, 2))); -typedef void (*connection_close_function) (struct connection *) __attribute__((__nonnull__ (1))); +typedef void (*connection_close_function) (void); enum { HANDLE_OPEN = 1, /* Set if .open passed, so .close is needed */ @@ -234,29 +231,22 @@ struct connection { }; extern void handle_single_connection (int sockin, int sockout); -extern int connection_get_status (struct connection *conn) - __attribute__((__nonnull__ (1))); -extern int connection_set_status (struct connection *conn, int value) - __attribute__((__nonnull__ (1))); +extern int connection_get_status (void); +extern int connection_set_status (int value); /* protocol-handshake.c */ -extern int protocol_handshake (struct connection *conn) - __attribute__((__nonnull__ (1))); -extern int protocol_common_open (struct connection *conn, - uint64_t *exportsize, uint16_t *flags) - __attribute__((__nonnull__ (1, 2, 3))); +extern int protocol_handshake (void); +extern int protocol_common_open (uint64_t *exportsize, uint16_t *flags) + __attribute__((__nonnull__ (1, 2))); /* protocol-handshake-oldstyle.c */ -extern int protocol_handshake_oldstyle (struct connection *conn) - __attribute__((__nonnull__ (1))); +extern int protocol_handshake_oldstyle (void); /* protocol-handshake-newstyle.c */ -extern int protocol_handshake_newstyle (struct connection *conn) - __attribute__((__nonnull__ (1))); +extern int protocol_handshake_newstyle (void); /* protocol.c */ -extern int protocol_recv_request_send_reply (struct connection *conn) - __attribute__((__nonnull__ (1))); +extern int protocol_recv_request_send_reply (void); /* The context ID of base:allocation. As far as I can tell it doesn't * matter what this is as long as nbdkit always returns the same @@ -268,9 +258,7 @@ extern int protocol_recv_request_send_reply (struct connection *conn) #define root_tls_certificates_dir sysconfdir "/pki/" PACKAGE_NAME extern void crypto_init (bool tls_set_on_cli); extern void crypto_free (void); -extern int crypto_negotiate_tls (struct connection *conn, - int sockin, int sockout) - __attribute__((__nonnull__ (1))); +extern int crypto_negotiate_tls (int sockin, int sockout); /* debug.c */ #define debug(fs, ...) \ @@ -332,44 +320,39 @@ 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 (*preconnect) (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); - void (*close) (struct backend *, struct connection *conn, void *handle); + int (*preconnect) (struct backend *, int readonly); + void *(*open) (struct backend *, int readonly); + int (*prepare) (struct backend *, void *handle, int readonly); + int (*finalize) (struct backend *, void *handle); + void (*close) (struct backend *, void *handle); - int64_t (*get_size) (struct backend *, struct connection *conn, void *handle); - int (*can_write) (struct backend *, struct connection *conn, void *handle); - int (*can_flush) (struct backend *, struct connection *conn, void *handle); - int (*is_rotational) (struct backend *, struct connection *conn, - void *handle); - int (*can_trim) (struct backend *, struct connection *conn, void *handle); - int (*can_zero) (struct backend *, struct connection *conn, void *handle); - int (*can_fast_zero) (struct backend *, struct connection *conn, - void *handle); - int (*can_extents) (struct backend *, struct connection *conn, void *handle); - int (*can_fua) (struct backend *, struct connection *conn, void *handle); - int (*can_multi_conn) (struct backend *, struct connection *conn, - void *handle); - int (*can_cache) (struct backend *, struct connection *conn, void *handle); + int64_t (*get_size) (struct backend *, void *handle); + int (*can_write) (struct backend *, void *handle); + int (*can_flush) (struct backend *, void *handle); + int (*is_rotational) (struct backend *, void *handle); + int (*can_trim) (struct backend *, void *handle); + int (*can_zero) (struct backend *, void *handle); + int (*can_fast_zero) (struct backend *, void *handle); + int (*can_extents) (struct backend *, void *handle); + int (*can_fua) (struct backend *, void *handle); + int (*can_multi_conn) (struct backend *, void *handle); + int (*can_cache) (struct backend *, void *handle); - int (*pread) (struct backend *, struct connection *conn, void *handle, + int (*pread) (struct backend *, void *handle, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*pwrite) (struct backend *, struct connection *conn, void *handle, + int (*pwrite) (struct backend *, void *handle, const void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*flush) (struct backend *, struct connection *conn, void *handle, - uint32_t flags, int *err); - int (*trim) (struct backend *, struct connection *conn, void *handle, + int (*flush) (struct backend *, void *handle, uint32_t flags, int *err); + int (*trim) (struct backend *, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*zero) (struct backend *, struct connection *conn, void *handle, + int (*zero) (struct backend *, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*extents) (struct backend *, struct connection *conn, void *handle, + int (*extents) (struct backend *, void *handle, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents *extents, int *err); - int (*cache) (struct backend *, struct connection *conn, void *handle, + int (*cache) (struct backend *, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err); }; @@ -382,72 +365,70 @@ extern void backend_load (struct backend *b, const char *name, extern void backend_unload (struct backend *b, void (*unload) (void)) __attribute__((__nonnull__ (1))); -extern int backend_open (struct backend *b, struct connection *conn, - int readonly) - __attribute__((__nonnull__ (1, 2))); -extern int backend_prepare (struct backend *b, struct connection *conn) - __attribute__((__nonnull__ (1, 2))); -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 bool backend_valid_range (struct backend *b, struct connection *conn, +extern int backend_open (struct backend *b, int readonly) + __attribute__((__nonnull__ (1))); +extern int backend_prepare (struct backend *b) + __attribute__((__nonnull__ (1))); +extern int backend_finalize (struct backend *b) + __attribute__((__nonnull__ (1))); +extern void backend_close (struct backend *b) + __attribute__((__nonnull__ (1))); +extern bool backend_valid_range (struct backend *b, uint64_t offset, uint32_t count) - __attribute__((__nonnull__ (1, 2))); + __attribute__((__nonnull__ (1))); -extern int backend_reopen (struct backend *b, struct connection *conn, - int readonly) - __attribute__((__nonnull__ (1, 2))); -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) - __attribute__((__nonnull__ (1, 2))); -extern int backend_can_flush (struct backend *b, struct connection *conn) - __attribute__((__nonnull__ (1, 2))); -extern int backend_is_rotational (struct backend *b, struct connection *conn) - __attribute__((__nonnull__ (1, 2))); -extern int backend_can_trim (struct backend *b, struct connection *conn) - __attribute__((__nonnull__ (1, 2))); -extern int backend_can_zero (struct backend *b, struct connection *conn) - __attribute__((__nonnull__ (1, 2))); -extern int backend_can_fast_zero (struct backend *b, struct connection *conn) - __attribute__((__nonnull__ (1, 2))); -extern int backend_can_extents (struct backend *b, struct connection *conn) - __attribute__((__nonnull__ (1, 2))); -extern int backend_can_fua (struct backend *b, struct connection *conn) - __attribute__((__nonnull__ (1, 2))); -extern int backend_can_multi_conn (struct backend *b, struct connection *conn) - __attribute__((__nonnull__ (1, 2))); -extern int backend_can_cache (struct backend *b, struct connection *conn) - __attribute__((__nonnull__ (1, 2))); +extern int backend_reopen (struct backend *b, int readonly) + __attribute__((__nonnull__ (1))); +extern int64_t backend_get_size (struct backend *b) + __attribute__((__nonnull__ (1))); +extern int backend_can_write (struct backend *b) + __attribute__((__nonnull__ (1))); +extern int backend_can_flush (struct backend *b) + __attribute__((__nonnull__ (1))); +extern int backend_is_rotational (struct backend *b) + __attribute__((__nonnull__ (1))); +extern int backend_can_trim (struct backend *b) + __attribute__((__nonnull__ (1))); +extern int backend_can_zero (struct backend *b) + __attribute__((__nonnull__ (1))); +extern int backend_can_fast_zero (struct backend *b) + __attribute__((__nonnull__ (1))); +extern int backend_can_extents (struct backend *b) + __attribute__((__nonnull__ (1))); +extern int backend_can_fua (struct backend *b) + __attribute__((__nonnull__ (1))); +extern int backend_can_multi_conn (struct backend *b) + __attribute__((__nonnull__ (1))); +extern int backend_can_cache (struct backend *b) + __attribute__((__nonnull__ (1))); -extern int backend_pread (struct backend *b, struct connection *conn, +extern int backend_pread (struct backend *b, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) - __attribute__((__nonnull__ (1, 2, 3, 7))); -extern int backend_pwrite (struct backend *b, struct connection *conn, + __attribute__((__nonnull__ (1, 2, 6))); +extern int backend_pwrite (struct backend *b, const void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) - __attribute__((__nonnull__ (1, 2, 3, 7))); -extern int backend_flush (struct backend *b, struct connection *conn, + __attribute__((__nonnull__ (1, 2, 6))); +extern int backend_flush (struct backend *b, uint32_t flags, int *err) - __attribute__((__nonnull__ (1, 2, 4))); -extern int backend_trim (struct backend *b, struct connection *conn, + __attribute__((__nonnull__ (1, 3))); +extern int backend_trim (struct backend *b, uint32_t count, uint64_t offset, uint32_t flags, int *err) - __attribute__((__nonnull__ (1, 2, 6))); -extern int backend_zero (struct backend *b, struct connection *conn, + __attribute__((__nonnull__ (1, 5))); +extern int backend_zero (struct backend *b, uint32_t count, uint64_t offset, uint32_t flags, int *err) - __attribute__((__nonnull__ (1, 2, 6))); -extern int backend_extents (struct backend *b, struct connection *conn, + __attribute__((__nonnull__ (1, 5))); +extern int backend_extents (struct backend *b, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents *extents, int *err) - __attribute__((__nonnull__ (1, 2, 6, 7))); -extern int backend_cache (struct backend *b, struct connection *conn, + __attribute__((__nonnull__ (1, 5, 6))); +extern int backend_cache (struct backend *b, uint32_t count, uint64_t offset, uint32_t flags, int *err) - __attribute__((__nonnull__ (1, 2, 6))); + __attribute__((__nonnull__ (1, 5))); /* plugins.c */ extern struct backend *plugin_register (size_t index, const char *filename, @@ -465,8 +446,8 @@ extern void lock_init_thread_model (void); extern const char *name_of_thread_model (int model); 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_request (void); +extern void unlock_request (void); extern void lock_unload (void); extern void unlock_unload (void); diff --git a/server/backend.c b/server/backend.c index 208c07b1..7a9a7ec8 100644 --- a/server/backend.c +++ b/server/backend.c @@ -151,8 +151,9 @@ backend_unload (struct backend *b, void (*unload) (void)) } int -backend_open (struct backend *b, struct connection *conn, int readonly) +backend_open (struct backend *b, int readonly) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; controlpath_debug ("%s: open readonly=%d", b->name, readonly); @@ -166,12 +167,12 @@ backend_open (struct backend *b, struct connection *conn, int readonly) /* Most filters will call next_open first, resulting in * inner-to-outer ordering. */ - h->handle = b->open (b, conn, readonly); + h->handle = b->open (b, readonly); controlpath_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); + backend_close (b->next); return -1; } @@ -182,8 +183,9 @@ backend_open (struct backend *b, struct connection *conn, int readonly) } int -backend_prepare (struct backend *b, struct connection *conn) +backend_prepare (struct backend *b) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; assert (h->handle); @@ -192,20 +194,21 @@ backend_prepare (struct backend *b, struct connection *conn) /* 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) + if (b->i && backend_prepare (b->next) == -1) return -1; controlpath_debug ("%s: prepare readonly=%d", b->name, h->can_write == 0); - if (b->prepare (b, conn, h->handle, h->can_write == 0) == -1) + if (b->prepare (b, h->handle, h->can_write == 0) == -1) return -1; h->state |= HANDLE_CONNECTED; return 0; } int -backend_finalize (struct backend *b, struct connection *conn) +backend_finalize (struct backend *b) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; /* Call these in reverse order to .prepare above, starting from the @@ -220,7 +223,7 @@ backend_finalize (struct backend *b, struct connection *conn) if (h->handle) { assert (h->state & HANDLE_CONNECTED); - if (b->finalize (b, conn, h->handle) == -1) { + if (b->finalize (b, h->handle) == -1) { h->state |= HANDLE_FAILED; return -1; } @@ -229,13 +232,14 @@ backend_finalize (struct backend *b, struct connection *conn) assert (! (h->state & HANDLE_CONNECTED)); if (b->i) - return backend_finalize (b->next, conn); + return backend_finalize (b->next); return 0; } void -backend_close (struct backend *b, struct connection *conn) +backend_close (struct backend *b) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; /* outer-to-inner order, opposite .open */ @@ -243,19 +247,19 @@ backend_close (struct backend *b, struct connection *conn) if (h->handle) { assert (h->state & HANDLE_OPEN); - b->close (b, conn, h->handle); + b->close (b, h->handle); } else assert (! (h->state & HANDLE_OPEN)); reset_b_conn_handle (h); if (b->i) - backend_close (b->next, conn); + backend_close (b->next); } bool -backend_valid_range (struct backend *b, struct connection *conn, - uint64_t offset, uint32_t count) +backend_valid_range (struct backend *b, uint64_t offset, uint32_t count) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; assert (h->exportsize <= INT64_MAX); /* Guaranteed by negotiation phase */ @@ -266,80 +270,85 @@ backend_valid_range (struct backend *b, struct connection *conn, /* Wrappers for all callbacks in a filter's struct nbdkit_next_ops. */ int -backend_reopen (struct backend *b, struct connection *conn, int readonly) +backend_reopen (struct backend *b, int readonly) { controlpath_debug ("%s: reopen readonly=%d", b->name, readonly); - if (backend_finalize (b, conn) == -1) + if (backend_finalize (b) == -1) return -1; - backend_close (b, conn); - if (backend_open (b, conn, readonly) == -1) { - backend_close (b, conn); + backend_close (b); + if (backend_open (b, readonly) == -1) { + backend_close (b); return -1; } - if (backend_prepare (b, conn) == -1) { - backend_finalize (b, conn); - backend_close (b, conn); + if (backend_prepare (b) == -1) { + backend_finalize (b); + backend_close (b); return -1; } return 0; } int64_t -backend_get_size (struct backend *b, struct connection *conn) +backend_get_size (struct backend *b) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; controlpath_debug ("%s: get_size", b->name); assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->exportsize == -1) - h->exportsize = b->get_size (b, conn, h->handle); + h->exportsize = b->get_size (b, h->handle); return h->exportsize; } int -backend_can_write (struct backend *b, struct connection *conn) +backend_can_write (struct backend *b) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; controlpath_debug ("%s: can_write", b->name); assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->can_write == -1) - h->can_write = b->can_write (b, conn, h->handle); + h->can_write = b->can_write (b, h->handle); return h->can_write; } int -backend_can_flush (struct backend *b, struct connection *conn) +backend_can_flush (struct backend *b) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; controlpath_debug ("%s: can_flush", b->name); assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->can_flush == -1) - h->can_flush = b->can_flush (b, conn, h->handle); + h->can_flush = b->can_flush (b, h->handle); return h->can_flush; } int -backend_is_rotational (struct backend *b, struct connection *conn) +backend_is_rotational (struct backend *b) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; controlpath_debug ("%s: is_rotational", b->name); assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->is_rotational == -1) - h->is_rotational = b->is_rotational (b, conn, h->handle); + h->is_rotational = b->is_rotational (b, h->handle); return h->is_rotational; } int -backend_can_trim (struct backend *b, struct connection *conn) +backend_can_trim (struct backend *b) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; int r; @@ -347,19 +356,20 @@ backend_can_trim (struct backend *b, struct connection *conn) assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->can_trim == -1) { - r = backend_can_write (b, conn); + r = backend_can_write (b); if (r != 1) { h->can_trim = 0; return r; } - h->can_trim = b->can_trim (b, conn, h->handle); + h->can_trim = b->can_trim (b, h->handle); } return h->can_trim; } int -backend_can_zero (struct backend *b, struct connection *conn) +backend_can_zero (struct backend *b) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; int r; @@ -367,19 +377,20 @@ backend_can_zero (struct backend *b, struct connection *conn) assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->can_zero == -1) { - r = backend_can_write (b, conn); + r = backend_can_write (b); if (r != 1) { h->can_zero = NBDKIT_ZERO_NONE; return r; /* Relies on 0 == NBDKIT_ZERO_NONE */ } - h->can_zero = b->can_zero (b, conn, h->handle); + h->can_zero = b->can_zero (b, h->handle); } return h->can_zero; } int -backend_can_fast_zero (struct backend *b, struct connection *conn) +backend_can_fast_zero (struct backend *b) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; int r; @@ -387,32 +398,34 @@ backend_can_fast_zero (struct backend *b, struct connection *conn) assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->can_fast_zero == -1) { - r = backend_can_zero (b, conn); + r = backend_can_zero (b); if (r < NBDKIT_ZERO_EMULATE) { h->can_fast_zero = 0; return r; /* Relies on 0 == NBDKIT_ZERO_NONE */ } - h->can_fast_zero = b->can_fast_zero (b, conn, h->handle); + h->can_fast_zero = b->can_fast_zero (b, h->handle); } return h->can_fast_zero; } int -backend_can_extents (struct backend *b, struct connection *conn) +backend_can_extents (struct backend *b) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; controlpath_debug ("%s: can_extents", b->name); assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->can_extents == -1) - h->can_extents = b->can_extents (b, conn, h->handle); + h->can_extents = b->can_extents (b, h->handle); return h->can_extents; } int -backend_can_fua (struct backend *b, struct connection *conn) +backend_can_fua (struct backend *b) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; int r; @@ -420,90 +433,95 @@ backend_can_fua (struct backend *b, struct connection *conn) assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->can_fua == -1) { - r = backend_can_write (b, conn); + r = backend_can_write (b); if (r != 1) { h->can_fua = NBDKIT_FUA_NONE; return r; /* Relies on 0 == NBDKIT_FUA_NONE */ } - h->can_fua = b->can_fua (b, conn, h->handle); + h->can_fua = b->can_fua (b, h->handle); } return h->can_fua; } int -backend_can_multi_conn (struct backend *b, struct connection *conn) +backend_can_multi_conn (struct backend *b) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; assert (h->handle && (h->state & HANDLE_CONNECTED)); controlpath_debug ("%s: can_multi_conn", b->name); if (h->can_multi_conn == -1) - h->can_multi_conn = b->can_multi_conn (b, conn, h->handle); + h->can_multi_conn = b->can_multi_conn (b, h->handle); return h->can_multi_conn; } int -backend_can_cache (struct backend *b, struct connection *conn) +backend_can_cache (struct backend *b) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; controlpath_debug ("%s: can_cache", b->name); assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->can_cache == -1) - h->can_cache = b->can_cache (b, conn, h->handle); + h->can_cache = b->can_cache (b, h->handle); return h->can_cache; } int -backend_pread (struct backend *b, struct connection *conn, +backend_pread (struct backend *b, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; int r; assert (h->handle && (h->state & HANDLE_CONNECTED)); - assert (backend_valid_range (b, conn, offset, count)); + assert (backend_valid_range (b, offset, count)); assert (flags == 0); datapath_debug ("%s: pread count=%" PRIu32 " offset=%" PRIu64, b->name, count, offset); - r = b->pread (b, conn, h->handle, buf, count, offset, flags, err); + r = b->pread (b, h->handle, buf, count, offset, flags, err); if (r == -1) assert (*err); return r; } int -backend_pwrite (struct backend *b, struct connection *conn, +backend_pwrite (struct backend *b, const void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; bool fua = !!(flags & NBDKIT_FLAG_FUA); int r; assert (h->handle && (h->state & HANDLE_CONNECTED)); assert (h->can_write == 1); - assert (backend_valid_range (b, conn, offset, count)); + assert (backend_valid_range (b, offset, count)); assert (!(flags & ~NBDKIT_FLAG_FUA)); if (fua) assert (h->can_fua > NBDKIT_FUA_NONE); datapath_debug ("%s: pwrite count=%" PRIu32 " offset=%" PRIu64 " fua=%d", b->name, count, offset, fua); - r = b->pwrite (b, conn, h->handle, buf, count, offset, flags, err); + r = b->pwrite (b, h->handle, buf, count, offset, flags, err); if (r == -1) assert (*err); return r; } int -backend_flush (struct backend *b, struct connection *conn, +backend_flush (struct backend *b, uint32_t flags, int *err) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; int r; @@ -512,17 +530,18 @@ backend_flush (struct backend *b, struct connection *conn, assert (flags == 0); datapath_debug ("%s: flush", b->name); - r = b->flush (b, conn, h->handle, flags, err); + r = b->flush (b, h->handle, flags, err); if (r == -1) assert (*err); return r; } int -backend_trim (struct backend *b, struct connection *conn, +backend_trim (struct backend *b, uint32_t count, uint64_t offset, uint32_t flags, int *err) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; bool fua = !!(flags & NBDKIT_FLAG_FUA); int r; @@ -530,24 +549,25 @@ backend_trim (struct backend *b, struct connection *conn, assert (h->handle && (h->state & HANDLE_CONNECTED)); assert (h->can_write == 1); assert (h->can_trim == 1); - assert (backend_valid_range (b, conn, offset, count)); + assert (backend_valid_range (b, offset, count)); assert (!(flags & ~NBDKIT_FLAG_FUA)); if (fua) assert (h->can_fua > NBDKIT_FUA_NONE); datapath_debug ("%s: trim count=%" PRIu32 " offset=%" PRIu64 " fua=%d", b->name, count, offset, fua); - r = b->trim (b, conn, h->handle, count, offset, flags, err); + r = b->trim (b, h->handle, count, offset, flags, err); if (r == -1) assert (*err); return r; } int -backend_zero (struct backend *b, struct connection *conn, +backend_zero (struct backend *b, uint32_t count, uint64_t offset, uint32_t flags, int *err) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; bool fua = !!(flags & NBDKIT_FLAG_FUA); bool fast = !!(flags & NBDKIT_FLAG_FAST_ZERO); @@ -556,7 +576,7 @@ backend_zero (struct backend *b, struct connection *conn, assert (h->handle && (h->state & HANDLE_CONNECTED)); assert (h->can_write == 1); assert (h->can_zero > NBDKIT_ZERO_NONE); - assert (backend_valid_range (b, conn, offset, count)); + assert (backend_valid_range (b, offset, count)); assert (!(flags & ~(NBDKIT_FLAG_MAY_TRIM | NBDKIT_FLAG_FUA | NBDKIT_FLAG_FAST_ZERO))); if (fua) @@ -568,7 +588,7 @@ backend_zero (struct backend *b, struct connection *conn, b->name, count, offset, !!(flags & NBDKIT_FLAG_MAY_TRIM), fua, fast); - r = b->zero (b, conn, h->handle, count, offset, flags, err); + r = b->zero (b, h->handle, count, offset, flags, err); if (r == -1) { assert (*err); if (!fast) @@ -578,16 +598,17 @@ backend_zero (struct backend *b, struct connection *conn, } int -backend_extents (struct backend *b, struct connection *conn, +backend_extents (struct backend *b, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents *extents, int *err) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; int r; assert (h->handle && (h->state & HANDLE_CONNECTED)); assert (h->can_extents >= 0); - assert (backend_valid_range (b, conn, offset, count)); + assert (backend_valid_range (b, offset, count)); assert (!(flags & ~NBDKIT_FLAG_REQ_ONE)); datapath_debug ("%s: extents count=%" PRIu32 " offset=%" PRIu64 " req_one=%d", b->name, count, offset, !!(flags & NBDKIT_FLAG_REQ_ONE)); @@ -601,23 +622,24 @@ backend_extents (struct backend *b, struct connection *conn, *err = errno; return r; } - r = b->extents (b, conn, h->handle, count, offset, flags, extents, err); + r = b->extents (b, h->handle, count, offset, flags, extents, err); if (r == -1) assert (*err); return r; } int -backend_cache (struct backend *b, struct connection *conn, +backend_cache (struct backend *b, uint32_t count, uint64_t offset, uint32_t flags, int *err) { + struct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; int r; assert (h->handle && (h->state & HANDLE_CONNECTED)); assert (h->can_cache > NBDKIT_CACHE_NONE); - assert (backend_valid_range (b, conn, offset, count)); + assert (backend_valid_range (b, offset, count)); assert (flags == 0); datapath_debug ("%s: cache count=%" PRIu32 " offset=%" PRIu64, b->name, count, offset); @@ -628,13 +650,13 @@ backend_cache (struct backend *b, struct connection *conn, while (count) { limit = MIN (count, sizeof buf); - if (backend_pread (b, conn, buf, limit, offset, flags, err) == -1) + if (backend_pread (b, buf, limit, offset, flags, err) == -1) return -1; count -= limit; } return 0; } - r = b->cache (b, conn, h->handle, count, offset, flags, err); + r = b->cache (b, h->handle, count, offset, flags, err); if (r == -1) assert (*err); return r; diff --git a/server/connections.c b/server/connections.c index 9978afb4..d21c9db7 100644 --- a/server/connections.c +++ b/server/connections.c @@ -53,23 +53,24 @@ static struct connection *new_connection (int sockin, int sockout, static void free_connection (struct connection *conn); /* Don't call these raw socket functions directly. Use conn->recv etc. */ -static int raw_recv (struct connection *, void *buf, size_t len); -static int raw_send_socket (struct connection *, const void *buf, size_t len, - int flags); -static int raw_send_other (struct connection *, const void *buf, size_t len, - int flags); -static void raw_close (struct connection *); +static int raw_recv ( void *buf, size_t len); +static int raw_send_socket (const void *buf, size_t len, int flags); +static int raw_send_other (const void *buf, size_t len, int flags); +static void raw_close (void); void * -connection_get_handle (struct connection *conn, size_t i) +connection_get_handle (size_t i) { + struct connection *conn = GET_CONN; + assert (i < conn->nr_handles); return conn->handles[i].handle; } int -connection_get_status (struct connection *conn) +connection_get_status (void) { + struct connection *conn = GET_CONN; int r; if (conn->nworkers && @@ -86,8 +87,10 @@ connection_get_status (struct connection *conn) * For convenience, return the incoming value. */ int -connection_set_status (struct connection *conn, int value) +connection_set_status (int value) { + struct connection *conn = GET_CONN; + if (conn->nworkers && pthread_mutex_lock (&conn->status_lock)) abort (); @@ -125,8 +128,8 @@ connection_worker (void *data) threadlocal_set_conn (conn); free (worker); - while (!quit && connection_get_status (conn) > 0) - protocol_recv_request_send_reply (conn); + while (!quit && connection_get_status () > 0) + protocol_recv_request_send_reply (); debug ("exiting worker thread %s", threadlocal_get_name ()); free (name); return NULL; @@ -159,7 +162,7 @@ handle_single_connection (int sockin, int sockout) plugin_name = "(unknown)"; threadlocal_set_name (plugin_name); - if (backend && backend->preconnect (backend, conn, read_only) == -1) + if (backend && backend->preconnect (backend, read_only) == -1) goto done; /* NBD handshake. @@ -167,14 +170,14 @@ handle_single_connection (int sockin, int sockout) * Note that this calls the backend .open callback when it is safe * to do so (eg. after TLS authentication). */ - if (protocol_handshake (conn) == -1) + if (protocol_handshake () == -1) goto done; if (!nworkers) { /* No need for a separate thread. */ debug ("handshake complete, processing requests serially"); - while (!quit && connection_get_status (conn) > 0) - protocol_recv_request_send_reply (conn); + while (!quit && connection_get_status () > 0) + protocol_recv_request_send_reply (); } else { /* Create thread pool to process requests. */ @@ -192,13 +195,13 @@ handle_single_connection (int sockin, int sockout) if (unlikely (!worker)) { perror ("malloc"); - connection_set_status (conn, -1); + connection_set_status (-1); goto wait; } if (unlikely (asprintf (&worker->name, "%s.%d", plugin_name, nworkers) < 0)) { perror ("asprintf"); - connection_set_status (conn, -1); + connection_set_status (-1); free (worker); goto wait; } @@ -208,7 +211,7 @@ handle_single_connection (int sockin, int sockout) if (unlikely (err)) { errno = err; perror ("pthread_create"); - connection_set_status (conn, -1); + connection_set_status (-1); free (worker); goto wait; } @@ -221,9 +224,9 @@ handle_single_connection (int sockin, int sockout) } /* Finalize (for filters), called just before close. */ - lock_request (conn); - r = backend_finalize (backend, conn); - unlock_request (conn); + lock_request (); + r = backend_finalize (backend); + unlock_request (); if (r == -1) goto done; @@ -331,7 +334,7 @@ free_connection (struct connection *conn) if (!conn) return; - conn->close (conn); + conn->close (); if (listen_stdin) { int fd; @@ -350,9 +353,9 @@ free_connection (struct connection *conn) * callback should always be called. */ if (!quit) { - lock_request (conn); - backend_close (backend, conn); - unlock_request (conn); + lock_request (); + backend_close (backend); + unlock_request (); } if (conn->status_pipe[0] >= 0) { @@ -375,9 +378,9 @@ free_connection (struct connection *conn) * that this send will be followed by related data. */ static int -raw_send_socket (struct connection *conn, const void *vbuf, size_t len, - int flags) +raw_send_socket (const void *vbuf, size_t len, int flags) { + struct connection *conn = GET_CONN; int sock = conn->sockout; const char *buf = vbuf; ssize_t r; @@ -405,9 +408,9 @@ raw_send_socket (struct connection *conn, const void *vbuf, size_t len, * (returns 0) or fail (returns -1). flags is ignored. */ static int -raw_send_other (struct connection *conn, const void *vbuf, size_t len, - int flags) +raw_send_other (const void *vbuf, size_t len, int flags) { + struct connection *conn = GET_CONN; int sock = conn->sockout; const char *buf = vbuf; ssize_t r; @@ -430,8 +433,9 @@ raw_send_other (struct connection *conn, const void *vbuf, size_t len, * (returns > 0), read an EOF (returns 0), or fail (returns -1). */ static int -raw_recv (struct connection *conn, void *vbuf, size_t len) +raw_recv (void *vbuf, size_t len) { + struct connection *conn = GET_CONN; int sock = conn->sockin; char *buf = vbuf; ssize_t r; @@ -463,8 +467,10 @@ raw_recv (struct connection *conn, void *vbuf, size_t len) * close, so this function ignores errors. */ static void -raw_close (struct connection *conn) +raw_close (void) { + struct connection *conn = GET_CONN; + if (conn->sockin >= 0) close (conn->sockin); if (conn->sockout >= 0 && conn->sockin != conn->sockout) diff --git a/server/crypto.c b/server/crypto.c index 9cd1bb08..5ba3930e 100644 --- a/server/crypto.c +++ b/server/crypto.c @@ -312,8 +312,9 @@ crypto_free (void) * (returns > 0), read an EOF (returns 0), or fail (returns -1). */ static int -crypto_recv (struct connection *conn, void *vbuf, size_t len) +crypto_recv (void *vbuf, size_t len) { + struct connection *conn = GET_CONN; gnutls_session_t session = conn->crypto_session; char *buf = vbuf; ssize_t r; @@ -355,8 +356,9 @@ crypto_recv (struct connection *conn, void *vbuf, size_t len) * (returns 0) or fail (returns -1). flags is ignored for now. */ static int -crypto_send (struct connection *conn, const void *vbuf, size_t len, int flags) +crypto_send (const void *vbuf, size_t len, int flags) { + struct connection *conn = GET_CONN; gnutls_session_t session = conn->crypto_session; const char *buf = vbuf; ssize_t r; @@ -392,8 +394,9 @@ crypto_send (struct connection *conn, const void *vbuf, size_t len, int flags) * close, so this function ignores errors. */ static void -crypto_close (struct connection *conn) +crypto_close (void) { + struct connection *conn = GET_CONN; gnutls_session_t session = conn->crypto_session; int sockin, sockout; @@ -417,8 +420,9 @@ crypto_close (struct connection *conn) * only be called once per connection. */ int -crypto_negotiate_tls (struct connection *conn, int sockin, int sockout) +crypto_negotiate_tls (int sockin, int sockout) { + struct connection *conn = GET_CONN; gnutls_session_t session; CLEANUP_FREE char *priority = NULL; int err; @@ -559,7 +563,7 @@ crypto_free (void) } int -crypto_negotiate_tls (struct connection *conn, int sockin, int sockout) +crypto_negotiate_tls (int sockin, int sockout) { /* Should never be called because tls == 0. */ abort (); diff --git a/server/filters.c b/server/filters.c index 2f65818e..c916217c 100644 --- a/server/filters.c +++ b/server/filters.c @@ -49,13 +49,13 @@ struct backend_filter { struct nbdkit_filter filter; }; -/* Literally a backend, a connection pointer, and the filter's handle. +/* Literally a backend and the filter's handle. + * * This is the implementation of our handle in .open, and serves as * a stable ‘void *nxdata’ in the filter API. */ -struct b_conn { +struct b_h { struct backend *b; - struct connection *conn; void *handle; }; @@ -186,22 +186,22 @@ filter_config_complete (struct backend *b) static int next_preconnect (void *nxdata, int readonly) { - struct b_conn *b_conn = nxdata; - return b_conn->b->preconnect (b_conn->b, b_conn->conn, readonly); + struct b_h *b_h = nxdata; + return b_h->b->preconnect (b_h->b, readonly); } static int -filter_preconnect (struct backend *b, struct connection *conn, int readonly) +filter_preconnect (struct backend *b, int readonly) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn nxdata = { .b = b->next, .conn = conn }; + struct b_h nxdata = { .b = b->next }; debug ("%s: preconnect", b->name); if (f->filter.preconnect) return f->filter.preconnect (next_preconnect, &nxdata, readonly); else - return b->next->preconnect (b->next, conn, readonly); + return b->next->preconnect (b->next, readonly); } /* magic_config_key only applies to plugins, so this passes the @@ -216,16 +216,16 @@ plugin_magic_config_key (struct backend *b) static int next_open (void *nxdata, int readonly) { - struct b_conn *b_conn = nxdata; + struct b_h *b_h = nxdata; - return backend_open (b_conn->b, b_conn->conn, readonly); + return backend_open (b_h->b, readonly); } static void * -filter_open (struct backend *b, struct connection *conn, int readonly) +filter_open (struct backend *b, int readonly) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = malloc (sizeof *nxdata); + struct b_h *nxdata = malloc (sizeof *nxdata); if (!nxdata) { nbdkit_error ("malloc: %m"); @@ -233,7 +233,6 @@ filter_open (struct backend *b, struct connection *conn, int readonly) } nxdata->b = b->next; - nxdata->conn = conn; nxdata->handle = NULL; /* Most filters will call next_open first, resulting in @@ -241,7 +240,7 @@ filter_open (struct backend *b, struct connection *conn, int readonly) */ if (f->filter.open) nxdata->handle = f->filter.open (next_open, nxdata, readonly); - else if (backend_open (b->next, conn, readonly) == -1) + else if (backend_open (b->next, readonly) == -1) nxdata->handle = NULL; else nxdata->handle = NBDKIT_HANDLE_NOT_NEEDED; @@ -253,13 +252,13 @@ filter_open (struct backend *b, struct connection *conn, int readonly) } static void -filter_close (struct backend *b, struct connection *conn, void *handle) +filter_close (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; if (handle && f->filter.close) { - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); f->filter.close (nxdata->handle); free (nxdata); } @@ -267,101 +266,101 @@ filter_close (struct backend *b, struct connection *conn, void *handle) /* The next_functions structure contains pointers to backend * functions. However because these functions are all expecting a - * backend and a connection, we cannot call them directly, but must + * backend and a handle, we cannot call them directly, but must * write some next_* functions that unpack the two parameters from a - * single ‘void *nxdata’ struct pointer (‘b_conn’). + * single ‘void *nxdata’ struct pointer (‘b_h’). */ static int next_reopen (void *nxdata, int readonly) { - struct b_conn *b_conn = nxdata; - return backend_reopen (b_conn->b, b_conn->conn, readonly); + struct b_h *b_h = nxdata; + return backend_reopen (b_h->b, readonly); } static int64_t next_get_size (void *nxdata) { - struct b_conn *b_conn = nxdata; - return backend_get_size (b_conn->b, b_conn->conn); + struct b_h *b_h = nxdata; + return backend_get_size (b_h->b); } static int next_can_write (void *nxdata) { - struct b_conn *b_conn = nxdata; - return backend_can_write (b_conn->b, b_conn->conn); + struct b_h *b_h = nxdata; + return backend_can_write (b_h->b); } static int next_can_flush (void *nxdata) { - struct b_conn *b_conn = nxdata; - return backend_can_flush (b_conn->b, b_conn->conn); + struct b_h *b_h = nxdata; + return backend_can_flush (b_h->b); } static int next_is_rotational (void *nxdata) { - struct b_conn *b_conn = nxdata; - return backend_is_rotational (b_conn->b, b_conn->conn); + struct b_h *b_h = nxdata; + return backend_is_rotational (b_h->b); } static int next_can_trim (void *nxdata) { - struct b_conn *b_conn = nxdata; - return backend_can_trim (b_conn->b, b_conn->conn); + struct b_h *b_h = nxdata; + return backend_can_trim (b_h->b); } static int next_can_zero (void *nxdata) { - struct b_conn *b_conn = nxdata; - return backend_can_zero (b_conn->b, b_conn->conn); + struct b_h *b_h = nxdata; + return backend_can_zero (b_h->b); } static int next_can_fast_zero (void *nxdata) { - struct b_conn *b_conn = nxdata; - return backend_can_fast_zero (b_conn->b, b_conn->conn); + struct b_h *b_h = nxdata; + return backend_can_fast_zero (b_h->b); } static int next_can_extents (void *nxdata) { - struct b_conn *b_conn = nxdata; - return backend_can_extents (b_conn->b, b_conn->conn); + struct b_h *b_h = nxdata; + return backend_can_extents (b_h->b); } static int next_can_fua (void *nxdata) { - struct b_conn *b_conn = nxdata; - return backend_can_fua (b_conn->b, b_conn->conn); + struct b_h *b_h = nxdata; + return backend_can_fua (b_h->b); } static int next_can_multi_conn (void *nxdata) { - struct b_conn *b_conn = nxdata; - return backend_can_multi_conn (b_conn->b, b_conn->conn); + struct b_h *b_h = nxdata; + return backend_can_multi_conn (b_h->b); } static int next_can_cache (void *nxdata) { - struct b_conn *b_conn = nxdata; - return backend_can_cache (b_conn->b, b_conn->conn); + struct b_h *b_h = nxdata; + return backend_can_cache (b_h->b); } static int next_pread (void *nxdata, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) { - struct b_conn *b_conn = nxdata; - return backend_pread (b_conn->b, b_conn->conn, buf, count, offset, flags, + struct b_h *b_h = nxdata; + return backend_pread (b_h->b, buf, count, offset, flags, err); } @@ -369,40 +368,40 @@ static int next_pwrite (void *nxdata, const void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) { - struct b_conn *b_conn = nxdata; - return backend_pwrite (b_conn->b, b_conn->conn, buf, count, offset, flags, + struct b_h *b_h = nxdata; + return backend_pwrite (b_h->b, buf, count, offset, flags, err); } static int next_flush (void *nxdata, uint32_t flags, int *err) { - struct b_conn *b_conn = nxdata; - return backend_flush (b_conn->b, b_conn->conn, flags, err); + struct b_h *b_h = nxdata; + return backend_flush (b_h->b, flags, err); } static int next_trim (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, int *err) { - struct b_conn *b_conn = nxdata; - return backend_trim (b_conn->b, b_conn->conn, count, offset, flags, err); + struct b_h *b_h = nxdata; + return backend_trim (b_h->b, count, offset, flags, err); } static int next_zero (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, int *err) { - struct b_conn *b_conn = nxdata; - return backend_zero (b_conn->b, b_conn->conn, count, offset, flags, err); + struct b_h *b_h = nxdata; + return backend_zero (b_h->b, count, offset, flags, err); } static int next_extents (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents *extents, int *err) { - struct b_conn *b_conn = nxdata; - return backend_extents (b_conn->b, b_conn->conn, count, offset, flags, + struct b_h *b_h = nxdata; + return backend_extents (b_h->b, count, offset, flags, extents, err); } @@ -410,8 +409,8 @@ static int next_cache (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, int *err) { - struct b_conn *b_conn = nxdata; - return backend_cache (b_conn->b, b_conn->conn, count, offset, flags, err); + struct b_h *b_h = nxdata; + return backend_cache (b_h->b, count, offset, flags, err); } static struct nbdkit_next_ops next_ops = { @@ -437,13 +436,12 @@ static struct nbdkit_next_ops next_ops = { }; static int -filter_prepare (struct backend *b, struct connection *conn, void *handle, - int readonly) +filter_prepare (struct backend *b, void *handle, int readonly) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.prepare && f->filter.prepare (&next_ops, nxdata, nxdata->handle, readonly) == -1) return -1; @@ -452,12 +450,12 @@ filter_prepare (struct backend *b, struct connection *conn, void *handle, } static int -filter_finalize (struct backend *b, struct connection *conn, void *handle) +filter_finalize (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.finalize && f->filter.finalize (&next_ops, nxdata, nxdata->handle) == -1) return -1; @@ -465,257 +463,257 @@ filter_finalize (struct backend *b, struct connection *conn, void *handle) } static int64_t -filter_get_size (struct backend *b, struct connection *conn, void *handle) +filter_get_size (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.get_size) return f->filter.get_size (&next_ops, nxdata, nxdata->handle); else - return backend_get_size (b->next, conn); + return backend_get_size (b->next); } static int -filter_can_write (struct backend *b, struct connection *conn, void *handle) +filter_can_write (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.can_write) return f->filter.can_write (&next_ops, nxdata, nxdata->handle); else - return backend_can_write (b->next, conn); + return backend_can_write (b->next); } static int -filter_can_flush (struct backend *b, struct connection *conn, void *handle) +filter_can_flush (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.can_flush) return f->filter.can_flush (&next_ops, nxdata, nxdata->handle); else - return backend_can_flush (b->next, conn); + return backend_can_flush (b->next); } static int -filter_is_rotational (struct backend *b, struct connection *conn, void *handle) +filter_is_rotational (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.is_rotational) return f->filter.is_rotational (&next_ops, nxdata, nxdata->handle); else - return backend_is_rotational (b->next, conn); + return backend_is_rotational (b->next); } static int -filter_can_trim (struct backend *b, struct connection *conn, void *handle) +filter_can_trim (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.can_trim) return f->filter.can_trim (&next_ops, nxdata, nxdata->handle); else - return backend_can_trim (b->next, conn); + return backend_can_trim (b->next); } static int -filter_can_zero (struct backend *b, struct connection *conn, void *handle) +filter_can_zero (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.can_zero) return f->filter.can_zero (&next_ops, nxdata, nxdata->handle); else - return backend_can_zero (b->next, conn); + return backend_can_zero (b->next); } static int -filter_can_fast_zero (struct backend *b, struct connection *conn, void *handle) +filter_can_fast_zero (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.can_fast_zero) return f->filter.can_fast_zero (&next_ops, nxdata, nxdata->handle); else - return backend_can_fast_zero (b->next, conn); + return backend_can_fast_zero (b->next); } static int -filter_can_extents (struct backend *b, struct connection *conn, void *handle) +filter_can_extents (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.can_extents) return f->filter.can_extents (&next_ops, nxdata, nxdata->handle); else - return backend_can_extents (b->next, conn); + return backend_can_extents (b->next); } static int -filter_can_fua (struct backend *b, struct connection *conn, void *handle) +filter_can_fua (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.can_fua) return f->filter.can_fua (&next_ops, nxdata, nxdata->handle); else - return backend_can_fua (b->next, conn); + return backend_can_fua (b->next); } static int -filter_can_multi_conn (struct backend *b, struct connection *conn, void *handle) +filter_can_multi_conn (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.can_multi_conn) return f->filter.can_multi_conn (&next_ops, nxdata, nxdata->handle); else - return backend_can_multi_conn (b->next, conn); + return backend_can_multi_conn (b->next); } static int -filter_can_cache (struct backend *b, struct connection *conn, void *handle) +filter_can_cache (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.can_cache) return f->filter.can_cache (&next_ops, nxdata, nxdata->handle); else - return backend_can_cache (b->next, conn); + return backend_can_cache (b->next); } static int -filter_pread (struct backend *b, struct connection *conn, void *handle, +filter_pread (struct backend *b, void *handle, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.pread) return f->filter.pread (&next_ops, nxdata, nxdata->handle, buf, count, offset, flags, err); else - return backend_pread (b->next, conn, buf, count, offset, flags, err); + return backend_pread (b->next, buf, count, offset, flags, err); } static int -filter_pwrite (struct backend *b, struct connection *conn, void *handle, +filter_pwrite (struct backend *b, void *handle, const void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.pwrite) return f->filter.pwrite (&next_ops, nxdata, nxdata->handle, buf, count, offset, flags, err); else - return backend_pwrite (b->next, conn, buf, count, offset, flags, err); + return backend_pwrite (b->next, buf, count, offset, flags, err); } static int -filter_flush (struct backend *b, struct connection *conn, void *handle, +filter_flush (struct backend *b, void *handle, uint32_t flags, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.flush) return f->filter.flush (&next_ops, nxdata, nxdata->handle, flags, err); else - return backend_flush (b->next, conn, flags, err); + return backend_flush (b->next, flags, err); } static int -filter_trim (struct backend *b, struct connection *conn, void *handle, +filter_trim (struct backend *b, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.trim) return f->filter.trim (&next_ops, nxdata, nxdata->handle, count, offset, flags, err); else - return backend_trim (b->next, conn, count, offset, flags, err); + return backend_trim (b->next, count, offset, flags, err); } static int -filter_zero (struct backend *b, struct connection *conn, void *handle, +filter_zero (struct backend *b, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.zero) return f->filter.zero (&next_ops, nxdata, nxdata->handle, count, offset, flags, err); else - return backend_zero (b->next, conn, count, offset, flags, err); + return backend_zero (b->next, count, offset, flags, err); } static int -filter_extents (struct backend *b, struct connection *conn, void *handle, +filter_extents (struct backend *b, void *handle, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents *extents, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.extents) return f->filter.extents (&next_ops, nxdata, nxdata->handle, count, offset, flags, extents, err); else - return backend_extents (b->next, conn, count, offset, flags, + return backend_extents (b->next, count, offset, flags, extents, err); } static int -filter_cache (struct backend *b, struct connection *conn, void *handle, +filter_cache (struct backend *b, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle; - assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.cache) return f->filter.cache (&next_ops, nxdata, nxdata->handle, count, offset, flags, err); else - return backend_cache (b->next, conn, count, offset, flags, err); + return backend_cache (b->next, count, offset, flags, err); } static struct backend filter_functions = { diff --git a/server/locks.c b/server/locks.c index ef6726d8..d187b422 100644 --- a/server/locks.c +++ b/server/locks.c @@ -91,8 +91,12 @@ unlock_connection (void) } void -lock_request (struct connection *conn) +lock_request (void) { + struct connection *conn = GET_CONN; + + assert (conn != NULL); + assert (thread_model >= 0); if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS && pthread_mutex_lock (&all_requests_lock)) @@ -107,8 +111,12 @@ lock_request (struct connection *conn) } void -unlock_request (struct connection *conn) +unlock_request () { + struct connection *conn = GET_CONN; + + assert (conn != NULL); + if (pthread_rwlock_unlock (&unload_prevention_lock)) abort (); diff --git a/server/plugins.c b/server/plugins.c index 79d98b8c..9595269c 100644 --- a/server/plugins.c +++ b/server/plugins.c @@ -235,7 +235,7 @@ plugin_magic_config_key (struct backend *b) } static int -plugin_preconnect (struct backend *b, struct connection *conn, int readonly) +plugin_preconnect (struct backend *b, int readonly) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); @@ -248,7 +248,7 @@ plugin_preconnect (struct backend *b, struct connection *conn, int readonly) } static void * -plugin_open (struct backend *b, struct connection *conn, int readonly) +plugin_open (struct backend *b, int readonly) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); @@ -262,20 +262,20 @@ plugin_open (struct backend *b, struct connection *conn, int readonly) * .close. */ static int -plugin_prepare (struct backend *b, struct connection *conn, void *handle, +plugin_prepare (struct backend *b, void *handle, int readonly) { return 0; } static int -plugin_finalize (struct backend *b, struct connection *conn, void *handle) +plugin_finalize (struct backend *b, void *handle) { return 0; } static void -plugin_close (struct backend *b, struct connection *conn, void *handle) +plugin_close (struct backend *b, void *handle) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); @@ -284,7 +284,7 @@ plugin_close (struct backend *b, struct connection *conn, void *handle) } static int64_t -plugin_get_size (struct backend *b, struct connection *conn, void *handle) +plugin_get_size (struct backend *b, void *handle) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); @@ -294,7 +294,7 @@ plugin_get_size (struct backend *b, struct connection *conn, void *handle) } static int -plugin_can_write (struct backend *b, struct connection *conn, void *handle) +plugin_can_write (struct backend *b, void *handle) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); @@ -305,7 +305,7 @@ plugin_can_write (struct backend *b, struct connection *conn, void *handle) } static int -plugin_can_flush (struct backend *b, struct connection *conn, void *handle) +plugin_can_flush (struct backend *b, void *handle) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); @@ -316,7 +316,7 @@ plugin_can_flush (struct backend *b, struct connection *conn, void *handle) } static int -plugin_is_rotational (struct backend *b, struct connection *conn, void *handle) +plugin_is_rotational (struct backend *b, void *handle) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); @@ -327,7 +327,7 @@ plugin_is_rotational (struct backend *b, struct connection *conn, void *handle) } static int -plugin_can_trim (struct backend *b, struct connection *conn, void *handle) +plugin_can_trim (struct backend *b, void *handle) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); @@ -338,7 +338,7 @@ plugin_can_trim (struct backend *b, struct connection *conn, void *handle) } static int -plugin_can_zero (struct backend *b, struct connection *conn, void *handle) +plugin_can_zero (struct backend *b, void *handle) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); int r; @@ -359,7 +359,7 @@ plugin_can_zero (struct backend *b, struct connection *conn, void *handle) } static int -plugin_can_fast_zero (struct backend *b, struct connection *conn, void *handle) +plugin_can_fast_zero (struct backend *b, void *handle) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); int r; @@ -372,14 +372,14 @@ plugin_can_fast_zero (struct backend *b, struct connection *conn, void *handle) */ if (p->plugin.zero == NULL) return 1; - r = backend_can_zero (b, conn); + r = backend_can_zero (b); if (r == -1) return -1; return !r; } static int -plugin_can_extents (struct backend *b, struct connection *conn, void *handle) +plugin_can_extents (struct backend *b, void *handle) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); @@ -390,7 +390,7 @@ plugin_can_extents (struct backend *b, struct connection *conn, void *handle) } static int -plugin_can_fua (struct backend *b, struct connection *conn, void *handle) +plugin_can_fua (struct backend *b, void *handle) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); int r; @@ -410,7 +410,7 @@ plugin_can_fua (struct backend *b, struct connection *conn, void *handle) } static int -plugin_can_multi_conn (struct backend *b, struct connection *conn, void *handle) +plugin_can_multi_conn (struct backend *b, void *handle) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); @@ -421,7 +421,7 @@ plugin_can_multi_conn (struct backend *b, struct connection *conn, void *handle) } static int -plugin_can_cache (struct backend *b, struct connection *conn, void *handle) +plugin_can_cache (struct backend *b, void *handle) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); @@ -454,7 +454,7 @@ get_error (struct backend_plugin *p) } static int -plugin_pread (struct backend *b, struct connection *conn, void *handle, +plugin_pread (struct backend *b, void *handle, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) { @@ -473,7 +473,7 @@ plugin_pread (struct backend *b, struct connection *conn, void *handle, } static int -plugin_flush (struct backend *b, struct connection *conn, void *handle, +plugin_flush (struct backend *b, void *handle, uint32_t flags, int *err) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); @@ -493,7 +493,7 @@ plugin_flush (struct backend *b, struct connection *conn, void *handle, } static int -plugin_pwrite (struct backend *b, struct connection *conn, void *handle, +plugin_pwrite (struct backend *b, void *handle, const void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) { @@ -502,7 +502,7 @@ plugin_pwrite (struct backend *b, struct connection *conn, void *handle, bool fua = flags & NBDKIT_FLAG_FUA; bool need_flush = false; - if (fua && backend_can_fua (b, conn) != NBDKIT_FUA_NATIVE) { + if (fua && backend_can_fua (b) != NBDKIT_FUA_NATIVE) { flags &= ~NBDKIT_FLAG_FUA; need_flush = true; } @@ -515,14 +515,14 @@ plugin_pwrite (struct backend *b, struct connection *conn, void *handle, return -1; } if (r != -1 && need_flush) - r = plugin_flush (b, conn, handle, 0, err); + r = plugin_flush (b, handle, 0, err); if (r == -1 && !*err) *err = get_error (p); return r; } static int -plugin_trim (struct backend *b, struct connection *conn, void *handle, +plugin_trim (struct backend *b, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err) { int r; @@ -530,7 +530,7 @@ plugin_trim (struct backend *b, struct connection *conn, void *handle, bool fua = flags & NBDKIT_FLAG_FUA; bool need_flush = false; - if (fua && backend_can_fua (b, conn) != NBDKIT_FUA_NATIVE) { + if (fua && backend_can_fua (b) != NBDKIT_FUA_NATIVE) { flags &= ~NBDKIT_FLAG_FUA; need_flush = true; } @@ -543,14 +543,14 @@ plugin_trim (struct backend *b, struct connection *conn, void *handle, return -1; } if (r != -1 && need_flush) - r = plugin_flush (b, conn, handle, 0, err); + r = plugin_flush (b, handle, 0, err); if (r == -1 && !*err) *err = get_error (p); return r; } static int -plugin_zero (struct backend *b, struct connection *conn, void *handle, +plugin_zero (struct backend *b, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); @@ -561,14 +561,14 @@ plugin_zero (struct backend *b, struct connection *conn, void *handle, bool emulate = false; bool need_flush = false; - if (fua && backend_can_fua (b, conn) != NBDKIT_FUA_NATIVE) { + if (fua && backend_can_fua (b) != NBDKIT_FUA_NATIVE) { flags &= ~NBDKIT_FLAG_FUA; need_flush = true; } if (!count) return 0; - if (backend_can_zero (b, conn) == NBDKIT_ZERO_NATIVE) { + if (backend_can_zero (b) == NBDKIT_ZERO_NATIVE) { errno = 0; if (p->plugin.zero) r = p->plugin.zero (handle, count, offset, flags); @@ -605,7 +605,7 @@ plugin_zero (struct backend *b, struct connection *conn, void *handle, static /* const */ char buf[MAX_REQUEST_SIZE]; uint32_t limit = MIN (count, sizeof buf); - r = plugin_pwrite (b, conn, handle, buf, limit, offset, flags, err); + r = plugin_pwrite (b, handle, buf, limit, offset, flags, err); if (r == -1) break; count -= limit; @@ -613,14 +613,14 @@ plugin_zero (struct backend *b, struct connection *conn, void *handle, done: if (r != -1 && need_flush) - r = plugin_flush (b, conn, handle, 0, err); + r = plugin_flush (b, handle, 0, err); if (r == -1 && !*err) *err = get_error (p); return r; } static int -plugin_extents (struct backend *b, struct connection *conn, void *handle, +plugin_extents (struct backend *b, void *handle, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents *extents, int *err) { @@ -642,7 +642,7 @@ plugin_extents (struct backend *b, struct connection *conn, void *handle, } static int -plugin_cache (struct backend *b, struct connection *conn, void *handle, +plugin_cache (struct backend *b, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err) { diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c index 7179186f..aa817be7 100644 --- a/server/protocol-handshake-newstyle.c +++ b/server/protocol-handshake-newstyle.c @@ -49,9 +49,9 @@ /* Receive newstyle options. */ static int -send_newstyle_option_reply (struct connection *conn, - uint32_t option, uint32_t reply) +send_newstyle_option_reply (uint32_t option, uint32_t reply) { + struct connection *conn = GET_CONN; struct nbd_fixed_new_option_reply fixed_new_option_reply; fixed_new_option_reply.magic = htobe64 (NBD_REP_MAGIC); @@ -59,8 +59,7 @@ send_newstyle_option_reply (struct connection *conn, fixed_new_option_reply.reply = htobe32 (reply); fixed_new_option_reply.replylen = htobe32 (0); - if (conn->send (conn, - &fixed_new_option_reply, + if (conn->send (&fixed_new_option_reply, sizeof fixed_new_option_reply, 0) == -1) { /* The protocol document says that the client is allowed to simply * drop the connection after sending NBD_OPT_ABORT, or may read @@ -77,9 +76,9 @@ send_newstyle_option_reply (struct connection *conn, } static int -send_newstyle_option_reply_exportname (struct connection *conn, - uint32_t option, uint32_t reply) +send_newstyle_option_reply_exportname (uint32_t option, uint32_t reply) { + struct connection *conn = GET_CONN; struct nbd_fixed_new_option_reply fixed_new_option_reply; size_t name_len = strlen (exportname); uint32_t len; @@ -89,20 +88,19 @@ send_newstyle_option_reply_exportname (struct connection *conn, fixed_new_option_reply.reply = htobe32 (reply); fixed_new_option_reply.replylen = htobe32 (name_len + sizeof (len)); - if (conn->send (conn, - &fixed_new_option_reply, + if (conn->send (&fixed_new_option_reply, sizeof fixed_new_option_reply, SEND_MORE) == -1) { nbdkit_error ("write: %s: %m", name_of_nbd_opt (option)); return -1; } len = htobe32 (name_len); - if (conn->send (conn, &len, sizeof len, SEND_MORE) == -1) { + if (conn->send (&len, sizeof len, SEND_MORE) == -1) { nbdkit_error ("write: %s: %s: %m", name_of_nbd_opt (option), "sending length"); return -1; } - if (conn->send (conn, exportname, name_len, 0) == -1) { + if (conn->send (exportname, name_len, 0) == -1) { nbdkit_error ("write: %s: %s: %m", name_of_nbd_opt (option), "sending export name"); return -1; @@ -112,10 +110,10 @@ send_newstyle_option_reply_exportname (struct connection *conn, } static int -send_newstyle_option_reply_info_export (struct connection *conn, - uint32_t option, uint32_t reply, +send_newstyle_option_reply_info_export (uint32_t option, uint32_t reply, uint16_t info, uint64_t exportsize) { + struct connection *conn = GET_CONN; struct nbd_fixed_new_option_reply fixed_new_option_reply; struct nbd_fixed_new_option_reply_info_export export; @@ -127,10 +125,9 @@ send_newstyle_option_reply_info_export (struct connection *conn, export.exportsize = htobe64 (exportsize); export.eflags = htobe16 (conn->eflags); - if (conn->send (conn, - &fixed_new_option_reply, + if (conn->send (&fixed_new_option_reply, sizeof fixed_new_option_reply, SEND_MORE) == -1 || - conn->send (conn, &export, sizeof export, 0) == -1) { + conn->send (&export, sizeof export, 0) == -1) { nbdkit_error ("write: %s: %m", name_of_nbd_opt (option)); return -1; } @@ -139,11 +136,11 @@ send_newstyle_option_reply_info_export (struct connection *conn, } static int -send_newstyle_option_reply_meta_context (struct connection *conn, - uint32_t option, uint32_t reply, +send_newstyle_option_reply_meta_context (uint32_t option, uint32_t reply, uint32_t context_id, const char *name) { + struct connection *conn = GET_CONN; struct nbd_fixed_new_option_reply fixed_new_option_reply; struct nbd_fixed_new_option_reply_meta_context context; const size_t namelen = strlen (name); @@ -156,11 +153,10 @@ send_newstyle_option_reply_meta_context (struct connection *conn, fixed_new_option_reply.replylen = htobe32 (sizeof context + namelen); context.context_id = htobe32 (context_id); - if (conn->send (conn, - &fixed_new_option_reply, + if (conn->send (&fixed_new_option_reply, sizeof fixed_new_option_reply, SEND_MORE) == -1 || - conn->send (conn, &context, sizeof context, SEND_MORE) == -1 || - conn->send (conn, name, namelen, 0) == -1) { + conn->send (&context, sizeof context, SEND_MORE) == -1 || + conn->send (name, namelen, 0) == -1) { nbdkit_error ("write: %s: %m", name_of_nbd_opt (option)); return -1; } @@ -171,11 +167,11 @@ send_newstyle_option_reply_meta_context (struct connection *conn, /* Sub-function during negotiate_handshake_newstyle, to uniformly handle * a client hanging up on a message boundary. */ -static int __attribute__ ((format (printf, 4, 5))) -conn_recv_full (struct connection *conn, void *buf, size_t len, - const char *fmt, ...) +static int __attribute__ ((format (printf, 3, 4))) +conn_recv_full (void *buf, size_t len, const char *fmt, ...) { - int r = conn->recv (conn, buf, len); + struct connection *conn = GET_CONN; + int r = conn->recv (buf, len); va_list args; if (r == -1) { @@ -198,9 +194,11 @@ conn_recv_full (struct connection *conn, void *buf, size_t len, * in that function, and must not cause any wire traffic. */ static int -finish_newstyle_options (struct connection *conn, uint64_t *exportsize) +finish_newstyle_options (uint64_t *exportsize) { - if (protocol_common_open (conn, exportsize, &conn->eflags) == -1) + struct connection *conn = GET_CONN; + + if (protocol_common_open (exportsize, &conn->eflags) == -1) return -1; debug ("newstyle negotiation: flags: export 0x%x", conn->eflags); @@ -233,9 +231,11 @@ check_string (uint32_t option, char *buf, uint32_t len, uint32_t maxlen, * validate an export name. */ static int -check_export_name (struct connection *conn, uint32_t option, char *buf, +check_export_name (uint32_t option, char *buf, uint32_t exportnamelen, uint32_t maxlen, bool save) { + struct connection *conn = GET_CONN; + if (check_string (option, buf, exportnamelen, maxlen, "export name") == -1) return -1; @@ -254,8 +254,9 @@ check_export_name (struct connection *conn, uint32_t option, char *buf, } static int -negotiate_handshake_newstyle_options (struct connection *conn) +negotiate_handshake_newstyle_options (void) { + struct connection *conn = GET_CONN; struct nbd_new_option new_option; size_t nr_options; uint64_t version; @@ -268,7 +269,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) for (nr_options = 0; nr_options < MAX_NR_OPTIONS; ++nr_options) { CLEANUP_FREE char *data = NULL; - if (conn_recv_full (conn, &new_option, sizeof new_option, + if (conn_recv_full (&new_option, sizeof new_option, "reading option: conn->recv: %m") == -1) return -1; @@ -302,7 +303,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) */ if (!(conn->cflags & NBD_FLAG_FIXED_NEWSTYLE) && option != NBD_OPT_EXPORT_NAME) { - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID)) + if (send_newstyle_option_reply (option, NBD_REP_ERR_INVALID)) return -1; continue; } @@ -312,31 +313,30 @@ negotiate_handshake_newstyle_options (struct connection *conn) */ if (tls == 2 && !conn->using_tls && !(option == NBD_OPT_ABORT || option == NBD_OPT_STARTTLS)) { - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_TLS_REQD)) + if (send_newstyle_option_reply (option, NBD_REP_ERR_TLS_REQD)) return -1; continue; } switch (option) { case NBD_OPT_EXPORT_NAME: - if (conn_recv_full (conn, data, optlen, + if (conn_recv_full (data, optlen, "read: %s: %m", name_of_nbd_opt (option)) == -1) return -1; - if (check_export_name (conn, option, data, optlen, optlen, true) == -1) + if (check_export_name (option, data, optlen, optlen, true) == -1) return -1; /* We have to finish the handshake by sending handshake_finish. * On failure, we have to disconnect. */ - if (finish_newstyle_options (conn, &exportsize) == -1) + if (finish_newstyle_options (&exportsize) == -1) return -1; memset (&handshake_finish, 0, sizeof handshake_finish); handshake_finish.exportsize = htobe64 (exportsize); handshake_finish.eflags = htobe16 (conn->eflags); - if (conn->send (conn, - &handshake_finish, + if (conn->send (&handshake_finish, (conn->cflags & NBD_FLAG_NO_ZEROES) ? offsetof (struct nbd_export_name_option_reply, zeroes) : sizeof handshake_finish, 0) == -1) { @@ -346,7 +346,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) break; case NBD_OPT_ABORT: - if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) + if (send_newstyle_option_reply (option, NBD_REP_ACK) == -1) return -1; debug ("client sent %s to abort the connection", name_of_nbd_opt (option)); @@ -354,10 +354,10 @@ negotiate_handshake_newstyle_options (struct connection *conn) case NBD_OPT_LIST: if (optlen != 0) { - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) + if (send_newstyle_option_reply (option, NBD_REP_ERR_INVALID) == -1) return -1; - if (conn_recv_full (conn, data, optlen, + if (conn_recv_full (data, optlen, "read: %s: %m", name_of_nbd_opt (option)) == -1) return -1; continue; @@ -366,20 +366,19 @@ negotiate_handshake_newstyle_options (struct connection *conn) /* Send back the exportname. */ debug ("newstyle negotiation: %s: advertising export '%s'", name_of_nbd_opt (option), exportname); - if (send_newstyle_option_reply_exportname (conn, option, - NBD_REP_SERVER) == -1) + if (send_newstyle_option_reply_exportname (option, NBD_REP_SERVER) == -1) return -1; - if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) + if (send_newstyle_option_reply (option, NBD_REP_ACK) == -1) return -1; break; case NBD_OPT_STARTTLS: if (optlen != 0) { - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) + if (send_newstyle_option_reply (option, NBD_REP_ERR_INVALID) == -1) return -1; - if (conn_recv_full (conn, data, optlen, + if (conn_recv_full (data, optlen, "read: %s: %m", name_of_nbd_opt (option)) == -1) return -1; continue; @@ -391,14 +390,13 @@ negotiate_handshake_newstyle_options (struct connection *conn) #else #define NO_TLS_REPLY NBD_REP_ERR_UNSUP #endif - if (send_newstyle_option_reply (conn, option, NO_TLS_REPLY) == -1) + if (send_newstyle_option_reply (option, NO_TLS_REPLY) == -1) return -1; } else /* --tls=on or --tls=require */ { /* We can't upgrade to TLS twice on the same connection. */ if (conn->using_tls) { - if (send_newstyle_option_reply (conn, option, - NBD_REP_ERR_INVALID) == -1) + if (send_newstyle_option_reply (option, NBD_REP_ERR_INVALID) == -1) return -1; continue; } @@ -406,11 +404,11 @@ negotiate_handshake_newstyle_options (struct connection *conn) /* We have to send the (unencrypted) reply before starting * the handshake. */ - if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) + if (send_newstyle_option_reply (option, NBD_REP_ACK) == -1) return -1; /* Upgrade the connection to TLS. Also performs access control. */ - if (crypto_negotiate_tls (conn, conn->sockin, conn->sockout) == -1) + if (crypto_negotiate_tls (conn->sockin, conn->sockout) == -1) return -1; conn->using_tls = true; debug ("using TLS on this connection"); @@ -419,14 +417,13 @@ negotiate_handshake_newstyle_options (struct connection *conn) case NBD_OPT_INFO: case NBD_OPT_GO: - if (conn_recv_full (conn, data, optlen, - "read: %s: %m", optname) == -1) + if (conn_recv_full (data, optlen, "read: %s: %m", optname) == -1) return -1; if (optlen < 6) { /* 32 bit export length + 16 bit nr info */ debug ("newstyle negotiation: %s option length < 6", optname); - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) + if (send_newstyle_option_reply (option, NBD_REP_ERR_INVALID) == -1) return -1; continue; @@ -443,7 +440,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) exportnamelen = be32toh (exportnamelen); if (exportnamelen > optlen-6 /* NB optlen >= 6, see above */) { debug ("newstyle negotiation: %s: export name too long", optname); - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) + if (send_newstyle_option_reply (option, NBD_REP_ERR_INVALID) == -1) return -1; continue; @@ -453,7 +450,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) if (optlen != 4 + exportnamelen + 2 + 2*nrinfos) { debug ("newstyle negotiation: %s: " "number of information requests incorrect", optname); - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) + if (send_newstyle_option_reply (option, NBD_REP_ERR_INVALID) == -1) return -1; continue; @@ -464,9 +461,9 @@ negotiate_handshake_newstyle_options (struct connection *conn) * NBD_OPT_SET_META_CONTEXT used an export name, it must match * or else we drop the support for that context. */ - if (check_export_name (conn, option, &data[4], exportnamelen, + if (check_export_name (option, &data[4], exportnamelen, optlen - 6, true) == -1) { - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) + if (send_newstyle_option_reply (option, NBD_REP_ERR_INVALID) == -1) return -1; continue; @@ -480,17 +477,16 @@ negotiate_handshake_newstyle_options (struct connection *conn) * client and let them try another NBD_OPT, rather than * disconnecting. */ - if (finish_newstyle_options (conn, &exportsize) == -1) { - if (backend_finalize (backend, conn) == -1) + if (finish_newstyle_options (&exportsize) == -1) { + if (backend_finalize (backend) == -1) return -1; - backend_close (backend, conn); - if (send_newstyle_option_reply (conn, option, - NBD_REP_ERR_UNKNOWN) == -1) + backend_close (backend); + if (send_newstyle_option_reply (option, NBD_REP_ERR_UNKNOWN) == -1) return -1; continue; } - if (send_newstyle_option_reply_info_export (conn, option, + if (send_newstyle_option_reply_info_export (option, NBD_REP_INFO, NBD_INFO_EXPORT, exportsize) == -1) @@ -518,23 +514,23 @@ negotiate_handshake_newstyle_options (struct connection *conn) /* Unlike NBD_OPT_EXPORT_NAME, NBD_OPT_GO sends back an ACK * or ERROR packet. If this was NBD_OPT_LIST, call .close. */ - if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) + if (send_newstyle_option_reply (option, NBD_REP_ACK) == -1) return -1; if (option == NBD_OPT_INFO) { - if (backend_finalize (backend, conn) == -1) + if (backend_finalize (backend) == -1) return -1; - backend_close (backend, conn); + backend_close (backend); } break; case NBD_OPT_STRUCTURED_REPLY: if (optlen != 0) { - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) + if (send_newstyle_option_reply (option, NBD_REP_ERR_INVALID) == -1) return -1; - if (conn_recv_full (conn, data, optlen, + if (conn_recv_full (data, optlen, "read: %s: %m", name_of_nbd_opt (option)) == -1) return -1; continue; @@ -547,14 +543,14 @@ negotiate_handshake_newstyle_options (struct connection *conn) /* Must fail with ERR_UNSUP for qemu 4.2 to remain happy; * but failing with ERR_POLICY would have been nicer. */ - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_UNSUP) == -1) + if (send_newstyle_option_reply (option, NBD_REP_ERR_UNSUP) == -1) return -1; debug ("newstyle negotiation: %s: structured replies are disabled", name_of_nbd_opt (option)); break; } - if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) + if (send_newstyle_option_reply (option, NBD_REP_ACK) == -1) return -1; conn->structured_replies = true; @@ -569,14 +565,14 @@ negotiate_handshake_newstyle_options (struct connection *conn) uint32_t querylen; const char *what; - if (conn_recv_full (conn, data, optlen, "read: %s: %m", optname) == -1) + if (conn_recv_full (data, optlen, "read: %s: %m", optname) == -1) return -1; /* Note that we support base:allocation whether or not the plugin * supports can_extents. */ if (!conn->structured_replies) { - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) + if (send_newstyle_option_reply (option, NBD_REP_ERR_INVALID) == -1) return -1; continue; @@ -593,7 +589,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) debug ("newstyle negotiation: %s: invalid option length: %s", optname, what); - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) + if (send_newstyle_option_reply (option, NBD_REP_ERR_INVALID) == -1) return -1; continue; @@ -606,7 +602,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) memcpy (&exportnamelen, &data[0], 4); exportnamelen = be32toh (exportnamelen); what = "validating export name"; - if (check_export_name (conn, option, &data[4], exportnamelen, + if (check_export_name (option, &data[4], exportnamelen, optlen - 8, option == NBD_OPT_SET_META_CONTEXT) == -1) goto opt_meta_invalid_option_len; @@ -630,13 +626,14 @@ negotiate_handshake_newstyle_options (struct connection *conn) conn->meta_context_base_allocation = false; if (nr_queries == 0) { if (option == NBD_OPT_LIST_META_CONTEXT) { - if (send_newstyle_option_reply_meta_context - (conn, option, NBD_REP_META_CONTEXT, - 0, "base:allocation") == -1) + if (send_newstyle_option_reply_meta_context (option, + NBD_REP_META_CONTEXT, + 0, "base:allocation") + == -1) return -1; } - if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) + if (send_newstyle_option_reply (option, NBD_REP_ACK) == -1) return -1; } else { @@ -665,7 +662,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) querylen == 5 && strncmp (&data[opt_index], "base:", 5) == 0) { if (send_newstyle_option_reply_meta_context - (conn, option, NBD_REP_META_CONTEXT, + (option, NBD_REP_META_CONTEXT, 0, "base:allocation") == -1) return -1; } @@ -673,7 +670,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) else if (querylen == 15 && strncmp (&data[opt_index], "base:allocation", 15) == 0) { if (send_newstyle_option_reply_meta_context - (conn, option, NBD_REP_META_CONTEXT, + (option, NBD_REP_META_CONTEXT, option == NBD_OPT_SET_META_CONTEXT ? base_allocation_id : 0, "base:allocation") == -1) @@ -686,7 +683,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) opt_index += querylen; nr_queries--; } - if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) + if (send_newstyle_option_reply (option, NBD_REP_ACK) == -1) return -1; } debug ("newstyle negotiation: %s: reply complete", optname); @@ -695,9 +692,9 @@ negotiate_handshake_newstyle_options (struct connection *conn) default: /* Unknown option. */ - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_UNSUP) == -1) + if (send_newstyle_option_reply (option, NBD_REP_ERR_UNSUP) == -1) return -1; - if (conn_recv_full (conn, data, optlen, + if (conn_recv_full (data, optlen, "reading unknown option data: conn->recv: %m") == -1) return -1; } @@ -728,8 +725,9 @@ negotiate_handshake_newstyle_options (struct connection *conn) } int -protocol_handshake_newstyle (struct connection *conn) +protocol_handshake_newstyle (void) { + struct connection *conn = GET_CONN; struct nbd_new_handshake handshake; uint16_t gflags; @@ -741,13 +739,13 @@ protocol_handshake_newstyle (struct connection *conn) handshake.version = htobe64 (NBD_NEW_VERSION); handshake.gflags = htobe16 (gflags); - if (conn->send (conn, &handshake, sizeof handshake, 0) == -1) { + if (conn->send (&handshake, sizeof handshake, 0) == -1) { nbdkit_error ("write: %s: %m", "sending newstyle handshake"); return -1; } /* Client now sends us its 32 bit flags word ... */ - if (conn_recv_full (conn, &conn->cflags, sizeof conn->cflags, + if (conn_recv_full (&conn->cflags, sizeof conn->cflags, "reading initial client flags: conn->recv: %m") == -1) return -1; conn->cflags = be32toh (conn->cflags); @@ -759,7 +757,7 @@ protocol_handshake_newstyle (struct connection *conn) } /* Receive newstyle options. */ - if (negotiate_handshake_newstyle_options (conn) == -1) + if (negotiate_handshake_newstyle_options () == -1) return -1; return 0; diff --git a/server/protocol-handshake-oldstyle.c b/server/protocol-handshake-oldstyle.c index 45a1a486..bba21cc4 100644 --- a/server/protocol-handshake-oldstyle.c +++ b/server/protocol-handshake-oldstyle.c @@ -44,8 +44,9 @@ #include "nbd-protocol.h" int -protocol_handshake_oldstyle (struct connection *conn) +protocol_handshake_oldstyle (void) { + struct connection *conn = GET_CONN; struct nbd_old_handshake handshake; uint64_t exportsize; uint16_t gflags, eflags; @@ -55,7 +56,7 @@ protocol_handshake_oldstyle (struct connection *conn) /* With oldstyle, our only option if .open or friends fail is to * disconnect, as we cannot report the problem to the client. */ - if (protocol_common_open (conn, &exportsize, &eflags) == -1) + if (protocol_common_open (&exportsize, &eflags) == -1) return -1; gflags = 0; @@ -69,7 +70,7 @@ protocol_handshake_oldstyle (struct connection *conn) handshake.gflags = htobe16 (gflags); handshake.eflags = htobe16 (eflags); - if (conn->send (conn, &handshake, sizeof handshake, 0) == -1) { + if (conn->send (&handshake, sizeof handshake, 0) == -1) { nbdkit_error ("write: %m"); return -1; } diff --git a/server/protocol-handshake.c b/server/protocol-handshake.c index 2c2f35ee..a208dc1d 100644 --- a/server/protocol-handshake.c +++ b/server/protocol-handshake.c @@ -44,16 +44,16 @@ #include "nbd-protocol.h" int -protocol_handshake (struct connection *conn) +protocol_handshake () { int r; - lock_request (conn); + lock_request (); if (!newstyle) - r = protocol_handshake_oldstyle (conn); + r = protocol_handshake_oldstyle (); else - r = protocol_handshake_newstyle (conn); - unlock_request (conn); + r = protocol_handshake_newstyle (); + unlock_request (); return r; } @@ -72,21 +72,21 @@ protocol_handshake (struct connection *conn) * simply opening a TCP connection. */ int -protocol_common_open (struct connection *conn, - uint64_t *exportsize, uint16_t *flags) +protocol_common_open (uint64_t *exportsize, uint16_t *flags) { + struct connection *conn = GET_CONN; int64_t size; uint16_t eflags = NBD_FLAG_HAS_FLAGS; int fl; - if (backend_open (backend, conn, read_only) == -1) + if (backend_open (backend, read_only) == -1) return -1; /* Prepare (for filters), called just after open. */ - if (backend_prepare (backend, conn) == -1) + if (backend_prepare (backend) == -1) return -1; - size = backend_get_size (backend, conn); + size = backend_get_size (backend); if (size == -1) return -1; if (size < 0) { @@ -98,57 +98,57 @@ protocol_common_open (struct connection *conn, /* Check all flags even if they won't be advertised, to prime the * cache and make later request validation easier. */ - fl = backend_can_write (backend, conn); + fl = backend_can_write (backend); if (fl == -1) return -1; if (!fl) eflags |= NBD_FLAG_READ_ONLY; - fl = backend_can_zero (backend, conn); + fl = backend_can_zero (backend); if (fl == -1) return -1; if (fl) eflags |= NBD_FLAG_SEND_WRITE_ZEROES; - fl = backend_can_fast_zero (backend, conn); + fl = backend_can_fast_zero (backend); if (fl == -1) return -1; if (fl) eflags |= NBD_FLAG_SEND_FAST_ZERO; - fl = backend_can_trim (backend, conn); + fl = backend_can_trim (backend); if (fl == -1) return -1; if (fl) eflags |= NBD_FLAG_SEND_TRIM; - fl = backend_can_fua (backend, conn); + fl = backend_can_fua (backend); if (fl == -1) return -1; if (fl) eflags |= NBD_FLAG_SEND_FUA; - fl = backend_can_flush (backend, conn); + fl = backend_can_flush (backend); if (fl == -1) return -1; if (fl) eflags |= NBD_FLAG_SEND_FLUSH; - fl = backend_is_rotational (backend, conn); + fl = backend_is_rotational (backend); if (fl == -1) return -1; if (fl) eflags |= NBD_FLAG_ROTATIONAL; /* multi-conn is useless if parallel connections are not allowed. */ - fl = backend_can_multi_conn (backend, conn); + fl = backend_can_multi_conn (backend); if (fl == -1) return -1; if (fl && (backend->thread_model (backend) > NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS)) eflags |= NBD_FLAG_CAN_MULTI_CONN; - fl = backend_can_cache (backend, conn); + fl = backend_can_cache (backend); if (fl == -1) return -1; if (fl) @@ -159,7 +159,7 @@ protocol_common_open (struct connection *conn, * not have to worry about errors, and makes test-layers easier to * write. */ - fl = backend_can_extents (backend, conn); + fl = backend_can_extents (backend); if (fl == -1) return -1; diff --git a/server/protocol.c b/server/protocol.c index f6ea35cb..dd06d1f6 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -49,10 +49,11 @@ #include "protostrings.h" static bool -validate_request (struct connection *conn, - uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count, +validate_request (uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count, uint32_t *error) { + struct connection *conn = GET_CONN; + /* Readonly connection? */ if (conn->eflags & NBD_FLAG_READ_ONLY && (cmd == NBD_CMD_WRITE || cmd == NBD_CMD_TRIM || @@ -71,7 +72,7 @@ validate_request (struct connection *conn, case NBD_CMD_TRIM: case NBD_CMD_WRITE_ZEROES: case NBD_CMD_BLOCK_STATUS: - if (!backend_valid_range (backend, conn, offset, count)) { + if (!backend_valid_range (backend, offset, count)) { /* XXX Allow writes to extend the disk? */ nbdkit_error ("invalid request: %s: offset and count are out of range: " "offset=%" PRIu64 " count=%" PRIu32, @@ -225,8 +226,7 @@ validate_request (struct connection *conn, * for success). */ static uint32_t -handle_request (struct connection *conn, - uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count, +handle_request (uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count, void *buf, struct nbdkit_extents *extents) { uint32_t f = 0; @@ -238,31 +238,31 @@ handle_request (struct connection *conn, switch (cmd) { case NBD_CMD_READ: - if (backend_pread (backend, conn, buf, count, offset, 0, &err) == -1) + if (backend_pread (backend, buf, count, offset, 0, &err) == -1) return err; break; case NBD_CMD_WRITE: if (flags & NBD_CMD_FLAG_FUA) f |= NBDKIT_FLAG_FUA; - if (backend_pwrite (backend, conn, buf, count, offset, f, &err) == -1) + if (backend_pwrite (backend, buf, count, offset, f, &err) == -1) return err; break; case NBD_CMD_FLUSH: - if (backend_flush (backend, conn, 0, &err) == -1) + if (backend_flush (backend, 0, &err) == -1) return err; break; case NBD_CMD_TRIM: if (flags & NBD_CMD_FLAG_FUA) f |= NBDKIT_FLAG_FUA; - if (backend_trim (backend, conn, count, offset, f, &err) == -1) + if (backend_trim (backend, count, offset, f, &err) == -1) return err; break; case NBD_CMD_CACHE: - if (backend_cache (backend, conn, count, offset, 0, &err) == -1) + if (backend_cache (backend, count, offset, 0, &err) == -1) return err; break; @@ -273,14 +273,14 @@ handle_request (struct connection *conn, f |= NBDKIT_FLAG_FUA; if (flags & NBD_CMD_FLAG_FAST_ZERO) f |= NBDKIT_FLAG_FAST_ZERO; - if (backend_zero (backend, conn, count, offset, f, &err) == -1) + if (backend_zero (backend, count, offset, f, &err) == -1) return err; break; case NBD_CMD_BLOCK_STATUS: if (flags & NBD_CMD_FLAG_REQ_ONE) f |= NBDKIT_FLAG_REQ_ONE; - if (backend_extents (backend, conn, count, offset, f, + if (backend_extents (backend, count, offset, f, extents, &err) == -1) return err; break; @@ -361,11 +361,11 @@ nbd_errno (int error, uint16_t flags) } static int -send_simple_reply (struct connection *conn, - uint64_t handle, uint16_t cmd, uint16_t flags, +send_simple_reply (uint64_t handle, uint16_t cmd, uint16_t flags, const char *buf, uint32_t count, uint32_t error) { + struct connection *conn = GET_CONN; ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); struct nbd_simple_reply reply; int r; @@ -375,18 +375,18 @@ send_simple_reply (struct connection *conn, reply.handle = handle; reply.error = htobe32 (nbd_errno (error, flags)); - r = conn->send (conn, &reply, sizeof reply, f); + r = conn->send (&reply, sizeof reply, f); if (r == -1) { nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd)); - return connection_set_status (conn, -1); + return connection_set_status (-1); } /* Send the read data buffer. */ if (cmd == NBD_CMD_READ && !error) { - r = conn->send (conn, buf, count, 0); + r = conn->send (buf, count, 0); if (r == -1) { nbdkit_error ("write data: %s: %m", name_of_nbd_cmd (cmd)); - return connection_set_status (conn, -1); + return connection_set_status (-1); } } @@ -394,10 +394,10 @@ send_simple_reply (struct connection *conn, } static int -send_structured_reply_read (struct connection *conn, - uint64_t handle, uint16_t cmd, +send_structured_reply_read (uint64_t handle, uint16_t cmd, const char *buf, uint32_t count, uint64_t offset) { + struct connection *conn = GET_CONN; /* Once we are really using structured replies and sending data back * in chunks, we'll be able to grab the write lock for each chunk, * allowing other threads to interleave replies. As we're not doing @@ -416,24 +416,24 @@ send_structured_reply_read (struct connection *conn, reply.type = htobe16 (NBD_REPLY_TYPE_OFFSET_DATA); reply.length = htobe32 (count + sizeof offset_data); - r = conn->send (conn, &reply, sizeof reply, SEND_MORE); + r = conn->send (&reply, sizeof reply, SEND_MORE); if (r == -1) { nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd)); - return connection_set_status (conn, -1); + return connection_set_status (-1); } /* Send the offset + read data buffer. */ offset_data.offset = htobe64 (offset); - r = conn->send (conn, &offset_data, sizeof offset_data, SEND_MORE); + r = conn->send (&offset_data, sizeof offset_data, SEND_MORE); if (r == -1) { nbdkit_error ("write data: %s: %m", name_of_nbd_cmd (cmd)); - return connection_set_status (conn, -1); + return connection_set_status (-1); } - r = conn->send (conn, buf, count, 0); + r = conn->send (buf, count, 0); if (r == -1) { nbdkit_error ("write data: %s: %m", name_of_nbd_cmd (cmd)); - return connection_set_status (conn, -1); + return connection_set_status (-1); } return 1; /* command processed ok */ @@ -522,12 +522,12 @@ extents_to_block_descriptors (struct nbdkit_extents *extents, } static int -send_structured_reply_block_status (struct connection *conn, - uint64_t handle, +send_structured_reply_block_status (uint64_t handle, uint16_t cmd, uint16_t flags, uint32_t count, uint64_t offset, struct nbdkit_extents *extents) { + struct connection *conn = GET_CONN; ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); struct nbd_structured_reply reply; CLEANUP_FREE struct nbd_block_descriptor *blocks = NULL; @@ -542,7 +542,7 @@ send_structured_reply_block_status (struct connection *conn, blocks = extents_to_block_descriptors (extents, flags, count, offset, &nr_blocks); if (blocks == NULL) - return connection_set_status (conn, -1); + return connection_set_status (-1); reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC); reply.handle = handle; @@ -551,27 +551,27 @@ send_structured_reply_block_status (struct connection *conn, reply.length = htobe32 (sizeof context_id + nr_blocks * sizeof (struct nbd_block_descriptor)); - r = conn->send (conn, &reply, sizeof reply, SEND_MORE); + r = conn->send (&reply, sizeof reply, SEND_MORE); if (r == -1) { nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd)); - return connection_set_status (conn, -1); + return connection_set_status (-1); } /* Send the base:allocation context ID. */ context_id = htobe32 (base_allocation_id); - r = conn->send (conn, &context_id, sizeof context_id, SEND_MORE); + r = conn->send (&context_id, sizeof context_id, SEND_MORE); if (r == -1) { nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd)); - return connection_set_status (conn, -1); + return connection_set_status (-1); } /* Send each block descriptor. */ for (i = 0; i < nr_blocks; ++i) { - r = conn->send (conn, &blocks[i], sizeof blocks[i], + r = conn->send (&blocks[i], sizeof blocks[i], i == nr_blocks - 1 ? 0 : SEND_MORE); if (r == -1) { nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd)); - return connection_set_status (conn, -1); + return connection_set_status (-1); } } @@ -579,10 +579,10 @@ send_structured_reply_block_status (struct connection *conn, } static int -send_structured_reply_error (struct connection *conn, - uint64_t handle, uint16_t cmd, uint16_t flags, +send_structured_reply_error (uint64_t handle, uint16_t cmd, uint16_t flags, uint32_t error) { + struct connection *conn = GET_CONN; ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); struct nbd_structured_reply reply; struct nbd_structured_reply_error error_data; @@ -594,19 +594,19 @@ send_structured_reply_error (struct connection *conn, reply.type = htobe16 (NBD_REPLY_TYPE_ERROR); reply.length = htobe32 (0 /* no human readable error */ + sizeof error_data); - r = conn->send (conn, &reply, sizeof reply, SEND_MORE); + r = conn->send (&reply, sizeof reply, SEND_MORE); if (r == -1) { nbdkit_error ("write error reply: %m"); - return connection_set_status (conn, -1); + return connection_set_status (-1); } /* Send the error. */ error_data.error = htobe32 (nbd_errno (error, flags)); error_data.len = htobe16 (0); - r = conn->send (conn, &error_data, sizeof error_data, 0); + r = conn->send (&error_data, sizeof error_data, 0); if (r == -1) { nbdkit_error ("write data: %s: %m", name_of_nbd_cmd (cmd)); - return connection_set_status (conn, -1); + return connection_set_status (-1); } /* No human readable error message at the moment. */ @@ -614,8 +614,9 @@ send_structured_reply_error (struct connection *conn, } int -protocol_recv_request_send_reply (struct connection *conn) +protocol_recv_request_send_reply (void) { + struct connection *conn = GET_CONN; int r; struct nbd_request request; uint16_t cmd, flags; @@ -627,24 +628,24 @@ protocol_recv_request_send_reply (struct connection *conn) /* Read the request packet. */ { ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->read_lock); - r = connection_get_status (conn); + r = connection_get_status (); if (r <= 0) return r; - r = conn->recv (conn, &request, sizeof request); + r = conn->recv (&request, sizeof request); if (r == -1) { nbdkit_error ("read request: %m"); - return connection_set_status (conn, -1); + return connection_set_status (-1); } if (r == 0) { debug ("client closed input socket, closing connection"); - return connection_set_status (conn, 0); /* disconnect */ + return connection_set_status (0); /* disconnect */ } magic = be32toh (request.magic); if (magic != NBD_REQUEST_MAGIC) { nbdkit_error ("invalid request: 'magic' field is incorrect (0x%x)", magic); - return connection_set_status (conn, -1); + return connection_set_status (-1); } flags = be16toh (request.flags); @@ -655,14 +656,14 @@ protocol_recv_request_send_reply (struct connection *conn) if (cmd == NBD_CMD_DISC) { debug ("client sent %s, closing connection", name_of_nbd_cmd (cmd)); - return connection_set_status (conn, 0); /* disconnect */ + return connection_set_status (0); /* disconnect */ } /* Validate the request. */ - if (!validate_request (conn, cmd, flags, offset, count, &error)) { + if (!validate_request (cmd, flags, offset, count, &error)) { if (cmd == NBD_CMD_WRITE && skip_over_write_buffer (conn->sockin, count) < 0) - return connection_set_status (conn, -1); + return connection_set_status (-1); goto send_reply; } @@ -675,14 +676,14 @@ protocol_recv_request_send_reply (struct connection *conn) error = ENOMEM; if (cmd == NBD_CMD_WRITE && skip_over_write_buffer (conn->sockin, count) < 0) - return connection_set_status (conn, -1); + return connection_set_status (-1); goto send_reply; } } /* Allocate the extents list for block status only. */ if (cmd == NBD_CMD_BLOCK_STATUS) { - extents = nbdkit_extents_new (offset, backend_get_size (backend, conn)); + extents = nbdkit_extents_new (offset, backend_get_size (backend)); if (extents == NULL) { error = ENOMEM; goto send_reply; @@ -691,32 +692,32 @@ protocol_recv_request_send_reply (struct connection *conn) /* Receive the write data buffer. */ if (cmd == NBD_CMD_WRITE) { - r = conn->recv (conn, buf, count); + r = conn->recv (buf, count); if (r == 0) { errno = EBADMSG; r = -1; } if (r == -1) { nbdkit_error ("read data: %s: %m", name_of_nbd_cmd (cmd)); - return connection_set_status (conn, -1); + return connection_set_status (-1); } } } /* Perform the request. Only this part happens inside the request lock. */ - if (quit || !connection_get_status (conn)) { + if (quit || !connection_get_status ()) { error = ESHUTDOWN; } else { - lock_request (conn); - error = handle_request (conn, cmd, flags, offset, count, buf, extents); + lock_request (); + error = handle_request (cmd, flags, offset, count, buf, extents); assert ((int) error >= 0); - unlock_request (conn); + unlock_request (); } /* Send the reply packet. */ send_reply: - if (connection_get_status (conn) < 0) + if (connection_get_status () < 0) return -1; if (error != 0) { @@ -738,19 +739,19 @@ protocol_recv_request_send_reply (struct connection *conn) (cmd == NBD_CMD_READ || cmd == NBD_CMD_BLOCK_STATUS)) { if (!error) { if (cmd == NBD_CMD_READ) - return send_structured_reply_read (conn, request.handle, cmd, + return send_structured_reply_read (request.handle, cmd, buf, count, offset); else /* NBD_CMD_BLOCK_STATUS */ - return send_structured_reply_block_status (conn, request.handle, + return send_structured_reply_block_status (request.handle, cmd, flags, count, offset, extents); } else - return send_structured_reply_error (conn, request.handle, cmd, flags, + return send_structured_reply_error (request.handle, cmd, flags, error); } else - return send_simple_reply (conn, request.handle, cmd, flags, buf, count, + return send_simple_reply (request.handle, cmd, flags, buf, count, error); } diff --git a/server/public.c b/server/public.c index 8fa7e21b..97de4a42 100644 --- a/server/public.c +++ b/server/public.c @@ -562,7 +562,7 @@ nbdkit_nanosleep (unsigned sec, unsigned nsec) * event, we know the connection should be shutting down. */ assert (quit || - (conn && conn->nworkers > 0 && connection_get_status (conn) < 1) || + (conn && conn->nworkers > 0 && connection_get_status () < 1) || (conn && (fds[2].revents & (POLLRDHUP | POLLHUP | POLLERR)))); nbdkit_error ("aborting sleep to shut down"); errno = ESHUTDOWN; diff --git a/server/test-public.c b/server/test-public.c index 4a7eb173..fe347d44 100644 --- a/server/test-public.c +++ b/server/test-public.c @@ -62,7 +62,7 @@ threadlocal_get_conn (void) abort (); } -int connection_get_status (struct connection *conn) +int connection_get_status (void) { abort (); } -- 2.25.0

Eric Blake

5:46 p.m.

New subject: [PATCH nbdkit 3/3] server: Remove explicit connection parameter, use TLS instead.

On 2/11/20 11:15 AM, Richard W.M. Jones wrote:

...

Since commit 86fdb48c6a5362d66865493d9d2172166f99722e we have stored the connection object in thread-local storage.

In this very large, but mostly mechanical change we stop passing the connection pointer around everywhere, and instead use the value stored in thread-local storage.

This assumes a 1-1 mapping between the connection and the current thread which is true in *most* places. Occasionally we still have the explicit connection pointer, especially just before we launch a thread or call threadlocal_set_conn. --- server/internal.h | 191 +++++++++---------- server/backend.c | 160 +++++++++------- server/connections.c | 68 ++++--- server/crypto.c | 14 +- server/filters.c | 270 +++++++++++++-------------- server/locks.c | 12 +- server/plugins.c | 64 +++---- server/protocol-handshake-newstyle.c | 172 +++++++++-------- server/protocol-handshake-oldstyle.c | 7 +- server/protocol-handshake.c | 40 ++-- server/protocol.c | 127 ++++++------- server/public.c | 2 +- server/test-public.c | 2 +- 13 files changed, 574 insertions(+), 555 deletions(-)

Makes sense to me (and not too hard to rebase my NBD_INFO_INIT_STATE stuff on top of it). Are we sure that there is not going to be a speed penalty (from frequent access to the thread-local storage, compared to previous access through a parameter stored in a register)? A few comments:

...

+++ b/server/filters.c @@ -49,13 +49,13 @@ struct backend_filter { struct nbdkit_filter filter; };

-/* Literally a backend, a connection pointer, and the filter's handle. +/* Literally a backend and the filter's handle. + * * This is the implementation of our handle in .open, and serves as * a stable ‘void *nxdata’ in the filter API. */ -struct b_conn { +struct b_h { struct backend *b; - struct connection *conn; void *handle; };

@@ -186,22 +186,22 @@ filter_config_complete (struct backend *b) static int next_preconnect (void *nxdata, int readonly) { - struct b_conn *b_conn = nxdata; - return b_conn->b->preconnect (b_conn->b, b_conn->conn, readonly); + struct b_h *b_h = nxdata; + return b_h->b->preconnect (b_h->b, readonly); }

None of the next_*() wrappers use b_h->handle, they all stick to b_h->b. I don't think that matters too much, other than...

...

@@ -267,101 +266,101 @@ filter_close (struct backend *b, struct connection *conn, void *handle)

/* The next_functions structure contains pointers to backend * functions. However because these functions are all expecting a - * backend and a connection, we cannot call them directly, but must + * backend and a handle, we cannot call them directly, but must * write some next_* functions that unpack the two parameters from a - * single ‘void *nxdata’ struct pointer (‘b_conn’). + * single ‘void *nxdata’ struct pointer (‘b_h’).

...this comment is slightly off. In fact, if ALL we use is b_h->b, we could probably populate next_ops with backend_* functions rather than next_* wrapper functions:

...

@@ -410,8 +409,8 @@ static int next_cache (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, int *err) { - struct b_conn *b_conn = nxdata; - return backend_cache (b_conn->b, b_conn->conn, count, offset, flags, err); + struct b_h *b_h = nxdata; + return backend_cache (b_h->b, count, offset, flags, err); }

static struct nbdkit_next_ops next_ops = {

as in next_ops = { ... .cache = backend_cache, };

...

static int -filter_cache (struct backend *b, struct connection *conn, void *handle, +filter_cache (struct backend *b, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - struct b_conn *nxdata = handle; + struct b_h *nxdata = handle;

- assert (nxdata->b == b->next && nxdata->conn == conn); + assert (nxdata->b == b->next); if (f->filter.cache) return f->filter.cache (&next_ops, nxdata, nxdata->handle, count, offset, flags, err); else - return backend_cache (b->next, conn, count, offset, flags, err); + return backend_cache (b->next, count, offset, flags, err);

and here, we could call: struct backend_filter *f = container_of (b, struct backend_filter, backend); if (f->filter.cache) return f->filter.cache (&next_ops, b->next, handle, count, offset, flags, err); else return backend_cache (b->next, count, offset, flags, err); and drop the malloc() wrapper around the handle altogether (test-layers.sh will confirm that we still have a stable pointer). That can be a separate patch; for the sake of _this_ patch, keeping things mechanical (with maybe a tweak to the comment) is fine.

...

}

static struct backend filter_functions = { diff --git a/server/locks.c b/server/locks.c index ef6726d8..d187b422 100644 --- a/server/locks.c +++ b/server/locks.c @@ -91,8 +91,12 @@ unlock_connection (void) }

void -lock_request (struct connection *conn) +lock_request (void) { + struct connection *conn = GET_CONN; + + assert (conn != NULL); +

Here's a site where we could exploit the macro guaranteeing a non-null result. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

Richard W.M. Jones

6:15 p.m.

New subject: [PATCH nbdkit 3/3] server: Remove explicit connection parameter, use TLS instead.

On Tue, Feb 11, 2020 at 11:46:06AM -0600, Eric Blake wrote:

...

On 2/11/20 11:15 AM, Richard W.M. Jones wrote:

...
Since commit 86fdb48c6a5362d66865493d9d2172166f99722e we have stored the connection object in thread-local storage.

In this very large, but mostly mechanical change we stop passing the connection pointer around everywhere, and instead use the value stored in thread-local storage.

This assumes a 1-1 mapping between the connection and the current thread which is true in *most* places. Occasionally we still have the explicit connection pointer, especially just before we launch a thread or call threadlocal_set_conn. --- server/internal.h | 191 +++++++++---------- server/backend.c | 160 +++++++++------- server/connections.c | 68 ++++--- server/crypto.c | 14 +- server/filters.c | 270 +++++++++++++-------------- server/locks.c | 12 +- server/plugins.c | 64 +++---- server/protocol-handshake-newstyle.c | 172 +++++++++-------- server/protocol-handshake-oldstyle.c | 7 +- server/protocol-handshake.c | 40 ++-- server/protocol.c | 127 ++++++------- server/public.c | 2 +- server/test-public.c | 2 +- 13 files changed, 574 insertions(+), 555 deletions(-)

Makes sense to me (and not too hard to rebase my NBD_INFO_INIT_STATE stuff on top of it). Are we sure that there is not going to be a speed penalty (from frequent access to the thread-local storage, compared to previous access through a parameter stored in a register)?

In v2 the new GET_CONN macro still makes a non-inline call to threadlocal_get_conn (). I changed threadlocal_get_conn so it's inlined in the header file internal.h - this is quite an ugly change because you have to pull a few private structures into that header file. Anyway it still makes a non-inline call to pthread_getspecific. So I went back to v2 and instead enabled LTO, but sadly something (probably libtool) breaks LTO. Anyway TLS data is basically a dereference through a register: https://sourceware.org/git/?p=glibc.git;a=blob;f=nptl/pthread_getspecific.c;... (note that THREAD_SELF is a macro which returns a constant offset in the %fs segment) so it should eventually be possible to make this fast.

...

A few comments:

...
+++ b/server/filters.c @@ -49,13 +49,13 @@ struct backend_filter { struct nbdkit_filter filter; }; -/* Literally a backend, a connection pointer, and the filter's handle. +/* Literally a backend and the filter's handle. + * * This is the implementation of our handle in .open, and serves as * a stable ‘void *nxdata’ in the filter API. */ -struct b_conn { +struct b_h { struct backend *b; - struct connection *conn; void *handle; }; @@ -186,22 +186,22 @@ filter_config_complete (struct backend *b) static int next_preconnect (void *nxdata, int readonly) { - struct b_conn *b_conn = nxdata; - return b_conn->b->preconnect (b_conn->b, b_conn->conn, readonly); + struct b_h *b_h = nxdata; + return b_h->b->preconnect (b_h->b, readonly); }

None of the next_*() wrappers use b_h->handle, they all stick to b_h->b. I don't think that matters too much, other than...

...
@@ -267,101 +266,101 @@ filter_close (struct backend *b, struct connection *conn, void *handle) /* The next_functions structure contains pointers to backend * functions. However because these functions are all expecting a - * backend and a connection, we cannot call them directly, but must + * backend and a handle, we cannot call them directly, but must * write some next_* functions that unpack the two parameters from a - * single ‘void *nxdata’ struct pointer (‘b_conn’). + * single ‘void *nxdata’ struct pointer (‘b_h’).

...this comment is slightly off. In fact, if ALL we use is b_h->b, we could probably populate next_ops with backend_* functions rather than next_* wrapper functions:

This was in fact my original motivation for this patch series, except I was expecting we'd use struct backend * instead of the function. As you say in the snipped bit, this should be a separate patch once we get this one right. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v

2467

Age (days ago)

2467

Last active (days ago)

List overview

Download

7 comments

2 participants

participants (2)

Eric Blake
Richard W.M. Jones

[PATCH nbdkit 0/3] server: Remove explicit connection parameter.

tags

participants (2)