[PATCH 1/3] Socket Activation: stash $LISTEN_FDNAMES

[PATCH 1/3] Socket Activation: stash $LISTEN_FDNAMES

[Daniel Kahn Gillmor]

By recording a copy of LISTEN_FDNAMES, we make it possible to learn
mappings from file descriptor labels (e.g., as set by
FileDescriptorName= in systemd.socket(5)).

This also makes it possible to invoke check_socket_activation() more
than once and have it return the same value each time.

This is one step toward addressing
https://gitlab.com/qemu-project/qemu/-/issues/3011

Since we can't count on the buffer returned from getenv
persisting (getenv is documented as non-re-entrant), we need to keep a
copy of it around in case multiple subsystems want to interrogate it.

This proposed implementation uses a static buffer, and breaks socket
activation with a visible error_report if LISTEN_FDNAMES is too large.
Another approach would be to g_strdup the value returned by getenv,
which would have failure modes on heap exhaustion, and would introduce
a memory leak as there's no clear opportunity to g_free the copy.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 util/systemd.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/util/systemd.c b/util/systemd.c
index ced518f771..1eca2bd69f 100644
--- a/util/systemd.c
+++ b/util/systemd.c
@@ -16,37 +16,62 @@
 #include "qemu/error-report.h"
 
 #ifndef _WIN32
+static char fdnames[256];
+
 unsigned int check_socket_activation(void)
 {
+    static unsigned int nr_fds = -1;
     const char *s;
     unsigned long pid;
-    unsigned long nr_fds;
+    unsigned long nr_fdsl;
     unsigned int i;
     int fd;
     int f;
     int err;
 
+    if (nr_fds != -1) {
+        return nr_fds;
+    }
     s = getenv("LISTEN_PID");
     if (s == NULL) {
+        nr_fds = 0;
         return 0;
     }
     err = qemu_strtoul(s, NULL, 10, &pid);
     if (err) {
+        nr_fds = 0;
         return 0;
     }
     if (pid != getpid()) {
+        nr_fds = 0;
         return 0;
     }
 
     s = getenv("LISTEN_FDS");
     if (s == NULL) {
+        nr_fds = 0;
         return 0;
     }
-    err = qemu_strtoul(s, NULL, 10, &nr_fds);
+    err = qemu_strtoul(s, NULL, 10, &nr_fdsl);
     if (err) {
+        nr_fds = 0;
         return 0;
     }
-    assert(nr_fds <= UINT_MAX);
+    assert(nr_fdsl <= UINT_MAX);
+    nr_fds = (unsigned int) nr_fdsl;
+    s = getenv("LISTEN_FDNAMES");
+    if (s != NULL) {
+        size_t fdnames_len = strlen(s);
+        if (fdnames_len + 1 > sizeof(fdnames)) {
+            error_report("LISTEN_FDNAMES is larger than %ldu bytes, "
+                         "ignoring socket activation.",
+                         sizeof(fdnames));
+            nr_fds = 0;
+            return 0;
+        } else {
+            memcpy(fdnames, s, fdnames_len + 1);
+        }
+    }
 
     /* So these are not passed to any child processes we might start. */
     unsetenv("LISTEN_FDS");
@@ -69,7 +94,7 @@ unsigned int check_socket_activation(void)
         }
     }
 
-    return (unsigned int) nr_fds;
+    return nr_fds;
 }
 
 #else /* !_WIN32 */
-- 
2.47.2

[PATCH 2/3] Socket activation: get FD by label

[Daniel Kahn Gillmor]

This uses the cached copy of LISTEN_FDNAMES to find the first file
descriptorlabel with a matching label.

Note that if two file descriptors are given the same label this will
ignore all but the first.

This is another step toward addressing
https://gitlab.com/qemu-project/qemu/-/issues/3011

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 include/qemu/systemd.h | 15 +++++++++++++++
 util/systemd.c         | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/include/qemu/systemd.h b/include/qemu/systemd.h
index f0ea1266d5..9f2bfdc7c7 100644
--- a/include/qemu/systemd.h
+++ b/include/qemu/systemd.h
@@ -23,4 +23,19 @@
  */
 unsigned int check_socket_activation(void);
 
+
+/*
+ * Check if socket activation indicates named file descriptor based on
+ * the colon-delimited LISTEN_FDNAMES.  The "label" must not be NULL,
+ * and should be a simple text string that does not contain a colon,
+ * matching the FileDescriptorName= directive in systemd.socket(5)
+ *
+ * It is acceptable to ask for the empty string as a label.
+ *
+ * Returns -1 if no socket activation is in use, or if the label does
+ * not match any file descriptor.  Otherwise, returns the lowest
+ * numeric value for a file descriptor matching the label exactly.
+ */
+unsigned int socket_activated_fd_by_label(const char *label);
+
 #endif
