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

>+#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.

>+ if (h->argv) >+ nbd_internal_free_string_list (h->argv); How can h->argv ever be previously set?

>--- a/lib/handle.c >+++ b/lib/handle.c >@@ -129,6 +129,16 @@ nbd_close (struct nbd_handle *h) > free_cmd_list (h->cmds_in_flight); > free_cmd_list (h->cmds_done); > nbd_internal_free_string_list (h->argv); >+ if (h->sa_sockpath) { >+ if (h->pid > 0) >+ kill (h->pid, SIGTERM); >+ unlink (h->sa_sockpath); >+ free (h->sa_sockpath); >+ } >+ if (h->sa_tmpdir) { >+ rmdir (h->sa_tmpdir); >+ free (h->sa_tmpdir); >+ } > free (h->unixsocket); > free (h->hostname); > free (h->port); Somewhat pre-existing: we have a waitpid() here (good, so we don't hang on to a zombie process), but we are relying on the child process to gracefully go away (whether for connect_command when stdin closes, or for connect_sa on receipt of SIGTERM). Do we need a retry loop that escalates to SIGKILL if the child process does not quickly respond to the initial condition? On the other hand, the fact that our waitpid() blocks until the child changes status means that if a child ever wedges, the fact that we wedge too gives some visibility to the client that it's not libnbd's fault and that they need to get the bug fixed in their child process.