search for: fork_into_background

...s even work? It works in the same way as nbdkit_absolute_path() did: when calling it on a relative path, nbdkit_absolute_path() will prepend $PWD to it, while the new nbdkit_realpath() will do something similar. At least the usages that I changed are called before start_serving() (and thus before fork_into_background()), so I think there should be no issue wrt paths. Did I miss anything? -- Pino Toscano

...t = realpath (path, NULL); Wait. Does this even work? Remember, in nbdkit_absolute_path(), we are prepending get_current_dir_name() (why the GNU spelling, instead of getcwd(NULL, 0), since either way is a GNU extension?). But in main.c, we call an unconditional chdir("/") as part of fork_into_background, which has the annoying result that when using 'nbdkit -f' you honor your current $PWD, but when running in the background, relative filenames are now parsed against /. Shouldn't we fix it so that nbdkit saves off getcwd() up front, prior to fork_into_background, and then both nbd...

Introduce a new helper function to resolve a path name, calling nbdkit_error on failure: other than doing what nbdkit_absolute_path does, it also checks that the file exists (and thus avoids errors later on). To help distinguish it from nbdkit_absolute_path, improve the documentation of the latter. Apply it where an existing path is required, both in nbdkit itself and in plugins. Related to:

...would be trivial to rewrite it so that we manually prepend something cached from when the program started, so that this is then conceptually correct no matter when we change directories later. > > At least the usages that I changed are called before start_serving() > (and thus before fork_into_background()), so I think there should be no > issue wrt paths. Oh. Hmm. Interesting observation, that .config and .config_complete finish PRIOR to start_serving() (and makes sense, too, as if we're going to diagnose any errors, printing to stderr is a LOT easier at the point where we haven't...

...llow the same conventions. If the value is a relative path, then note that the server changes directory when it starts up. See L</FILENAMES AND PATHS> above. diff --git a/src/main.c b/src/main.c index 9c18d6f..c523854 100644 --- a/src/main.c +++ b/src/main.c @@ -72,6 +72,7 @@ static void fork_into_background (void); static uid_t parseuser (const char *); static gid_t parsegroup (const char *); static unsigned int get_socket_activation (void); +static int is_config_key (const char *key, size_t len); struct debug_flag *debug_flags; /* -D */ int exit_with_parent; /* --exit-with-parent */...

...llow the same conventions. If the value is a relative path, then note that the server changes directory when it starts up. See L</FILENAMES AND PATHS> above. diff --git a/src/main.c b/src/main.c index 9c18d6f..f4a800f 100644 --- a/src/main.c +++ b/src/main.c @@ -72,6 +72,7 @@ static void fork_into_background (void); static uid_t parseuser (const char *); static gid_t parsegroup (const char *); static unsigned int get_socket_activation (void); +static int is_config_key (const char *key, size_t len); struct debug_flag *debug_flags; /* -D */ int exit_with_parent; /* --exit-with-parent */...

...+++ b/server/background.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2019 Red Hat Inc. + * Copyright (C) 2019-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -70,15 +70,11 @@ fork_into_background (void) chdir ("/"); #pragma GCC diagnostic pop - /* Close stdin/stdout and redirect to /dev/null. */ - close (0); - close (1); - open ("/dev/null", O_RDONLY); - open ("/dev/null", O_WRONLY); - - /* If not verbose, set stderr to the same as stdout as well. */...

...+++ b/server/background.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2019 Red Hat Inc. + * Copyright (C) 2019-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -70,13 +70,9 @@ fork_into_background (void) chdir ("/"); #pragma GCC diagnostic pop - /* Close stdin/stdout and redirect to /dev/null. */ - close (0); - close (1); - open ("/dev/null", O_RDONLY); - open ("/dev/null", O_WRONLY); - - /* If not verbose, set stderr to the same as stdout as well. */...

This fixes the long-standing crash on close when nbdkit exits. I did try first to fix threads so we're using a proper thread pool, but that's difficult to implement. So this does the minimal change needed to fix the crash instead. There are still two segfaults that happen during running the test suite. One is deliberately caused (tests/test-captive.sh). The other appears to be an

...on stdin/stdout. */ if (listen_stdin) { change_user (); + backend->ready_to_serve (backend); write_pidfile (); threadlocal_new_server_thread (); if (handle_single_connection (0, 1) == -1) @@ -882,6 +884,7 @@ start_serving (void) run_command (); change_user (); fork_into_background (); + backend->ready_to_serve (backend); write_pidfile (); accept_incoming_connections (socks, nr_socks); free_listening_sockets (socks, nr_socks); diff --git a/server/plugins.c b/server/plugins.c index 87daaf2..bb0269d 100644 --- a/server/plugins.c +++ b/server/plugins.c @@ -158,6 +15...

This is what I've been playing with in response to my earlier question about what to do with 'nbdkit -s sh -' (https://www.redhat.com/archives/libguestfs/2020-April/msg00032.html) I'm still open to ideas on a better name, and/or whether adding <stdbool.h> to our public include files is a good idea (if not, returning int instead of bool is tolerable). Eric Blake (2):

This is an incomplete port to Windows. Currently the server compiles and starts up successfully, but goes into an infinite loop when you connect to it. Nevertheless I think the approach is ready for feedback. This being Windows the changes go quite deep. Rich.

...init) (void); char *error; @@ -792,7 +793,7 @@ open_filter_so (struct backend *next, size_t i, perror ("asprintf"); exit (EXIT_FAILURE); } - free_filename = 1; + free_filename = true; } dl = dlopen (filename, RTLD_NOW|RTLD_GLOBAL); @@ -1023,7 +1024,7 @@ fork_into_background (void) if (!verbose) dup2 (1, 2); - forked_into_background = 1; + forked_into_background = true; debug ("forked into background (new pid = %d)", getpid ()); } diff --git a/server/plugins.c b/server/plugins.c index 52dd3a5..701c18e 100644 --- a/server/plugins.c +++ b/serv...