diff --git a/util/systemd.c b/util/systemd.c
index 1eca2bd69f..d6675725be 100644
--- a/util/systemd.c
+++ b/util/systemd.c
@@ -97,9 +97,44 @@ unsigned int check_socket_activation(void)
     return nr_fds;
 }
 
+unsigned int socket_activated_fd_by_label(const char *label)
+{
+    int nr_fds = check_socket_activation();
+    if (!nr_fds) {
+        return -1;
+    }
+    int curfd;
+    const char *nameend;
+    const char *nameptr;
+    size_t labellen, namelen;
+
+    labellen = sizeof(label);
+    curfd = 0;
+    nameptr = fdnames;
+    do {
+        nameend = strchr(nameptr, ':');
+        if (nameend) {
+            namelen = nameend - nameptr;
+            nameend++;
+        } else {
+            namelen = strlen(nameptr);
+        }
+        if (labellen == namelen && memcmp(nameptr, label, namelen) == 0) {
+            return curfd + FIRST_SOCKET_ACTIVATION_FD;
+        }
+        curfd++;
+        nameptr = nameend;
+    } while (nameptr && curfd < nr_fds);
+    return -1;
+}
+
 #else /* !_WIN32 */
 unsigned int check_socket_activation(void)
 {
     return 0;
 }
+unsigned int socket_activated_fd_by_label(const char *label)
+{
+    return 0;
+}
 #endif
-- 
2.47.2

[PATCH 3/3] Socket activation: enable spice listener.

[Daniel Kahn Gillmor]

Enable qemu to be socket-activated based on a spice connection.

Note that this depends on un-deprecating
spice_server_set_listen_socket_fd, see
https://gitlab.freedesktop.org/spice/spice/-/merge_requests/240

This partially addresses
https://gitlab.com/qemu-project/qemu/-/issues/3011

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>

More fixup for spice systemd socket activation
---
 qemu-options.hx |  7 +++++--
 ui/spice-core.c | 47 +++++++++++++++++++++++++++++++++++------------
 2 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 1f862b19a6..d17c5bc5ff 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2270,6 +2270,7 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice,
     "       [,x509-cert-file=<file>][,x509-cacert-file=<file>]\n"
     "       [,x509-dh-key-file=<file>][,addr=addr]\n"
     "       [,ipv4=on|off][,ipv6=on|off][,unix=on|off]\n"
+    "       [,socket-activated=<str>]\n"
     "       [,tls-ciphers=<list>]\n"
     "       [,tls-channel=[main|display|cursor|inputs|record|playback]]\n"
     "       [,plaintext-channel=[main|display|cursor|inputs|record|playback]]\n"
@@ -2297,8 +2298,10 @@ SRST
         Set the IP address spice is listening on. Default is any
         address.
 
-    ``ipv4=on|off``; \ ``ipv6=on|off``; \ ``unix=on|off``
-        Force using the specified IP version.
+    ``ipv4=on|off``; \ ``ipv6=on|off``; \ ``unix=on|off`` ; \ ``socket-activated=<str>``
+        Force using the specified IP version.  Or, use a unix-domain socket.
+        Or, listen using the passed file descriptor from systemd-style socket
+        activation associated with FileDescriptorName ``str``.
 
     ``password-secret=<secret-id>``
         Set the ID of the ``secret`` object containing the password
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 0326c63bec..1f817142f0 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -38,6 +38,7 @@
 #include "migration/misc.h"
 #include "hw/pci/pci_bus.h"
 #include "ui/spice-display.h"
+#include "qemu/systemd.h"
 
 /* core bits */
 
@@ -433,6 +434,9 @@ static QemuOptsList qemu_spice_opts = {
         },{
             .name = "unix",
             .type = QEMU_OPT_BOOL,
+        },{
+            .name = "socket-activated",
+            .type = QEMU_OPT_STRING,
 #endif
         },{
             .name = "password-secret",
@@ -736,18 +740,37 @@ static void qemu_spice_init(void)
     }
 
     spice_server = spice_server_new();
