Libguestfs - Mar 2023 - [libnbd PATCH v3 19/19] socket activation: 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.

https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg06114.html

[Original commit message and upstream discussion reference by Rich Jones;
at
<https://listman.redhat.com/archives/libguestfs/2023-January/030558.html>
/ msgid <20230130225521.1771496-5-rjones at redhat.com>.]

Signed-off-by: Laszlo Ersek <lersek at redhat.com>
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
---

Notes:
    v4:
    
    - pick up Rich's R-b
    
    - resolve rebase conflict near the
      prepare_socket_activation_environment() call site, due to keeping
      set_error() internal to that callee, in patch "socket activation:
      clean up responsibilities of prep.sock.act.env.()"

 generator/states-connect-socket-activation.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/generator/states-connect-socket-activation.c
b/generator/states-connect-socket-activation.c
index 10ccc3119299..f9dbd433ce00 100644
--- a/generator/states-connect-socket-activation.c
+++ b/generator/states-connect-socket-activation.c
@@ -185,7 +185,7 @@  CONNECT_SA.START:
   struct sockaddr_un addr;
   struct execvpe execvpe_ctx;
   size_t num_vars;
-  struct sact_var sact_var[2];
+  struct sact_var sact_var[3];
   size_t pid_ofs;
   string_vector env;
   pid_t pid;
@@ -245,6 +245,9 @@  CONNECT_SA.START:
                  "LISTEN_PID=",
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", &pid_ofs);
   SACT_VAR_PUSH (sact_var, &num_vars,
                  "LISTEN_FDS=", "1", NULL);
+  if (h->sact_name != NULL)
+    SACT_VAR_PUSH (sact_var, &num_vars,
+                   "LISTEN_FDNAMES=", h->sact_name, NULL);
   if (prepare_socket_activation_environment (&env, sact_var, num_vars) ==
-1)
     /* prepare_socket_activation_environment() calls set_error() internally */
     goto uninit_execvpe;

On Thu, Mar 23, 2023 at 01:10:16PM +0100, Laszlo Ersek
wrote:
> 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.
> 
> https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg06114.html
> 
> [Original commit message and upstream discussion reference by Rich Jones;
> at
>
<https://listman.redhat.com/archives/libguestfs/2023-January/030558.html>
> / msgid <20230130225521.1771496-5-rjones at redhat.com>.]
> 
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
> ---
> 
> @@ -245,6 +245,9 @@  CONNECT_SA.START:
>                   "LISTEN_PID=",
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", &pid_ofs);
>    SACT_VAR_PUSH (sact_var, &num_vars,
>                   "LISTEN_FDS=", "1", NULL);
> +  if (h->sact_name != NULL)
> +    SACT_VAR_PUSH (sact_var, &num_vars,
> +                   "LISTEN_FDNAMES=", h->sact_name, NULL);
>    if (prepare_socket_activation_environment (&env, sact_var, num_vars)
== -1)
If I'm reading this correctly, this does wipe an inherited
LISTEN_FDNAMES from the environment in the case where the application
linked with libnbd started life with a (different) socket activation,
but now the user wants to connect to an nbd server without setting a
name (default usage, or explicitly requested a name of "").

Put another way, SACT_VAR_PUSH as written appears to be only additive
for replacement purposes (if I pushed a variable, I intend to override
it in the child process, so don't copy it from environ if one was
previously there), but not effective for deletion purposes (I don't
intend to set the variable, but if it is already set in environ, I
want it omitted in the child's copy).

Is there a way to rework this so that you can pass NULL as the fourth
parameter as an indication of an unset request (vs. "" when you want
it set to the empty string)?  At which point, you would drop the 'if
(h->sact_name != NULL)', and just blindly use SACT_VAR_PUSH(,
h->sact_name,).  That has ripple effects earlier in the series to
support those semantics.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org