Re: [Libguestfs] [nbdkit PATCH v3 03/16] server: Move default_exportname from handle to connection

struct handle represents things that can differ per export (such as the results of .can_write), but .default_exportname is really something that should be global to all possible exports of the connection. Moving it also gets rid of the leaky abstraction where NBD_OPT_STARTTLS has to reset the cached default when changing TLS even though there is no current handle at that time. It also lets us simplify reset_handle(), as it no longer tracks allocated memory, and prepares for an upcoming patch refactoring how backend_open operates. --- server/internal.h | 6 ++---- server/backend.c | 8 ++++---- server/connections.c | 15 ++++++++++++--- server/protocol-handshake-newstyle.c | 7 +++---- 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/server/internal.h b/server/internal.h index 906f0690..1f37703f 100644 --- a/server/internal.h +++ b/server/internal.h @@ -211,8 +211,6 @@ struct handle { unsigned char state; /* Bitmask of HANDLE_* values */ - char *default_exportname; - uint64_t exportsize; int can_write; int can_flush; @@ -231,8 +229,6 @@ reset_handle (struct handle *h) { h->handle = NULL; h->state = 0; - free (h->default_exportname); - h->default_exportname = NULL; h->exportsize = -1; h->can_write = -1; h->can_flush = -1; @@ -261,6 +257,8 @@ struct connection { struct handle *handles; /* One per plugin and filter. */ size_t nr_handles; + char **default_exportname; /* One per plugin and filter. */ + uint32_t cflags; uint16_t eflags; bool handshake_complete; diff --git a/server/backend.c b/server/backend.c index e9fcd696..d08a2b6b 100644 --- a/server/backend.c +++ b/server/backend.c @@ -193,7 +193,7 @@ backend_default_export (struct backend *b, int readonly) controlpath_debug ("%s: default_export readonly=%d tls=%d", b->name, readonly, conn->using_tls); - if (h->default_exportname == NULL) { + if (conn->default_exportname[b->i] == NULL) { assert (h->handle == NULL); assert ((h->state & HANDLE_OPEN) == 0); s = b->default_export (b, readonly, conn->using_tls); @@ -205,12 +205,12 @@ backend_default_export (struct backend *b, int readonly) } if (s) { /* Best effort caching */ - h->default_exportname = strdup (s); - if (h->default_exportname == NULL) + conn->default_exportname[b->i] = strdup (s); + if (conn->default_exportname[b->i] == NULL) return s; } } - return h->default_exportname; + return conn->default_exportname[b->i]; } int diff --git a/server/connections.c b/server/connections.c index 38b32742..b56f89f8 100644 --- a/server/connections.c +++ b/server/connections.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013-2020 Red Hat Inc. + * Copyright (C) 2013-2021 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -263,6 +263,14 @@ new_connection (int sockin, int sockout, int nworkers) for_each_backend (b) reset_handle (get_handle (conn, b->i)); + conn->default_exportname = calloc (top->i + 1, + sizeof *conn->default_exportname); + if (conn->default_exportname == NULL) { + perror ("malloc"); + free (conn->handles); + goto error1; + } + conn->status = 1; conn->nworkers = nworkers; if (nworkers) { @@ -330,6 +338,7 @@ new_connection (int sockin, int sockout, int nworkers) if (conn->status_pipe[1] >= 0) close (conn->status_pipe[1]); free (conn->handles); + free (conn->default_exportname); error1: pthread_mutex_destroy (&conn->request_lock); @@ -373,9 +382,9 @@ free_connection (struct connection *conn) free (conn->exportname_from_set_meta_context); free_interns (); - /* This is needed in order to free a field in struct handle. */ for_each_backend (b) - reset_handle (get_handle (conn, b->i)); + free (conn->default_exportname[b->i]); + free (conn->default_exportname); free (conn->handles); free (conn); diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c index 0a76a814..fb4a93bc 100644 --- a/server/protocol-handshake-newstyle.c +++ b/server/protocol-handshake-newstyle.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013-2020 Red Hat Inc. + * Copyright (C) 2013-2021 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -497,9 +497,8 @@ negotiate_handshake_newstyle_options (void) debug ("using TLS on this connection"); /* Wipe out any cached default export name. */ for_each_backend (b) { - struct handle *h = get_handle (conn, b->i); - free (h->default_exportname); - h->default_exportname = NULL; + free (conn->default_exportname[b->i]); + conn->default_exportname[b->i] = NULL; } } break;