-    spice_server_set_addr(spice_server, addr ? addr : "", addr_flags);
-    if (port) {
-        spice_server_set_port(spice_server, port);
-    }
-    if (tls_port) {
-        spice_server_set_tls(spice_server, tls_port,
-                             x509_cacert_file,
-                             x509_cert_file,
-                             x509_key_file,
-                             x509_key_password,
-                             x509_dh_file,
-                             tls_ciphers);
+    str = qemu_opt_get(opts, "socket-activated");
+    if (str) {
+        int fd = socket_activated_fd_by_label(str);
+        if (fd == -1) {
+            error_report("socket-activated spice failed: No FD found with label '%s'",
+                         str);
+            exit(1);
+        }
+
+        if (addr || addr_flags) {
+            error_report("When spice is socket-activated, do not set addr or ipv4 or ipv6 or unix");
+            exit(1);
+        }
+        if (spice_server_set_listen_socket_fd(spice_server, fd) == -1) {
+            error_report("spice_server_set_listen_socket_fd failed!");
+            exit(1);
+        }
+    } else {
+        spice_server_set_addr(spice_server, addr ? addr : "", addr_flags);
+        if (port) {
+            spice_server_set_port(spice_server, port);
+        }
+        if (tls_port) {
+            spice_server_set_tls(spice_server, tls_port,
+                                 x509_cacert_file,
+                                 x509_cert_file,
+                                 x509_key_file,
+                                 x509_key_password,
+                                 x509_dh_file,
+                                 tls_ciphers);
+        }
     }
     if (password) {
         qemu_spice.set_passwd(password, false, false);
-- 
2.47.2

Clean up (v2) of Socket Activation series

[Daniel Kahn Gillmor]

This is a clean-up of the code I offered yesterday to improve socket
activation for qemu.  It resolves a couple dumb bugs i had initially
introduced, and reduces the line-width of some of the new code.

This is addressing https://gitlab.com/qemu-project/qemu/-/issues/3011

I welcome reviews and feedback!

  --dkg

[PATCH v2 1/3] Socket Activation: stash $LISTEN_FDNAMES

[Daniel Kahn Gillmor]

By recording a copy of LISTEN_FDNAMES, we make it possible to learn
mappings from file descriptor labels (e.g., as set by
FileDescriptorName= in systemd.socket(5)).

This also makes it possible to invoke check_socket_activation() more
than once and have it return the same value each time.

This is one step toward addressing
https://gitlab.com/qemu-project/qemu/-/issues/3011

Since we can't count on the buffer returned from getenv
persisting (getenv is documented as non-re-entrant), we need to keep a
copy of it around in case multiple subsystems want to interrogate it.

This proposed implementation uses a static buffer, and breaks socket
activation with a visible error_report if LISTEN_FDNAMES is too large.
Another approach would be to g_strdup the value returned by getenv,
which would have failure modes on heap exhaustion, and would introduce
a memory leak as there's no clear opportunity to g_free the copy.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 util/systemd.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/util/systemd.c b/util/systemd.c
index ced518f771..1eca2bd69f 100644
--- a/util/systemd.c
+++ b/util/systemd.c
@@ -16,37 +16,62 @@
 #include "qemu/error-report.h"
 
 #ifndef _WIN32
+static char fdnames[256];
+
 unsigned int check_socket_activation(void)
 {
+    static unsigned int nr_fds = -1;
     const char *s;
     unsigned long pid;
-    unsigned long nr_fds;
+    unsigned long nr_fdsl;
     unsigned int i;
     int fd;
     int f;
     int err;
 
+    if (nr_fds != -1) {
+        return nr_fds;
+    }
     s = getenv("LISTEN_PID");
     if (s == NULL) {
+        nr_fds = 0;
         return 0;
     }
     err = qemu_strtoul(s, NULL, 10, &pid);
     if (err) {
+        nr_fds = 0;
         return 0;
     }
     if (pid != getpid()) {
+        nr_fds = 0;
         return 0;
     }
 
     s = getenv("LISTEN_FDS");
     if (s == NULL) {
+        nr_fds = 0;
         return 0;
     }
-    err = qemu_strtoul(s, NULL, 10, &nr_fds);
+    err = qemu_strtoul(s, NULL, 10, &nr_fdsl);
     if (err) {
+        nr_fds = 0;
         return 0;
     }
-    assert(nr_fds <= UINT_MAX);
+    assert(nr_fdsl <= UINT_MAX);
+    nr_fds = (unsigned int) nr_fdsl;
+    s = getenv("LISTEN_FDNAMES");
+    if (s != NULL) {
+        size_t fdnames_len = strlen(s);
+        if (fdnames_len + 1 > sizeof(fdnames)) {
+            error_report("LISTEN_FDNAMES is larger than %ldu bytes, "
+                         "ignoring socket activation.",
+                         sizeof(fdnames));
+            nr_fds = 0;
+            return 0;
+        } else {
+            memcpy(fdnames, s, fdnames_len + 1);
+        }
+    }
 
     /* So these are not passed to any child processes we might start. */
     unsetenv("LISTEN_FDS");
@@ -69,7 +94,7 @@ unsigned int check_socket_activation(void)
         }
     }
 
-    return (unsigned int) nr_fds;
+    return nr_fds;
 }
 
 #else /* !_WIN32 */
-- 
2.47.2

[PATCH v2 2/3] Socket activation: get FD by label

[Daniel Kahn Gillmor]

This uses the cached copy of LISTEN_FDNAMES to find the first file
descriptorlabel with a matching label.

Note that if two file descriptors are given the same label this will
ignore all but the first.

This is another step toward addressing
https://gitlab.com/qemu-project/qemu/-/issues/3011

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 include/qemu/systemd.h | 15 +++++++++++++++
 util/systemd.c         | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/include/qemu/systemd.h b/include/qemu/systemd.h
