Laszlo Ersek
2023-Apr-18 10:00 UTC
[Libguestfs] [PATCH nbdkit] server: Fix exit with parent behaviour if flag is not specified
On 4/18/23 09:25, Richard W.M. Jones wrote:> Commit 6b03ec8ad6 ("common: Move exit-with-parent code from include/ > to utils/") was supposed to be a largely neutral refactoring of the > --exit-with-parent flag. However I made a mistake in one hunk of that > patch: > > -#ifdef HAVE_EXIT_WITH_PARENT > - if (exit_with_parent) { > - if (set_exit_with_parent () == -1) { > - perror ("nbdkit: --exit-with-parent"); > - exit (EXIT_FAILURE); > - } > + if (set_exit_with_parent () == -1) { > + perror ("nbdkit: --exit-with-parent"); > + exit (EXIT_FAILURE); > } > -#endif > > by removing the test for exit_with_parent. The effect is similar to > if we always specified --exit-with-parent when nbdkit is running in > the foreground. > > Somewhat surprisingly this didn't have any noticable effect. That's > for a few of reasons: (1) Mostly if you don't fork then you should be > using --exit-with-parent (and the bulk of the tests do this). (2) If > we do fork into the background then the PR_SET_PDEATHSIG flag is > cleared by Linux across the fork. (3) It actually _did_ break Windows > because there set_exit_with_parent calls abort, but for unrelated > reasons our Windows CI was broken (and never tested running > nbdkit.exe). > > Fix this with the obvious adjustment. > > This broke nbdkit 1.32 and 1.34. > > Fixes: commit 6b03ec8ad672e5956b41cbacae9c307e5acd786b > --- > server/main.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/server/main.c b/server/main.c > index c3b9bf384..1df5d69ac 100644 > --- a/server/main.c > +++ b/server/main.c > @@ -574,9 +574,11 @@ main (int argc, char *argv[]) > /* Implement --exit-with-parent early in case plugin initialization > * takes a long time and the parent exits during that time. > */ > - if (set_exit_with_parent () == -1) { > - perror ("nbdkit: --exit-with-parent"); > - exit (EXIT_FAILURE); > + if (exit_with_parent) { > + if (set_exit_with_parent () == -1) { > + perror ("nbdkit: --exit-with-parent"); > + exit (EXIT_FAILURE); > + } > } > > /* If the user has mixed up -p/--run/-s/-U/--vsock options, thenReviewed-by: Laszlo Ersek <lersek at redhat.com>
Richard W.M. Jones
2023-Apr-18 10:09 UTC
[Libguestfs] [PATCH nbdkit] server: Fix exit with parent behaviour if flag is not specified
On Tue, Apr 18, 2023 at 12:00:22PM +0200, Laszlo Ersek wrote:> On 4/18/23 09:25, Richard W.M. Jones wrote: > > Commit 6b03ec8ad6 ("common: Move exit-with-parent code from include/ > > to utils/") was supposed to be a largely neutral refactoring of the > > --exit-with-parent flag. However I made a mistake in one hunk of that > > patch: > > > > -#ifdef HAVE_EXIT_WITH_PARENT > > - if (exit_with_parent) { > > - if (set_exit_with_parent () == -1) { > > - perror ("nbdkit: --exit-with-parent"); > > - exit (EXIT_FAILURE); > > - } > > + if (set_exit_with_parent () == -1) { > > + perror ("nbdkit: --exit-with-parent"); > > + exit (EXIT_FAILURE); > > } > > -#endif > > > > by removing the test for exit_with_parent. The effect is similar to > > if we always specified --exit-with-parent when nbdkit is running in > > the foreground. > > > > Somewhat surprisingly this didn't have any noticable effect. That's > > for a few of reasons: (1) Mostly if you don't fork then you should be > > using --exit-with-parent (and the bulk of the tests do this). (2) If > > we do fork into the background then the PR_SET_PDEATHSIG flag is > > cleared by Linux across the fork. (3) It actually _did_ break Windows > > because there set_exit_with_parent calls abort, but for unrelated > > reasons our Windows CI was broken (and never tested running > > nbdkit.exe). > > > > Fix this with the obvious adjustment. > > > > This broke nbdkit 1.32 and 1.34. > > > > Fixes: commit 6b03ec8ad672e5956b41cbacae9c307e5acd786b > > --- > > server/main.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/server/main.c b/server/main.c > > index c3b9bf384..1df5d69ac 100644 > > --- a/server/main.c > > +++ b/server/main.c > > @@ -574,9 +574,11 @@ main (int argc, char *argv[]) > > /* Implement --exit-with-parent early in case plugin initialization > > * takes a long time and the parent exits during that time. > > */ > > - if (set_exit_with_parent () == -1) { > > - perror ("nbdkit: --exit-with-parent"); > > - exit (EXIT_FAILURE); > > + if (exit_with_parent) { > > + if (set_exit_with_parent () == -1) { > > + perror ("nbdkit: --exit-with-parent"); > > + exit (EXIT_FAILURE); > > + } > > } > > > > /* If the user has mixed up -p/--run/-s/-U/--vsock options, then > > Reviewed-by: Laszlo Ersek <lersek at redhat.com>Thanks, pushed as beae44398ddd5386add34b43353ed16e2df0f30e I have nearly got all the tests passing on Wine (with my out of tree AF_UNIX patch). And in other news ... https://www.winehq.org/mailman3/hyperkitty/list/wine-devel at winehq.org/thread/CWFIHT4ZP5WG7FKFZZWO4W7L477BFEKJ/ https://bugs.winehq.org/show_bug.cgi?id=52568 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit