search for: nbd_internal_free_string_list

v1 was posted here: https://www.redhat.com/archives/libguestfs/2019-September/thread.html#00337 v2: - Drop the first patch. - Hopefully fix the multiple issues with fork-safety and general behaviour on error paths. Note this requires execvpe for which there seems to be no equivalent on FreeBSD, except some kind of tedious path parsing (but can we assign to environ?) Rich.

...ctivation_environment (); > + if (!env) { > + SET_NEXT_STATE (%.DEAD); > + close (s); > + return 0; > + } > + > + pid = fork (); > + if (pid == -1) { > + SET_NEXT_STATE (%.DEAD); > + set_error (errno, "fork"); > + close (s); > + nbd_internal_free_string_list (env); > + return 0; > + } > + if (pid == 0) { /* child - run command */ > + if (s != FIRST_SOCKET_ACTIVATION_FD) { > + dup2 (s, FIRST_SOCKET_ACTIVATION_FD); > + close (s); > + } > + else { > + /* We must unset CLOEXEC on the fd. (dup...

..., 11) == 0 || + strncmp (env[i], "LISTEN_FDS=", 11) == 0) { + memmove (&env[i], &env[i+1], + sizeof (char *) * (nbd_internal_string_list_length (&env[i]))); + i--; + } + } + + return env; + + err: + set_error (errno, "malloc"); + nbd_internal_free_string_list (env); + free (p0); + free (p1); + return NULL; +} + +STATE_MACHINE { + CONNECT_SA.START: +#ifdef HAVE_EXECVPE + size_t len; + int s; + struct sockaddr_un addr; + char **env; + pid_t pid; + int flags; + + assert (!h->sock); + assert (h->argv); + assert (h->argv[0]); + + /* Use...

...ding 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. > >+ if (h->argv) > >+ nbd_internal_free_string_list (h->argv); > > How can h->argv ever be previously set? Probably not right now, but it might happen if we ever implement error recovery for either nbd_connect_socket_activation or nbd_connect_command. At the moment these functions move the handle to the DEAD state if they fail, but th...

...dex bdb0e83..31bc3d4 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -384,5 +384,7 @@ extern void nbd_internal_hexdump (const void *data, size_t len, FILE *fp); extern size_t nbd_internal_string_list_length (char **argv); extern char **nbd_internal_copy_string_list (char **argv); extern void nbd_internal_free_string_list (char **argv); +extern const char *nbd_internal_fork_safe_itoa (long v, char *buf, size_t len); +extern void nbd_internal_fork_safe_perror (const char *s); #endif /* LIBNBD_INTERNAL_H */ diff --git a/lib/utils.c b/lib/utils.c index 2d7fbf3..644781b 100644 --- a/lib/utils.c +++ b/lib/utils.c @@ -...

This adds coalescing of adjacent extents of the same type, as mentioned by Eric Blake in the commit message here: https://github.com/libguestfs/libnbd/commit/46072f6611f80245846a445766da071e457b00cd The patch series is rather long because it detours through adding the <vector.h> library from nbdkit into libnbd and replacing ad hoc uses of realloc, char ** etc in various places. Rich.

..., cmd_connect_command); } + +int +nbd_unlocked_aio_connect_socket_activation (struct nbd_handle *h, char **argv) +{ + char **copy; + + copy = nbd_internal_copy_string_list (argv); + if (!copy) { + set_error (errno, "copy_string_list"); + return -1; + } + + if (h->argv) + nbd_internal_free_string_list (h->argv); + h->argv = copy; + + return nbd_internal_run (h, cmd_connect_sa); +} diff --git a/lib/handle.c b/lib/handle.c index 5ad818e..18cc8d0 100644 --- 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); fre...

If the user calls nbd_kill_subprocess, we shouldn't kill the process again when we close the handle (since the process has likely gone and we might be killing a different process). --- lib/handle.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/handle.c b/lib/handle.c index 2af25fe..5ad818e 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -315,6 +315,8 @@

...set_error (EINVAL, "meta context request out of range"); + return NULL; + } + + ret = strdup (h->request_meta_contexts[i]); + if (ret == NULL) + set_error (errno, "strdup"); + + return ret; +} + +int +nbd_unlocked_clear_meta_contexts (struct nbd_handle *h) +{ + nbd_internal_free_string_list (h->request_meta_contexts); + h->request_meta_contexts = NULL; + return 0; +} + int nbd_unlocked_set_request_structured_replies (struct nbd_handle *h, bool request) diff --git a/tests/meta-base-allocation.c b/tests/meta-base-allocation.c index...

...@ nbd_close (struct nbd_handle *h) free_cmd_list (h->cmds_to_issue); free_cmd_list (h->cmds_in_flight); free_cmd_list (h->cmds_done); + + /* Any remaining free callbacks indicate an error. */ + if (h->nr_free_callbacks != 0) + abort (); + free (h->free_callbacks); + nbd_internal_free_string_list (h->argv); free (h->unixsocket); free (h->hostname); diff --git a/lib/internal.h b/lib/internal.h index 301b798..d8b0eed 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -47,6 +47,7 @@ struct meta_context; struct socket; struct command; +struct free_callback; struct nbd_hand...

...> +} > diff --git a/lib/connect.c b/lib/connect.c > index f98bcdb..c1cbef7 100644 > --- a/lib/connect.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); Are we sure that SIGTERM Is always going to be sufficient? Or do we need a tiered approach where we try SIGTERM, but followup with SIGKILL if too much time elapses? I guess it...

This isn't quite finished because not all of the tests or examples have been updated, but it demonstrates an idea: Should we forget about the concept of having multiple connections managed under a single handle? In this patch there is a single ‘struct nbd_handle *’ which manages a single state machine and connection (and therefore no nbd_connection). To connect to a multi-conn server you must

I'm posting this now, as I'm at the end of a workday and I got things working for manual experimentation. Still to do: - write interop tests for qemu-nbd and nbdkit (including my proposed patch addition of qemu-nbd -A to show qemu:allocation-depth) - figure out if we can make 'nbdinfo --map' use the new API to automatically select all contexts advertised by the server Eric Blake

As proposed here: https://www.redhat.com/archives/libguestfs/2019-August/msg00130.html I didn't actually read Eric's replies to that yet because I've been concentrating on writing these patches all day. Anyway here they are and I'll look at what Eric said about the proposal next. Rich.