From a997c8611e11a752698d53e26f11fa7d5c5486ea Mon Sep 17 00:00:00 2001
From: Emoly Liu <emoly.liu@intel.com>
Date: Tue, 2 Apr 2013 15:18:29 +0800
Subject: [PATCH] LU-2826 tests: add useful text to error() calls

Many tests just call "error" without any arguments, but this does not
provide any information to Maloo about why the test fails.  This
in turn causes autovet to match the empty failure message to many
different and irrelevant bugs, making Maloo statistics inaccurate.

Change a number of error() calls to have some valid error text, and
print something more useful if error() is called without arguments.
Not all of the error() calls in sanity.sh and sanityn.sh have been
converted, but it is at least a good start at cleaning this up, and
catches the most frequent failure cases currently.

Signed-off-by: Andreas Dilger <andreas.dilger@intel.com>
Signed-off-by: Liu Ying <emoly.liu@intel.com>
Change-Id: Ie148e3e26e1ecb888ac021a8df7cd995183ebbe5
Reviewed-on: http://review.whamcloud.com/5750
Tested-by: Hudson
Reviewed-by: Nathaniel Clark <nathaniel.l.clark@intel.com>
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Jian Yu <jian.yu@intel.com>
---
 lustre/tests/conf-sanity.sh    |   4 +-
 lustre/tests/sanity-gss.sh     |  58 +++--
 lustre/tests/sanity.sh         | 484 +++++++++++++++++++++--------------------
 lustre/tests/test-framework.sh |  58 ++---
 4 files changed, 309 insertions(+), 295 deletions(-)

diff --git a/lustre/tests/conf-sanity.sh b/lustre/tests/conf-sanity.sh
index 655329f..671f82f 100644
--- a/lustre/tests/conf-sanity.sh
+++ b/lustre/tests/conf-sanity.sh
@@ -871,8 +871,8 @@ test_23a() {	# was test_23
 		"MOUNT_LUSTRE_PID $MOUNT_LUSTRE_PID still not killed in $WAIT secs"
 		ps -ef | grep mount
 	fi
-	stop_mds || error
-	stop_ost || error
+	stop_mds || error "stopping MDSes failed"
+	stop_ost || error "stopping OSSes failed"
 }
 run_test 23a "interrupt client during recovery mount delay"
 
diff --git a/lustre/tests/sanity-gss.sh b/lustre/tests/sanity-gss.sh
index 550a1ff..8b695b6 100644
--- a/lustre/tests/sanity-gss.sh
+++ b/lustre/tests/sanity-gss.sh
@@ -1,5 +1,4 @@
 #!/bin/bash
-# vim:shiftwidth=4:softtabstop=4:tabstop=4:
 #
 # Run select tests by setting ONLY, or as arguments to the script.
 # Skip specific tests by setting EXCEPT.
