glusterd/shd: Return null proc if process is not running.

We were ruturning first proc entry even if it is not running.
This was in an assumption that the process could have just
started and not updated the pidfile.

Now we that we have introduced the states for process state,
we can take decision based on that.

Change-Id: Ibfc11c966b0db599a8d6a08d8b975233b2bbfb8c
Fixes: bz#1728766
Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>

4 files changed, 65 insertions, 18 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-shd-svc.c b/xlators/mgmt/glusterd/src/glusterd-shd-svc.c
index aad47203d5f..81594e3d105 100644
--- a/xlators/mgmt/glusterd/src/glusterd-shd-svc.c
+++ b/xlators/mgmt/glusterd/src/glusterd-shd-svc.c
@@ -491,6 +491,8 @@ glusterd_shdsvc_start(glusterd_svc_t *svc, int flags)
         shd->attached = _gf_true;
     }
 out:
+    if (ret && volinfo)
+        glusterd_shd_svcproc_cleanup(&volinfo->shd);
     if (volinfo)
         glusterd_volinfo_unref(volinfo);
     gf_msg_debug(THIS->name, 0, "Returning %d", ret);
@@ -703,6 +705,7 @@ glusterd_shdsvc_stop(glusterd_svc_t *svc, int sig)
         cds_list_del_init(&svc->mux_svc);
         empty = cds_list_empty(&svc_proc->svcs);
         if (empty) {
+            svc_proc->status = GF_SVC_STOPPING;
             cds_list_del_init(&svc_proc->svc_proc_list);
         }
     }
diff --git a/xlators/mgmt/glusterd/src/glusterd-svc-helper.c b/xlators/mgmt/glusterd/src/glusterd-svc-helper.c
index b98892af61e..cf40a758082 100644
--- a/xlators/mgmt/glusterd/src/glusterd-svc-helper.c
+++ b/xlators/mgmt/glusterd/src/glusterd-svc-helper.c
@@ -396,15 +396,38 @@ out:
     return ret;
 }
 
+gf_boolean_t
+glusterd_is_svcproc_attachable(glusterd_svc_proc_t *svc_proc)
+{
+    int pid = -1;
+    glusterd_svc_t *parent_svc = NULL;
+
+    if (!svc_proc)
+        return _gf_false;
+
+    if (svc_proc->status == GF_SVC_STARTING)
+        return _gf_true;
+
+    if (svc_proc->status == GF_SVC_STARTED ||
+        svc_proc->status == GF_SVC_DISCONNECTED) {
+        parent_svc = cds_list_entry(svc_proc->svcs.next, glusterd_svc_t,
+                                    mux_svc);
+        if (parent_svc && gf_is_service_running(parent_svc->proc.pidfile, &pid))
+            return _gf_true;
+    }
+
+    if (svc_proc->status == GF_SVC_DIED || svc_proc->status == GF_SVC_STOPPING)
+        return _gf_false;
+
+    return _gf_false;
+}
+
 void *
 __gf_find_compatible_svc(gd_node_type daemon)
 {
     glusterd_svc_proc_t *svc_proc = NULL;
-    glusterd_svc_proc_t *return_proc = NULL;
-    glusterd_svc_t *parent_svc = NULL;
     struct cds_list_head *svc_procs = NULL;
     glusterd_conf_t *conf = NULL;
-    int pid = -1;
 
     conf = THIS->private;
     GF_VALIDATE_OR_GOTO("glusterd", conf, out);
@@ -422,15 +445,7 @@ __gf_find_compatible_svc(gd_node_type daemon)
 
     cds_list_for_each_entry(svc_proc, svc_procs, svc_proc_list)
     {
-        parent_svc = cds_list_entry(svc_proc->svcs.next, glusterd_svc_t,
-                                    mux_svc);
-        if (!return_proc)
-            return_proc = svc_proc;
-
-        /* If there is an  already running shd daemons, select it. Otehrwise
-         * select the first one.
-         */
-        if (parent_svc && gf_is_service_running(parent_svc->proc.pidfile, &pid))
+        if (glusterd_is_svcproc_attachable(svc_proc))
             return (void *)svc_proc;
         /*
          * Logic to select one process goes here. Currently there is only one
@@ -438,7 +453,7 @@ __gf_find_compatible_svc(gd_node_type daemon)
          */
     }
 out:
-    return return_proc;
+    return NULL;
 }
 
 glusterd_svc_proc_t *
@@ -455,6 +470,7 @@ glusterd_svcprocess_new()
     CDS_INIT_LIST_HEAD(&new_svcprocess->svc_proc_list);
     CDS_INIT_LIST_HEAD(&new_svcprocess->svcs);
     new_svcprocess->notify = glusterd_muxsvc_common_rpc_notify;
+    new_svcprocess->status = GF_SVC_STARTING;
     return new_svcprocess;
 }
 
