From bd8fc26a278697c30537d879ea5402db7ebab577 Mon Sep 17 00:00:00 2001 From: Mohit Agrawal Date: Sat, 4 Aug 2018 12:05:03 +0530 Subject: glusterd: Compare volume_id before start/attach a brick Problem: After reboot a node brick is not coming up because fsid comparison is failed before start a brick Solution: Instead of comparing fsid compare volume_id to resolve the same because fsid is changed after reboot a node but volume_id persist as a xattr on brick_root path at the time of creating a volume. Change-Id: Ic289aab1b4ebfd83bbcae8438fee26ae61a0fff4 fixes: bz#1612418 Signed-off-by: Mohit Agrawal --- tests/basic/bug-1595320.t | 92 ----------------------------- tests/bugs/glusterd/bug-1595320.t | 93 ++++++++++++++++++++++++++++++ xlators/mgmt/glusterd/src/glusterd-utils.c | 47 ++++++++------- 3 files changed, 120 insertions(+), 112 deletions(-) delete mode 100644 tests/basic/bug-1595320.t create mode 100644 tests/bugs/glusterd/bug-1595320.t diff --git a/tests/basic/bug-1595320.t b/tests/basic/bug-1595320.t deleted file mode 100644 index 9d856eeadec..00000000000 --- a/tests/basic/bug-1595320.t +++ /dev/null @@ -1,92 +0,0 @@ -#!/bin/bash - -. $(dirname $0)/../include.rc -. $(dirname $0)/../volume.rc -. $(dirname $0)/../snapshot.rc - -cleanup - -function count_up_bricks { - $CLI --xml volume status $V0 | grep '1' | wc -l -} - -function count_brick_processes { - pgrep glusterfsd | wc -l -} - -# Setup 3 LVMS -LVM_PREFIX="test" -TEST init_n_bricks 3 -TEST setup_lvm 3 - -# Start glusterd -TEST glusterd -TEST pidof glusterd - -# Create volume and enable brick multiplexing -TEST $CLI volume create $V0 $H0:$L1 $H0:$L2 $H0:$L3 -gluster v set all cluster.brick-multiplex on - -# Start the volume -TEST $CLI volume start $V0 -EXPECT_WITHIN $PROCESS_UP_TIMEOUT 3 count_up_bricks -EXPECT 1 count_brick_processes - -# Kill volume ungracefully -brick_pid=`pgrep glusterfsd` - -# Make sure every brick root should be consumed by a brick process -n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L1 | grep -v ".glusterfs" | wc -l` -TEST [ $n -eq 1 ] -n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L2 | grep -v ".glusterfs" | wc -l` -TEST [ $n -eq 1 ] -n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L3 | grep -v ".glusterfs" | wc -l` -TEST [ $n -eq 1 ] - -b1_pid_file=$(ls $GLUSTERD_PIDFILEDIR/vols/$V0/*d-backends-1*.pid) -b2_pid_file=$(ls $GLUSTERD_PIDFILEDIR/vols/$V0/*d-backends-2*.pid) -b3_pid_file=$(ls $GLUSTERD_PIDFILEDIR/vols/$V0/*d-backends-3*.pid) - -kill -9 $brick_pid -EXPECT 0 count_brick_processes - -# Unmount 3rd brick root from node -brick_root=$L3 -TEST umount -l $brick_root 2>/dev/null - -# Start the volume only 2 brick should be start -TEST $CLI volume start $V0 force -EXPECT_WITHIN $PROCESS_UP_TIMEOUT 2 count_up_bricks -EXPECT 1 count_brick_processes - -brick_pid=`pgrep glusterfsd` - -# Make sure only two brick root should be consumed by a brick process -n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L1 | grep -v ".glusterfs" | wc -l` -TEST [ $n -eq 1 ] -n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L2 | grep -v ".glusterfs" | wc -l` -TEST [ $n -eq 1 ] -n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L3 | grep -v ".glusterfs" | wc -l` -TEST [ $n -eq 0 ] - -# Mount the brick root -TEST mount -t xfs -o nouuid /dev/test_vg_3/brick_lvm $brick_root - -# Replace brick_pid file to test brick_attach code -TEST cp $b1_pid_file $b3_pid_file - -# Start the volume all brick should be up -TEST $CLI volume start $V0 force - -EXPECT_WITHIN $PROCESS_UP_TIMEOUT 3 count_up_bricks -EXPECT 1 count_brick_processes - -# Make sure every brick root should be consumed by a brick process -n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L1 | grep -v ".glusterfs" | wc -l` -TEST [ $n -eq 1 ] -n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L2 | grep -v ".glusterfs" | wc -l` -TEST [ $n -eq 1 ] -n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L3 | grep -v ".glusterfs" | wc -l` -TEST [ $n -eq 1 ] - -cleanup diff --git a/tests/bugs/glusterd/bug-1595320.t b/tests/bugs/glusterd/bug-1595320.t new file mode 100644 index 00000000000..f41df9d0ffa --- /dev/null +++ b/tests/bugs/glusterd/bug-1595320.t @@ -0,0 +1,93 @@ +#!/bin/bash + +. $(dirname $0)/../../include.rc +. $(dirname $0)/../../volume.rc +. $(dirname $0)/../../snapshot.rc + +cleanup + +function count_up_bricks { + $CLI --xml volume status $V0 | grep '1' | wc -l +} + +function count_brick_processes { + pgrep glusterfsd | wc -l +} + +# Setup 3 LVMS +LVM_PREFIX="test" +TEST init_n_bricks 3 +TEST setup_lvm 3 + +# Start glusterd +TEST glusterd +TEST pidof glusterd + +# Create volume and enable brick multiplexing +TEST $CLI volume create $V0 $H0:$L1 $H0:$L2 $H0:$L3 +gluster v set all cluster.brick-multiplex on + +# Start the volume +TEST $CLI volume start $V0 +EXPECT_WITHIN $PROCESS_UP_TIMEOUT 3 count_up_bricks +EXPECT 1 count_brick_processes + +# Kill volume ungracefully +brick_pid=`pgrep glusterfsd` + +# Make sure every brick root should be consumed by a brick process +n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L1 | grep -v ".glusterfs" | wc -l` +TEST [ $n -eq 1 ] +n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L2 | grep -v ".glusterfs" | wc -l` +TEST [ $n -eq 1 ] +n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L3 | grep -v ".glusterfs" | wc -l` +TEST [ $n -eq 1 ] + +b1_pid_file=$(ls $GLUSTERD_PIDFILEDIR/vols/$V0/*d-backends-1*.pid) +b2_pid_file=$(ls $GLUSTERD_PIDFILEDIR/vols/$V0/*d-backends-2*.pid) +b3_pid_file=$(ls $GLUSTERD_PIDFILEDIR/vols/$V0/*d-backends-3*.pid) + +kill -9 $brick_pid +EXPECT 0 count_brick_processes + +# Unmount 3rd brick root from node +brick_root=$L3 +_umount_lv 3 + +# Start the volume only 2 brick should be start +TEST $CLI volume start $V0 force +EXPECT_WITHIN $PROCESS_UP_TIMEOUT 2 count_up_bricks +EXPECT 1 count_brick_processes + +brick_pid=`pgrep glusterfsd` + +# Make sure only two brick root should be consumed by a brick process +n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L1 | grep -v ".glusterfs" | wc -l` +TEST [ $n -eq 1 ] +n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L2 | grep -v ".glusterfs" | wc -l` +TEST [ $n -eq 1 ] +n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L3 | grep -v ".glusterfs" | wc -l` +TEST [ $n -eq 0 ] + +# Mount the brick root +TEST mkdir -p $brick_root +TEST mount -t xfs -o nouuid /dev/test_vg_3/brick_lvm $brick_root + +# Replace brick_pid file to test brick_attach code +TEST cp $b1_pid_file $b3_pid_file + +# Start the volume all brick should be up +TEST $CLI volume start $V0 force + +EXPECT_WITHIN $PROCESS_UP_TIMEOUT 3 count_up_bricks +EXPECT 1 count_brick_processes + +# Make sure every brick root should be consumed by a brick process +n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L1 | grep -v ".glusterfs" | wc -l` +TEST [ $n -eq 1 ] +n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L2 | grep -v ".glusterfs" | wc -l` +TEST [ $n -eq 1 ] +n=`ls -lrth /proc/$brick_pid/fd | grep -iw $L3 | grep -v ".glusterfs" | wc -l` +TEST [ $n -eq 1 ] + +cleanup diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c index 0ac075bcc87..352e445e28b 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-utils.c @@ -5522,6 +5522,12 @@ attach_brick_callback (struct rpc_req *req, struct iovec *iov, int count, frame->local = NULL; frame->cookie = NULL; + if (!iov) { + gf_log (frame->this->name, GF_LOG_ERROR, "iov is NULL"); + ret = -1; + goto out; + } + ret = xdr_to_generic (*iov, &rsp, (xdrproc_t)xdr_gf_getspec_rsp); if (ret < 0) { gf_log (frame->this->name, GF_LOG_ERROR, "XDR decoding error"); @@ -6164,17 +6170,19 @@ glusterd_brick_start (glusterd_volinfo_t *volinfo, gf_boolean_t wait, gf_boolean_t only_connect) { - int ret = -1; + int ret = -1; xlator_t *this = NULL; glusterd_brickinfo_t *other_brick; glusterd_conf_t *conf = NULL; - int32_t pid = -1; - char pidfile[PATH_MAX] = {0}; - char socketpath[PATH_MAX] = {0}; - char *brickpath = NULL; + int32_t pid = -1; + char pidfile[PATH_MAX] = {0}; + char socketpath[PATH_MAX] = {0}; + char *brickpath = NULL; glusterd_volinfo_t *other_vol; - struct statvfs brickstat = {0,}; gf_boolean_t is_service_running = _gf_false; + uuid_t volid = {0,}; + ssize_t size = -1; + this = THIS; GF_ASSERT (this); @@ -6221,24 +6229,23 @@ glusterd_brick_start (glusterd_volinfo_t *volinfo, GLUSTERD_GET_BRICK_PIDFILE (pidfile, volinfo, brickinfo, conf); - ret = sys_statvfs (brickinfo->path, &brickstat); - if (ret) { - gf_msg (this->name, GF_LOG_ERROR, - errno, GD_MSG_BRICKINFO_CREATE_FAIL, - "failed to get statfs() call on brick %s", - brickinfo->path); + /* Compare volume-id xattr is helpful to ensure the existence of a brick_root + path before the start/attach a brick + */ + size = sys_lgetxattr (brickinfo->path, GF_XATTR_VOL_ID_KEY, volid, 16); + if (size != 16) { + gf_log (this->name, GF_LOG_ERROR, + "Missing %s extended attribute on brick root (%s)," + " brick is deemed not to be a part of the volume (%s) ", + GF_XATTR_VOL_ID_KEY, brickinfo->path, volinfo->volname); goto out; } - /* Compare fsid is helpful to ensure the existence of a brick_root - path before the start/attach a brick - */ - if (brickinfo->statfs_fsid && - (brickinfo->statfs_fsid != brickstat.f_fsid)) { + if (strncmp (uuid_utoa (volinfo->volume_id), uuid_utoa(volid), GF_UUID_BUF_SIZE)) { gf_log (this->name, GF_LOG_ERROR, - "fsid comparison is failed it means Brick root path" - " %s is not created by glusterd, start/attach will also fail", - brickinfo->path); + "Mismatching %s extended attribute on brick root (%s)," + " brick is deemed not to be a part of the volume (%s)", + GF_XATTR_VOL_ID_KEY, brickinfo->path, volinfo->volname); goto out; } is_service_running = gf_is_service_running (pidfile, &pid); -- cgit