Re: [PATCH] BUG/MEDIUM: stats: show servers state may show an servers from another backend

2016-05-26 Thread Willy Tarreau
On Fri, May 27, 2016 at 12:06:45AM +0200, Cyril Bonté wrote:
> Olivier Doucet reported that "show servers state" was producing an invalid
> output with some configurations where nbproc > 1.
(...)

merged into 1.7+1.6. Thanks Cyril!

Willy



[PATCH] BUG/MEDIUM: stats: show servers state may show an servers from another backend

2016-05-26 Thread Cyril Bonté
Olivier Doucet reported that "show servers state" was producing an invalid
output with some configurations where nbproc > 1.

Indeed, commit 76a99784f4 fixed some issues but unfortunately introduced a
regression when a backend bound to the same process as the stats socket and a
previous backend is bound to another one.

For example :
  global
daemon
nbproc 2
stats socket /var/run/haproxy-1.sock process 1
stats socket /var/run/haproxy-2.sock process 2

  listen proc1
bind 127.0.0.1:9001
bind-process 1
server WRONG 127.0.0.1:80

  listen proc2
bind 127.0.0.1:9002
bind-process 2
server RIGHT 127.0.0.1:80

Requesting "show servers state" on /var/run/haproxy-2.sock was producing a line
like :
3 proc2 1 WRONG 127.0.0.1 2 0 1 1 4 1 0 2 0 0 0 0

whereas the line below was awaited :
3 proc2 1 RIGHT 127.0.0.1 2 0 1 1 5 1 0 2 0 0 0 0

This was caused by the initialization of the server loop too early, before the
bind_proc filtering whereas it should be done after.

This fix should be backported to 1.6, where the regression has unfortunately
been backported.
---
 src/dumpstats.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/dumpstats.c b/src/dumpstats.c
index 19e2688..47fc4bc 100644
--- a/src/dumpstats.c
+++ b/src/dumpstats.c
@@ -3076,6 +3076,9 @@ static int dump_servers_state(struct stream_interface 
*si, struct chunk *buf)
if (appctx->ctx.server_state.px->bind_proc && 
!(appctx->ctx.server_state.px->bind_proc & (1UL << (relative_pid - 1
return 1;
 
+   if (!appctx->ctx.server_state.sv)
+   appctx->ctx.server_state.sv = appctx->ctx.server_state.px->srv;
+
for (; appctx->ctx.server_state.sv != NULL; appctx->ctx.server_state.sv 
= srv->next) {
srv = appctx->ctx.server_state.sv;
srv_addr[0] = '\0';
@@ -3178,8 +3181,6 @@ static int stats_dump_servers_state_to_buffer(struct 
stream_interface *si)
 
for (; appctx->ctx.server_state.px != NULL; appctx->ctx.server_state.px 
= curproxy->next) {
curproxy = appctx->ctx.server_state.px;
-   if (!appctx->ctx.server_state.sv)
-   appctx->ctx.server_state.sv = 
appctx->ctx.server_state.px->srv;
/* servers are only in backends */
if (curproxy->cap & PR_CAP_BE) {
if (!dump_servers_state(si, ))
-- 
2.8.1