@@ -467,6 +483,7 @@ glusterd_shd_svc_mux_init(glusterd_volinfo_t *volinfo, glusterd_svc_t *svc)
     glusterd_conf_t *conf = NULL;
     glusterd_svc_t *parent_svc = NULL;
     int pid = -1;
+    gf_boolean_t stop_daemon = _gf_false;
     char pidfile[PATH_MAX] = {
         0,
     };
@@ -507,9 +524,10 @@ glusterd_shd_svc_mux_init(glusterd_volinfo_t *volinfo, glusterd_svc_t *svc)
 
             if (!mux_proc) {
                 if (pid != -1 && sys_access(pidfile, R_OK) == 0) {
-                    /* stale pid file, stop and unlink it */
-                    glusterd_proc_stop(&svc->proc, SIGTERM, PROC_STOP_FORCE);
-                    glusterd_unlink_file(pidfile);
+                    /* stale pid file, stop and unlink it. This has to be
+                     * done outside the attach_lock.
+                     */
+                    stop_daemon = _gf_true;
                 }
                 mux_proc = __gf_find_compatible_svc(GD_NODE_SHD);
             }
@@ -517,7 +535,6 @@ glusterd_shd_svc_mux_init(glusterd_volinfo_t *volinfo, glusterd_svc_t *svc)
                 /* Take first entry from the process */
                 parent_svc = cds_list_entry(mux_proc->svcs.next, glusterd_svc_t,
                                             mux_svc);
-                glusterd_copy_file(parent_svc->proc.pidfile, svc->proc.pidfile);
                 mux_conn = &parent_svc->conn;
                 if (volinfo)
                     volinfo->shd.attached = _gf_true;
@@ -548,6 +565,10 @@ glusterd_shd_svc_mux_init(glusterd_volinfo_t *volinfo, glusterd_svc_t *svc)
 unlock:
     pthread_mutex_unlock(&conf->attach_lock);
 out:
+    if (stop_daemon) {
+        glusterd_proc_stop(&svc->proc, SIGTERM, PROC_STOP_FORCE);
+        glusterd_unlink_file(pidfile);
+    }
     return ret;
 }
 
@@ -580,7 +601,8 @@ __gf_find_compatible_svc_from_pid(gd_node_type daemon, pid_t pid)
         cds_list_for_each_entry(svc, &svc_proc->svcs, mux_svc)
         {
             if (gf_is_service_running(svc->proc.pidfile, &mux_pid)) {
-                if (mux_pid == pid) {
+                if (mux_pid == pid &&
+                    glusterd_is_svcproc_attachable(svc_proc)) {
                     /*TODO
                      * inefficient loop, but at the moment, there is only
                      * one shd.
diff --git a/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c b/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c
index 8b0755b05ce..c2b31490a6e 100644
--- a/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c
+++ b/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c
@@ -425,6 +425,9 @@ glusterd_muxsvc_common_rpc_notify(glusterd_svc_proc_t *mux_proc,
                     continue;
                 svc->online = _gf_true;
             }
+            if (mux_proc->status != GF_SVC_STARTED)
+                mux_proc->status = GF_SVC_STARTED;
+
             break;
 
         case RPC_CLNT_DISCONNECT:
@@ -436,6 +439,16 @@ glusterd_muxsvc_common_rpc_notify(glusterd_svc_proc_t *mux_proc,
                     svc->online = _gf_false;
                 }
             }
+            if (mux_proc->status != GF_SVC_DIED) {
+                svc = cds_list_entry(mux_proc->svcs.next, glusterd_svc_t,
+                                     mux_svc);
+                if (svc && !glusterd_proc_is_running(&svc->proc)) {
+                    mux_proc->status = GF_SVC_DIED;
+                } else {
+                    mux_proc->status = GF_SVC_DISCONNECTED;
+                }
+            }
+
             if (need_logging) {
                 gf_msg(this->name, GF_LOG_INFO, 0, GD_MSG_NODE_DISCONNECTED,
                        "glustershd has disconnected from glusterd.");
diff --git a/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.h b/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.h
index 5a5466a9b0f..4d96b3af682 100644
--- a/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.h
+++ b/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.h
@@ -31,12 +31,21 @@ typedef int (*glusterd_svc_reconfigure_t)(void *data);
 typedef int (*glusterd_muxsvc_conn_notify_t)(glusterd_svc_proc_t *mux_proc,
                                              rpc_clnt_event_t event);
 
+typedef enum gf_svc_status {
+    GF_SVC_STARTING,
+    GF_SVC_STARTED,
+    GF_SVC_STOPPING,
+    GF_SVC_DISCONNECTED,
+    GF_SVC_DIED,
+} gf_svc_status_t;
+
 struct glusterd_svc_proc_ {
     struct cds_list_head svc_proc_list;
     struct cds_list_head svcs;
     glusterd_muxsvc_conn_notify_t notify;
     rpc_clnt_t *rpc;
     void *data;
+    gf_svc_status_t status;
 };
 
 struct glusterd_svc_ {