index f0ea1266d5..6b3f9a97ff 100644
--- a/include/qemu/systemd.h
+++ b/include/qemu/systemd.h
@@ -23,4 +23,19 @@
  */
 unsigned int check_socket_activation(void);
 
+
+/*
+ * Check if socket activation indicates named file descriptor based on
+ * the colon-delimited LISTEN_FDNAMES.  The "label" must not be NULL,
+ * and should be a simple text string that does not contain a colon,
+ * matching the FileDescriptorName= directive in systemd.socket(5)
+ *
+ * It is acceptable to ask for the empty string as a label.
+ *
+ * Returns -1 if no socket activation is in use, or if the label does
+ * not match any file descriptor.  Otherwise, returns the lowest
+ * numeric value for a file descriptor matching the label exactly.
+ */
+int socket_activated_fd_by_label(const char *label);
+
 #endif
diff --git a/util/systemd.c b/util/systemd.c
index 1eca2bd69f..7bf4a847b2 100644
--- a/util/systemd.c
+++ b/util/systemd.c
@@ -97,9 +97,44 @@ unsigned int check_socket_activation(void)
     return nr_fds;
 }
 
+int socket_activated_fd_by_label(const char *label)
+{
+    int nr_fds = check_socket_activation();
+    if (!nr_fds) {
+        return -1;
+    }
+    int curfd;
+    const char *nameend;
+    const char *nameptr;
+    size_t labellen, namelen;
+
+    labellen = strlen(label);
+    curfd = 0;
+    nameptr = fdnames;
+    do {
+        nameend = strchr(nameptr, ':');
+        if (nameend) {
+            namelen = nameend - nameptr;
+            nameend++;
+        } else {
+            namelen = strlen(nameptr);
+        }
+        if (labellen == namelen && memcmp(nameptr, label, namelen) == 0) {
+            return curfd + FIRST_SOCKET_ACTIVATION_FD;
+        }
+        curfd++;
+        nameptr = nameend;
+    } while (nameptr && curfd < nr_fds);
+    return -1;
+}
+
 #else /* !_WIN32 */
 unsigned int check_socket_activation(void)
 {
     return 0;
 }
+unsigned int socket_activated_fd_by_label(const char *label)
+{
+    return 0;
+}
 #endif
-- 
2.47.2

[PATCH v2 3/3] Socket activation: enable spice listener.

[Daniel Kahn Gillmor]

Enable qemu to be socket-activated based on a spice connection.

Note that this depends on un-deprecating
spice_server_set_listen_socket_fd, see
https://gitlab.freedesktop.org/spice/spice/-/merge_requests/240

This partially addresses
https://gitlab.com/qemu-project/qemu/-/issues/3011

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 qemu-options.hx |  7 +++++--
 ui/spice-core.c | 49 +++++++++++++++++++++++++++++++++++++------------
 2 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 1f862b19a6..d17c5bc5ff 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2270,6 +2270,7 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice,
     "       [,x509-cert-file=<file>][,x509-cacert-file=<file>]\n"
     "       [,x509-dh-key-file=<file>][,addr=addr]\n"
     "       [,ipv4=on|off][,ipv6=on|off][,unix=on|off]\n"
+    "       [,socket-activated=<str>]\n"
     "       [,tls-ciphers=<list>]\n"
     "       [,tls-channel=[main|display|cursor|inputs|record|playback]]\n"
     "       [,plaintext-channel=[main|display|cursor|inputs|record|playback]]\n"
@@ -2297,8 +2298,10 @@ SRST
         Set the IP address spice is listening on. Default is any
         address.
 
-    ``ipv4=on|off``; \ ``ipv6=on|off``; \ ``unix=on|off``
-        Force using the specified IP version.
+    ``ipv4=on|off``; \ ``ipv6=on|off``; \ ``unix=on|off`` ; \ ``socket-activated=<str>``
+        Force using the specified IP version.  Or, use a unix-domain socket.
+        Or, listen using the passed file descriptor from systemd-style socket
+        activation associated with FileDescriptorName ``str``.
 
     ``password-secret=<secret-id>``
         Set the ID of the ``secret`` object containing the password
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 0326c63bec..d74930db45 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -38,6 +38,7 @@
 #include "migration/misc.h"
 #include "hw/pci/pci_bus.h"
 #include "ui/spice-display.h"
+#include "qemu/systemd.h"
 
 /* core bits */
 
@@ -433,6 +434,9 @@ static QemuOptsList qemu_spice_opts = {
         },{
             .name = "unix",
             .type = QEMU_OPT_BOOL,
+        },{
+            .name = "socket-activated",
+            .type = QEMU_OPT_STRING,
 #endif
         },{
             .name = "password-secret",
@@ -736,18 +740,39 @@ static void qemu_spice_init(void)
     }
 
     spice_server = spice_server_new();
