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.

Change-Id: I69949874435b02699e5708dab811777ccb297174
BUG: 1421590
Signed-off-by: Samikshan Bairagya <samikshan@gmail.com>
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>

5 files changed, 75 insertions, 14 deletions

diff --git a/glusterfsd/src/glusterfsd-mgmt.c b/glusterfsd/src/glusterfsd-mgmt.c
index 57f8dc0405..6871582abb 100644
--- a/glusterfsd/src/glusterfsd-mgmt.c
+++ b/glusterfsd/src/glusterfsd-mgmt.c
@@ -234,6 +234,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 {
                 /*
@@ -246,6 +247,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);
         }
@@ -2563,7 +2566,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, };
@@ -2574,7 +2577,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;
@@ -2585,8 +2588,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 f402246e78..e3699739c0 100644
--- a/glusterfsd/src/glusterfsd.c
+++ b/glusterfsd/src/glusterfsd.c
@@ -1332,7 +1332,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 e442bede5d..6a30ee9e3f 100644
--- a/glusterfsd/src/glusterfsd.h
+++ b/glusterfsd/src/glusterfsd.h
@@ -109,7 +109,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 0000000000..ed401f6e6a
--- /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 2e87ff6ecd..9b2954af64 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;