On 1/30/23 23:55, Richard W.M. Jones 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.
> ---
> 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;
As long as we don't use a table+loop for the special variables, this
seems OK to me.
(For patch#1, I meant to add: wherever we know string_vector_append()
can't fail, due to the reservation at the top, adding an assert() could
improve readability; the reservation can get visually quite far from the
repeated string_vector_append() calls.)
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
