-------- Original Message --------
Subject: [PATCH 01/15] btrfs: new test to run btrfs balance and 
subvolume test simultaneously
From: Eryu Guan <redacted>
To: <redacted>
Date: 2014年08月21日 01:33

I wonder should we add the mkfs options there.
Since xfstests already use environment MKFS_OPTIONS to do mkfs,
if really need to test all mkfs options, IMO it is better to change 
MKFS_OPTIONS on each test round.

+
+run_test()
+{
+	local mkfs_opts=$1
+	local saved_mkfs_opts=$MKFS_OPTIONS
+	local subvol_mnt=$tmp.mnt
+
+	echo "Test $mkfs_opts" >>$seqres.full
+
+	MKFS_OPTIONS="$MKFS_OPTIONS $mkfs_opts"
+	# dup only works on single device
+	if [[ "$mkfs_opts" =~ dup ]]; then
+		_scratch_mkfs >>$seqres.full 2>&1
+	else
+		_scratch_pool_mkfs >>$seqres.full 2>&1
+	fi
+	ret=$?
+	MKFS_OPTIONS=$saved_mkfs_opts
+	# make sure we created btrfs with desired options
+	if [ $ret -ne 0 ]; then
+		echo "mkfs $mkfs_opts failed"
+		return
+	fi
+
+	_scratch_mount >>$seqres.full 2>&1
+
+	args=`_scale_fsstress_args -p 20 -n 100 $FSSTRESS_AVOID -d $SCRATCH_MNT/stressdir`
+	echo "Run fsstress $args" >>$seqres.full
+	$FSSTRESS_PROG $args >/dev/null 2>&1 &
+	fsstress_pid=$!
+
+	echo -n "Start balance worker: " >>$seqres.full
+	(
+		while true; do
+			$BTRFS_UTIL_PROG balance start $SCRATCH_MNT
+		done
+	) >/dev/null 2>&1 &
+	balance_pid=$!
+	echo "$balance_pid" >>$seqres.full
+
+	echo -n "Start subvolume worker: " >>$seqres.full
+	mkdir -p $subvol_mnt
+	(
+		while true; do
+			$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subvol_$$
+			$MOUNT_PROG -o subvol=subvol_$$ $SCRATCH_DEV $subvol_mnt
+			$UMOUNT_PROG $subvol_mnt
+			$BTRFS_UTIL_PROG subvolume delete $SCRATCH_MNT/subvol_$$
+		done
+	) >/dev/null 2>&1 &
+	subvol_pid=$!
+	echo "$subvol_pid" >>$seqres.full
+
+	echo "Wait for fsstress to exit and kill all background workers" >>$seqres.full
+	wait $fsstress_pid

What about integrate this 'run in background; record PID; wait; kill' 
thing into one function or two?
and then thing would be like this:
add_test_background TEST_FUNC1 PID_RET1
add_test_background TEST_FUNC2 PID_RET2
...
stop_test_backgroupd PID_RET1
stop_test_backgroupd PID_RET2

Which will be much cleaner.

+
+	kill $balance_pid $subvol_pid
+	wait
+	# the balance process might be still in D state and cannot be killed
+	# which could block umount, wait for it to finish
+	while ps aux | grep "balance start" | grep -qv grep; do
+		sleep 1
+	done
+
+	echo "Scrub the filesystem" >>$seqres.full
+	$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1
+	if [ $? -ne 0 ]; then
+		echo "Scrub find errors in \"$mkfs_opts\" test" | tee -a $seqres.full
+	fi

All your testcases uses almost same scrub/replace/subvolume operations,
it would be better move them to common operations in common/ like 
_scratch_mount in common/rc.
(since all these are btrfs only operations, maybe common/btrfs is a good 
place for them?)

At least this should save some lines.

Thanks,
Qu

+
+	$BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT >/dev/null 2>&1
+	# in case the subvolume is still mounted
+	$UMOUNT_PROG $subvol_mnt >/dev/null 2>&1
+	_scratch_unmount
+	_check_scratch_fs
+}