@@ -347,35 +346,34 @@ test_6() {
 run_test 6 "test basic DLM callback works"
 
 test_7() {
-    local tdir=$DIR/d7
-    local num_osts
-
-    #
-    # for open(), client only reserve space for default stripe count lovea,
-    # and server may return larger lovea in reply (because of larger stripe
-    # count), client need call enlarge_reqbuf() and save the replied lovea
-    # in request for future possible replay.
-    #
-    # Note: current script does NOT guarantee enlarge_reqbuf() will be in
-    # the path, however it does work in local test which has 2 OSTs and
-    # default stripe count is 1.
-    #
-    num_osts=`$LFS getstripe $MOUNT | egrep "^[0-9]*:.*ACTIVE" | wc -l`
-    echo "found $num_osts active OSTs"
-    [ $num_osts -lt 2 ] && echo "skipping $TESTNAME (must have >= 2 OSTs)" && return
-
-    mkdir $tdir || error
-    $LFS setstripe -c $num_osts $tdir || error
-
-    echo "creating..."
-    for ((i=0;i<20;i++)); do
-        dd if=/dev/zero of=$tdir/f$i bs=4k count=16 2>/dev/null
-    done
-    echo "reading..."
-    for ((i=0;i<20;i++)); do
-        dd if=$tdir/f$i of=/dev/null bs=4k count=16 2>/dev/null
-    done
-    rm -rf $tdir
+	local tdir=$DIR/d7
+	local num_osts
+
+	# for open(), client only reserve space for default stripe count lovea,
+	# and server may return larger lovea in reply (because of larger stripe
+	# count), client need call enlarge_reqbuf() and save the replied lovea
+	# in request for future possible replay.
+	#
+	# Note: current script does NOT guarantee enlarge_reqbuf() will be in
+	# the path, however it does work in local test which has 2 OSTs and
+	# default stripe count is 1.
+	num_osts=$($LFS getstripe $MOUNT | egrep "^[0-9]*:.*ACTIVE" | wc -l)
+	echo "found $num_osts active OSTs"
+	[ $num_osts -lt 2 ] &&
+		echo "skipping $TESTNAME (must have >= 2 OSTs)" && return
+
+	mkdir $tdir || error "mkdir $tdir failed"
+	$LFS setstripe -c $num_osts $tdir || error "setstripe -c $num_osts"
+
+	echo "creating..."
+	for ((i = 0; i < 20; i++)); do
+		dd if=/dev/zero of=$tdir/f$i bs=4k count=16 2>/dev/null
+	done
+	echo "reading..."
+	for ((i = 0; i < 20; i++)); do
+		dd if=$tdir/f$i of=/dev/null bs=4k count=16 2>/dev/null
+	done
+	rm -rf $tdir
 }
 run_test 7 "exercise enlarge_reqbuf()"
 
diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index d5ce819..0f2556c 100644
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -139,105 +139,82 @@ echo # add a newline after mke2fs.
 
 umask 077
 
-OLDDEBUG="`lctl get_param -n debug 2> /dev/null`"
+OLDDEBUG=$(lctl get_param -n debug 2> /dev/null)
 lctl set_param debug=-1 2> /dev/null || true
 test_0() {
 	touch $DIR/$tfile
-	$CHECKSTAT -t file $DIR/$tfile || error
+	$CHECKSTAT -t file $DIR/$tfile || error "$tfile is not a file"
 	rm $DIR/$tfile
-	$CHECKSTAT -a $DIR/$tfile || error
+	$CHECKSTAT -a $DIR/$tfile || error "$tfile was not removed"
 }
 run_test 0 "touch .../$tfile ; rm .../$tfile ====================="
 
 test_0b() {
-	chmod 0755 $DIR || error
-	$CHECKSTAT -p 0755 $DIR || error
+	chmod 0755 $DIR || error "chmod 0755 $DIR failed"
+	$CHECKSTAT -p 0755 $DIR || error "$DIR permission is not 0755"
 }
 run_test 0b "chmod 0755 $DIR ============================="
 
 test_0c() {
-    $LCTL get_param mdc.*.import | grep  "state: FULL" || error "import not FULL"
-    $LCTL get_param mdc.*.import | grep  "target: $FSNAME-MDT" || error "bad target"
+	$LCTL get_param mdc.*.import | grep "state: FULL" ||
+		error "import not FULL"
+	$LCTL get_param mdc.*.import | grep "target: $FSNAME-MDT" ||
+		error "bad target"
 }
 run_test 0c "check import proc ============================="
 
-test_1a() {
-	test_mkdir -p $DIR/$tdir
-	test_mkdir -p $DIR/$tdir/d2
+test_1() {
+	test_mkdir -p $DIR/$tdir || error "mkdir $tdir failed"
+	test_mkdir -p $DIR/$tdir/d2 || error "mkdir $tdir/d2 failed"
 	test_mkdir $DIR/$tdir/d2 && error "we expect EEXIST, but not returned"
-	$CHECKSTAT -t dir $DIR/$tdir/d2 || error
-}
-run_test 1a "mkdir .../d1; mkdir .../d1/d2 ====================="
-
-test_1b() {
+	$CHECKSTAT -t dir $DIR/$tdir/d2 || error "$tdir/d2 is not a dir"
 	rmdir $DIR/$tdir/d2
 	rmdir $DIR/$tdir
-	$CHECKSTAT -a $DIR/$tdir || error
-}
-run_test 1b "rmdir .../d1/d2; rmdir .../d1 ====================="
-
-test_2a() {
-	test_mkdir $DIR/$tdir
-	touch $DIR/$tdir/$tfile
-	$CHECKSTAT -t file $DIR/$tdir/$tfile || error
+	$CHECKSTAT -a $DIR/$tdir || error "$tdir was not removed"
 }
-run_test 2a "mkdir .../d2; touch .../d2/f ======================"
+run_test 1 "mkdir; remkdir; rmdir =============================="
 
-test_2b() {
+test_2() {
+	test_mkdir -p $DIR/$tdir || error "mkdir $tdir failed"
+	touch $DIR/$tdir/$tfile || error "touch $tdir/$tfile failed"
+	$CHECKSTAT -t file $DIR/$tdir/$tfile || error "$tdir/$tfile not a file"
 	rm -r $DIR/$tdir
-	$CHECKSTAT -a $DIR/$tdir || error
+	$CHECKSTAT -a $DIR/$tdir/$tfile || error "$tdir/$file is not removed"
 }
-run_test 2b "rm -r .../d2; checkstat .../d2/f ======================"
+run_test 2 "mkdir; touch; rmdir; check file ===================="
 
-test_3a() {
-	test_mkdir -p $DIR/$tdir
-	$CHECKSTAT -t dir $DIR/$tdir || error
-}
-run_test 3a "mkdir .../d3 ======================================"
-
-test_3b() {
-	if [ ! -d $DIR/$tdir ]; then
-		mkdir $DIR/$tdir
-	fi
+test_3() {
+	test_mkdir -p $DIR/$tdir || error "mkdir $tdir failed"
+	$CHECKSTAT -t dir $DIR/$tdir || error "$tdir is not a directory"
 	touch $DIR/$tdir/$tfile
-	$CHECKSTAT -t file $DIR/$tdir/$tfile || error
-}
-run_test 3b "touch .../d3/f ===================================="
-
-test_3c() {
+	$CHECKSTAT -t file $DIR/$tdir/$tfile || error "$tdir/$tfile not a file"
 	rm -r $DIR/$tdir
-	$CHECKSTAT -a $DIR/$tdir || error
-}
-run_test 3c "rm -r .../d3 ======================================"
-
-test_4a() {
-	test_mkdir -p $DIR/$tdir
-	$CHECKSTAT -t dir $DIR/$tdir || error
+	$CHECKSTAT -a $DIR/$tdir || error "$tdir is not removed"
 }
-run_test 4a "mkdir .../d4 ======================================"
+run_test 3 "mkdir; touch; rmdir; check dir ====================="
 
-test_4b() {
-	if [ ! -d $DIR/$tdir ]; then
-		test_mkdir $DIR/$tdir
-	fi
-	test_mkdir $DIR/$tdir/d2
-	mkdir $DIR/$tdir/d2
-	$CHECKSTAT -t dir $DIR/$tdir/d2 || error
+test_4() {
+	test_mkdir -p $DIR/$tdir || error "mkdir $tdir failed"
+	$CHECKSTAT -t dir $DIR/$tdir || error "$tdir is not a directory"
+	test_mkdir $DIR/$tdir/d2 || error "mkdir $tdir/d2 failed"
+	$CHECKSTAT -t dir $DIR/$tdir/d2 || error "$tdir/d2 is not a directory"
 }
-run_test 4b "mkdir .../d4/d2 ==================================="
+run_test 4 "mkdir =============================================="
 
 test_5() {
-	test_mkdir $DIR/$tdir
-	test_mkdir $DIR/$tdir/d2
-	chmod 0707 $DIR/$tdir/d2
-	$CHECKSTAT -t dir -p 0707 $DIR/$tdir/d2 || error
+	test_mkdir -p $DIR/$tdir || error "mkdir $tdir failed"
+	test_mkdir $DIR/$tdir/d2 || error "mkdir $tdir/d2 failed"
+	chmod 0707 $DIR/$tdir/d2 || error "chmod 0707 $tdir/d2 failed"
+	$CHECKSTAT -t dir -p 0707 $DIR/$tdir/d2 || error "$tdir/d2 not mode 707"
+	$CHECKSTAT -t dir $DIR/$tdir/d2 || error "$tdir/d2 is not a directory"
 }
 run_test 5 "mkdir .../d5 .../d5/d2; chmod .../d5/d2 ============"
 
 test_6a() {
-	touch $DIR/$tfile
-	chmod 0666 $DIR/$tfile || error
-	$CHECKSTAT -t file -p 0666 -u \#$UID $DIR/$tfile || error
+	touch $DIR/$tfile || error "touch $DIR/$tfile failed"
+	chmod 0666 $DIR/$tfile || error "chmod 0666 $tfile failed"
+	$CHECKSTAT -t file -p 0666 -u \#$UID $DIR/$tfile ||
+		error "$tfile does not have perm 0666 or UID $UID"
 }
 run_test 6a "touch .../f6a; chmod .../f6a ======================"
 
@@ -247,16 +224,18 @@ test_6b() {
 		touch $DIR/$tfile
 		chmod 0666 $DIR/$tfile
 	fi
-	$RUNAS chmod 0444 $DIR/$tfile && error
-	$CHECKSTAT -t file -p 0666 -u \#$UID $DIR/$tfile || error
+	$RUNAS chmod 0444 $DIR/$tfile && error "chmod $tfile worked on UID $UID"
+	$CHECKSTAT -t file -p 0666 -u \#$UID $DIR/$tfile ||
+		error "$tfile should be 0666 and owned by UID $UID"
 }
 run_test 6b "$RUNAS chmod .../f6a (should return error) =="
 
 test_6c() {
 	[ $RUNAS_ID -eq $UID ] && skip_env "RUNAS_ID = UID = $UID" && return
 	touch $DIR/$tfile
-	chown $RUNAS_ID $DIR/$tfile || error
-	$CHECKSTAT -t file -u \#$RUNAS_ID $DIR/$tfile || error
+	chown $RUNAS_ID $DIR/$tfile || error "chown $RUNAS_ID $file failed"
+	$CHECKSTAT -t file -u \#$RUNAS_ID $DIR/$tfile ||
+		error "$tfile should be owned by UID $RUNAS_ID"
 }
 run_test 6c "touch .../f6c; chown .../f6c ======================"
 
@@ -266,16 +245,18 @@ test_6d() {
 		touch $DIR/$tfile
 		chown $RUNAS_ID $DIR/$tfile
 	fi
-	$RUNAS chown $UID $DIR/$tfile && error
-	$CHECKSTAT -t file -u \#$RUNAS_ID $DIR/$tfile || error
+	$RUNAS chown $UID $DIR/$tfile && error "chown $UID $file succeeded"
+	$CHECKSTAT -t file -u \#$RUNAS_ID $DIR/$tfile ||
+		error "$tfile should be owned by UID $RUNAS_ID"
 }
 run_test 6d "$RUNAS chown .../f6c (should return error) =="
 
 test_6e() {
 	[ $RUNAS_ID -eq $UID ] && skip_env "RUNAS_ID = UID = $UID" && return
 	touch $DIR/$tfile
-	chgrp $RUNAS_ID $DIR/$tfile || error
-	$CHECKSTAT -t file -u \#$UID -g \#$RUNAS_ID $DIR/$tfile || error
+	chgrp $RUNAS_ID $DIR/$tfile || error "chgrp $RUNAS_ID $file failed"
+	$CHECKSTAT -t file -u \#$UID -g \#$RUNAS_ID $DIR/$tfile ||
+		error "$tfile should be owned by GID $UID"
 }
 run_test 6e "touch .../f6e; chgrp .../f6e ======================"
 
@@ -285,19 +266,21 @@ test_6f() {
 		touch $DIR/$tfile
 		chgrp $RUNAS_ID $DIR/$tfile
 	fi
-	$RUNAS chgrp $UID $DIR/$tfile && error
-	$CHECKSTAT -t file -u \#$UID -g \#$RUNAS_ID $DIR/$tfile || error
+	$RUNAS chgrp $UID $DIR/$tfile && error "chgrp $UID $file succeeded"
+	$CHECKSTAT -t file -u \#$UID -g \#$RUNAS_ID $DIR/$tfile ||
+		error "$tfile should be owned by UID $UID and GID $RUNAS_ID"
 }
 run_test 6f "$RUNAS chgrp .../f6e (should return error) =="
 
 test_6g() {
 	[ $RUNAS_ID -eq $UID ] && skip_env "RUNAS_ID = UID = $UID" && return
-	test_mkdir $DIR/$tdir || error
-        chmod 777 $DIR/$tdir || error
-        $RUNAS mkdir $DIR/$tdir/d || error
-        chmod g+s $DIR/$tdir/d || error
-        test_mkdir $DIR/$tdir/d/subdir
-	$CHECKSTAT -g \#$RUNAS_GID $DIR/$tdir/d/subdir || error
+	test_mkdir $DIR/$tdir || error "mkdir $tfile failed"
+	chmod 777 $DIR/$tdir || error "chmod 0777 $tdir failed"
+	$RUNAS mkdir $DIR/$tdir/d || error "mkdir $tdir/d failed"
+	chmod g+s $DIR/$tdir/d || error "chmod g+s $tdir/d failed"
+	test_mkdir $DIR/$tdir/d/subdir || error "mkdir $tdir/d/subdir failed"
+	$CHECKSTAT -g \#$RUNAS_GID $DIR/$tdir/d/subdir ||
+		error "$tdir/d/subdir should be GID $RUNAS_GID"
 }
 run_test 6g "Is new dir in sgid dir inheriting group?"
 
@@ -306,8 +289,9 @@ test_6h() { # bug 7331
 	touch $DIR/$tfile || error "touch failed"
 	chown $RUNAS_ID:$RUNAS_GID $DIR/$tfile || error "initial chown failed"
 	$RUNAS -G$RUNAS_GID chown $RUNAS_ID:0 $DIR/$tfile &&
-		error "chown worked"
-	$CHECKSTAT -t file -u \#$RUNAS_ID -g \#$RUNAS_GID $DIR/$tfile || error
+		error "chown $RUNAS_ID:0 $tfile worked as GID $RUNAS_GID"
+	$CHECKSTAT -t file -u \#$RUNAS_ID -g \#$RUNAS_GID $DIR/$tfile ||
+		error "$tdir/$tfile should be UID $RUNAS_UID GID $RUNAS_GID"
 }
 run_test 6h "$RUNAS chown RUNAS_ID.0 .../f6h (should return error)"
 
@@ -315,7 +299,8 @@ test_7a() {
 	test_mkdir $DIR/$tdir
 	$MCREATE $DIR/$tdir/$tfile
 	chmod 0666 $DIR/$tdir/$tfile
-	$CHECKSTAT -t file -p 0666 $DIR/$tdir/$tfile || error
+	$CHECKSTAT -t file -p 0666 $DIR/$tdir/$tfile ||
+		error "$tdir/$tfile should be mode 0666"
 }
 run_test 7a "mkdir .../d7; mcreate .../d7/f; chmod .../d7/f ===="
 
@@ -325,8 +310,8 @@ test_7b() {
 	fi
 	$MCREATE $DIR/$tdir/$tfile
 	echo -n foo > $DIR/$tdir/$tfile
-	[ "`cat $DIR/$tdir/$tfile`" = "foo" ] || error
-	$CHECKSTAT -t file -s 3 $DIR/$tdir/$tfile || error
+	[ "$(cat $DIR/$tdir/$tfile)" = "foo" ] || error "$tdir/$tfile not 'foo'"
+	$CHECKSTAT -t file -s 3 $DIR/$tdir/$tfile || error "$tfile size not 3"
 }
 run_test 7b "mkdir .../d7; mcreate d7/f2; echo foo > d7/f2 ====="
 
@@ -334,7 +319,8 @@ test_8() {
 	test_mkdir $DIR/$tdir
 	touch $DIR/$tdir/$tfile
 	chmod 0666 $DIR/$tdir/$tfile
-	$CHECKSTAT -t file -p 0666 $DIR/$tdir/$tfile || error
+	$CHECKSTAT -t file -p 0666 $DIR/$tdir/$tfile ||
+		error "$tfile mode not 0666"
 }
 run_test 8 "mkdir .../d8; touch .../d8/f; chmod .../d8/f ======="
 
@@ -342,7 +328,7 @@ test_9() {
 	test_mkdir $DIR/$tdir
 	test_mkdir $DIR/$tdir/d2
 	test_mkdir $DIR/$tdir/d2/d3
-	$CHECKSTAT -t dir $DIR/$tdir/d2/d3 || error
+	$CHECKSTAT -t dir $DIR/$tdir/d2/d3 || error "$tdir/d2/d3 not a dir"
 }
 run_test 9 "mkdir .../d9 .../d9/d2 .../d9/d2/d3 ================"
 
@@ -350,7 +336,8 @@ test_10() {
 	test_mkdir $DIR/$tdir
 	test_mkdir $DIR/$tdir/d2
 	touch $DIR/$tdir/d2/$tfile
-	$CHECKSTAT -t file $DIR/$tdir/d2/$tfile || error
+	$CHECKSTAT -t file $DIR/$tdir/d2/$tfile ||
+		error "$tdir/d2/$tfile not a file"
 }
 run_test 10 "mkdir .../d10 .../d10/d2; touch .../d10/d2/f ======"
 
@@ -359,7 +346,8 @@ test_11() {
 	test_mkdir $DIR/$tdir/d2
 	chmod 0666 $DIR/$tdir/d2
 	chmod 0705 $DIR/$tdir/d2
-	$CHECKSTAT -t dir -p 0705 $DIR/$tdir/d2 || error
+	$CHECKSTAT -t dir -p 0705 $DIR/$tdir/d2 ||
+		error "$tdir/d2 mode not 0705"
 }
 run_test 11 "mkdir .../d11 d11/d2; chmod .../d11/d2 ============"
 
@@ -368,7 +356,8 @@ test_12() {
 	touch $DIR/$tdir/$tfile
 	chmod 0666 $DIR/$tdir/$tfile
 	chmod 0654 $DIR/$tdir/$tfile
-	$CHECKSTAT -t file -p 0654 $DIR/$tdir/$tfile || error
+	$CHECKSTAT -t file -p 0654 $DIR/$tdir/$tfile ||
+		error "$tdir/d2 mode not 0654"
 }
 run_test 12 "touch .../d12/f; chmod .../d12/f .../d12/f ========"
 
@@ -376,7 +365,8 @@ test_13() {
 	test_mkdir $DIR/$tdir
 	dd if=/dev/zero of=$DIR/$tdir/$tfile count=10
 	>  $DIR/$tdir/$tfile
-	$CHECKSTAT -t file -s 0 $DIR/$tdir/$tfile || error
+	$CHECKSTAT -t file -s 0 $DIR/$tdir/$tfile ||
+		error "$tdir/$tfile size not 0 after truncate"
 }
 run_test 13 "creat .../d13/f; dd .../d13/f; > .../d13/f ========"
 
@@ -384,7 +374,7 @@ test_14() {
 	test_mkdir $DIR/$tdir
 	touch $DIR/$tdir/$tfile
 	rm $DIR/$tdir/$tfile
-	$CHECKSTAT -a $DIR/$tdir/$tfile || error
+	$CHECKSTAT -a $DIR/$tdir/$tfile || error "$tdir/$tfile not removed"
 }
 run_test 14 "touch .../d14/f; rm .../d14/f; rm .../d14/f ======="
 
@@ -392,7 +382,8 @@ test_15() {
 	test_mkdir $DIR/$tdir
 	touch $DIR/$tdir/$tfile
 	mv $DIR/$tdir/$tfile $DIR/$tdir/${tfile}_2
-	$CHECKSTAT -t file $DIR/$tdir/${tfile}_2 || error
+	$CHECKSTAT -t file $DIR/$tdir/${tfile}_2 ||
+		error "$tdir/${tfile_2} not a file after rename"
 }
 run_test 15 "touch .../d15/f; mv .../d15/f .../d15/f2 =========="
 
@@ -400,7 +391,7 @@ test_16() {
 	test_mkdir $DIR/$tdir
 	touch $DIR/$tdir/$tfile
 	rm -rf $DIR/$tdir/$tfile
-	$CHECKSTAT -a $DIR/$tdir/$tfile || error
+	$CHECKSTAT -a $DIR/$tdir/$tfile || error "$tdir/$tfile not removed"
 }
 run_test 16 "touch .../d16/f; rm -rf .../d16/f ================="
 
@@ -409,10 +400,12 @@ test_17a() {
 	touch $DIR/$tdir/$tfile
 	ln -s $DIR/$tdir/$tfile $DIR/$tdir/l-exist
 	ls -l $DIR/$tdir
-	$CHECKSTAT -l $DIR/$tdir/$tfile $DIR/$tdir/l-exist || error
-	$CHECKSTAT -f -t f $DIR/$tdir/l-exist || error
+	$CHECKSTAT -l $DIR/$tdir/$tfile $DIR/$tdir/l-exist ||
+		error "$tdir/l-exist not a symlink"
+	$CHECKSTAT -f -t f $DIR/$tdir/l-exist ||
+		error "$tdir/l-exist not referencing a file"
 	rm -f $DIR/$tdir/l-exist
-	$CHECKSTAT -a $DIR/$tdir/l-exist || error
+	$CHECKSTAT -a $DIR/$tdir/l-exist || error "$tdir/l-exist not removed"
 }
 run_test 17a "symlinks: create, remove (real) =================="
 
@@ -420,10 +413,12 @@ test_17b() {
 	test_mkdir -p $DIR/$tdir
 	ln -s no-such-file $DIR/$tdir/l-dangle
 	ls -l $DIR/$tdir
-	$CHECKSTAT -l no-such-file $DIR/$tdir/l-dangle || error
-	$CHECKSTAT -fa $DIR/$tdir/l-dangle || error
+	$CHECKSTAT -l no-such-file $DIR/$tdir/l-dangle ||
+		error "$tdir/l-dangle not referencing no-such-file"
+	$CHECKSTAT -fa $DIR/$tdir/l-dangle ||
+		error "$tdir/l-dangle not referencing non-existent file"
 	rm -f $DIR/$tdir/l-dangle
-	$CHECKSTAT -a $DIR/$tdir/l-dangle || error
+	$CHECKSTAT -a $DIR/$tdir/l-dangle || error "$tdir/l-dangle not removed"
 }
 run_test 17b "symlinks: create, remove (dangling) =============="
 
@@ -708,21 +703,22 @@ test_18() {
 run_test 18 "touch .../f ; ls ... =============================="
 
 test_19a() {
-	touch $DIR/f19
+	touch $DIR/$tfile
 	ls -l $DIR
-	rm $DIR/f19
-	$CHECKSTAT -a $DIR/f19 || error
+	rm $DIR/$tfile
+	$CHECKSTAT -a $DIR/$tfile || error "$tfile was not removed"
 }
 run_test 19a "touch .../f19 ; ls -l ... ; rm .../f19 ==========="
 
 test_19b() {
-	ls -l $DIR/f19 && error || true
+	ls -l $DIR/$tfile && error "ls -l $tfile failed"|| true
 }
 run_test 19b "ls -l .../f19 (should return error) =============="
 
 test_19c() {
-	[ $RUNAS_ID -eq $UID ] && skip_env "RUNAS_ID = UID = $UID -- skipping" && return
-	$RUNAS touch $DIR/f19 && error || true
+	[ $RUNAS_ID -eq $UID ] &&
+		skip_env "RUNAS_ID = UID = $UID -- skipping" && return
+	$RUNAS touch $DIR/$tfile && error "create non-root file failed" || true
 }
 run_test 19c "$RUNAS touch .../f19 (should return error) =="
 
@@ -732,27 +728,28 @@ test_19d() {
 run_test 19d "cat .../f19 (should return error) =============="
 
 test_20() {
-	touch $DIR/f
-	rm $DIR/f
+	touch $DIR/$tfile
+	rm $DIR/$tfile
 	log "1 done"
-	touch $DIR/f
-	rm $DIR/f
+	touch $DIR/$tfile
+	rm $DIR/$tfile
 	log "2 done"
-	touch $DIR/f
-	rm $DIR/f
+	touch $DIR/$tfile
+	rm $DIR/$tfile
 	log "3 done"
-	$CHECKSTAT -a $DIR/f || error
+	$CHECKSTAT -a $DIR/$tfile || error "$tfile was not removed"
 }
 run_test 20 "touch .../f ; ls -l ... ==========================="
 
 test_21() {
-	test_mkdir $DIR/d21
-	[ -f $DIR/d21/dangle ] && rm -f $DIR/d21/dangle
-	ln -s dangle $DIR/d21/link
-	echo foo >> $DIR/d21/link
-	cat $DIR/d21/dangle
-	$CHECKSTAT -t link $DIR/d21/link || error
-	$CHECKSTAT -f -t file $DIR/d21/link || error
+	test_mkdir -p $DIR/$tdir
+	[ -f $DIR/$tdir/dangle ] && rm -f $DIR/$tdir/dangle
+	ln -s dangle $DIR/$tdir/link
+	echo foo >> $DIR/$tdir/link
+	cat $DIR/$tdir/dangle
+	$CHECKSTAT -t link $DIR/$tdir/link || error "$tdir/link not a link"
+	$CHECKSTAT -f -t file $DIR/$tdir/link ||
+		error "$tdir/link not linked to a file"
 }
 run_test 21 "write to dangling link ============================"
 
@@ -792,43 +789,43 @@ test_23b() { # bug 18988
 }
 run_test 23b "O_APPEND check =========================="
 
+# rename sanity
 test_24a() {
-	echo '== rename sanity =============================================='
 	echo '-- same directory rename'
-	test_mkdir $DIR/R1
-	touch $DIR/R1/f
-	mv $DIR/R1/f $DIR/R1/g
-	$CHECKSTAT -t file $DIR/R1/g || error
+	test_mkdir $DIR/$tdir
+	touch $DIR/$tdir/$tfile.1
+	mv $DIR/$tdir/$tfile.1 $DIR/$tdir/$tfile.2
+	$CHECKSTAT -t file $DIR/$tdir/$tfile.2 || error "$tfile.2 not a file"
 }
-run_test 24a "touch .../R1/f; rename .../R1/f .../R1/g ========="
+run_test 24a "rename file to non-existent target"
 
 test_24b() {
-	test_mkdir $DIR/R2
-	touch $DIR/R2/{f,g}
-	mv $DIR/R2/f $DIR/R2/g
-	$CHECKSTAT -a $DIR/R2/f || error
-	$CHECKSTAT -t file $DIR/R2/g || error
+	test_mkdir $DIR/$tdir
+	touch $DIR/$tdir/$tfile.{1,2}
+	mv $DIR/$tdir/$tfile.1 $DIR/$tdir/$tfile.2
+	$CHECKSTAT -a $DIR/$tdir/$tfile.1 || error "$tfile.1 exists"
+	$CHECKSTAT -t file $DIR/$tdir/$tfile.2 || error "$tfile.2 not a file"
 }
-run_test 24b "touch .../R2/{f,g}; rename .../R2/f .../R2/g ====="
+run_test 24b "rename file to existing target"
 
 test_24c() {
-	test_mkdir $DIR/R3
-	test_mkdir $DIR/R3/f
-	mv $DIR/R3/f $DIR/R3/g
-	$CHECKSTAT -a $DIR/R3/f || error
-	$CHECKSTAT -t dir $DIR/R3/g || error
+	test_mkdir $DIR/$tdir
+	test_mkdir $DIR/$tdir/d$testnum.1
+	mv $DIR/$tdir/d$testnum.1 $DIR/$tdir/d$testnum.2
+	$CHECKSTAT -a $DIR/$tdir/d$testnum.1 || error "d$testnum.1 exists"
+	$CHECKSTAT -t dir $DIR/$tdir/d$testnum.2 || error "d$testnum.2 not dir"
 }
-run_test 24c "mkdir .../R3/f; rename .../R3/f .../R3/g ========="
+run_test 24c "rename directory to non-existent target"
 
 test_24d() {
-	test_mkdir $DIR/R4
-	test_mkdir $DIR/R4/f
-	test_mkdir $DIR/R4/g
-	mrename $DIR/R4/f $DIR/R4/g
-	$CHECKSTAT -a $DIR/R4/f || error
-	$CHECKSTAT -t dir $DIR/R4/g || error
+	test_mkdir $DIR/$tdir
+	test_mkdir $DIR/$tdir/d$testnum.1
+	test_mkdir $DIR/$tdir/d$ttestnum.2
+	mrename $DIR/$tdir/d$testnum.1 $DIR/$tdir/d$testnum.2
+	$CHECKSTAT -a $DIR/$tdir/d$testnum.1 || error "d$testnum.1 exists"
+	$CHECKSTAT -t dir $DIR/$tdir/d$testnum.2 || error "d$testnum.2 not dir"
 }
-run_test 24d "mkdir .../R4/{f,g}; rename .../R4/f .../R4/g ====="
+run_test 24d "rename directory to existing target"
 
 test_24e() {
 	echo '-- cross directory renames --'
@@ -3096,9 +3093,11 @@ test_39m() {
 run_test 39m "test atime and mtime before 1970"
 
 test_40() {
-	dd if=/dev/zero of=$DIR/f40 bs=4096 count=1
-	$RUNAS $OPENFILE -f O_WRONLY:O_TRUNC $DIR/f40 && error
-	$CHECKSTAT -t file -s 4096 $DIR/f40 || error
+	dd if=/dev/zero of=$DIR/$tfile bs=4096 count=1
+	$RUNAS $OPENFILE -f O_WRONLY:O_TRUNC $DIR/$tfile &&
+		error "openfile O_WRONLY:O_TRUNC $tfile failed"
+	$CHECKSTAT -t file -s 4096 $DIR/$tfile ||
+		error "$tfile is not 4096 bytes in size"
 }
 run_test 40 "failed open(O_TRUNC) doesn't truncate ============="
 
@@ -3633,7 +3632,7 @@ test_50() {
 	# bug 1485
 	test_mkdir $DIR/$tdir
 	cd $DIR/$tdir
-	ls /proc/$$/cwd || error
+	ls /proc/$$/cwd || error "ls /proc/$$/cwd failed"
 }
 run_test 50 "special situations: /proc symlinks  ==============="
 
@@ -3645,13 +3644,13 @@ test_51a() {	# was test_51
 	rm $DIR/$tdir/foo
 	createmany -m $DIR/$tdir/longfile 201
 	FNUM=202
-	while [ `ls -sd $DIR/$tdir | awk '{ print $1 }'` -eq 4 ]; do
+	while [ $(ls -sd $DIR/$tdir | awk '{ print $1 }') -eq 4 ]; do
 		$MCREATE $DIR/$tdir/longfile$FNUM
 		FNUM=$(($FNUM + 1))
 		echo -n "+"
 	done
 	echo
-	ls -l $DIR/$tdir > /dev/null || error
+	ls -l $DIR/$tdir > /dev/null || error "ls -l $DIR/$tdir failed"
 }
 run_test 51a "special situations: split htree with empty entry =="
 
@@ -3790,13 +3789,13 @@ test_52b() {
 					error "link worked"
 	echo foo >> $DIR/$tdir/foo && error "echo worked"
 	mrename $DIR/$tdir/foo $DIR/$tdir/foo_ren && error "rename worked"
-	[ -f $DIR/$tdir/foo ] || error
-	[ -f $DIR/$tdir/foo_ren ] && error
+	[ -f $DIR/$tdir/foo ] || error "$tdir/foo is not a file"
+	[ -f $DIR/$tdir/foo_ren ] && error "$tdir/foo_ren is not a file"
 	lsattr $DIR/$tdir/foo | egrep -q "^-+i[-e]+ $DIR/$tdir/foo" ||
 							error "lsattr"
 	chattr -i $DIR/$tdir/foo || error "chattr failed"
 
-	rm -fr $DIR/$tdir || error
+	rm -fr $DIR/$tdir || error "unable to remove $DIR/$tdir"
 }
 run_test 52b "immutable flag test (should return errors) ======="
 
@@ -3851,11 +3850,13 @@ test_53() {
 run_test 53 "verify that MDS and OSTs agree on pre-creation ===="
 
 test_54a() {
-        [ ! -f "$SOCKETSERVER" ] && skip_env "no socketserver, skipping" && return
-        [ ! -f "$SOCKETCLIENT" ] && skip_env "no socketclient, skipping" && return
-     	$SOCKETSERVER $DIR/socket
-     	$SOCKETCLIENT $DIR/socket || error
-      	$MUNLINK $DIR/socket
+	[ ! -f "$SOCKETSERVER" ] &&
+		skip_env "no socketserver, skipping" && return
+	[ ! -f "$SOCKETCLIENT" ] &&
+		skip_env "no socketclient, skipping" && return
+	$SOCKETSERVER $DIR/socket
+	$SOCKETCLIENT $DIR/socket || error "$SOCKETCLIENT $DIR/socket failed"
+	$MUNLINK $DIR/socket
 }
 run_test 54a "unix domain socket test =========================="
 
@@ -3863,7 +3864,7 @@ test_54b() {
 	f="$DIR/f54b"
 	mknod $f c 1 3
 	chmod 0666 $f
-	dd if=/dev/zero of=$f bs=`page_size` count=1
+	dd if=/dev/zero of=$f bs=$(page_size) count=1
 }
 run_test 54b "char device works in lustre ======================"
 
@@ -3872,7 +3873,7 @@ find_loop_dev() {
 	[ -b /dev/loop0 ] && LOOPBASE=/dev/loop
 	[ -z "$LOOPBASE" ] && echo "/dev/loop/0 and /dev/loop0 gone?" && return
 
-	for i in `seq 3 7`; do
+	for i in $(seq 3 7); do
 		losetup $LOOPBASE$i > /dev/null 2>&1 && continue
 		LOOPDEV=$LOOPBASE$i
 		LOOPNUM=$i
@@ -3908,7 +3909,7 @@ test_54d() {
 	f="$DIR/f54d"
 	string="aaaaaa"
 	mknod $f p
-	[ "$string" = `echo $string > $f | cat $f` ] || error
+	[ "$string" = $(echo $string > $f | cat $f) ] || error "$f != $string"
 }
 run_test 54d "fifo device works in lustre ======================"
 
@@ -3917,7 +3918,7 @@ test_54e() {
 	f="$DIR/f54e"
 	string="aaaaaa"
 	cp -aL /dev/console $f
-	echo $string > $f || error
+	echo $string > $f || error "echo $string to $f failed"
 }
 run_test 54e "console/tty device works in lustre ======================"
 
@@ -4679,9 +4680,9 @@ run_test 60d "test printk console message masking"
 test_61() {
 	[ $PARALLEL == "yes" ] && skip "skip parallel run" && return
 	f="$DIR/f61"
-	dd if=/dev/zero of=$f bs=`page_size` count=1
+	dd if=/dev/zero of=$f bs=$(page_size) count=1 || error "dd $f failed"
 	cancel_lru_locks osc
-	$MULTIOP $f OSMWUc || error
+	$MULTIOP $f OSMWUc || error "$MULTIOP $f failed"
 	sync
 }
 run_test 61 "mmap() writes don't make sync hang ================"
@@ -5074,26 +5075,29 @@ run_test 71 "Running dbench on lustre (don't segment fault) ===="
 
 test_72a() { # bug 5695 - Test that on 2.6 remove_suid works properly
 	[ $PARALLEL == "yes" ] && skip "skip parallel run" && return
-	check_kernel_version 43 || return 0
-	[ "$RUNAS_ID" = "$UID" ] && skip_env "RUNAS_ID = UID = $UID -- skipping" && return
+	[ "$RUNAS_ID" = "$UID" ] &&
+		skip_env "RUNAS_ID = UID = $UID -- skipping" && return
 
-        # Check that testing environment is properly set up. Skip if not
-        FAIL_ON_ERROR=false check_runas_id_ret $RUNAS_ID $RUNAS_GID $RUNAS || {
-                skip_env "User $RUNAS_ID does not exist - skipping"
-                return 0
-        }
+	# Check that testing environment is properly set up. Skip if not
+	FAIL_ON_ERROR=false check_runas_id_ret $RUNAS_ID $RUNAS_GID $RUNAS || {
+		skip_env "User $RUNAS_ID does not exist - skipping"
+		return 0
+	}
 	# We had better clear the $DIR to get enough space for dd
 	rm -rf $DIR/*
-	touch $DIR/f72
-	chmod 777 $DIR/f72
-	chmod ug+s $DIR/f72
-	$RUNAS dd if=/dev/zero of=$DIR/f72 bs=512 count=1 || error
+	touch $DIR/$tfile
+	chmod 777 $DIR/$tfile
+	chmod ug+s $DIR/$tfile
+	$RUNAS dd if=/dev/zero of=$DIR/$tfile bs=512 count=1 ||
+		error "$RUNAS dd $DIR/$tfile failed"
 	# See if we are still setuid/sgid
-	test -u $DIR/f72 -o -g $DIR/f72 && error "S/gid is not dropped on write"
+	test -u $DIR/$tfile -o -g $DIR/$tfile &&
+		error "S/gid is not dropped on write"
 	# Now test that MDS is updated too
 	cancel_lru_locks mdc
-	test -u $DIR/f72 -o -g $DIR/f72 && error "S/gid is not dropped on MDS"
-	rm -f $DIR/f72
+	test -u $DIR/$tfile -o -g $DIR/$tfile &&
+		error "S/gid is not dropped on MDS"
+	rm -f $DIR/$tfile
 }
 run_test 72a "Test that remove suid works properly (bug5695) ===="
 
@@ -5595,20 +5599,21 @@ test_82() { # LU-1031
 run_test 82 "Basic grouplock test ==============================="
 
 test_99a() {
-        [ -z "$(which cvs 2>/dev/null)" ] && skip_env "could not find cvs" && \
-	    return
+	[ -z "$(which cvs 2>/dev/null)" ] && skip_env "could not find cvs" &&
+		return
 	test_mkdir -p $DIR/d99cvsroot
 	chown $RUNAS_ID $DIR/d99cvsroot
 	local oldPWD=$PWD	# bug 13584, use $TMP as working dir
 	cd $TMP
 
-	$RUNAS cvs -d $DIR/d99cvsroot init || error
+	$RUNAS cvs -d $DIR/d99cvsroot init || error "cvs init failed"
 	cd $oldPWD
 }
 run_test 99a "cvs init ========================================="
 
 test_99b() {
-        [ -z "$(which cvs 2>/dev/null)" ] && skip_env "could not find cvs" && return
+	[ -z "$(which cvs 2>/dev/null)" ] &&
+		skip_env "could not find cvs" && return
 	[ ! -d $DIR/d99cvsroot ] && test_99a
 	cd /etc/init.d
 	# some versions of cvs import exit(1) when asked to import links or
@@ -6021,19 +6026,21 @@ test_102a() {
 	touch $testfile
 
 	[ "$UID" != 0 ] && skip_env "must run as root" && return
-	[ -z "`lctl get_param -n mdc.*-mdc-*.connect_flags | grep xattr`" ] &&
+	[ -z "$(lctl get_param -n mdc.*-mdc-*.connect_flags | grep xattr)" ] &&
 		skip_env "must have user_xattr" && return
 
 	[ -z "$(which setfattr 2>/dev/null)" ] &&
 		skip_env "could not find setfattr" && return
 
 	echo "set/get xattr..."
-	setfattr -n trusted.name1 -v value1 $testfile || error
+	setfattr -n trusted.name1 -v value1 $testfile ||
+		error "setfattr -n trusted.name1=value1 $testfile failed"
 	getfattr -n trusted.name1 $testfile 2> /dev/null |
 	  grep "trusted.name1=.value1" ||
 		error "$testfile missing trusted.name1=value1"
 
-	setfattr -n user.author1 -v author1 $testfile || error
+	setfattr -n user.author1 -v author1 $testfile ||
+		error "setfattr -n user.author1=author1 $testfile failed"
 	getfattr -n user.author1 $testfile 2> /dev/null |
 	  grep "user.author1=.author1" ||
 		error "$testfile missing trusted.author1=author1"
@@ -6314,51 +6321,53 @@ run_acl_subtest()
 }
 
 test_103 () {
-    [ "$UID" != 0 ] && skip_env "must run as root" && return
-    [ -z "$(lctl get_param -n mdc.*-mdc-*.connect_flags | grep acl)" ] && skip "must have acl enabled" && return
-    [ -z "$(which setfacl 2>/dev/null)" ] && skip_env "could not find setfacl" && return
-    $GSS && skip "could not run under gss" && return
+	[ "$UID" != 0 ] && skip_env "must run as root" && return
+	[ -z "$(lctl get_param -n mdc.*-mdc-*.connect_flags | grep acl)" ] &&
+		skip "must have acl enabled" && return
+	[ -z "$(which setfacl 2>/dev/null)" ] &&
+		skip_env "could not find setfacl" && return
+	$GSS && skip "could not run under gss" && return
 
-    declare -a identity_old
+	declare -a identity_old
 
 	for num in $(seq $MDSCOUNT); do
 		switch_identity $num true || identity_old[$num]=$?
 	done
 
-    SAVE_UMASK=`umask`
-    umask 0022
-    cd $DIR
-
-    echo "performing cp ..."
-    run_acl_subtest cp || error
-    echo "performing getfacl-noacl..."
-    run_acl_subtest getfacl-noacl || error "getfacl-noacl test failed"
-    echo "performing misc..."
-    run_acl_subtest misc || error  "misc test failed"
-    echo "performing permissions..."
-    run_acl_subtest permissions || error "permissions failed"
-    echo "performing setfacl..."
-    run_acl_subtest setfacl || error  "setfacl test failed"
-
-    # inheritance test got from HP
-    echo "performing inheritance..."
-    cp $LUSTRE/tests/acl/make-tree . || error "cannot copy make-tree"
-    chmod +x make-tree || error "chmod +x failed"
-    run_acl_subtest inheritance || error "inheritance test failed"
-    rm -f make-tree
+	SAVE_UMASK=$(umask)
+	umask 0022
+	cd $DIR
+
+	echo "performing cp ..."
+	run_acl_subtest cp || error "run_acl_subtest cp failed"
+	echo "performing getfacl-noacl..."
+	run_acl_subtest getfacl-noacl || error "getfacl-noacl test failed"
+	echo "performing misc..."
+	run_acl_subtest misc || error  "misc test failed"
+	echo "performing permissions..."
+	run_acl_subtest permissions || error "permissions failed"
+	echo "performing setfacl..."
+	run_acl_subtest setfacl || error  "setfacl test failed"
+
+	# inheritance test got from HP
+	echo "performing inheritance..."
+	cp $LUSTRE/tests/acl/make-tree . || error "cannot copy make-tree"
+	chmod +x make-tree || error "chmod +x failed"
+	run_acl_subtest inheritance || error "inheritance test failed"
+	rm -f make-tree
 
 	echo "LU-974 ignore umask when acl is enabled..."
-	run_acl_subtest 974 || error "LU-974 test failed"
+	run_acl_subtest 974 || error "LU-974 umask test failed"
 	if [ $MDSCOUNT -ge 2 ]; then
 		run_acl_subtest 974_remote ||
-			error "LU-974 test failed under remote dir"
+			error "LU-974 umask test failed under remote dir"
 	fi
 
-    echo "LU-2561 newly created file is same size as directory..."
-    run_acl_subtest 2561 || error "LU-2561 test failed"
+	echo "LU-2561 newly created file is same size as directory..."
+	run_acl_subtest 2561 || error "LU-2561 test failed"
 
-    cd $SAVE_PWD
-    umask $SAVE_UMASK
+	cd $SAVE_PWD
+	umask $SAVE_UMASK
 
 	for num in $(seq $MDSCOUNT); do
 		if [ "${identity_old[$num]}" = 1 ]; then
@@ -6378,12 +6387,12 @@ test_104a() {
 	lfs df $DIR/$tfile || error "lfs df $DIR/$tfile failed"
 	lfs df -ih $DIR/$tfile || error "lfs df -ih $DIR/$tfile failed"
 
-        OSC=`lctl dl |grep OST0000-osc-[^M] |awk '{print $4}'`
+	local OSC=$(lctl dl | grep OST0000-osc-[^M] | awk '{ print $4 }')
 	lctl --device %$OSC deactivate
 	lfs df || error "lfs df with deactivated OSC failed"
 	lctl --device %$OSC activate
-        # wait the osc back to normal
-        wait_osc_import_state client ost FULL
+	# wait the osc back to normal
+	wait_osc_import_state client ost FULL
 
 	lfs df || error "lfs df with reactivated OSC failed"
 	rm -f $DIR/$tfile
@@ -6392,12 +6401,13 @@ run_test 104a "lfs df [-ih] [path] test ========================="
 
 test_104b() {
 	[ $PARALLEL == "yes" ] && skip "skip parallel run" && return
-	[ $RUNAS_ID -eq $UID ] && skip_env "RUNAS_ID = UID = $UID -- skipping" && return
+	[ $RUNAS_ID -eq $UID ] &&
+		skip_env "RUNAS_ID = UID = $UID -- skipping" && return
 	chmod 666 /dev/obd
-	denied_cnt=$((`$RUNAS $LFS check servers 2>&1 | grep "Permission denied" | wc -l`))
-	if [ $denied_cnt -ne 0 ];
-	then
-	            error "lfs check servers test failed"
+	denied_cnt=$(($($RUNAS $LFS check servers 2>&1 |
+			grep "Permission denied" | wc -l)))
+	if [ $denied_cnt -ne 0 ]; then
+		error "lfs check servers test failed"
 	fi
 }
 run_test 104b "$RUNAS lfs check servers test ===================="
@@ -6405,7 +6415,7 @@ run_test 104b "$RUNAS lfs check servers test ===================="
 test_105a() {
 	# doesn't work on 2.4 kernels
 	touch $DIR/$tfile
-	if [ -n "`mount | grep \"$MOUNT.*flock\" | grep -v noflock`" ]; then
+	if [ -n "$(mount | grep "$MOUNT.*flock" | grep -v noflock)" ]; then
 		flocks_test 1 on -f $DIR/$tfile || error "fail flock on"
 	else
 		flocks_test 1 off -f $DIR/$tfile || error "fail flock off"
@@ -6416,7 +6426,7 @@ run_test 105a "flock when mounted without -o flock test ========"
 
 test_105b() {
 	touch $DIR/$tfile
-	if [ -n "`mount | grep \"$MOUNT.*flock\" | grep -v noflock`" ]; then
+	if [ -n "$(mount | grep "$MOUNT.*flock" | grep -v noflock)" ]; then
 		flocks_test 1 on -c $DIR/$tfile || error "fail flock on"
 	else
 		flocks_test 1 off -c $DIR/$tfile || error "fail flock off"
@@ -6427,7 +6437,7 @@ run_test 105b "fcntl when mounted without -o flock test ========"
 
 test_105c() {
 	touch $DIR/$tfile
-	if [ -n "`mount | grep \"$MOUNT.*flock\" | grep -v noflock`" ]; then
+	if [ -n "$(mount | grep "$MOUNT.*flock" | grep -v noflock)" ]; then
 		flocks_test 1 on -l $DIR/$tfile || error "fail flock on"
 	else
 		flocks_test 1 off -l $DIR/$tfile || error "fail flock off"
@@ -6439,7 +6449,7 @@ run_test 105c "lockf when mounted without -o flock test ========"
 test_105d() { # bug 15924
 	[ $PARALLEL == "yes" ] && skip "skip parallel run" && return
 	test_mkdir -p $DIR/$tdir
-	[ -z "`mount | grep \"$MOUNT.*flock\" | grep -v noflock`" ] && \
+	[ -z "$(mount | grep "$MOUNT.*flock" | grep -v noflock)" ] &&
 		skip "mount w/o flock enabled" && return
 	#define OBD_FAIL_LDLM_CP_CB_WAIT  0x315
 	$LCTL set_param fail_loc=0x80000315
@@ -6448,7 +6458,7 @@ test_105d() { # bug 15924
 run_test 105d "flock race (should not freeze) ========"
 
 test_105e() { # bug 22660 && 22040
-	[ -z "`mount | grep \"$MOUNT.*flock\" | grep -v noflock`" ] && \
+	[ -z "$(mount | grep "$MOUNT.*flock" | grep -v noflock)" ] &&
 		skip "mount w/o flock enabled" && return
 	touch $DIR/$tfile
 	flocks_test 3 $DIR/$tfile
@@ -8634,17 +8644,17 @@ test_151() {
 		return 0
 	fi
 
-#define OBD_FAIL_OBD_NO_LRU  0x609
+	#define OBD_FAIL_OBD_NO_LRU  0x609
 	do_nodes $list $LCTL set_param fail_loc=0x609
 
 	# pages should be in the case right after write
 	dd if=/dev/urandom of=$DIR/$tfile bs=4k count=$CPAGES ||
 		error "dd failed"
 
-	local BEFORE=`roc_hit`
+	local BEFORE=$(roc_hit)
 	cancel_lru_locks osc
 	cat $DIR/$tfile >/dev/null
-	local AFTER=`roc_hit`
+	local AFTER=$(roc_hit)
 
 	do_nodes $list $LCTL set_param fail_loc=0
 
@@ -8658,10 +8668,10 @@ test_151() {
         cat $DIR/$tfile >/dev/null
 
         # now data shouldn't be found in the cache
-        BEFORE=`roc_hit`
+	BEFORE=$(roc_hit)
         cancel_lru_locks osc
         cat $DIR/$tfile >/dev/null
-        AFTER=`roc_hit`
+	AFTER=$(roc_hit)
         if let "AFTER - BEFORE != 0"; then
                 error "IN CACHE: before: $BEFORE, after: $AFTER"
         fi
diff --git a/lustre/tests/test-framework.sh b/lustre/tests/test-framework.sh
index afc2488..a128b2c 100644
--- a/lustre/tests/test-framework.sh
+++ b/lustre/tests/test-framework.sh
@@ -4036,28 +4036,33 @@ log_trace_dump() {
 ##################################
 
 error_noexit() {
-    local TYPE=${TYPE:-"FAIL"}
+	local TYPE=${TYPE:-"FAIL"}
 
-    local dump=true
-    # do not dump logs if $1=false
-    if [ "x$1" = "xfalse" ]; then
-        shift
-        dump=false
-    fi
+	local dump=true
+	# do not dump logs if $1=false
+	if [ "x$1" = "xfalse" ]; then
+		shift
+		dump=false
+	fi
 
-    log " ${TESTSUITE} ${TESTNAME}: @@@@@@ ${TYPE}: $@ "
-    log_trace_dump
 
-    mkdir -p $LOGDIR
-    # We need to dump the logs on all nodes
-    if $dump; then
-        gather_logs $(comma_list $(nodes_list))
-    fi
+	log " ${TESTSUITE} ${TESTNAME}: @@@@@@ ${TYPE}: $@ "
+	log_trace_dump
+
+	mkdir -p $LOGDIR
+	# We need to dump the logs on all nodes
+	if $dump; then
+		gather_logs $(comma_list $(nodes_list))
+	fi
 
 	debugrestore
 	[ "$TESTSUITELOG" ] &&
 		echo "$TESTSUITE: $TYPE: $TESTNAME $@" >> $TESTSUITELOG
-	echo "$@" > $LOGDIR/err
+	if [ -z "$*" ]; then
+		echo "error() without useful message, please fix" > $LOGDIR/err
+	else
+		echo "$@" > $LOGDIR/err
+	fi
 }
 
 exit_status () {
@@ -4069,21 +4074,21 @@ exit_status () {
 }
 
 error() {
-    error_noexit "$@"
-    exit 1
+	error_noexit "$@"
+	exit 1
 }
 
 error_exit() {
-    error "$@"
+	error "$@"
 }
 
 # use only if we are ignoring failures for this test, bugno required.
 # (like ALWAYS_EXCEPT, but run the test and ignore the results.)
 # e.g. error_ignore 5494 "your message"
 error_ignore() {
-    local TYPE="IGNORE (bz$1)"
-    shift
-    error_noexit "$@"
+	local TYPE="IGNORE (bz$1)"
+	shift
+	error_noexit "$@"
 }
 
 error_and_remount() {
@@ -4093,7 +4098,7 @@ error_and_remount() {
 }
 
 skip_env () {
-    $FAIL_ON_SKIP_ENV && error false $@ || skip $@
+	$FAIL_ON_SKIP_ENV && error false $@ || skip $@
 }
 
 skip() {
@@ -4744,10 +4749,11 @@ generate_machine_file() {
 }
 
 get_stripe () {
-    local file=$1/stripe
-    touch $file
-    $LFS getstripe -v $file || error
-    rm -f $file
+	local file=$1/stripe
+
+	touch $file
+	$LFS getstripe -v $file || error "getstripe $file failed"
+	rm -f $file
 }
 
 setstripe_nfsserver () {
-- 
1.8.3.1