glusterd: op state machine shouldn't use global peer list

Problem : op state machine was relying on the global peer list while sending
lock/stage/unlock commit rpc requests to the peers in the cluster. Trusting on
global peer list structure is dangerous as this structure gets modified if any
peer modification command is attempted in the cluster when there is a ongoing
transaction going through the state machine. An ideal usecase of this problem
when rebalance is in progress and peer probe is executed rebalance op-sm and
peer probe may run into race making peerinfo structure go for toss.

Solution: Use local copy of peer list (xaction_peers) in glusterd op-sm.

Change-Id: I1ff7118dc6a9a72633e2e87b7ab7bae1796595e0
BUG: 1152890
Signed-off-by: Atin Mukherjee <amukherj@redhat.com>
Reviewed-on: http://review.gluster.org/8932
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Krishnan Parthasarathi <kparthas@redhat.com>
Reviewed-by: Kaushal M <kaushal@redhat.com>

4 files changed, 42 insertions, 11 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-handler.c b/xlators/mgmt/glusterd/src/glusterd-handler.c
index d97cc4bdf32..46efb58f275 100644
--- a/xlators/mgmt/glusterd/src/glusterd-handler.c
+++ b/xlators/mgmt/glusterd/src/glusterd-handler.c
@@ -517,6 +517,7 @@ glusterd_op_txn_begin (rpcsvc_request_t *req, glusterd_op_t op, void *ctx,
                        char *err_str, size_t err_len)
 {
         int32_t                     ret             = -1;
+        int                         npeers          = 0;
         dict_t                     *dict            = NULL;
         xlator_t                   *this            = NULL;
         glusterd_conf_t            *priv            = NULL;
@@ -605,6 +606,10 @@ glusterd_op_txn_begin (rpcsvc_request_t *req, glusterd_op_t op, void *ctx,
 
 local_locking_done:
 
+        INIT_LIST_HEAD (&priv->xaction_peers);
+
+        npeers = gd_build_peers_list (&priv->peers, &priv->xaction_peers, op);
+
         /* If no volname is given as a part of the command, locks will
          * not be held, hence sending stage event. */
         if (volname || (priv->op_version < GD_OP_VERSION_3_6_0))
diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
index c3ef67e0c5c..f39c0ea555f 100644
--- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c
+++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
@@ -2810,7 +2810,7 @@ glusterd_op_ac_send_lock (glusterd_op_sm_event_t *event, void *ctx)
         priv = this->private;
         GF_ASSERT (priv);
 
-        list_for_each_entry (peerinfo, &priv->peers, uuid_list) {
+        list_for_each_entry (peerinfo, &priv->xaction_peers, op_peers_list) {
                 GF_ASSERT (peerinfo);
 
                 if (!peerinfo->connected || !peerinfo->mgmt)
@@ -2880,19 +2880,19 @@ out:
 static int
 glusterd_op_ac_send_unlock (glusterd_op_sm_event_t *event, void *ctx)
 {
-        int                   ret      = 0;
-        rpc_clnt_procedure_t *proc     = NULL;
-        glusterd_conf_t      *priv     = NULL;
-        xlator_t             *this     = NULL;
-        glusterd_peerinfo_t  *peerinfo = NULL;
-        uint32_t             pending_count = 0;
-        dict_t               *dict     = NULL;
+        int                   ret           = 0;
+        rpc_clnt_procedure_t *proc          = NULL;
+        glusterd_conf_t      *priv          = NULL;
+        xlator_t             *this          = NULL;
+        glusterd_peerinfo_t  *peerinfo      = NULL;
+        uint32_t              pending_count = 0;
+        dict_t               *dict          = NULL;
 
         this = THIS;
         priv = this->private;
         GF_ASSERT (priv);
 
-        list_for_each_entry (peerinfo, &priv->peers, uuid_list) {
+        list_for_each_entry (peerinfo, &priv->xaction_peers, op_peers_list) {
                 GF_ASSERT (peerinfo);
 
                 if (!peerinfo->connected || !peerinfo->mgmt)
@@ -3522,7 +3522,7 @@ glusterd_op_ac_send_stage_op (glusterd_op_sm_event_t *event, void *ctx)
         if (op == GD_OP_REPLACE_BRICK)
                 glusterd_rb_use_rsp_dict (NULL, rsp_dict);
 
-        list_for_each_entry (peerinfo, &priv->peers, uuid_list) {
+        list_for_each_entry (peerinfo, &priv->xaction_peers, op_peers_list) {
                 GF_ASSERT (peerinfo);
 
                 if (!peerinfo->connected || !peerinfo->mgmt)
@@ -4115,7 +4115,7 @@ glusterd_op_ac_send_commit_op (glusterd_op_sm_event_t *event, void *ctx)
         }
 
 
-        list_for_each_entry (peerinfo, &priv->peers, uuid_list) {
+        list_for_each_entry (peerinfo, &priv->xaction_peers, op_peers_list) {
                 GF_ASSERT (peerinfo);
 
                 if (!peerinfo->connected || !peerinfo->mgmt)
@@ -4430,6 +4430,7 @@ glusterd_op_txn_complete (uuid_t *txn_id)
         glusterd_op_clear_op ();
         glusterd_op_reset_ctx ();
         glusterd_op_clear_errstr ();
+        glusterd_op_clear_xaction_peers ();
 
         /* Based on the op-version, we release the cluster or mgmt_v3 lock */
         if (priv->op_version < GD_OP_VERSION_3_6_0) {
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index 494f982861d..a455cc03a30 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -13973,3 +13973,25 @@ glusterd_have_peers ()
 
         return !list_empty (&conf->peers);
 }
+
+void
+glusterd_op_clear_xaction_peers ()
+{
+        xlator_t               *this     = NULL;
+        glusterd_conf_t        *priv     = NULL;
+        glusterd_peerinfo_t    *peerinfo = NULL;
+        glusterd_peerinfo_t    *tmp      = NULL;
+
+        this = THIS;
+        priv = this->private;
+
+        GF_ASSERT (this);
+        GF_ASSERT (priv);
+
+        list_for_each_entry_safe (peerinfo, tmp, &priv->xaction_peers,
+                                  op_peers_list) {
+                GF_ASSERT (peerinfo);
+                list_del_init (&peerinfo->op_peers_list);
+        }
+
+}
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.h b/xlators/mgmt/glusterd/src/glusterd-utils.h
index 605d7e05124..f1a8469700c 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.h
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.h
@@ -915,4 +915,7 @@ glusterd_check_client_op_version_support (char *volname, uint32_t op_version,
 
 gf_boolean_t
 glusterd_have_peers ();
+
+void
+glusterd_op_clear_xaction_peers ();
 #endif