Re: [Libguestfs] [p2v PATCH v2 09/11] nbd.c: pass "--exit-with-parent" to nbdkit if nbdkit supports it

If virt-p2v goes away without killing nbdkit in cleanup_data_conns(), eg. it segfaults, then "--exit-with-parent" is a second chance to clean up nbdkit. Pass this option to nbdkit if nbdkit supports it. Track "nbd_exit_with_parent" internally to "nbd.c" (as static data), similarly to "nbd_local_port". It would be possible pass it up from test_nbd_server() and then pass it down to start_nbd_server(). However, that would affect (for example) the structure that the pthread entry point function start_conversion_thread() [gui.c] accepts. Right now, that structure is "struct config", which is auto-generated (and exposed to multiple consumers in various formats). "nbd_exit_with_parent" does not belong there -- thus, we'd need to introduce a wrapper structure just for this, which is overkill. Ref: https://listman.redhat.com/archives/libguestfs/2022-March/028475.html Suggested-by: Richard W.M. Jones <rjones(a)redhat.com&gt; Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- Notes: v2: - rewrite the patch completely [Rich] - keep the tracking by PID - detect whether nbdkit supports "--exit-with-parent" - include justification for "--exit-with-parent" in the commit message nbd.c | 22 ++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/nbd.c b/nbd.c index e30b4a2195cb..b836e50aa03d 100644 --- a/nbd.c +++ b/nbd.c @@ -44,6 +44,9 @@ */ static int nbd_local_port; +/* Whether nbdkit recognizes "--exit-with-parent". */ +static bool nbd_exit_with_parent; + static pid_t start_nbdkit (const char *device, int *fds, size_t nr_fds); static int open_listening_socket (int **fds, size_t *nr_fds); static int bind_tcpip_socket (const char *port, int **fds, size_t *nr_fds); @@ -86,44 +89,52 @@ void test_nbd_server (void) { int r; /* Initialize nbd_local_port. */ if (is_iso_environment) /* The p2v ISO should allow us to open up just about any port, so * we can fix a port number in that case. Using a predictable * port number in this case should avoid rare errors if the port * colides with another (ie. it'll either always fail or never * fail). */ nbd_local_port = 50123; else /* When testing on the local machine, choose a random port. */ nbd_local_port = 50000 + (random () % 10000); #if DEBUG_STDERR fprintf (stderr, "checking for nbdkit ...\n"); #endif r = system ("nbdkit file --version" #ifndef DEBUG_STDERR " >/dev/null 2>&1" #endif ); if (r != 0) { fprintf (stderr, _("%s: nbdkit was not found, cannot continue.\n"), g_get_prgname ()); exit (EXIT_FAILURE); } + r = system ("nbdkit --exit-with-parent --version" +#ifndef DEBUG_STDERR + " >/dev/null 2>&1" +#endif + ); + nbd_exit_with_parent = (r == 0); + #if DEBUG_STDERR - fprintf (stderr, "found nbdkit\n"); + fprintf (stderr, "found nbdkit (%s exit with parent)\n", + nbd_exit_with_parent ? "can" : "cannot"); #endif } /** * Start nbdkit. * * We previously tested nbdkit (see C<test_nbd_server>). * * Returns the process ID (E<gt> 0) or C<0> if there is an error. */ @@ -187,50 +198,57 @@ static pid_t start_nbdkit (const char *device, int *fds, size_t nr_fds) { pid_t pid; CLEANUP_FREE char *file_str = NULL; #if DEBUG_STDERR fprintf (stderr, "starting nbdkit for %s using socket activation\n", device); #endif if (asprintf (&file_str, "file=%s", device) == -1) error (EXIT_FAILURE, errno, "asprintf"); pid = fork (); if (pid == -1) { set_nbd_error ("fork: %m"); return 0; } if (pid == 0) { /* Child. */ + const char *nofork_opt; + close (0); if (open ("/dev/null", O_RDONLY) == -1) { perror ("open: /dev/null"); _exit (EXIT_FAILURE); } socket_activation (fds, nr_fds); + nofork_opt = nbd_exit_with_parent ? + "--exit-with-parent" : /* don't fork, and exit when the parent + * thread does */ + "-f"; /* don't fork */ + execlp ("nbdkit", "nbdkit", "-r", /* readonly (vital!) */ - "-f", /* don't fork */ + nofork_opt, "file", /* file plugin */ file_str, /* a device like file=/dev/sda */ NULL);