-    spice_server_set_addr(spice_server, addr ? addr : "", addr_flags);
-    if (port) {
-        spice_server_set_port(spice_server, port);
-    }
-    if (tls_port) {
-        spice_server_set_tls(spice_server, tls_port,
-                             x509_cacert_file,
-                             x509_cert_file,
-                             x509_key_file,
-                             x509_key_password,
-                             x509_dh_file,
-                             tls_ciphers);
+    str = qemu_opt_get(opts, "socket-activated");
+    if (str) {
+        int fd = socket_activated_fd_by_label(str);
+        if (fd == -1) {
+            error_report("socket-activated spice failed: "
+                         "No FD found with label '%s'",
+                         str);
+            exit(1);
+        }
+
+        if (addr || addr_flags) {
+            error_report("When spice is socket-activated, do not set "
+                         "addr or ipv4 or ipv6 or unix");
+            exit(1);
+        }
+        if (spice_server_set_listen_socket_fd(spice_server, fd) == -1) {
+            error_report("spice_server_set_listen_socket_fd failed!");
+            exit(1);
+        }
+    } else {
+        spice_server_set_addr(spice_server, addr ? addr : "", addr_flags);
+        if (port) {
+            spice_server_set_port(spice_server, port);
+        }
+        if (tls_port) {
+            spice_server_set_tls(spice_server, tls_port,
+                                 x509_cacert_file,
+                                 x509_cert_file,
+                                 x509_key_file,
+                                 x509_key_password,
+                                 x509_dh_file,
+                                 tls_ciphers);
+        }
     }
     if (password) {
         qemu_spice.set_passwd(password, false, false);
-- 
2.47.2

Re: [PATCH 3/3] Socket activation: enable spice listener.

[Daniel P. Berrangé]

On Fri, Jun 27, 2025 at 02:03:31PM -0400, Daniel Kahn Gillmor wrote:
> Enable qemu to be socket-activated based on a spice connection.
> 
> Note that this depends on un-deprecating
> spice_server_set_listen_socket_fd, see
> https://gitlab.freedesktop.org/spice/spice/-/merge_requests/240
> 
> This partially addresses
> https://gitlab.com/qemu-project/qemu/-/issues/3011

I don't think this is showing the right approach for systemd socket
activation. Spice is rather an odd-ball part of QEMU today that isn't
using modern best practice config design, so what looks simple from a
Spice POV is not aligned with what we need for QEMU.

Historically we've long supported passing pre-opened FDs of any kind
into QEMU at launch time, but we didn't rely on systemd socket
activation. We just passed the FDs in and referenced them numerically
in the config.

At runtime we then also had the ability to pass in pre-opend FDs over
QMP, which we referenced by name.

IMHO for systemd socket activation, we want to introduce the ability
to reference sockets by name at startup, so it is aligned with what
we can do at runtime.

Primarily this comes down to having logic present in the socket_get_fd
method, such that when there is no current monitor, we use the systemd
named sockets.

We should also have logic to validate that we have consumed all
systemd sockets, before we move out of startup phase, in order to
detect config errors. This indicates should we proactively parse
the socket activation env at starutp and record all FDs, and keep
track of which are consumed by the config.

Meanwhile both -vnc and -spice need updating to have their CLI
modelled in QAPI, and use the SocketAdddress struct config, which
would unlock FD passing both with & without systemd socket activation.

And then there are some hard questions about how we integrate this with
the various helper programs like qemu-nbd, and friends, which all
already support systemd socket activation but fail to validate the
names, making it hard to add propert support while retaining back compat.

This is all a major amount of work, which is partly why we've not addressed
this yet, despite it being on the wishlist for years already


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 3/3] Socket activation: enable spice listener.

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 5208 bytes --]

Hi Daniel--

Thanks for the followup and the background.  What you say makes sense to
me, but i don't know enough about the plumbing to know how i would go
about trying to help make it happen.  I ask a few questions below for
hints on how i might move forward.

On Tue 2025-07-01 10:38:02 +0100, Daniel P. Berrangé wrote:
> Spice is rather an odd-ball part of QEMU today that isn't using modern
> best practice config design, so what looks simple from a Spice POV is
> not aligned with what we need for QEMU.

Fair enough, i'd be happy to put the spice work on the back burner and
get the systemd socket activation by named file descriptor normalized
first.

> IMHO for systemd socket activation, we want to introduce the ability
> to reference sockets by name at startup, so it is aligned with what
> we can do at runtime.

Agreed, this makes sense to me.

> Primarily this comes down to having logic present in the socket_get_fd
> method, such that when there is no current monitor, we use the systemd
> named sockets.

I'm not sure how the monitor plays a role here.  Are you talking about
having a socket-activated monitor specifically, or do you see the
monitor playing some special role in socket activation beyond just being
yet another file descriptor that might be passed in via systemd-style
supervision?  why wouldn't it be OK to have a monitor *and* use systemd
named sockets? Looking at socket_get_fd, i see that it's resolving the
fdstr differently if there is a current monitor; is that codepath only
active after all command-line arguments have been processed?

