From 86a24b1c3d749721ba54137abb8c9b62e87be77a Mon Sep 17 00:00:00 2001

From: Samikshan Bairagya <samikshan@gmail.com>

Date: Mon, 20 Feb 2017 18:35:01 +0530

Subject: [PATCH 341/361] core: Clean up pmap registry up correctly on

 volume/brick stop

This commit changes the following:

1. In glusterfs_handle_terminate, send out individual pmap signout

requests to glusterd for every brick.

2. Add another parameter to glusterfs_mgmt_pmap_signout function to

pass the brickname that needs to be removed from the pmap registry.

3. Make sure pmap_registry_search doesn't break out from the loop

iterating over the list of bricks per port if the first brick entry

corresponding to a port is whitespaced out.

4. Make sure the pmap registry entries are removed for other

daemons like snapd.

mainline:

> BUG: 1421590

> Reviewed-on: https://review.gluster.org/16689

> Smoke: Gluster Build System <jenkins@build.gluster.org>

> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>

> CentOS-regression: Gluster Build System <jenkins@build.gluster.org>

> Reviewed-by: Gaurav Yadav <gyadav@redhat.com>

> Reviewed-by: Jeff Darcy <jdarcy@redhat.com>

(cherry picked from commit 2cf0062984ae2c159cc94bfa7093f6e38bfd99fe)

BUG: 1417815

Change-Id: I69949874435b02699e5708dab811777ccb297174

Signed-off-by: Samikshan Bairagya <samikshan@gmail.com>

Reviewed-on: https://code.engineering.redhat.com/gerrit/101322

Tested-by: Milind Changire <mchangir@redhat.com>

Reviewed-by: Atin Mukherjee <amukherj@redhat.com>

---

 glusterfsd/src/glusterfsd-mgmt.c                   | 15 ++++--

 glusterfsd/src/glusterfsd.c                        |  2 +-

 glusterfsd/src/glusterfsd.h                        |  2 +-

 .../bugs/core/bug-1421590-brick-mux-resuse-ports.t | 55 ++++++++++++++++++++++

 xlators/mgmt/glusterd/src/glusterd-pmap.c          | 15 +++---

 5 files changed, 75 insertions(+), 14 deletions(-)

 create mode 100644 tests/bugs/core/bug-1421590-brick-mux-resuse-ports.t

diff --git a/glusterfsd/src/glusterfsd-mgmt.c b/glusterfsd/src/glusterfsd-mgmt.c

index 44ddc64..9cb8b41 100644

--- a/glusterfsd/src/glusterfsd-mgmt.c

+++ b/glusterfsd/src/glusterfsd-mgmt.c

@@ -236,6 +236,7 @@ glusterfs_handle_terminate (rpcsvc_request_t *req)

                 gf_log (THIS->name, GF_LOG_INFO,

                         "terminating after loss of last child %s",

                         xlator_req.name);

