VDDK plugin + --run was broken because of the following sequence of
events:
(1) After .config_complete, server redirects stdin/stdout to /dev/null.
(2) Server then calls vddk_get_ready which reexecs.
(3) We restart the server from the top, but with stdin/stdout
redirected to /dev/null. So saved_stdin/saved_stdout save
/dev/null.
(4) run_command is called which "restores" /dev/null as stdin/stdout.
(5) The output of the --run option is sent to /dev/null.
In addition to fixing this problem with VDDK, it also makes general
sense not to redirect stdin/stdout before calling .get_ready since
this callback is supposed to be the last chance before the server
daemonizes, and redirecting stdin/stdout is part of daemonization.
Before this commit:
$ ./nbdkit vddk libdir=tests/.libs /dev/null --run 'qemu-img info $nbd'
[no output]
After this commit:
$ ./nbdkit vddk libdir=tests/.libs /dev/null --run 'qemu-img info $nbd'
image: nbd://localhost:10809
file format: raw
virtual size: 512 KiB (524288 bytes)
disk size: unavailable
One test (test-stdio.sh) made the assumption that stdin/stdout had
already been redirected to /dev/null in .get_ready. I adjusted this
test so it checks for redirection in .after_fork instead.
---
server/main.c | 10 +++++-----
tests/test-stdio-plugin.c | 9 +++++++++
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/server/main.c b/server/main.c
index 2f0aaf07..11c1614b 100644
--- a/server/main.c
+++ b/server/main.c
@@ -693,6 +693,11 @@ main (int argc, char *argv[])
/* Select the correct thread model based on config. */
lock_init_thread_model ();
+ /* Tell the plugin that we are about to start serving. This must be
+ * called before we change user, fork, or open any sockets.
+ */
+ top->get_ready (top);
+
/* Sanitize stdin/stdout to /dev/null, after saving the originals
* when needed. We are still single-threaded at this point, and
* already checked that stdin/out were open, so we don't have to
@@ -726,12 +731,7 @@ main (int argc, char *argv[])
perror ("open");
exit (EXIT_FAILURE);
}
-
- /* Tell the plugin that we are about to start serving. This must be
- * called before we change user, fork, or open any sockets.
- */
configured = true;
- top->get_ready (top);
start_serving ();
diff --git a/tests/test-stdio-plugin.c b/tests/test-stdio-plugin.c
index 618eae83..86447278 100644
--- a/tests/test-stdio-plugin.c
+++ b/tests/test-stdio-plugin.c
@@ -122,6 +122,14 @@ stdio_config_complete (void)
static int
stdio_get_ready (void)
+{
+ bool check = stdio_check ();
+ assert (check == false);
+ return 0;
+}
+
+static int
+stdio_after_fork (void)
{
bool check = stdio_check ();
assert (check == true);
@@ -163,6 +171,7 @@ static struct nbdkit_plugin plugin = {
.config = stdio_config,
.config_complete = stdio_config_complete,
.get_ready = stdio_get_ready,
+ .after_fork = stdio_after_fork,
.open = stdio_open,
.get_size = stdio_get_size,
.pread = stdio_pread,
--
2.27.0