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>
Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
---
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);
I think this patch is reasonable and beneficial.
Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com>
Rich.
--
Richard Jones, Virtualization Group, Red Hat