From f74ff011fce5959884a4f47f0c87ae8795d64a24 Mon Sep 17 00:00:00 2001 From: Raghavendra Talur Date: Wed, 31 Dec 2014 14:26:37 +0530 Subject: glusterd: Fix strtok_r parsing. Found a bug where a replica 2 volume creation prompts saying the bricks are in the same hosts even when they are in different hosts. Change-Id: Ie55addae55c55e32ad2b5339530ab71f0e3711ab BUG: 1091935 Signed-off-by: Raghavendra Talur Reviewed-on: http://review.gluster.org/9373 Tested-by: Gluster Build System Reviewed-by: Ravishankar N Reviewed-by: Gaurav Kumar Garg Reviewed-by: Krishnan Parthasarathi Tested-by: Krishnan Parthasarathi --- ...091935-brick-order-check-from-cli-to-glusterd.t | 84 +++++++++++++++++++--- xlators/mgmt/glusterd/src/glusterd-volume-ops.c | 7 +- 2 files changed, 79 insertions(+), 12 deletions(-) diff --git a/tests/bugs/glusterd/bug-1091935-brick-order-check-from-cli-to-glusterd.t b/tests/bugs/glusterd/bug-1091935-brick-order-check-from-cli-to-glusterd.t index 01cc5b56097..9ac9d8fedd9 100755 --- a/tests/bugs/glusterd/bug-1091935-brick-order-check-from-cli-to-glusterd.t +++ b/tests/bugs/glusterd/bug-1091935-brick-order-check-from-cli-to-glusterd.t @@ -2,26 +2,92 @@ . $(dirname $0)/../../include.rc . $(dirname $0)/../../volume.rc +. $(dirname $0)/../../cluster.rc + +function check_peers { + $CLI_1 peer status | grep 'Peer in Cluster (Connected)' | wc -l +} cleanup; -TEST glusterd -TEST pidof glusterd +## Lets create partitions for bricks +TEST truncate -s 100M $B0/brick1 +TEST truncate -s 200M $B0/brick2 +TEST truncate -s 200M $B0/brick3 +TEST truncate -s 200M $B0/brick4 + + +TEST LO1=`SETUP_LOOP $B0/brick1` +TEST LO2=`SETUP_LOOP $B0/brick2` +TEST LO3=`SETUP_LOOP $B0/brick3` +TEST LO4=`SETUP_LOOP $B0/brick4` + + +TEST MKFS_LOOP $LO1 +TEST MKFS_LOOP $LO2 +TEST MKFS_LOOP $LO3 +TEST MKFS_LOOP $LO4 + +TEST mkdir -p ${B0}/${V0}{0..3} + + +TEST MOUNT_LOOP $LO1 $B0/${V0}0 +TEST MOUNT_LOOP $LO2 $B0/${V0}1 +TEST MOUNT_LOOP $LO3 $B0/${V0}2 +TEST MOUNT_LOOP $LO4 $B0/${V0}3 + + +TEST launch_cluster 2; +TEST $CLI_1 peer probe $H2; +EXPECT_WITHIN $PROBE_TIMEOUT 1 check_peers; + + +CLI_1_WITHOUT_WIGNORE=$(echo $CLI_1 | sed 's/ --wignore//') -cli1=$(echo $CLI | sed 's/ --wignore//') # Creating volume with non resolvable host name -TEST ! $cli1 volume create $V0 replica 2 $H0:$B0/${V0}0 redhat:$B0/${V0}1 \ - $H0:$B0/${V0}2 redhat:$B0/${V0}3 +TEST ! $CLI_1_WITHOUT_WIGNORE volume create $V0 replica 2 \ + $H1:$B0/${V0}0/brick redhat:$B0/${V0}1/brick \ + $H1:$B0/${V0}2/brick redhat:$B0/${V0}3/brick + + +#Workaround for Bug:1091935 +#Failure to create volume above leaves 1st brick with xattrs. +rm -rf $B0/${V0}{0..3}/brick; + # Creating distribute-replica volume with bad brick order. It will fail # due to bad brick order. -TEST ! $cli1 volume create $V0 replica 2 $H0:$B0/${V0}0 $H0:$B0/${V0}1 \ - $H0:$B0/${V0}2 $H0:$B0/${V0}3 +TEST ! $CLI_1_WITHOUT_WIGNORE volume create $V0 replica 2 \ + $H1:$B0/${V0}0/brick $H1:$B0/${V0}1/brick \ + $H1:$B0/${V0}2/brick $H1:$B0/${V0}3/brick + + + +#Workaround for Bug:1091935 +#Failure to create volume above leaves 1st brick with xattrs. +rm -rf $B0/${V0}{0..3}/brick; + + + +# Test for positive case, volume create should pass for +# resolved hostnames and bricks in order. +TEST $CLI_1_WITHOUT_WIGNORE volume create $V0 replica 2 \ + $H1:$B0/${V0}0/brick $H2:$B0/${V0}1/brick \ + $H1:$B0/${V0}2/brick $H2:$B0/${V0}3/brick + +# Delete the volume as we want to reuse bricks +TEST $CLI_1_WITHOUT_WIGNORE volume delete $V0 + # Now with force at the end of command it will bypass brick-order check # for replicate or distribute-replicate volume. and it will create volume -TEST $cli1 volume create $V0 replica 2 $H0:$B0/${V0}0 $H0:$B0/${V0}1 \ - $H0:$B0/${V0}2 $H0:$B0/${V0}3 force +TEST $CLI_1_WITHOUT_WIGNORE volume create $V0 replica 2 \ + $H1:$B0/${V0}0/brick $H1:$B0/${V0}1/brick \ + $H1:$B0/${V0}2/brick $H1:$B0/${V0}3/brick force + +# Need to cleanup the loop back devices. +UMOUNT_LOOP ${B0}/${V0}{0..3} +rm -f ${B0}/brick{1..4} cleanup; diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c index 0535fedd753..4c2c895decb 100644 --- a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c @@ -88,6 +88,7 @@ glusterd_check_brick_order(dict_t *dict, char *err_str) char *brick = NULL; char *brick_list = NULL; char *brick_list_dup = NULL; + char *brick_list_ptr = NULL; char *tmpptr = NULL; char *volname = NULL; int32_t brick_count = 0; @@ -160,12 +161,12 @@ glusterd_check_brick_order(dict_t *dict, char *err_str) " found. Checking brick order."); } - brick_list_dup = gf_strdup(brick_list); + brick_list_dup = brick_list_ptr = gf_strdup(brick_list); /* Resolve hostnames and get addrinfo */ while (i < brick_count) { ++i; brick = strtok_r (brick_list_dup, " \n", &tmpptr); - brick_list = tmpptr; + brick_list_dup = tmpptr; if (brick == NULL) goto check_failed; brick = strtok_r (brick, ":", &tmpptr); @@ -234,7 +235,7 @@ found_bad_brick_order: ret = -1; out: ai_list_tmp2 = NULL; - GF_FREE (brick_list_dup); + GF_FREE (brick_list_ptr); list_for_each_entry (ai_list_tmp1, &ai_list->list, list) { if (ai_list_tmp1->info) freeaddrinfo (ai_list_tmp1->info); -- cgit