+                glusterfs_mgmt_pmap_signout (glusterfsd_ctx, xlator_req.name);

                 cleanup_and_exit (SIGTERM);

         } else {

                 /*

@@ -248,6 +249,8 @@ glusterfs_handle_terminate (rpcsvc_request_t *req)

                 gf_log (THIS->name, GF_LOG_INFO, "detaching not-only child %s",

                         xlator_req.name);

                 top->notify (top, GF_EVENT_TRANSPORT_CLEANUP, victim);

+                glusterfs_mgmt_pmap_signout (glusterfsd_ctx, xlator_req.name);

+

                 *trav_p = (*trav_p)->next;

                 glusterfs_autoscale_threads (THIS->ctx, -1);

         }

@@ -2560,7 +2563,7 @@ out:

 int

-glusterfs_mgmt_pmap_signout (glusterfs_ctx_t *ctx)

+glusterfs_mgmt_pmap_signout (glusterfs_ctx_t *ctx, char *brickname)

 {

         int               ret = 0;

         pmap_signout_req  req = {0, };

@@ -2571,7 +2574,7 @@ glusterfs_mgmt_pmap_signout (glusterfs_ctx_t *ctx)

         frame = create_frame (THIS, ctx->pool);

         cmd_args = &ctx->cmd_args;

-        if (!cmd_args->brick_port || !cmd_args->brick_name) {

+        if (!cmd_args->brick_port && (!cmd_args->brick_name || !brickname)) {

                 gf_log ("fsd-mgmt", GF_LOG_DEBUG,

                         "portmapper signout arguments not given");

                 goto out;

@@ -2582,8 +2585,12 @@ glusterfs_mgmt_pmap_signout (glusterfs_ctx_t *ctx)

                 snprintf (brick_name, sizeof(brick_name), "%s.rdma",

                           cmd_args->brick_name);

                 req.brick = brick_name;

-        } else

-                req.brick = cmd_args->brick_name;

+        } else {

+                if (brickname)

+                        req.brick = brickname;

+                else

+                        req.brick = cmd_args->brick_name;

+        }

         req.port  = cmd_args->brick_port;

         req.rdma_port = cmd_args->brick_port2;

diff --git a/glusterfsd/src/glusterfsd.c b/glusterfsd/src/glusterfsd.c

index 485da4e..cbc1b93 100644

--- a/glusterfsd/src/glusterfsd.c

+++ b/glusterfsd/src/glusterfsd.c

@@ -1291,7 +1291,7 @@ cleanup_and_exit (int signum)

                 return;

         ctx->cleanup_started = 1;

-        glusterfs_mgmt_pmap_signout (ctx);

+        glusterfs_mgmt_pmap_signout (ctx, NULL);

         /* below part is a racy code where the rpcsvc object is freed.

          * But in another thread (epoll thread), upon poll error in the

diff --git a/glusterfsd/src/glusterfsd.h b/glusterfsd/src/glusterfsd.h

index 62aae2a..e13896d 100644

--- a/glusterfsd/src/glusterfsd.h

+++ b/glusterfsd/src/glusterfsd.h

@@ -111,7 +111,7 @@ struct _gfd_vol_top_priv_t {

 };

 typedef struct _gfd_vol_top_priv_t gfd_vol_top_priv_t;

-int glusterfs_mgmt_pmap_signout (glusterfs_ctx_t *ctx);

+int glusterfs_mgmt_pmap_signout (glusterfs_ctx_t *ctx, char *brick_name);

 int glusterfs_mgmt_pmap_signin (glusterfs_ctx_t *ctx);

 int glusterfs_volfile_fetch (glusterfs_ctx_t *ctx);

 void cleanup_and_exit (int signum);

diff --git a/tests/bugs/core/bug-1421590-brick-mux-resuse-ports.t b/tests/bugs/core/bug-1421590-brick-mux-resuse-ports.t

new file mode 100644

index 0000000..ed401f6

--- /dev/null

+++ b/tests/bugs/core/bug-1421590-brick-mux-resuse-ports.t

@@ -0,0 +1,55 @@

+#!/bin/bash

+

+. $(dirname $0)/../../include.rc

+. $(dirname $0)/../../traps.rc

+. $(dirname $0)/../../volume.rc

+

+function get_nth_brick_port_for_volume () {

+        local VOL=$1

+        local n=$2

+

+        $CLI volume status $VOL --xml | sed -ne 's/.*<port>\([-0-9]*\)<\/port>/\1/p' \

+                                      | head -n $n | tail -n 1

+}

+

+TEST glusterd

+

+TEST $CLI volume set all cluster.brick-multiplex on

+push_trapfunc "$CLI volume set all cluster.brick-multiplex off"

+push_trapfunc "cleanup"

+

+TEST $CLI volume create $V0 $H0:$B0/brick{0,1}

+TEST $CLI volume start $V0

+

+port_brick0=$(get_nth_brick_port_for_volume $V0 1)

+

+# restart the volume

+TEST $CLI volume stop $V0

+TEST $CLI volume start $V0

+

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT $port_brick0 get_nth_brick_port_for_volume $V0 1

+

+TEST $CLI volume stop $V0

+TEST $CLI volume set all cluster.brick-multiplex off

+

+TEST $CLI volume start $V0

+

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT $port_brick0 get_nth_brick_port_for_volume $V0 1

+

+port_brick1=$(get_nth_brick_port_for_volume $V0 2)

+

+# restart the volume

+TEST $CLI volume stop $V0

+TEST $CLI volume start $V0

+

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT $port_brick0 get_nth_brick_port_for_volume $V0 1

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT $port_brick1 get_nth_brick_port_for_volume $V0 2

+

+TEST $CLI volume stop $V0

+

+TEST $CLI volume set all cluster.brick-multiplex on

+

+TEST $CLI volume start $V0

+

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT $port_brick0 get_nth_brick_port_for_volume $V0 1

+

diff --git a/xlators/mgmt/glusterd/src/glusterd-pmap.c b/xlators/mgmt/glusterd/src/glusterd-pmap.c

index 8a39fc2..d87c31e 100644

--- a/xlators/mgmt/glusterd/src/glusterd-pmap.c

+++ b/xlators/mgmt/glusterd/src/glusterd-pmap.c

@@ -119,9 +119,9 @@ pmap_registry_search (xlator_t *this, const char *brickname,

                 for (;;) {

                         for (i = 0; brck[i] && !isspace (brck[i]); ++i)

                                 ;

-                        if (!i) {

+                        if (i == 0 && brck[i] == '\0')

                                 break;

-                        }

+

                         if (strncmp (brck, brickname, i) == 0) {

                                 /*

                                  * Without this check, we'd break when brck

@@ -134,7 +134,9 @@ pmap_registry_search (xlator_t *this, const char *brickname,

                                         return p;

                                 }

                         }

+

                         brck += i;

+

                         /*

                          * Skip over *any* amount of whitespace, including

                          * none (if we're already at the end of the string).

@@ -260,7 +262,6 @@ pmap_registry_bind (xlator_t *this, int port, const char *brickname,

                 goto out;

         p = port;

-        pmap->ports[p].type = type;

         if (pmap->ports[p].brickname) {

                 char *tmp = pmap->ports[p].brickname;

                 asprintf (&pmap->ports[p].brickname, "%s %s", tmp, brickname);

@@ -356,10 +357,9 @@ pmap_registry_remove (xlator_t *this, int port, const char *brickname,

                         goto out;

                 p = port;

-                goto remove;

         }

-        if (brickname && strchr (brickname, '/')) {

+        if (brickname) {

                 p = pmap_registry_search (this, brickname, type, _gf_true);

                 if (p)

                         goto remove;

@@ -373,9 +373,8 @@ pmap_registry_remove (xlator_t *this, int port, const char *brickname,

         goto out;

 remove:

-        gf_msg ("pmap", GF_LOG_INFO, 0,

-                GD_MSG_BRICK_REMOVE, "removing brick %s on port %d",

-                pmap->ports[p].brickname, p);

+        gf_msg ("pmap", GF_LOG_INFO, 0, GD_MSG_BRICK_REMOVE,

+                "removing brick %s on port %d", brickname, p);

         if (xprt && (xprt == pmap->ports[p].xprt)) {

                 pmap->ports[p].xprt = NULL;

-- 

1.8.3.1