Blob Blame History Raw
From 216ac7a1bd22db08cc02d7b8688a3338e78c71cd Mon Sep 17 00:00:00 2001
From: Sanju Rakonde <srakonde@redhat.com>
Date: Tue, 11 Sep 2018 14:19:42 +0530
Subject: [PATCH 423/444] glusterd: acquire lock to update volinfo structure

Problem: With commit cb0339f92, we are using a separate syntask
for restart_bricks. There can be a situation where two threads
are accessing the same volinfo structure at the same time and
updating volinfo structure. This can lead volinfo to have
inconsistent values and assertion failures because of unexpected
values.

Solution: While updating the volinfo structure, acquire a
store_volinfo_lock, and release the lock only when the thread
completed its critical section part.

> Fixes: bz#1627610
> Signed-off-by: Sanju Rakonde <srakonde@redhat.com>
> Change-Id: I545e4e2368e3285d8f7aa28081ff4448abb72f5d

upstream patch: https://review.gluster.org/#/c/glusterfs/+/21150/

Change-Id: I545e4e2368e3285d8f7aa28081ff4448abb72f5d
BUG: 1631418
Signed-off-by: Sanju Rakonde <srakonde@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/154885
Tested-by: RHGS Build Bot <nigelb@redhat.com>
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
---
 xlators/mgmt/glusterd/src/glusterd-store.c      | 67 +++++++++++++------------
 xlators/mgmt/glusterd/src/glusterd-volume-ops.c |  2 +
 xlators/mgmt/glusterd/src/glusterd.h            |  3 ++
 3 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c
index 015f6c2..37542e7 100644
--- a/xlators/mgmt/glusterd/src/glusterd-store.c
+++ b/xlators/mgmt/glusterd/src/glusterd-store.c
@@ -1796,46 +1796,49 @@ glusterd_store_volinfo (glusterd_volinfo_t *volinfo, glusterd_volinfo_ver_ac_t a
 
         GF_ASSERT (volinfo);
 
-        glusterd_perform_volinfo_version_action (volinfo, ac);
-        ret = glusterd_store_create_volume_dir (volinfo);
-        if (ret)
-                goto out;
-
-        ret = glusterd_store_create_volume_run_dir (volinfo);
-        if (ret)
-                goto out;
+        pthread_mutex_lock(&volinfo->store_volinfo_lock);
+        {
+                glusterd_perform_volinfo_version_action(volinfo, ac);
+                ret = glusterd_store_create_volume_dir(volinfo);
+                if (ret)
+                        goto unlock;
 
-        ret = glusterd_store_create_vol_shandle_on_absence (volinfo);
-        if (ret)
-                goto out;
+                ret = glusterd_store_create_volume_run_dir(volinfo);
+                if (ret)
+                        goto unlock;
 
-        ret = glusterd_store_create_nodestate_sh_on_absence (volinfo);
-        if (ret)
-                goto out;
+                ret = glusterd_store_create_vol_shandle_on_absence(volinfo);
+                if (ret)
+                        goto unlock;
 
-        ret = glusterd_store_perform_volume_store (volinfo);
-        if (ret)
-                goto out;
+                ret = glusterd_store_create_nodestate_sh_on_absence(volinfo);
+                if (ret)
+                        goto unlock;
 
-        ret = glusterd_store_volume_atomic_update (volinfo);
-        if (ret) {
-                glusterd_perform_volinfo_version_action (volinfo,
-                                                         GLUSTERD_VOLINFO_VER_AC_DECREMENT);
-                goto out;
-        }
+                ret = glusterd_store_perform_volume_store(volinfo);
+                if (ret)
+                        goto unlock;
 
-        ret = glusterd_store_perform_node_state_store (volinfo);
-        if (ret)
-                goto out;
+                ret = glusterd_store_volume_atomic_update(volinfo);
+                if (ret) {
+                        glusterd_perform_volinfo_version_action(volinfo,
+                                                                GLUSTERD_VOLINFO_VER_AC_DECREMENT);
+                        goto unlock;
+                }
 
-        /* checksum should be computed at the end */
-        ret = glusterd_compute_cksum (volinfo, _gf_false);
-        if (ret)
-                goto out;
+                ret = glusterd_store_perform_node_state_store(volinfo);
+                if (ret)
+                        goto unlock;
 
-out:
+                /* checksum should be computed at the end */
+                ret = glusterd_compute_cksum(volinfo, _gf_false);
+                if (ret)
+                        goto unlock;
+        }
+unlock:
+        pthread_mutex_unlock(&volinfo->store_volinfo_lock);
         if (ret)
-                glusterd_store_volume_cleanup_tmp (volinfo);
+                glusterd_store_volume_cleanup_tmp(volinfo);
 
         gf_msg_debug (THIS->name, 0, "Returning %d", ret);
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
index 87b7acc..b91a516 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
@@ -2198,6 +2198,8 @@ glusterd_op_create_volume (dict_t *dict, char **op_errstr)
                 goto out;
         }
 
+        pthread_mutex_init(&volinfo->store_volinfo_lock, NULL);
+
         ret = dict_get_str (dict, "volname", &volname);
 
         if (ret) {
diff --git a/xlators/mgmt/glusterd/src/glusterd.h b/xlators/mgmt/glusterd/src/glusterd.h
index 8c70d48..edd41aa 100644
--- a/xlators/mgmt/glusterd/src/glusterd.h
+++ b/xlators/mgmt/glusterd/src/glusterd.h
@@ -478,6 +478,9 @@ struct glusterd_volinfo_ {
         gf_boolean_t              stage_deleted; /* volume has passed staging
                                                   * for delete operation
                                                   */
+        pthread_mutex_t store_volinfo_lock; /* acquire lock for
+                                             * updating the volinfo
+                                             */
 };
 
 typedef enum gd_snap_status_ {
-- 
1.8.3.1