Re: [PATCH] BUG/MINOR: when master-worker is in daemon mode, detach from tty
On Sat, Dec 02, 2017 at 01:53:14PM +0100, William Lallemand wrote: > I made a few comments inside the patch. thanks for the review, I've applied it according to your comments. Willy
Re: [PATCH] BUG/MINOR: when master-worker is in daemon mode, detach from tty
On Wed, Nov 29, 2017 at 10:26:06PM +0100, PiBa-NL wrote:
> Hi William,
>
Hi,
> FDs for the master-worker pipe can still be 0 and 1 if running in quiet
> mode as the stdin/stdout/stderr are still closed before creating the
> pipe then. Should the pipe be created earlier?
Well, thinking about it that's not a real problem, those FDs will be reused
anyway, and your other fix allows to have a FD 0, so that's fine.
> I've moved the code to just before the mworker_wait() in new attached
> patch. This should allow (all?) possible warnings to be output before
> closing stdX, and still 'seems' to work properly..
Good idea.
> > we also need to rely on
> > setsid() to do a proper tty detach.
> I've added a setsid(), but i must admit i have no clue what its doing
> exactly...
It creates a new session, and make the new process the leader of this
new session, detaching it from the terminal.
That's the common way to do a daemon, but I just notice that we still do a
setsid() in the children in mworker mode, we should share the same session with
the master. So maybe we can fix that in another patch!
> [...]
> I'm not sure if the attached patch is OK for you like this, or needs to
> be implemented completely differently.
> I have made and tried to test the changed patch with above cases but am
> sure there are many things / combinations with other features i have not
> included..
Looks fine to me, we can't use the same code for the -D and the -D + -W, so
that's good!
> If i need to change it slightly somehow please let me know, if you need
> time to look into it further, i can certainly wait :) i do not 'need'
> the feature urgently or perhaps won't need it at all..
>
I made a few comments inside the patch.
> Anyhow when you have time to look into it, i look forward to your
> feedback :) . Thanks in advance.
>
> Regards,
> PiBa-NL / Pieter
> From c103dbd7837d49721ccadfb1aee9520e821a020f Mon Sep 17 00:00:00 2001
> From: PiBa-NL <pba_...@yahoo.com>
> Date: Tue, 28 Nov 2017 23:26:08 +0100
> Subject: [PATCH] BUG/MINOR: when master-worker is in daemon mode, detach from
> tty
>
> This allows a calling script to show the first startup output and know when
> to stop reading from stdout so haproxy can daemonize.
> ---
> src/haproxy.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/src/haproxy.c b/src/haproxy.c
> index 891a021..702501d 100644
> --- a/src/haproxy.c
> +++ b/src/haproxy.c
> @@ -2749,7 +2749,7 @@ int main(int argc, char **argv)
> //lseek(pidfd, 0, SEEK_SET); /* debug: emulate eglibc
> bug */
> close(pidfd);
> }
> -
> +
Be careful there, you have some useless space, you should try to configure your
editor or git to see the trailing spaces.
> /* We won't ever use this anymore */
> free(global.pidfile); global.pidfile = NULL;
>
> @@ -2757,6 +2757,16 @@ int main(int argc, char **argv)
> if (global.mode & MODE_MWORKER) {
> mworker_cleanlisteners();
> deinit_pollers();
> +
> + if ((!(global.mode & MODE_QUIET) ||
> (global.mode & MODE_VERBOSE)) &&
> + ((global.mode & (MODE_DAEMON |
> MODE_MWORKER)) == (MODE_DAEMON | MODE_MWORKER))) {
The test can be simplified there, we already know that we are in MWORKER, you
can just check the DAEMON one.
> + /* detach from the tty, this is
> required to properly daemonize. */
> + fclose(stdin); fclose(stdout);
> fclose(stderr);
> + global.mode &= ~MODE_VERBOSE;
> + global.mode |= MODE_QUIET; /* ensure
> that we won't say anything from now */
> + setsid();
> + }
> +
> mworker_wait();
> /* should never get there */
> exit(EXIT_FAILURE);
Cheers,
--
William Lallemand
Re: [PATCH] BUG/MINOR: when master-worker is in daemon mode, detach from tty
Hi William,
When you have time, please take a look below & attached :) .
Op 29-11-2017 om 1:28 schreef William Lallemand:
Hi Pieter,
diff --git a/src/haproxy.c b/src/haproxy.c
index c3c8281..a811577 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2648,6 +2648,13 @@ int main(int argc, char **argv)
}
if (global.mode & (MODE_DAEMON | MODE_MWORKER)) {
+ if ((!(global.mode & MODE_QUIET) || (global.mode & MODE_VERBOSE))
&&
+ ((global.mode & (MODE_DAEMON | MODE_MWORKER)) ==
(MODE_DAEMON | MODE_MWORKER))) {
+ /* detach from the tty, this is required to properly
daemonize. */
+ fclose(stdin); fclose(stdout); fclose(stderr);
+ global.mode &= ~MODE_VERBOSE;
+ global.mode |= MODE_QUIET; /* ensure that we won't say
anything from now */
+ }
struct proxy *px;
struct peers *curpeers;
int ret = 0;
I need to check that again later, in my opinion it should be done after the
pipe() so we don't inherit the 0 and 1 FDs in the pipe,
FDs for the master-worker pipe can still be 0 and 1 if running in quiet
mode as the stdin/stdout/stderr are still closed before creating the
pipe then. Should the pipe be created earlier?
I've moved the code to just before the mworker_wait() in new attached
patch. This should allow (all?) possible warnings to be output before
closing stdX, and still 'seems' to work properly..
we also need to rely on
setsid() to do a proper tty detach.
I've added a setsid(), but i must admit i have no clue what its doing
exactly...
This is already done in -D mode without -W, maybe
this part of the code should me moved elsewhere, but we have to be careful not
to break the daemon mode w/o mworker.
I've tried most combinations of parameters like these:
1: -W
2: -W -q
3: -D -W
4: -D -W -q
5: -D
6: -D -q
7: -q
8: (without parameters)
Both by starting directly from a ssh console, and by running from my php
script that reads the stdout/stderr output. And reloading it with USR2
with the -W mode..
It seemed that the expected output or lack thereof was being produced in
all cases.
But it preferably also needs to be tested under systemd itself as that
is the intended use-case, which i did not test at all :/ ..
Also i did not change the config while running to include/exclude
'quiet' or 'daemon' option or something like that. Seems like a odd
thing to do..
I'm not sure if the attached patch is OK for you like this, or needs to
be implemented completely differently.
I have made and tried to test the changed patch with above cases but am
sure there are many things / combinations with other features i have not
included..
If i need to change it slightly somehow please let me know, if you need
time to look into it further, i can certainly wait :) i do not 'need'
the feature urgently or perhaps won't need it at all..
Anyhow when you have time to look into it, i look forward to your
feedback :) . Thanks in advance.
Regards,
PiBa-NL / Pieter
From c103dbd7837d49721ccadfb1aee9520e821a020f Mon Sep 17 00:00:00 2001
From: PiBa-NL <pba_...@yahoo.com>
Date: Tue, 28 Nov 2017 23:26:08 +0100
Subject: [PATCH] BUG/MINOR: when master-worker is in daemon mode, detach from
tty
This allows a calling script to show the first startup output and know when to
stop reading from stdout so haproxy can daemonize.
---
src/haproxy.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/haproxy.c b/src/haproxy.c
index 891a021..702501d 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2749,7 +2749,7 @@ int main(int argc, char **argv)
//lseek(pidfd, 0, SEEK_SET); /* debug: emulate eglibc
bug */
close(pidfd);
}
-
+
/* We won't ever use this anymore */
free(global.pidfile); global.pidfile = NULL;
@@ -2757,6 +2757,16 @@ int main(int argc, char **argv)
if (global.mode & MODE_MWORKER) {
mworker_cleanlisteners();
deinit_pollers();
+
+ if ((!(global.mode & MODE_QUIET) ||
(global.mode & MODE_VERBOSE)) &&
+ ((global.mode & (MODE_DAEMON |
MODE_MWORKER)) == (MODE_DAEMON | MODE_MWORKER))) {
+ /* detach from the tty, this is
required to properly daemonize. */
+ fclose(stdin); fclose(stdout);
fclose(stderr);
+ global.mode &= ~MODE_VERBOSE;
+ global.mode |= MODE_QUIET; /* ensure
that we won't say anything from now */
+ setsid();
+ }
+
Re: [PATCH] BUG/MINOR: when master-worker is in daemon mode, detach from tty
On Wed, Nov 29, 2017 at 12:05:43AM +0100, PiBa-NL wrote:
> Hi List,
>
Hi Pieter,
> Made a patch that makes the master-worker detach from tty when it is
> also combined with daemon mode to allow a script to start haproxy with
> daemon mode, closing stdout so the calling process knows when to stop
> reading from it and allow the master to properly daemonize.
>
> diff --git a/src/haproxy.c b/src/haproxy.c
> index c3c8281..a811577 100644
> --- a/src/haproxy.c
> +++ b/src/haproxy.c
> @@ -2648,6 +2648,13 @@ int main(int argc, char **argv)
> }
>
> if (global.mode & (MODE_DAEMON | MODE_MWORKER)) {
> + if ((!(global.mode & MODE_QUIET) || (global.mode &
> MODE_VERBOSE)) &&
> + ((global.mode & (MODE_DAEMON | MODE_MWORKER)) ==
> (MODE_DAEMON | MODE_MWORKER))) {
> + /* detach from the tty, this is required to properly
> daemonize. */
> + fclose(stdin); fclose(stdout); fclose(stderr);
> + global.mode &= ~MODE_VERBOSE;
> + global.mode |= MODE_QUIET; /* ensure that we won't say
> anything from now */
> + }
> struct proxy *px;
> struct peers *curpeers;
> int ret = 0;
I need to check that again later, in my opinion it should be done after the
pipe() so we don't inherit the 0 and 1 FDs in the pipe, we also need to rely on
setsid() to do a proper tty detach. This is already done in -D mode without -W,
maybe
this part of the code should me moved elsewhere, but we have to be careful not
to break the daemon mode w/o mworker.
--
William Lallemand
[PATCH] BUG/MINOR: when master-worker is in daemon mode, detach from tty
Hi List,
Made a patch that makes the master-worker detach from tty when it is
also combined with daemon mode to allow a script to start haproxy with
daemon mode, closing stdout so the calling process knows when to stop
reading from it and allow the master to properly daemonize.
This is intended to solve my previously reported 'issue' :
https://www.mail-archive.com/haproxy@formilux.org/msg27963.html
Let me know if something about it needs fixing..
Thanks
PiBa-NL / Pieter
From 06224a3fcf7b39bf1bf0128a5bac3d0209bc2aab Mon Sep 17 00:00:00 2001
From: PiBa-NL <pba_...@yahoo.com>
Date: Tue, 28 Nov 2017 23:26:08 +0100
Subject: [PATCH] [PATCH] BUG/MINOR: when master-worker is in daemon mode,
detach from tty
This allows a calling script to show the first startup output and know when to
stop reading from stdout so haproxy can daemonize.
---
src/haproxy.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/haproxy.c b/src/haproxy.c
index c3c8281..a811577 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2648,6 +2648,13 @@ int main(int argc, char **argv)
}
if (global.mode & (MODE_DAEMON | MODE_MWORKER)) {
+ if ((!(global.mode & MODE_QUIET) || (global.mode &
MODE_VERBOSE)) &&
+ ((global.mode & (MODE_DAEMON | MODE_MWORKER)) ==
(MODE_DAEMON | MODE_MWORKER))) {
+ /* detach from the tty, this is required to properly
daemonize. */
+ fclose(stdin); fclose(stdout); fclose(stderr);
+ global.mode &= ~MODE_VERBOSE;
+ global.mode |= MODE_QUIET; /* ensure that we won't say
anything from now */
+ }
struct proxy *px;
struct peers *curpeers;
int ret = 0;
--
2.10.1.windows.1