> We should also have logic to validate that we have consumed all
> systemd sockets, before we move out of startup phase, in order to
> detect config errors. This indicates should we proactively parse
> the socket activation env at starutp and record all FDs, and keep
> track of which are consumed by the config.

Could you be a bit more specific about how the "startup phase" is
delimited within the codebase?  I'd be happy to try to build out
something similar to what you describe here, if you think that would be
useful.  I'd want to make sure i place the check for all passed file
descriptors being accounted for in the right place.

> Meanwhile both -vnc and -spice need updating to have their CLI
> modelled in QAPI, and use the SocketAdddress struct config, which
> would unlock FD passing both with & without systemd socket activation.

I'm happy to leave the -vnc and -spice alone for now, if we can get
name-based socket activation normalized and stable.

> And then there are some hard questions about how we integrate this with
> the various helper programs like qemu-nbd, and friends, which all
> already support systemd socket activation but fail to validate the
> names, making it hard to add propert support while retaining back compat.

Understood -- from looking at the sources i think that means this
specific list of four helper programs:

 - qemu-nbd (network block device server)
 - qemu-ga (the qemu guest agent)
 - qemu-pr-helper (qemu SCSI persistent reservation helper)
 - qemu-vmsr-helper (i386 only?)

all of these processes currently just accept a single listening socket,
and ignore the names.  They all abort if they are passed more than one
socket via systemd-style supervision.  With the exception of qga, they
all abort with an error if they are passed listener configuration
information while also being launched under systemd-style supervision
with a socket.

I don't see this narrow scope of functionality as being a difficult to
maintain for backward-compatibility.

We can simply offer a mechanism that these tools can use with the
semantics of "if only one socket-activated listener, claim it".

As far as i can tell, none of these four helper daemons is designed to
listen on more than one socket anyway.

          --dkg

PS in the course of thinking through this patch, one alternate approach
   did occur to me, but i'm not inclined to follow it as it might be too
   radical.  I thought i'd note it here anyway, in case anyone thinks
   it's interesting, or (alternately) wants to definitively close the
   door on it.

   The idea is that rather than just being able to pass a file
   descriptor by name anywhere that you can pass a file descriptor by
   number, qemu could use the name of the file descriptor to decide what
   to do with it.

   So, for example, rather than running:

      qemu -chardev socket,id=foo,opt=123,server=on ...

   the administrator could set up a systemd .socket file with:

      FileDescriptorName=id=foo,opt=123

   and have the corresponding systemd .service file would launch:

      ExecStart=/usr/bin/qemu ...

   One of the logistical challenges for that is that the colon (":")
   isn't permitted in FileDescriptorName, and some qemu options might
   want a colon in them.  And, this approach with -chardev doesn't
   necessarily translate well to all the various places that might also
   want a file descriptor (e.g. -incoming, -object, -spice, etc).

   So like i said, probably too radical, but i thought i'd mention it.

Re: [PATCH 3/3] Socket activation: enable spice listener.

[Daniel P. Berrangé]

On Tue, Jul 01, 2025 at 02:02:05PM -0400, Daniel Kahn Gillmor wrote:
> On Tue 2025-07-01 10:38:02 +0100, Daniel P. Berrangé wrote:
> > Primarily this comes down to having logic present in the socket_get_fd
> > method, such that when there is no current monitor, we use the systemd
> > named sockets.
> 
> I'm not sure how the monitor plays a role here.  Are you talking about
> having a socket-activated monitor specifically, or do you see the
> monitor playing some special role in socket activation beyond just being
> yet another file descriptor that might be passed in via systemd-style
> supervision?  why wouldn't it be OK to have a monitor *and* use systemd
> named sockets? Looking at socket_get_fd, i see that it's resolving the
> fdstr differently if there is a current monitor; is that codepath only
> active after all command-line arguments have been processed?

The monitor is involved in so much as the socket_get_fd() method
is what internal QEMU code uses to resolve FD names into FD numbers.

The first thing it does is check whether the caller is runing in
the context of a monitor command. If so, it resolves FD names using
the monitor, otherwise it'll assume an FD has been passed in by the
parent process.  The latter codepath is the one we need to enhance
to use systemd named FDs. Essentially this latter codepath will only
run during initial QEMU startup, as at runtimne, all callers will
be in the context of the monitor.

> > We should also have logic to validate that we have consumed all
> > systemd sockets, before we move out of startup phase, in order to
> > detect config errors. This indicates should we proactively parse
> > the socket activation env at starutp and record all FDs, and keep
> > track of which are consumed by the config.
> 
> Could you be a bit more specific about how the "startup phase" is
> delimited within the codebase?  I'd be happy to try to build out
> something similar to what you describe here, if you think that would be
> useful.  I'd want to make sure i place the check for all passed file
> descriptors being accounted for in the right place.

In system/vl.c  the 'qemu_init' method takes care of system
startup - so at the very end of that method we should check
for unclaimed FDs.

> > And then there are some hard questions about how we integrate this with
> > the various helper programs like qemu-nbd, and friends, which all
> > already support systemd socket activation but fail to validate the
> > names, making it hard to add propert support while retaining back compat.
> 
> Understood -- from looking at the sources i think that means this
> specific list of four helper programs:
> 
>  - qemu-nbd (network block device server)
>  - qemu-ga (the qemu guest agent)
>  - qemu-pr-helper (qemu SCSI persistent reservation helper)
>  - qemu-vmsr-helper (i386 only?)
> 
> all of these processes currently just accept a single listening socket,
> and ignore the names.  They all abort if they are passed more than one
> socket via systemd-style supervision.  With the exception of qga, they
> all abort with an error if they are passed listener configuration
> information while also being launched under systemd-style supervision
> with a socket.
> 
> I don't see this narrow scope of functionality as being a difficult to
> maintain for backward-compatibility.
> 
> We can simply offer a mechanism that these tools can use with the
> semantics of "if only one socket-activated listener, claim it".
> 
> As far as i can tell, none of these four helper daemons is designed to
> listen on more than one socket anyway.

Well that's their current impl, but conceptually it is interesting
to be able to listen on multiple sockets, so that machines with
multiple public facing IP addresses can be made to listen on a
selection of IPs, instead of the wildcard address, or a single
address. So it is an interesting facility to gain if it is easy.

>    The idea is that rather than just being able to pass a file
>    descriptor by name anywhere that you can pass a file descriptor by
>    number, qemu could use the name of the file descriptor to decide what
>    to do with it.
> 
>    So, for example, rather than running:
> 
>       qemu -chardev socket,id=foo,opt=123,server=on ...
> 
>    the administrator could set up a systemd .socket file with:
> 
>       FileDescriptorName=id=foo,opt=123
> 
>    and have the corresponding systemd .service file would launch:
> 
>       ExecStart=/usr/bin/qemu ...
> 
>    One of the logistical challenges for that is that the colon (":")
>    isn't permitted in FileDescriptorName, and some qemu options might
>    want a colon in them.  And, this approach with -chardev doesn't
>    necessarily translate well to all the various places that might also
>    want a file descriptor (e.g. -incoming, -object, -spice, etc).
> 
>    So like i said, probably too radical, but i thought i'd mention it.

It gets even more troublesome when we consider our long term direction
is to support JSON documents for command line args. The plain FD names
are a good decoupling of CLI syntax from the service manager.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 1/3] Socket Activation: stash $LISTEN_FDNAMES

[Eric Blake]

On Sun, Jun 29, 2025 at 12:03:59AM -0400, Daniel Kahn Gillmor wrote:
> By recording a copy of LISTEN_FDNAMES, we make it possible to learn
> mappings from file descriptor labels (e.g., as set by
> FileDescriptorName= in systemd.socket(5)).
> 

Just now noticing that this hasn't been reviewed yet...

In general, it's okay to ping the list after a week or so if you feel
like your patch has been overlooked.

> This also makes it possible to invoke check_socket_activation() more
> than once and have it return the same value each time.
> 
> This is one step toward addressing
> https://gitlab.com/qemu-project/qemu/-/issues/3011
> 
> Since we can't count on the buffer returned from getenv
> persisting (getenv is documented as non-re-entrant), we need to keep a
> copy of it around in case multiple subsystems want to interrogate it.

getenv()'s result is non-reliable IF the application is calling
setenv()/putenv() to be changing the environment.  But if you treat
the environment as a read-only block of memory, then the getenv()
result has a lifetime equivalent to the entire process.  And a quick
grep:

$ git grep '\b[sp][eu]tenv('
tests/tcg/multiarch/linux/linux-sigrtminmax.c:        setenv("QEMU_RTSIG_MAP", rt_sigmap, 0);
util/envlist.c: * than putenv(3).

shows that we aren't generally modifying qemu's environ.  Okay, there
ARE a few calls to unsetenv() in util/systemd.c, but that's the file
you're modifying, and before this patch it was already using getenv()
without worrying about copying its results.  We _do_ need to modify
the environment after reading it, at which point, you might be right
that calling getenv(), then unsetenv(), then relying on the former
pointer may be unsafe; but if this is the only file manipulating the
environment we have quite a bit of control over safety aspects, and
may still be able to design something that doesn't have to copy
getenv() results.

> 
> This proposed implementation uses a static buffer, and breaks socket
> activation with a visible error_report if LISTEN_FDNAMES is too large.
> Another approach would be to g_strdup the value returned by getenv,
> which would have failure modes on heap exhaustion, and would introduce
> a memory leak as there's no clear opportunity to g_free the copy.

