On Mon, Sep 30, 2019 at 12:15:41PM -0500, Eric Blake wrote:
diff --git a/server/captive.c b/server/captive.c index 90e42050..1606eb1a 100644 --- a/server/captive.c +++ b/server/captive.c @@ -54,7 +54,7 @@ run_command (void) FILE *fp; char *cmd = NULL; size_t len = 0; - int r; + int r, status; pid_t pid;
if (!run) @@ -135,20 +135,43 @@ run_command (void)
if (pid > 0) { /* Parent process is the run command. */ r = system (cmd); - if (WIFEXITED (r)) + if (r == -1) { + nbdkit_error ("failure to execute external command: %m"); + r = 1;
Although this is copying what the old code did, I wonder if we should use r = EXIT_FAILURE here? In any case the series is fine and I tested it, so: ACK series I don't know if you've noticed but because our tests use MALLOC_PERTURB_ and because we have a race along the shutdown path, some tests cause coredumps, eg: $ make check $ coredumpctl list Mon 2019-09-30 21:20:45 BST 310684 1000 1000 6 present /home/rjones/d/nbdkit/server/nbdkit Mon 2019-09-30 21:21:07 BST 317022 1000 1000 11 present /home/rjones/d/nbdkit/server/nbdkit Mon 2019-09-30 21:21:08 BST 317217 1000 1000 11 present /home/rjones/d/nbdkit/server/nbdkit Mon 2019-09-30 21:21:16 BST 323530 1000 1000 11 present /home/rjones/d/nbdkit/server/nbdkit Mon 2019-09-30 21:21:17 BST 322363 1000 1000 11 present /home/rjones/d/nbdkit/server/nbdkit Mon 2019-09-30 21:21:19 BST 325833 1000 1000 11 present /home/rjones/d/nbdkit/server/nbdkit Mon 2019-09-30 21:21:22 BST 327989 1000 1000 11 present /home/rjones/d/nbdkit/server/nbdkit Mon 2019-09-30 21:21:22 BST 328290 1000 1000 11 present /home/rjones/d/nbdkit/server/nbdkit Mon 2019-09-30 21:21:23 BST 328520 1000 1000 11 present /home/rjones/d/nbdkit/server/nbdkit Mon 2019-09-30 21:21:24 BST 328914 1000 1000 11 present /home/rjones/d/nbdkit/server/nbdkit $ coredumpctl gdb [...] Program terminated with signal SIGSEGV, Segmentation fault. #0 0x0000565098005718 in filter_finalize (b=0x5650a0693500, conn=0x5650a0694940) at filters.c:440 440 return b->next->finalize (b->next, conn); [Current thread is 1 (Thread 0x7fdacb08c700 (LWP 328918))] Missing separate debuginfos, use: dnf debuginfo-install glibc-2.30-4.fc31.x86_64 gmp-6.1.2-10.fc31.x86_64 gnutls-3.6.9-1.fc31.x86_64 libffi-3.1-23.fc31.x86_64 libgcc-9.1.1-2.fc31.1.x86_64 libidn2-2.2.0-2.fc31.x86_64 libselinux-2.9-5.fc31.x86_64 libtasn1-4.14-2.fc31.x86_64 libunistring-0.9.10-6.fc31.x86_64 nettle-3.5.1-3.fc31.x86_64 p11-kit-0.23.16.1-2.fc31.x86_64 pcre2-10.33-12.fc31.x86_64 (gdb) bt #0 0x0000565098005718 in filter_finalize (b=0x5650a0693500, conn=0x5650a0694940) at filters.c:440 #1 0x000056509800316c in _handle_single_connection (sockout=<optimized out>, sockin=<optimized out>) at connections.c:220 #2 handle_single_connection (sockin=<optimized out>, sockout=<optimized out>) at connections.c:239 #3 0x000056509800c7be in start_thread (datav=0x5650a06756f0) at sockets.c:276 #4 0x00007fdacb6dd4e2 in start_thread () from /lib64/libpthread.so.0 #5 0x00007fdacb60c643 in clone () from /lib64/libc.so.6 (This has happened for a long time and has nothing to do with this patch series.) I thought that this series might expose some of these crashes and cause test failures, but it seems as if it does not. So I guess that these shutdown failures are not happening with --run, but in test cleanup functions instead. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/