On Mon, Jun 19, 2017 at 03:35:08PM +0200, Emmanuel Hocdet wrote:
> In case of upgrade from haproxy without -x option to -x, i suppose it will do
> cleanly.
> I try to play with -x, multi-proc (add and remove), upgrade pre -x without
> master-worker and is painful.
> Perhaps i misunderstood (and i used multi -x). Now, i only test from legacy
> to -x with master-worker.
>
> > I think that's just a matter of documentation, maybe I can write a longer
> > paragraph in the management.txt doc explaining how to upgrade for the legacy
> > mode, and what changed between this one and the master worker.
>
> Yes, documentation must be sufficient to avoid bad usages. What about case of
> multi -x?
>
Well, the -x should be unique, you don't need several -x, one socket is enough
because you have access to every FD of every processes from it. I add a warning
about that.
I fixed the few issues you had in the attached patches.
Willy, could you merge them? thanks
--
William Lallemand
>From 9927f4a1e864638e16dcb3c61de23bcc8f256a91 Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallem...@haproxy.com>
Date: Mon, 19 Jun 2017 15:57:55 +0200
Subject: [PATCH 1/4] BUG/MEDIUM: fix segfault when no argument to -x option
This patch fixes a segfault in the command line parser.
When haproxy is launched with -x with no argument and -x is the latest
option in argv it segfaults.
Use usage() insteads of exit() on error.
---
src/haproxy.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/haproxy.c b/src/haproxy.c
index d64058da..2fd387f3 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -1279,9 +1279,9 @@ static void init(int argc, char **argv)
else if (*flag == 'q')
arg_mode |= MODE_QUIET;
else if (*flag == 'x') {
- if (argv[1][0] == '-') {
- Alert("Unix socket path expected with the -x flag\n");
- exit(1);
+ if (argc <= 1 || argv[1][0] == '-') {
+ Alert("Unix socket path expected with the -x flag\n\n");
+ usage(progname);
}
old_unixsocket = argv[1];
argv++;
--
2.13.0
>From 0dc2e1f497f475e541056e0769431528a6edb39c Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallem...@haproxy.com>
Date: Mon, 19 Jun 2017 16:37:19 +0200
Subject: [PATCH 2/4] MINOR: warning on multiple -x
Multiple use of the -x option is useless, emit a warning.
---
src/haproxy.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/haproxy.c b/src/haproxy.c
index 2fd387f3..1eabb552 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -1283,7 +1283,10 @@ static void init(int argc, char **argv)
Alert("Unix socket path expected with the -x flag\n\n");
usage(progname);
}
+ if (old_unixsocket)
+ Warning("-x option already set, overwriting the value\n");
old_unixsocket = argv[1];
+
argv++;
argc--;
}
--
2.13.0
>From 17f2decfe5d667d77415d390c61f839db8470065 Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallem...@haproxy.com>
Date: Tue, 20 Jun 2017 11:20:23 +0200
Subject: [PATCH 3/4] MINOR: mworker: don't copy -x argument anymore in
copy_argv()
Don't copy the -x argument anymore in copy_argv() since it's already
allocated in mworker_reload().
Make the copy_argv() more consistent when used with multiple arguments
to strip.
It prevents multiple -x on reload, which is not supported.
---
src/haproxy.c | 36 +++++++++++++-----------------------
1 file changed, 13 insertions(+), 23 deletions(-)
diff --git a/src/haproxy.c b/src/haproxy.c
index 1eabb552..cdb6066b 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -596,23 +596,12 @@ static void mworker_reload()
}
next_argv[next_argc] = NULL;
- /* if -x was used, try to update the stat socket if not available anymore */
+ /* add the -x option with the stat socket */
if (cur_unixsocket) {
- if (old_unixsocket) {
-
- /* look for -x <path> */
- for (j = 0; next_argv[j]; j++) {
- if (!strcmp(next_argv[j], "-x"))
- next_argv[j + 1] = (char *)cur_unixsocket;
- }
- } else {
- /* if -x is not specified but we know the socket, add -x with it */
- next_argv[next_argc++] = "-x";
- next_argv[next_argc++] = (char *)cur_unixsocket;
- next_argv[next_argc++] = NULL;
-
- }
+ next_argv[next_argc++] = "-x";
+ next_argv[next_argc++] = (char *)cur_unixsocket;
+ next_argv[next_argc++] = NULL;
}
deinit(); /* we don't want to leak FD there */
@@ -1101,7 +1090,7 @@ out:
static char **copy_argv(int argc, char **argv)
{
char **newargv;
- int i, j;
+ int i = 0, j = 0;
newargv = calloc(argc + 2, sizeof(char *));
if (newargv == NULL) {
@@ -1109,19 +1098,20 @@ static char **copy_argv(int argc, char **argv)
return NULL;
}
- for (i = 0, j = 0; i < argc; i++, j++) {
- char *flag = *(argv + i) + 1;
-
- /* -sf or -st */
- if (*flag == 's' && (flag[1] == 'f' || flag[1] == 't')) {
- /* list of pids to finish ('f') or terminate ('t') */
+ while (i < argc) {
+ /* -sf or -st or -x */
+ if ((argv[i][1] == 's' && (argv[i][2] == 'f' || argv[i][2] == 't')) || argv[i][1] == 'x' ) {
+ /* list of pids to finish ('f') or terminate ('t') or unix socket (-x) */
i++;
while (i < argc && argv[i][0] != '-') {
i++;
}
+ continue;
}
- newargv[j] = argv[i];
+
+ newargv[j++] = argv[i++];
}
+
return newargv;
}
--
2.13.0
>From 0cef1904a2a5c96cf22f1dcba75e4084859b14bd Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallem...@haproxy.com>
Date: Tue, 20 Jun 2017 11:20:33 +0200
Subject: [PATCH 4/4] BUG/MEDIUM: mworker: don't reuse PIDs passed to the
master
When starting the master worker with -sf or -st, the PIDs will be reused
on the next reload, which is a problem if new processes on the system
took those PIDs.
This patch ensures that we don't register old PIDs in the reload system
when launching the master worker.
---
src/haproxy.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/haproxy.c b/src/haproxy.c
index cdb6066b..a4257443 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2444,6 +2444,13 @@ int main(int argc, char **argv)
if (nb_oldpids)
nb_oldpids = tell_old_pids(oldpids_sig);
+ if ((getenv("HAPROXY_MWORKER_REEXEC") == NULL)) {
+ nb_oldpids = 0;
+ free(oldpids);
+ oldpids = NULL;
+ }
+
+
/* Note that any error at this stage will be fatal because we will not
* be able to restart the old pids.
*/
--
2.13.0
