tests: spurious failure fix for bug-948686.t

Ensured import volume and volume start doesn't race in volinfo by refcounting
volinfo.

Change-Id: I7467eccaba9a00fd63ba0121d8157df24d1c00a6
BUG: 1258714
Signed-off-by: Atin Mukherjee <amukherj@redhat.com>
Reviewed-on: http://review.gluster.org/12329
Tested-by: NetBSD Build System <jenkins@build.gluster.org>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Jeff Darcy <jdarcy@redhat.com>

3 files changed, 27 insertions, 1 deletions

diff --git a/run-tests.sh b/run-tests.sh
index b0a3c65a55e..2b934f9134d 100755
--- a/run-tests.sh
+++ b/run-tests.sh
@@ -202,7 +202,6 @@ function is_bad_test ()
               ./tests/bugs/snapshot/bug-1109889.t \
               ./tests/bugs/distribute/bug-1066798.t \
               ./tests/bugs/glusterd/bug-1238706-daemons-stop-on-peer-cleanup.t \
-              ./tests/bugs/glusterd/bug-948686.t \
               ./tests/geo-rep/georep-basic-dr-rsync.t \
               ./tests/geo-rep/georep-basic-dr-tarssh.t \
               ./tests/bugs/replicate/bug-1221481-allow-fops-on-dir-split-brain.t \
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index 8f8b3b36abd..a529fd3ca3f 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -3878,9 +3878,14 @@ glusterd_import_friend_volume (dict_t *peer_data, size_t count)
 
         ret = glusterd_volinfo_find (new_volinfo->volname, &old_volinfo);
         if (0 == ret) {
+                /* Ref count the old_volinfo such that deleting it doesn't crash
+                 * if its been already in use by other thread
+                 */
+                glusterd_volinfo_ref (old_volinfo);
                 (void) gd_check_and_update_rebalance_info (old_volinfo,
                                                            new_volinfo);
                 (void) glusterd_delete_stale_volume (old_volinfo, new_volinfo);
+                glusterd_volinfo_unref (old_volinfo);
         }
 
         if (glusterd_is_volume_started (new_volinfo)) {
diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
index 75f11e742b6..d997d60cd18 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
@@ -1450,6 +1450,14 @@ glusterd_op_stage_start_volume (dict_t *dict, char **op_errstr,
                         volname);
                 goto out;
         }
+        /* This is an incremental approach to have all the volinfo objects ref
+         * count. The first attempt is made in volume start transaction to
+         * ensure it doesn't race with import volume where stale volume is
+         * deleted. There are multiple instances of GlusterD crashing in
+         * bug-948686.t because of this. Once this approach is full proof, all
+         * other volinfo objects will be refcounted.
+         */
+        glusterd_volinfo_ref (volinfo);
 
         ret = glusterd_validate_volume_id (dict, volinfo);
         if (ret)
@@ -1580,6 +1588,9 @@ glusterd_op_stage_start_volume (dict_t *dict, char **op_errstr,
         volinfo->caps = caps;
         ret = 0;
 out:
+        if (volinfo)
+                glusterd_volinfo_unref (volinfo);
+
         if (ret && (msg[0] != '\0')) {
                 gf_msg (this->name, GF_LOG_ERROR, 0,
                         GD_MSG_OP_STAGE_START_VOL_FAIL, "%s", msg);
@@ -2463,6 +2474,14 @@ glusterd_op_start_volume (dict_t *dict, char **op_errstr)
                         volname);
                 goto out;
         }
+        /* This is an incremental approach to have all the volinfo objects ref
+         * count. The first attempt is made in volume start transaction to
+         * ensure it doesn't race with import volume where stale volume is
+         * deleted. There are multiple instances of GlusterD crashing in
+         * bug-948686.t because of this. Once this approach is full proof, all
+         * other volinfo objects will be refcounted.
+         */
+        glusterd_volinfo_ref (volinfo);
 
         /* A bricks mount dir is required only by snapshots which were
          * introduced in gluster-3.6.0
@@ -2537,6 +2556,9 @@ glusterd_op_start_volume (dict_t *dict, char **op_errstr)
         ret = glusterd_svcs_manager (volinfo);
 
 out:
+        if (!volinfo)
+                glusterd_volinfo_unref (volinfo);
+
         gf_msg_trace (this->name, 0, "returning %d ", ret);
         return ret;
 }