...uot; #include "internal.h" +#define FIRST_SOCKET_ACTIVATION_FD 3 /* defined by systemd ABI */ + static char *make_random_fifo (void); static void open_plugin_so (const char *filename); static void start_serving (void); @@ -64,6 +66,7 @@ static void write_pidfile (void); static void fork_into_background (void); static uid_t parseuser (const char *); static gid_t parsegroup (const char *); +static unsigned int get_socket_activation (void); const char *exportname; /* -e */ int foreground; /* -f */ @@ -77,6 +80,7 @@ int listen_stdin; /* -s */ char *unixsoc...

The patches following do indeed allow you to compile nbdkit.exe, but it does not actually work yet. I'm posting this experimental series more as a work in progress and to get feedback. Note this does not require Windows itself to build or test. You can cross-compile it using mingw64-* packages on Fedora or Debian, and test it [spoiler alert: it fails] using Wine. Rich.

In v2: - use int instead of bool in the public header - split the tests from the code - don't overload test-layers; instead, add new tests - add a missing fflush exposed by the new tests - other minor cleanups Eric Blake (3): server: Add nbdkit_stdio_safe server: Sanitize stdin/out before running plugin code server: More tests of stdin/out handling docs/nbdkit-plugin.pod |

...8,7 @@ start_serving (void) if (listen_stdin) { change_user (); write_pidfile (); + top->after_fork (top); threadlocal_new_server_thread (); handle_single_connection (saved_stdin, saved_stdout); return; @@ -986,6 +988,7 @@ start_serving (void) change_user (); fork_into_background (); write_pidfile (); + top->after_fork (top); accept_incoming_connections (&socks); } diff --git a/server/plugins.c b/server/plugins.c index fa572a6a..285569bb 100644 --- a/server/plugins.c +++ b/server/plugins.c @@ -159,6 +159,7 @@ plugin_dump_fields (struct backend *b) HAS (c...

This is my second attempt at this. The first version (also not working) was here: https://www.redhat.com/archives/libguestfs/2019-October/msg00062.html In part 1/2 I introduce a new .ready_to_serve plugin method which is called after forking and just before accepting any client connection. The idea would be that plugins could start background threads here. However this doesn't work well in

v1 was here: https://www.redhat.com/archives/libguestfs/2018-September/msg00024.html v2: - As discussed in the patch review, tighten up the characters permitted in keys. - Update documentation to note that relative paths can be made safe by prefixing with ./ and absolute paths do not need any extra steps. - I pushed patch 1/6 from the v1 series since it was just a trivial

v1: https://www.redhat.com/archives/libguestfs/2018-September/msg00024.html v2: https://www.redhat.com/archives/libguestfs/2018-September/msg00034.html v3: - Fixed is_config_key zero length test. - Fixed is_config_key so it uses strspn and is not O(n^2). - Changed >= 1.7 to >= 1.8 in the documentation. Rich.