From 0f35b301fcb539dd85a1dae202b3ce532852d4d6 Mon Sep 17 00:00:00 2001 From: Atin Mukherjee Date: Fri, 9 Oct 2015 22:31:28 +0530 Subject: 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 Reviewed-on: http://review.gluster.org/12329 Tested-by: NetBSD Build System Tested-by: Gluster Build System Reviewed-by: Jeff Darcy --- run-tests.sh | 1 - xlators/mgmt/glusterd/src/glusterd-utils.c | 5 +++++ xlators/mgmt/glusterd/src/glusterd-volume-ops.c | 22 ++++++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) 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; } -- cgit