Re: [Libguestfs] [libnbd PATCH v3 19/19] socket activation: set LISTEN_FDNAMES

>> @@ -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 ""). Good observation; this is a functionality gap that goes back to v1 of this patch. (I've not investigated the specifics of systemd socket activation before; but see below.) > 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. For understanding your point, I have had to read up on systemd socket activation: https://www.freedesktop.org/software/systemd/man/sd_listen_fds.html Let me quote a part: > Under specific conditions, the following automatic file descriptor names are returned: > > [...] > "unknown" -- The process received no name for the specific file > descriptor from the service manager. > [...] I've also checked the implementation of sd_listen_fds_with_names(): https://github.com/systemd/systemd/blob/main/src/libsystemd/sd-daemon/sd-...

(2) If we pass LISTEN_PID=xxx and LISTEN_FDS=1, and just "pass through" an *inherited* LISTEN_FDNAMES variable, then it will (in the general case) confuse the nbd server that we start. Namely, if LISTEN_FDNAMES has multiple colon-separated elements (more than 1), or is the empty string (= 0 elements), then sd_listen_fds_with_names() in the nbd server will fail the "n_names != n_fds" check, and return (-EINVAL). If LISTEN_FDNAMES happens to have one element, then sd_listen_fds_with_names() will succeed, but the returned name will confuse the nbd server.