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