A single g_strdup at process startup is not a memory leak, even if it
doesn't get explicitly freed, if you can't trigger it more than once.
An inability to g_free the copy is not fatal, as long as you are not
repeatedly duplicating the string each time a different caller wants
to also check what you already learned from getenv() on the first
caller.


> 
> Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
> ---
>  util/systemd.c | 33 +++++++++++++++++++++++++++++----
>  1 file changed, 29 insertions(+), 4 deletions(-)

> 
> diff --git a/util/systemd.c b/util/systemd.c
> index ced518f771..1eca2bd69f 100644
> --- a/util/systemd.c
> +++ b/util/systemd.c
> @@ -16,37 +16,62 @@
>  #include "qemu/error-report.h"
>  
>  #ifndef _WIN32
> +static char fdnames[256];

Does systemd guarantee this particular size?  Should it be a #define?

> +
>  unsigned int check_socket_activation(void)
>  {
> +    static unsigned int nr_fds = -1;
>      const char *s;
>      unsigned long pid;
> -    unsigned long nr_fds;
> +    unsigned long nr_fdsl;
>      unsigned int i;
>      int fd;
>      int f;
>      int err;
>  
> +    if (nr_fds != -1) {
> +        return nr_fds;
> +    }
>      s = getenv("LISTEN_PID");
>      if (s == NULL) {
> +        nr_fds = 0;
>          return 0;
>      }
>      err = qemu_strtoul(s, NULL, 10, &pid);
>      if (err) {
> +        nr_fds = 0;
>          return 0;
>      }
>      if (pid != getpid()) {
> +        nr_fds = 0;
>          return 0;
>      }
>  
>      s = getenv("LISTEN_FDS");
>      if (s == NULL) {
> +        nr_fds = 0;
>          return 0;
>      }
> -    err = qemu_strtoul(s, NULL, 10, &nr_fds);
> +    err = qemu_strtoul(s, NULL, 10, &nr_fdsl);
>      if (err) {
> +        nr_fds = 0;
>          return 0;
>      }
> -    assert(nr_fds <= UINT_MAX);
> +    assert(nr_fdsl <= UINT_MAX);

I can put a value in the environment for LISTEN_FDS that will trip
this assertion, on a 64-bit platform.  While an early exit due to
garbage in the environment is okay, it should probably be cleaner than
by an assertion failure.

> +    nr_fds = (unsigned int) nr_fdsl;
> +    s = getenv("LISTEN_FDNAMES");
> +    if (s != NULL) {
> +        size_t fdnames_len = strlen(s);
> +        if (fdnames_len + 1 > sizeof(fdnames)) {
> +            error_report("LISTEN_FDNAMES is larger than %ldu bytes, "
> +                         "ignoring socket activation.",

error_report() should not use trailing '.'.  A g_strdup'd buffer won't
suffer from this arbitrary length limitation.

> +                         sizeof(fdnames));
> +            nr_fds = 0;
> +            return 0;
> +        } else {
> +            memcpy(fdnames, s, fdnames_len + 1);
> +        }
> +    }
>  
>      /* So these are not passed to any child processes we might start. */
>      unsetenv("LISTEN_FDS");
> @@ -69,7 +94,7 @@ unsigned int check_socket_activation(void)
>          }
>      }
>  
> -    return (unsigned int) nr_fds;
> +    return nr_fds;
>  }
>  
>  #else /* !_WIN32 */
> -- 
> 2.47.2
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH v2 2/3] Socket activation: get FD by label

[Eric Blake]

On Sun, Jun 29, 2025 at 12:04:00AM -0400, Daniel Kahn Gillmor wrote:
> This uses the cached copy of LISTEN_FDNAMES to find the first file
> descriptorlabel with a matching label.
> 
> Note that if two file descriptors are given the same label this will
> ignore all but the first.
> 
> This is another step toward addressing
> https://gitlab.com/qemu-project/qemu/-/issues/3011
> 
> Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
> ---
>  include/qemu/systemd.h | 15 +++++++++++++++
>  util/systemd.c         | 35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+)
> 

> +/*
> + * Check if socket activation indicates named file descriptor based on
> + * the colon-delimited LISTEN_FDNAMES.  The "label" must not be NULL,
> + * and should be a simple text string that does not contain a colon,
> + * matching the FileDescriptorName= directive in systemd.socket(5)
> + *
> + * It is acceptable to ask for the empty string as a label.
> + *
> + * Returns -1 if no socket activation is in use, or if the label does
> + * not match any file descriptor.  Otherwise, returns the lowest
> + * numeric value for a file descriptor matching the label exactly.

Per this description...

>  #else /* !_WIN32 */
>  unsigned int check_socket_activation(void)
>  {
>      return 0;
>  }
> +unsigned int socket_activated_fd_by_label(const char *label)
> +{
> +    return 0;
> +}
>  #endif

this stub should probably return -1, not 0.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org