Richard W.M. Jones
2023-Jan-30 22:55 UTC
[Libguestfs] [PATCH libnbd v2 0/4] Pass LISTEN_FDNAMES with systemd socket activation
This is an alternative approach to https://listman.redhat.com/archives/libguestfs/2023-January/030535.html After discussing this with Dan Berrange we came to the conclusion that you really might want to set LISTEN_FDNAMES to arbitrary short strings (or not set it). Especially when talking to qemu-storage-daemon which would allow you to use these names on the command line. Rich.
Richard W.M. Jones
2023-Jan-30 22:55 UTC
[Libguestfs] [PATCH libnbd v2 1/4] generator/states-connect-socket-activation.c: Refactor environment prep
Some small refactorings which should not affect the code: - Use string_vector_reserve instead of checking each time we append. - Get rid of the hard-coded length, and use strncmp (..., s, strlen (s)). The compiler should compile this to the same code. --- generator/states-connect-socket-activation.c | 23 +++++++------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/generator/states-connect-socket-activation.c b/generator/states-connect-socket-activation.c index 9a83834915..24544018fb 100644 --- a/generator/states-connect-socket-activation.c +++ b/generator/states-connect-socket-activation.c @@ -34,9 +34,6 @@ /* This is baked into the systemd socket activation API. */ #define FIRST_SOCKET_ACTIVATION_FD 3 -/* == strlen ("LISTEN_PID=") | strlen ("LISTEN_FDS=") */ -#define PREFIX_LENGTH 11 - extern char **environ; /* Prepare environment for calling execvp when doing systemd socket @@ -53,26 +50,22 @@ prepare_socket_activation_environment (string_vector *env) assert (env->len == 0); - /* Reserve slots env[0] and env[1]. */ + /* Reserve slots env[0]..env[1] */ + if (string_vector_reserve (env, 2) == -1) + goto err; p = strdup ("LISTEN_PID=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"); if (p == NULL) goto err; - if (string_vector_append (env, p) == -1) { - free (p); - goto err; - } + string_vector_append (env, p); p = strdup ("LISTEN_FDS=1"); if (p == NULL) goto err; - if (string_vector_append (env, p) == -1) { - free (p); - goto err; - } + string_vector_append (env, p); /* Append the current environment, but remove LISTEN_PID, LISTEN_FDS. */ for (i = 0; environ[i] != NULL; ++i) { - if (strncmp (environ[i], "LISTEN_PID=", PREFIX_LENGTH) != 0 && - strncmp (environ[i], "LISTEN_FDS=", PREFIX_LENGTH) != 0) { + if (strncmp (environ[i], "LISTEN_PID=", strlen ("LISTEN_PID=")) != 0 && + strncmp (environ[i], "LISTEN_FDS=", strlen ("LISTEN_FDS=")) != 0) { char *copy = strdup (environ[i]); if (copy == NULL) goto err; @@ -194,7 +187,7 @@ CONNECT_SA.START: char buf[32]; const char *v nbd_internal_fork_safe_itoa ((long) getpid (), buf, sizeof buf); - strcpy (&env.ptr[0][PREFIX_LENGTH], v); + strcpy (&env.ptr[0][strlen ("LISTEN_FDS=")], v); /* Restore SIGPIPE back to SIG_DFL. */ signal (SIGPIPE, SIG_DFL); -- 2.39.0
Richard W.M. Jones
2023-Jan-30 22:55 UTC
[Libguestfs] [PATCH libnbd v2 2/4] common/include: Copy ascii-ctype functions from nbdkit
--- .gitignore | 1 + common/include/Makefile.am | 6 +++ common/include/ascii-ctype.h | 75 ++++++++++++++++++++++++++ common/include/test-ascii-ctype.c | 88 +++++++++++++++++++++++++++++++ 4 files changed, 170 insertions(+) diff --git a/.gitignore b/.gitignore index f42737131d..90eeadb793 100644 --- a/.gitignore +++ b/.gitignore @@ -34,6 +34,7 @@ Makefile.in /bash-completion/nbdinfo /bash-completion/nbdublk /common/include/test-array-size +/common/include/test-ascii-ctype /common/include/test-checked-overflow /common/include/test-ispowerof2 /common/utils/test-human-size diff --git a/common/include/Makefile.am b/common/include/Makefile.am index fa2633c25a..04553a367d 100644 --- a/common/include/Makefile.am +++ b/common/include/Makefile.am @@ -20,6 +20,7 @@ include $(top_srcdir)/subdir-rules.mk EXTRA_DIST = \ ansi-colours.h \ array-size.h \ + ascii-ctype.h \ byte-swapping.h \ checked-overflow.h \ compiler-macros.h \ @@ -35,6 +36,7 @@ EXTRA_DIST = \ TESTS = \ test-array-size \ + test-ascii-ctype \ test-checked-overflow \ test-ispowerof2 \ $(NULL) @@ -44,6 +46,10 @@ test_array_size_SOURCES = test-array-size.c array-size.h test_array_size_CPPFLAGS = -I$(srcdir) test_array_size_CFLAGS = $(WARNINGS_CFLAGS) +test_ascii_ctype_SOURCES = test-ascii-ctype.c ascii-ctype.h +test_ascii_ctype_CPPFLAGS = -I$(srcdir) +test_ascii_ctype_CFLAGS = $(WARNINGS_CFLAGS) + test_checked_overflow_SOURCES = test-checked-overflow.c checked-overflow.h test_checked_overflow_CPPFLAGS = -I$(srcdir) test_checked_overflow_CFLAGS = $(WARNINGS_CFLAGS) diff --git a/common/include/ascii-ctype.h b/common/include/ascii-ctype.h new file mode 100644 index 0000000000..a55dee11b8 --- /dev/null +++ b/common/include/ascii-ctype.h @@ -0,0 +1,75 @@ +/* nbdkit + * Copyright (C) 2013-2020 Red Hat Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * * Neither the name of Red Hat nor the names of its contributors may be + * used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +/* Normal ctype functions are affected by the current locale. For + * example isupper() might recognize ? in some but not all locales. + * These functions match only 7 bit ASCII characters. + */ + +#ifndef NBDKIT_ASCII_CTYPE_H +#define NBDKIT_ASCII_CTYPE_H + +#define ascii_isalnum(c) (ascii_isalpha (c) || ascii_isdigit (c)) + +#define ascii_isalpha(c) \ + (((c) >= 'a' && (c) <= 'z') || ((c) >= 'A' && (c) <= 'Z')) + +#define ascii_isdigit(c) \ + ((c) >= '0' && (c) <= '9') + +#define ascii_isspace(c) \ + ((c) == '\t' || (c) == '\n' || (c) == '\f' || (c) == '\r' || (c) == ' ') + +#define ascii_isupper(c) \ + ((c) >= 'A' && (c) <= 'Z') + +#define ascii_islower(c) \ + ((c) >= 'a' && (c) <= 'z') + +/* See also hexdigit.h */ +#define ascii_isxdigit(c) \ + ((c) == '0' || (c) == '1' || (c) == '2' || (c) == '3' || (c) == '4' || \ + (c) == '5' || (c) == '6' || (c) == '7' || (c) == '8' || (c) == '9' || \ + (c) == 'a' || (c) == 'b' || (c) == 'c' || \ + (c) == 'd' || (c) == 'e' || (c) == 'f' || \ + (c) == 'A' || (c) == 'B' || (c) == 'C' || \ + (c) == 'D' || (c) == 'E' || (c) == 'F') + +#define ascii_tolower(c) \ + (ascii_isupper ((c)) ? (c) + 32 : (c)) + +#define ascii_toupper(c) \ + (ascii_islower ((c)) ? (c) - 32 : (c)) + +#define ascii_isprint(c) ((c) >= 32 && (c) <= 126) + +#endif /* NBDKIT_ASCII_CTYPE_H */ diff --git a/common/include/test-ascii-ctype.c b/common/include/test-ascii-ctype.c new file mode 100644 index 0000000000..4fbb0259f8 --- /dev/null +++ b/common/include/test-ascii-ctype.c @@ -0,0 +1,88 @@ +/* nbdkit + * Copyright (C) 2020 Red Hat Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * * Neither the name of Red Hat nor the names of its contributors may be + * used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#undef NDEBUG /* Keep test strong even for nbdkit built without assertions */ +#include <assert.h> + +#include "ascii-ctype.h" + +int +main (void) +{ + assert (ascii_isspace (' ')); + assert (ascii_isspace ('\t')); + assert (ascii_isspace ('\n')); + assert (! ascii_isspace ('a')); + + assert (ascii_isalpha ('a')); + assert (ascii_isalpha ('Z')); + assert (ascii_isalpha ('z')); + assert (! ascii_isalpha (' ')); + assert (! ascii_isalpha ('0')); + { const char *s = "?"; assert (! ascii_isalpha (s[0])); } + { const char *s = "?"; assert (! ascii_isalpha (s[0])); } + + assert (ascii_isdigit ('0')); + assert (ascii_isdigit ('9')); + { const char *s = "?"; assert (! ascii_isdigit (s[0])); } /* U+00D8 */ + { const char *s = "?"; assert (! ascii_isdigit (s[0])); } /* U+FF19 */ + + assert (ascii_islower ('a')); + assert (ascii_islower ('z')); + assert (! ascii_islower ('Z')); + { const char *s = "?"; assert (! ascii_islower (s[0])); } + + assert (ascii_isupper ('A')); + assert (ascii_isupper ('Z')); + assert (! ascii_isupper ('z')); + { const char *s = "?"; assert (! ascii_isupper (s[0])); } + + assert (ascii_tolower ('A') == 'a'); + assert (ascii_tolower ('Z') == 'z'); + assert (ascii_tolower ('a') == 'a'); + assert (ascii_tolower ('z') == 'z'); + assert (ascii_tolower ('0') == '0'); + { const char *s = "?"; assert (ascii_tolower (s[0]) == s[0]); } + + assert (ascii_toupper ('a') == 'A'); + assert (ascii_toupper ('z') == 'Z'); + assert (ascii_toupper ('A') == 'A'); + assert (ascii_toupper ('Z') == 'Z'); + assert (ascii_toupper ('0') == '0'); + { const char *s = "?"; assert (ascii_toupper (s[0]) == s[0]); } + + exit (EXIT_SUCCESS); +} -- 2.39.0
Richard W.M. Jones
2023-Jan-30 22:55 UTC
[Libguestfs] [PATCH libnbd v2 3/4] generator: Add APIs to get/set the socket activation socket name
To allow us to name the socket passed down to the NBD server when calling nbd_connect_systemd_socket_activation(3), we need to add the field to the handle and add access functions. --- generator/API.ml | 49 ++++++++++++++++++++++++++++++++++++++++++ lib/handle.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++ lib/internal.h | 1 + 3 files changed, 106 insertions(+) diff --git a/generator/API.ml b/generator/API.ml index 25a612a2e8..08fc80960b 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -2036,15 +2036,62 @@ "connect_systemd_socket_activation", { When the NBD handle is closed the server subprocess is killed. + +=head3 Socket name + +The socket activation protocol lets you optionally give +the socket a name. If used, the name is passed to the +NBD server using the C<LISTEN_FDNAMES> environment +variable. To provide a socket name, call +L<nbd_set_socket_activation_name(3)> before calling +the connect function. " ^ blocking_connect_call_description; see_also = [Link "aio_connect_systemd_socket_activation"; Link "connect_command"; Link "kill_subprocess"; Link "set_opt_mode"; + Link "set_socket_activation_name"; + Link "get_socket_activation_name"; ExternalLink ("qemu-nbd", 1); URLLink "http://0pointer.de/blog/projects/socket-activation.html"]; example = Some "examples/open-qcow2.c"; }; + "set_socket_activation_name", { + default_call with + args = [ String "socket_name" ]; ret = RErr; + shortdesc = "set the socket activation name"; + longdesc = "\ +When running an NBD server using +L<nbd_connect_systemd_socket_activation(3)> you can optionally +name the socket. Call this function before connecting to the +server. + +Some servers such as L<qemu-storage-daemon(1)> +can use this information to associate the socket with a name +used on the command line, but most servers will ignore it. +The name is passed through the C<LISTEN_FDNAMES> environment +variable. + +The parameter C<socket_name> can be a short alphanumeric string. +If it is set to the empty string (also the default when the handle +is created) then no name is passed to the server."; + see_also = [Link "connect_systemd_socket_activation"; + Link "get_socket_activation_name"]; + }; + + "get_socket_activation_name", { + default_call with + args = []; ret = RString; + shortdesc = "get the socket activation name"; + longdesc = "\ +Return the socket name used when you call +L<nbd_connect_systemd_socket_activation(3)> on the same +handle. By default this will return the empty string +meaning that no name is passed to the server."; + see_also = [Link "connect_systemd_socket_activation"; + Link "set_socket_activation_name"]; + }; + "is_read_only", { default_call with args = []; ret = RBool; @@ -3844,6 +3891,8 @@ let first_version "aio_opt_structured_reply", (1, 16); "opt_starttls", (1, 16); "aio_opt_starttls", (1, 16); + "set_socket_activation_name", (1, 16); + "get_socket_activation_name", (1, 16); (* These calls are proposed for a future version of libnbd, but * have not been added to any released version so far. diff --git a/lib/handle.c b/lib/handle.c index 4a186f8fa9..96c8b1f2b1 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -28,6 +28,7 @@ #include <sys/types.h> #include <sys/wait.h> +#include "ascii-ctype.h" #include "internal.h" static void @@ -161,6 +162,7 @@ nbd_close (struct nbd_handle *h) waitpid (h->pid, NULL, 0); free (h->export_name); + free (h->sa_name); free (h->tls_certificates); free (h->tls_username); free (h->tls_psk_file); @@ -200,6 +202,60 @@ nbd_unlocked_get_handle_name (struct nbd_handle *h) return copy; } +int +nbd_unlocked_set_socket_activation_name (struct nbd_handle *h, + const char *name) +{ + size_t i, len; + char *new_name; + + len = strlen (name); + + /* Setting it to empty string stores NULL in the handle. */ + if (len == 0) { + free (h->sa_name); + h->sa_name = NULL; + return 0; + } + + /* Check the proposed name is short and alphanumeric. */ + if (len > 32) { + set_error (ENAMETOOLONG, "socket activation name should be " + "<= 32 characters"); + return -1; + } + for (i = 0; i < len; ++i) { + if (! ascii_isalnum (name[i])) { + set_error (EINVAL, "socket activation name should contain " + "only alphanumeric ASCII characters"); + return -1; + } + } + + new_name = strdup (name); + if (!new_name) { + set_error (errno, "strdup"); + return -1; + } + + free (h->sa_name); + h->sa_name = new_name; + return 0; +} + +char * +nbd_unlocked_get_socket_activation_name (struct nbd_handle *h) +{ + char *copy = strdup (h->sa_name ? h->sa_name : ""); + + if (!copy) { + set_error (errno, "strdup"); + return NULL; + } + + return copy; +} + uintptr_t nbd_unlocked_set_private_data (struct nbd_handle *h, uintptr_t data) { diff --git a/lib/internal.h b/lib/internal.h index bbbd26393f..19d7f0af60 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -101,6 +101,7 @@ struct nbd_handle { _Atomic uintptr_t private_data; char *export_name; /* Export name, never NULL. */ + char *sa_name; /* Socket activation name, can be NULL. */ /* TLS settings. */ int tls; /* 0 = disable, 1 = enable, 2 = require */ -- 2.39.0
Richard W.M. Jones
2023-Jan-30 22:55 UTC
[Libguestfs] [PATCH libnbd v2 4/4] generator/states-connect-socket-activation.c: Set LISTEN_FDNAMES
When the user calls nbd_set_socket_activation_name before calling nbd_connect_system_socket_activation, pass the name down to the server through LISTEN_FDNAMES. This has no effect unless the new API has been called to set the socket name to a non-empty string. --- generator/states-connect-socket-activation.c | 35 +++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/generator/states-connect-socket-activation.c b/generator/states-connect-socket-activation.c index 24544018fb..2ff191bb9f 100644 --- a/generator/states-connect-socket-activation.c +++ b/generator/states-connect-socket-activation.c @@ -22,6 +22,7 @@ #include <stdio.h> #include <stdlib.h> +#include <stdbool.h> #include <string.h> #include <unistd.h> #include <errno.h> @@ -38,20 +39,27 @@ extern char **environ; /* Prepare environment for calling execvp when doing systemd socket * activation. Takes the current environment and copies it. Removes - * any existing LISTEN_PID or LISTEN_FDS and replaces them with new - * variables. env[0] is "LISTEN_PID=..." which is filled in by - * CONNECT_SA.START, and env[1] is "LISTEN_FDS=1". + * any existing LISTEN_PID, LISTEN_FDS or LISTEN_FDNAAMES, and + * replaces them with new variables. + * + * env[0] is "LISTEN_PID=..." which is filled in by CONNECT_SA.START + * + * env[1] is "LISTEN_FDS=1" + * + * env[2] (if used) is "LISTEN_FDNAMES=" + h->sa_name */ static int -prepare_socket_activation_environment (string_vector *env) +prepare_socket_activation_environment (struct nbd_handle *h, + string_vector *env) { char *p; size_t i; + const bool using_name = h->sa_name != NULL; assert (env->len == 0); - /* Reserve slots env[0]..env[1] */ - if (string_vector_reserve (env, 2) == -1) + /* Reserve slots in env. */ + if (string_vector_reserve (env, using_name ? 3 : 2) == -1) goto err; p = strdup ("LISTEN_PID=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"); if (p == NULL) @@ -61,11 +69,20 @@ prepare_socket_activation_environment (string_vector *env) if (p == NULL) goto err; string_vector_append (env, p); + if (using_name) { + if (asprintf (&p, "LISTEN_FDNAMES=%s", h->sa_name) == -1) + goto err; + string_vector_append (env, p); + } - /* Append the current environment, but remove LISTEN_PID, LISTEN_FDS. */ + /* Append the current environment, but remove the special + * environment variables. + */ for (i = 0; environ[i] != NULL; ++i) { if (strncmp (environ[i], "LISTEN_PID=", strlen ("LISTEN_PID=")) != 0 && - strncmp (environ[i], "LISTEN_FDS=", strlen ("LISTEN_FDS=")) != 0) { + strncmp (environ[i], "LISTEN_FDS=", strlen ("LISTEN_FDS=")) != 0 && + strncmp (environ[i], "LISTEN_FDNAMES=", + strlen ("LISTEN_FDNAMES=")) != 0) { char *copy = strdup (environ[i]); if (copy == NULL) goto err; @@ -148,7 +165,7 @@ CONNECT_SA.START: return 0; } - if (prepare_socket_activation_environment (&env) == -1) { + if (prepare_socket_activation_environment (h, &env) == -1) { SET_NEXT_STATE (%.DEAD); close (s); return 0; -- 2.39.0
Apparently Analagous Threads
- [PATCH libnbd] generator: Pass LISTEN_FDNAMES=nbd with systemd socket activation
- [PATCH libnbd] generator: Pass LISTEN_FDNAMES=nbd with systemd socket activation
- [PATCH libnbd v2 0/4] Pass LISTEN_FDNAMES with systemd socket activation
- [PATCH libnbd] generator: Pass LISTEN_FDNAMES=nbd with systemd socket activation
- [libnbd PATCH v3 19/19] socket activation: set LISTEN_FDNAMES