Re: [Libguestfs] [PATCH libnbd v2 2/2] api: Implement local command with systemd socket activation.

On 10/1/19 9:11 AM, Richard W.M. Jones wrote: >On Tue, Oct 01, 2019 at 08:24:33AM -0500, Eric Blake wrote: >>>+#else /* !HAVE_EXECVPE */ >>>+ SET_NEXT_STATE (%.DEAD) >>>+ set_error (ENOTSUP, "platform does not support socket activation"); >>>+ return 0; >>>+#endif >> >>We probably ought to add a matching nbd_supports_socket_activation() >>feature function. >> >>Or, it would be possible to create a fallback for execvpe() on >>platforms that lack it by using execlpe() and our own path-walker >>utility function. Can be done as a followup patch. If we do that, >>then the mere presence of LIBNBD_HAVE_NBD_CONNECT_SA is witness >>enough of the functionality, rather than needing a runtime probe. > >I'm hoping we will find the time to write a replacement execvpe so >that we can implement this on all platforms. That way we can avoid >having a redundant nbd_supports_socket_activation() call that (in >future) always returns true. In fact, I just realized that it's quite easy to replace execvpe() - remember, POSIX says that execvp() sets the environment of the new process to the contents of 'environ', so all we really have to do is assign to environ after fork()ing. From 3a019cb65329f290f5be71219b2acb1c8b687a1a Mon Sep 17 00:00:00 2001 From: Eric Blake <eblake(a)redhat.com&gt; Date: Fri, 15 Nov 2019 09:22:11 -0600 Subject: [libnbd PATCH] states: Use execvp instead of execvpe execvpe is a handy GNU extension, but it is trivial to fall back to the POSIX-mandated execvp by manually assigning to environ. This allows socket activation to build on BSD systems. --- configure.ac | 4 ---- generator/states-connect-socket-activation.c | 16 +++++----------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/configure.ac b/configure.ac index 2dbe97d..9bd6f73 100644 --- a/configure.ac +++ b/configure.ac @@ -79,10 +79,6 @@ AC_CHECK_HEADERS([\ AC_CHECK_HEADERS([linux/vm_sockets.h], [], [], [#include <sys/socket.h>]) -dnl Check for functions, all optional. -AC_CHECK_FUNCS([\ - execvpe]) - dnl Check for sys_errlist (optional). AC_MSG_CHECKING([for sys_errlist]) AC_TRY_LINK([], [extern int sys_errlist; char *p = &sys_errlist;], [ diff --git a/generator/states-connect-socket-activation.c b/generator/states-connect-socket-activation.c index 243ba36..ee08dff 100644 --- a/generator/states-connect-socket-activation.c +++ b/generator/states-connect-socket-activation.c @@ -36,7 +36,9 @@ /* == strlen ("LISTEN_PID=") | strlen ("LISTEN_FDS=") */ #define PREFIX_LENGTH 11 -/* Prepare environment for calling execvpe when doing systemd socket +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 @@ -45,7 +47,6 @@ static char ** prepare_socket_activation_environment (void) { - extern char **environ; char **env = NULL; char *p0 = NULL, *p1 = NULL; size_t i, len; @@ -97,7 +98,6 @@ prepare_socket_activation_environment (void) STATE_MACHINE { CONNECT_SA.START: -#ifdef HAVE_EXECVPE int s; struct sockaddr_un addr; char **env; @@ -200,7 +200,8 @@ STATE_MACHINE { /* Restore SIGPIPE back to SIG_DFL. */ signal (SIGPIPE, SIG_DFL); - execvpe (h->argv[0], h->argv, env); + environ = env; + execvp (h->argv[0], h->argv); nbd_internal_fork_safe_perror (h->argv[0]); if (errno == ENOENT) _exit (127); @@ -217,11 +218,4 @@ STATE_MACHINE { memcpy (&h->connaddr, &addr, h->connaddrlen); SET_NEXT_STATE (%^CONNECT.START); return 0; - -#else /* !HAVE_EXECVPE */ - SET_NEXT_STATE (%.DEAD); - set_error (ENOTSUP, "platform does not support socket activation"); - return 0; -#endif - } /* END STATE MACHINE */