fstests: improved config section support

PATCH In review Dave Chinner 2 participants 10 messages Jan 21, 2016 – Feb 5, 2016

lore

Share & tools

lore

Dave Chinner

author

2016-01-21 01:44 UTC #0 lore raw

cover fstests: improved config section support

Hi folks,

This small series adds more functionality to the config file section support that I've been using locally. It adds support for external devices to the config section parsing, so we can configure external log and realtime devices for XFS tests, updates the "setup" diagnostic command to understand config sections, and adds the ability to exclude sections from the set that check will pull from the config file.

-Dave.

Dave Chinner

author

2016-01-21 01:44 UTC #1 lore raw

PATCH 1/3 xfs: support realtime/log device setup changes in config sections

From: Dave Chinner <[email protected]>

Currently changing the devices used by "USE_EXTERNAL" environmental variable is not supported by the config section parsing. Add the functionality so that we can use config sections to test external device configs successfully.

This required tracking down a bug in _check_xfs_filesystem() which was causing a log device to be passed to a test device without an external log device. This was caused by an uninitialised variable in the function. I also added full output file removals to the first couple of generic tests that were failing, because that's where the check failure output ends up in this case.

Signed-off-by Dave Chinner <[email protected]>

---
 common/config     | 63 +++++++++++++++++++++++++++++++++++++++----------------
 common/dmflakey   |  3 ++-
 common/rc         |  1 +
 tests/generic/002 |  2 ++
 tests/generic/004 |  2 ++
 5 files changed, 52 insertions(+), 19 deletions(-)

common/config +45 ~~-18~~

index cb34fd7..477753e 100644
@@ -416,6 +416,30 @@ if [ -f "$HOST_OPTIONS" ]; then
 	fi
 fi
 
+_check_device()
+{
+	local name=$1
+	local dev_needed=$2
+	local dev=$3
+
+	if [ -z "$dev" ]; then
+		if [ "$dev_needed" == "required" ]; then 
+			_fatal "common/config: $name is required but not defined!"
+		fi
+		return
+	fi
+
+	echo $dev | grep -qE ":|//" > /dev/null 2>&1
+	network_dev=$?
+	if [ "$FSTYP" == "overlay" ]; then
+		if [ ! -d "$dev" ]; then
+			_fatal "common/config: $name ($dev) is not a directory for overlay"
+		fi
+	elif [ ! -b "$dev" -a "$network_dev" != "0" ]; then
+		_fatal "common/config: $name ($dev) is not a block device or a network filesystem"
+	fi
+}
+
 # Parse config section options. This function will parse all the configuration
 # within a single section which name is passed as an argument. For section
 # name format see comments in get_config_sections().

@@ -449,10 +473,13 @@ get_next_config() {
 	local OLD_TEST_FS_MOUNT_OPTS=$TEST_FS_MOUNT_OPTS
 	local OLD_MKFS_OPTIONS=$MKFS_OPTIONS
 	local OLD_FSCK_OPTIONS=$FSCK_OPTIONS
+	local OLD_USE_EXTERNAL=$USE_EXTERNAL
 
 	unset MOUNT_OPTIONS
 	unset MKFS_OPTIONS
 	unset FSCK_OPTIONS
+	unset USE_EXTERNAL
+
 	# We might have deduced SCRATCH_DEV from the SCRATCH_DEV_POOL in the previous
 	# run, so we have to unset it now.
 	if [ "$SCRATCH_DEV_NOT_SET" == "true" ]; then

@@ -466,11 +493,20 @@ get_next_config() {
 		[ -z "$TEST_FS_MOUNT_OPTS" ] && _test_mount_opts
 		[ -z "$MKFS_OPTIONS" ] && _mkfs_opts
 		[ -z "$FSCK_OPTIONS" ] && _fsck_opts
+
+		# clear the external devices if we are not using them
+		if [ -z "$USE_EXTERNAL" ]; then
+			unset TEST_RTDEV
+			unset TEST_LOGDEV
+			unset SCRATCH_RTDEV
+			unset SCRATCH_LOGDEV
+		fi
 	else
 		[ -z "$MOUNT_OPTIONS" ] && export MOUNT_OPTIONS=$OLD_MOUNT_OPTIONS
 		[ -z "$TEST_FS_MOUNT_OPTS" ] && export TEST_FS_MOUNT_OPTS=$OLD_TEST_FS_MOUNT_OPTS
 		[ -z "$MKFS_OPTIONS" ] && export MKFS_OPTIONS=$OLD_MKFS_OPTIONS
 		[ -z "$FSCK_OPTIONS" ] && export FSCK_OPTIONS=$OLD_FSCK_OPTIONS
+		[ -z "$USE_EXTERNAL" ] && export USE_EXTERNAL=$OLD_USE_EXTERNAL
 	fi
 
 	# set default RESULT_BASE

@@ -491,15 +527,7 @@ get_next_config() {
 		exit 1
 	fi
 
-	echo $TEST_DEV | grep -qE ":|//" > /dev/null 2>&1
-	if [ ! -b "$TEST_DEV" -a "$?" != "0" -a "$FSTYP" != "overlay" ]; then
-		echo "common/config: Error: \$TEST_DEV ($TEST_DEV) is not a block device or a network filesystem"
-		exit 1
-	elif [ "$FSTYP" == "overlay" -a ! -d "$TEST_DEV" ]; then
-		echo "common/config: Error: \$TEST_DEV ($TEST_DEV) is not a directory for overlay"
-		exit 1
-	fi
-
+	_check_device TEST_DEV required $TEST_DEV
 	if [ ! -d "$TEST_DIR" ]; then
 		echo "common/config: Error: \$TEST_DIR ($TEST_DIR) is not a directory"
 		exit 1

@@ -517,19 +545,18 @@ get_next_config() {
 		export SCRATCH_DEV_NOT_SET=true
 	fi
 
-	echo $SCRATCH_DEV | grep -qE ":|//" > /dev/null 2>&1
-	if [ ! -z "$SCRATCH_DEV" -a ! -b "$SCRATCH_DEV" -a "$?" != "0" -a "$FSTYP" != "overlay" ]; then
-		echo "common/config: Error: \$SCRATCH_DEV ($SCRATCH_DEV) is not a block device or a network filesystem"
-		exit 1
-	elif [ ! -z "$SCRATCH_DEV" -a "$FSTYP" == "overlay" -a ! -d "$SCRATCH_DEV" ]; then
-		echo "common/config: Error: \$SCRATCH_DEV ($SCRATCH_DEV) is not a directory for overlay"
-		exit 1
-	fi
-
+	_check_device SCRATCH_DEV optional $SCRATCH_DEV
 	if [ ! -z "$SCRATCH_MNT" -a ! -d "$SCRATCH_MNT" ]; then
 		echo "common/config: Error: \$SCRATCH_MNT ($SCRATCH_MNT) is not a directory"
 		exit 1
 	fi
+
+	if [ -n "$USE_EXTERNAL" ]; then
+		_check_device TEST_RTDEV optional $TEST_RTDEV
+		_check_device TEST_LOGDEV optional $TEST_LOGDEV
+		_check_device SCRATCH_RTDEV optional $SCRATCH_RTDEV
+		_check_device SCRATCH_LOGDEV optional $SCRATCH_LOGDEV
+	fi
 }
 
 if [ -z "$CONFIG_INCLUDED" ]; then

common/dmflakey +2 -1

index 5a45d14..3b6521a 100644
@@ -39,7 +39,8 @@ _init_flakey()
 
 _mount_flakey()
 {
-	mount -t $FSTYP $MOUNT_OPTIONS $FLAKEY_DEV $SCRATCH_MNT
+	_scratch_options mount
+	mount -t $FSTYP $SCRATCH_OPTIONS $MOUNT_OPTIONS $FLAKEY_DEV $SCRATCH_MNT
 }
 
 _unmount_flakey()

common/rc +1 -0

index 5135260..716c215 100644
@@ -2017,6 +2017,7 @@ _check_xfs_filesystem()
     fi
 
     extra_mount_options=""
+    extra_log_options=""
     extra_options=""
     device=$1
     if [ -f $device ];then

tests/generic/002 +2 -0

index f63b208..0433882 100755
@@ -45,6 +45,8 @@ _supported_fs generic
 _supported_os IRIX Linux
 _require_test
 
+rm -f $seqres.full
+
 echo "Silence is goodness ..."
 
 # ensure target directory exists

tests/generic/004 +2 -0

index c7aa473..d0926f1 100755
@@ -47,6 +47,8 @@ _supported_os Linux
 _require_test
 _require_xfs_io_command "flink"
 
+rm -f $seqres.full
+
 testfile="${TEST_DIR}/tst-tmpfile-flink"
 
 # test creating a r/w tmpfile, do I/O and link it into the namespace

Signature · 1 line

2.5.0

Eryu Guan

2016-02-02 04:36 UTC #2 lore raw

Re: [PATCH 1/3] xfs: support realtime/log device setup changes in config sections

Quoted 7 lines · Dave Chinner

From: Dave Chinner <[email protected]>
Currently changing the devices used by "USE_EXTERNAL" environmental variable is not supported by the config section parsing. Add the functionality so that we can use config sections to test external device configs successfully.

Quoted 8 lines

This required tracking down a bug in _check_xfs_filesystem() which was causing a log device to be passed to a test device without an external log device. This was caused by an uninitialised variable in the function. I also added full output file removals to the first couple of generic tests that were failing, because that's where the check failure output ends up in this case.
Signed-off-by Dave Chinner <[email protected]>

Looks good to me. And I ran some tests on NFS/CIFS/XFS/overlay and the new _check_device function worked as expected. Just one white space issue below

Quoted 14 lines

---
 common/config     | 63 +++++++++++++++++++++++++++++++++++++++----------------
 common/dmflakey   |  3 ++-
 common/rc         |  1 +
 tests/generic/002 |  2 ++
 tests/generic/004 |  2 ++
 5 files changed, 52 insertions(+), 19 deletions(-)

common/config +0 -0

index cb34fd7..477753e 100644
@@ -416,6 +416,30 @@ if [ -f "$HOST_OPTIONS" ]; then
 	fi

common/config if [ -f "$HOST_OPTIONS" ]; then · line 426

Quoted 10 lines

common/config +8 -0

 fi
 
+_check_device()
+{
+	local name=$1
+	local dev_needed=$2
+	local dev=$3
+
+	if [ -z "$dev" ]; then
+		if [ "$dev_needed" == "required" ]; then

Above line introduced trailing white space.

Thanks,
Eryu

Quoted 158 lines

common/config +37 ~~-18~~

+			_fatal "common/config: $name is required but not defined!"
+		fi
+		return
+	fi
+
+	echo $dev | grep -qE ":|//" > /dev/null 2>&1
+	network_dev=$?
+	if [ "$FSTYP" == "overlay" ]; then
+		if [ ! -d "$dev" ]; then
+			_fatal "common/config: $name ($dev) is not a directory for overlay"
+		fi
+	elif [ ! -b "$dev" -a "$network_dev" != "0" ]; then
+		_fatal "common/config: $name ($dev) is not a block device or a network filesystem"
+	fi
+}
+
 # Parse config section options. This function will parse all the configuration
 # within a single section which name is passed as an argument. For section
 # name format see comments in get_config_sections().

@@ -449,10 +473,13 @@ get_next_config() {
 	local OLD_TEST_FS_MOUNT_OPTS=$TEST_FS_MOUNT_OPTS
 	local OLD_MKFS_OPTIONS=$MKFS_OPTIONS
 	local OLD_FSCK_OPTIONS=$FSCK_OPTIONS
+	local OLD_USE_EXTERNAL=$USE_EXTERNAL
 
 	unset MOUNT_OPTIONS
 	unset MKFS_OPTIONS
 	unset FSCK_OPTIONS
+	unset USE_EXTERNAL
+
 	# We might have deduced SCRATCH_DEV from the SCRATCH_DEV_POOL in the previous
 	# run, so we have to unset it now.
 	if [ "$SCRATCH_DEV_NOT_SET" == "true" ]; then

@@ -466,11 +493,20 @@ get_next_config() {
 		[ -z "$TEST_FS_MOUNT_OPTS" ] && _test_mount_opts
 		[ -z "$MKFS_OPTIONS" ] && _mkfs_opts
 		[ -z "$FSCK_OPTIONS" ] && _fsck_opts
+
+		# clear the external devices if we are not using them
+		if [ -z "$USE_EXTERNAL" ]; then
+			unset TEST_RTDEV
+			unset TEST_LOGDEV
+			unset SCRATCH_RTDEV
+			unset SCRATCH_LOGDEV
+		fi
 	else
 		[ -z "$MOUNT_OPTIONS" ] && export MOUNT_OPTIONS=$OLD_MOUNT_OPTIONS
 		[ -z "$TEST_FS_MOUNT_OPTS" ] && export TEST_FS_MOUNT_OPTS=$OLD_TEST_FS_MOUNT_OPTS
 		[ -z "$MKFS_OPTIONS" ] && export MKFS_OPTIONS=$OLD_MKFS_OPTIONS
 		[ -z "$FSCK_OPTIONS" ] && export FSCK_OPTIONS=$OLD_FSCK_OPTIONS
+		[ -z "$USE_EXTERNAL" ] && export USE_EXTERNAL=$OLD_USE_EXTERNAL
 	fi
 
 	# set default RESULT_BASE

@@ -491,15 +527,7 @@ get_next_config() {
 		exit 1
 	fi
 
-	echo $TEST_DEV | grep -qE ":|//" > /dev/null 2>&1
-	if [ ! -b "$TEST_DEV" -a "$?" != "0" -a "$FSTYP" != "overlay" ]; then
-		echo "common/config: Error: \$TEST_DEV ($TEST_DEV) is not a block device or a network filesystem"
-		exit 1
-	elif [ "$FSTYP" == "overlay" -a ! -d "$TEST_DEV" ]; then
-		echo "common/config: Error: \$TEST_DEV ($TEST_DEV) is not a directory for overlay"
-		exit 1
-	fi
-
+	_check_device TEST_DEV required $TEST_DEV
 	if [ ! -d "$TEST_DIR" ]; then
 		echo "common/config: Error: \$TEST_DIR ($TEST_DIR) is not a directory"
 		exit 1

@@ -517,19 +545,18 @@ get_next_config() {
 		export SCRATCH_DEV_NOT_SET=true
 	fi
 
-	echo $SCRATCH_DEV | grep -qE ":|//" > /dev/null 2>&1
-	if [ ! -z "$SCRATCH_DEV" -a ! -b "$SCRATCH_DEV" -a "$?" != "0" -a "$FSTYP" != "overlay" ]; then
-		echo "common/config: Error: \$SCRATCH_DEV ($SCRATCH_DEV) is not a block device or a network filesystem"
-		exit 1
-	elif [ ! -z "$SCRATCH_DEV" -a "$FSTYP" == "overlay" -a ! -d "$SCRATCH_DEV" ]; then
-		echo "common/config: Error: \$SCRATCH_DEV ($SCRATCH_DEV) is not a directory for overlay"
-		exit 1
-	fi
-
+	_check_device SCRATCH_DEV optional $SCRATCH_DEV
 	if [ ! -z "$SCRATCH_MNT" -a ! -d "$SCRATCH_MNT" ]; then
 		echo "common/config: Error: \$SCRATCH_MNT ($SCRATCH_MNT) is not a directory"
 		exit 1
 	fi
+
+	if [ -n "$USE_EXTERNAL" ]; then
+		_check_device TEST_RTDEV optional $TEST_RTDEV
+		_check_device TEST_LOGDEV optional $TEST_LOGDEV
+		_check_device SCRATCH_RTDEV optional $SCRATCH_RTDEV
+		_check_device SCRATCH_LOGDEV optional $SCRATCH_LOGDEV
+	fi
 }
 
 if [ -z "$CONFIG_INCLUDED" ]; then

common/dmflakey +2 -1

index 5a45d14..3b6521a 100644
@@ -39,7 +39,8 @@ _init_flakey()
 
 _mount_flakey()
 {
-	mount -t $FSTYP $MOUNT_OPTIONS $FLAKEY_DEV $SCRATCH_MNT
+	_scratch_options mount
+	mount -t $FSTYP $SCRATCH_OPTIONS $MOUNT_OPTIONS $FLAKEY_DEV $SCRATCH_MNT
 }
 
 _unmount_flakey()

common/rc +1 -0

index 5135260..716c215 100644
@@ -2017,6 +2017,7 @@ _check_xfs_filesystem()
     fi
 
     extra_mount_options=""
+    extra_log_options=""
     extra_options=""
     device=$1
     if [ -f $device ];then

tests/generic/002 +2 -0

index f63b208..0433882 100755
@@ -45,6 +45,8 @@ _supported_fs generic
 _supported_os IRIX Linux
 _require_test
 
+rm -f $seqres.full
+
 echo "Silence is goodness ..."
 
 # ensure target directory exists

tests/generic/004 +2 -0

index c7aa473..d0926f1 100755
@@ -47,6 +47,8 @@ _supported_os Linux
 _require_test
 _require_xfs_io_command "flink"
 
+rm -f $seqres.full
+
 testfile="${TEST_DIR}/tst-tmpfile-flink"
 
 # test creating a r/w tmpfile, do I/O and link it into the namespace

Signature · 6 lines

2.5.0

--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Dave Chinner

author

2016-02-04 21:06 UTC #3 lore raw

Re: [PATCH 1/3] xfs: support realtime/log device setup changes in config sections

Quoted 37 lines · Eryu Guan

Quoted 7 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Currently changing the devices used by "USE_EXTERNAL" environmental variable is not supported by the config section parsing. Add the functionality so that we can use config sections to test external device configs successfully.
Quoted 8 lines
This required tracking down a bug in _check_xfs_filesystem() which was causing a log device to be passed to a test device without an external log device. This was caused by an uninitialised variable in the function. I also added full output file removals to the first couple of generic tests that were failing, because that's where the check failure output ends up in this case.
Signed-off-by Dave Chinner <[email protected]>
Looks good to me. And I ran some tests on NFS/CIFS/XFS/overlay and the new _check_device function worked as expected. Just one white space issue below
Quoted 16 lines
---
 common/config     | 63 +++++++++++++++++++++++++++++++++++++++----------------
 common/dmflakey   |  3 ++-
 common/rc         |  1 +
 tests/generic/002 |  2 ++
 tests/generic/004 |  2 ++
 5 files changed, 52 insertions(+), 19 deletions(-)
common/config +0 -0
index cb34fd7..477753e 100644
@@ -416,6 +416,30 @@ if [ -f "$HOST_OPTIONS" ]; then
 	fi
 fi
 

Quoted 10 lines

common/config if [ -f "$HOST_OPTIONS" ]; then · line 426
Quoted 8 lines
common/config +8 -0
+_check_device()
+{
+	local name=$1
+	local dev_needed=$2
+	local dev=$3
+
+	if [ -z "$dev" ]; then
+		if [ "$dev_needed" == "required" ]; then 
Above line introduced trailing white space.

Fixed, so can I add your reviewed-by?

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]

Eryu Guan

2016-02-05 02:46 UTC #4 lore raw

Re: [PATCH 1/3] xfs: support realtime/log device setup changes in config sections

Quoted 49 lines · Dave Chinner

Quoted 37 lines · Eryu Guan
Quoted 7 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Currently changing the devices used by "USE_EXTERNAL" environmental variable is not supported by the config section parsing. Add the functionality so that we can use config sections to test external device configs successfully.
Quoted 8 lines
This required tracking down a bug in _check_xfs_filesystem() which was causing a log device to be passed to a test device without an external log device. This was caused by an uninitialised variable in the function. I also added full output file removals to the first couple of generic tests that were failing, because that's where the check failure output ends up in this case.
Signed-off-by Dave Chinner <[email protected]>
Looks good to me. And I ran some tests on NFS/CIFS/XFS/overlay and the new _check_device function worked as expected. Just one white space issue below
Quoted 16 lines
---
 common/config     | 63 +++++++++++++++++++++++++++++++++++++++----------------
 common/dmflakey   |  3 ++-
 common/rc         |  1 +
 tests/generic/002 |  2 ++
 tests/generic/004 |  2 ++
 5 files changed, 52 insertions(+), 19 deletions(-)
common/config +0 -0
index cb34fd7..477753e 100644
@@ -416,6 +416,30 @@ if [ -f "$HOST_OPTIONS" ]; then
 	fi
 fi
 
Quoted 10 lines
common/config if [ -f "$HOST_OPTIONS" ]; then · line 426
Quoted 8 lines
common/config +8 -0
+_check_device()
+{
+	local name=$1
+	local dev_needed=$2
+	local dev=$3
+
+	if [ -z "$dev" ]; then
+		if [ "$dev_needed" == "required" ]; then 
Above line introduced trailing white space.

Quoted 1 line

Fixed, so can I add your reviewed-by?

Sure, and I'll review the other two soon (got interrupted last time I reviewed).

Thanks,
Eryu

Dave Chinner

author

2016-01-21 01:44 UTC #5 lore raw

PATCH 2/3 setup: add section support

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

Make the setup command section aware so that it is easy to test whether the section config code is generating the correct configurations or not.

Signed-off-by Dave Chinner <[email protected]>

---
 setup | 53 +++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 47 insertions(+), 6 deletions(-)

setup +47 -6

index eb7bdb3..1a41d81 100755
@@ -15,22 +15,63 @@
 # along with this program; if not, write the Free Software Foundation,
 # Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 #
+usage()
+{
+
+	echo "Usage: $0 [options]"'
+
+	-s section		run only specified section(s) from config file
+'
+	exit 0
+}
+
+while [ $# -gt 0 ]; do
+	case "$1" in
+	-\? | -h | --help) usage ;;
+	-s)	RUN_SECTION="$RUN_SECTION $2"; shift ;;
+	*)	usage ;;
+	esac
+	shift
+done
+
 if ! . ./common/config
 then
     echo "check: failed to source common/config"
     exit 1
 fi
 
-[ "$USE_EXTERNAL"    = yes ] || USE_EXTERNAL=no
-[ "$USE_LBD_PATCH"   = yes ] || USE_LBD_PATCH=no
-[ "$LARGE_SCRATCH_DEV"  = yes ] || LARGE_SCRATCH_DEV=no
-[ "$USE_ATTR_SECURE" = yes ] || USE_ATTR_SECURE=no
-[ -z "$FSTYP" ] && FSTYP="xfs"
+for section in $HOST_OPTIONS_SECTIONS; do
+	OLD_FSTYP=$FSTYP
+	OLD_MOUNT_OPTIONS=$MOUNT_OPTIONS
+	get_next_config $section
+
+	# Do we need to run only some sections ?
+	if [ ! -z "$RUN_SECTION" ]; then
+		skip=true
+		for s in $RUN_SECTION; do
+			if [ $section == $s ]; then
+				skip=false
+			fi
+		done
+		if $skip; then
+			continue
+		fi
+	fi
+
+	[ "$USE_EXTERNAL"    = yes ] || USE_EXTERNAL=no
+	[ "$USE_LBD_PATCH"   = yes ] || USE_LBD_PATCH=no
+	[ "$LARGE_SCRATCH_DEV"  = yes ] || LARGE_SCRATCH_DEV=no
+	[ "$USE_ATTR_SECURE" = yes ] || USE_ATTR_SECURE=no
+	[ -z "$FSTYP" ] && FSTYP="xfs"
+
+	cat <<EOF
+SECTION       -- $section
 
-cat <<EOF
 TEST: DIR=$TEST_DIR DEV=$TEST_DEV rt=[$TEST_RTDEV] log=[$TEST_LOGDEV]
 TAPE: dev=[$TAPE_DEV] rmt=[$RMT_TAPE_DEV] rmtirix=[$RMT_TAPE_USER@$RMT_IRIXTAPE_DEV]
 SCRATCH: MNT=$SCRATCH_MNT DEV=$SCRATCH_DEV rt=[$SCRATCH_RTDEV] log=[$SCRATCH_LOGDEV]
 VARIABLES: external=$USE_EXTERNAL largeblk=$USE_LBD_PATCH fstyp=$FSTYP
 	   large_scratch_dev=$LARGE_SCRATCH_DEV attrsecure=$USE_ATTR_SECURE
+--------
 EOF
+done

Signature · 1 line

2.5.0

Eryu Guan

Reviewed-by on v1 2/3

2016-02-05 03:33 UTC #6 lore raw

Re: [PATCH 2/3] setup: add section support

Quoted 7 lines · Dave Chinner

From: Dave Chinner <[email protected]>
Make the setup command section aware so that it is easy to test whether the section config code is generating the correct configurations or not.
Signed-off-by Dave Chinner <[email protected]>

Looks good to me. To this and the third patch:

Reviewed-by Eryu Guan <[email protected]>

Just one minor issue inline

Quoted 43 lines

---
 setup | 53 +++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 47 insertions(+), 6 deletions(-)

setup +20 -5

index eb7bdb3..1a41d81 100755
@@ -15,22 +15,63 @@
 # along with this program; if not, write the Free Software Foundation,
 # Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 #
+usage()
+{
+
+	echo "Usage: $0 [options]"'
+
+	-s section		run only specified section(s) from config file
+'
+	exit 0
+}
+
+while [ $# -gt 0 ]; do
+	case "$1" in
+	-\? | -h | --help) usage ;;
+	-s)	RUN_SECTION="$RUN_SECTION $2"; shift ;;
+	*)	usage ;;
+	esac
+	shift
+done
+
 if ! . ./common/config
 then
     echo "check: failed to source common/config"
     exit 1
 fi
 
-[ "$USE_EXTERNAL"    = yes ] || USE_EXTERNAL=no
-[ "$USE_LBD_PATCH"   = yes ] || USE_LBD_PATCH=no
-[ "$LARGE_SCRATCH_DEV"  = yes ] || LARGE_SCRATCH_DEV=no
-[ "$USE_ATTR_SECURE" = yes ] || USE_ATTR_SECURE=no
-[ -z "$FSTYP" ] && FSTYP="xfs"
+for section in $HOST_OPTIONS_SECTIONS; do

setup line 53

Quoted 10 lines

setup +10 -0

+	OLD_FSTYP=$FSTYP
+	OLD_MOUNT_OPTIONS=$MOUNT_OPTIONS
+	get_next_config $section
+
+	# Do we need to run only some sections ?
+	if [ ! -z "$RUN_SECTION" ]; then
+		skip=true
+		for s in $RUN_SECTION; do
+			if [ $section == $s ]; then
+				skip=false

I think the for loop can be break once "skip" is false.

Also in the other two "for loop"s in the third patch, only break the loop once "skip" is true there.

Thanks,
Eryu

Quoted 32 lines

setup +17 -1

+			fi
+		done
+		if $skip; then
+			continue
+		fi
+	fi
+
+	[ "$USE_EXTERNAL"    = yes ] || USE_EXTERNAL=no
+	[ "$USE_LBD_PATCH"   = yes ] || USE_LBD_PATCH=no
+	[ "$LARGE_SCRATCH_DEV"  = yes ] || LARGE_SCRATCH_DEV=no
+	[ "$USE_ATTR_SECURE" = yes ] || USE_ATTR_SECURE=no
+	[ -z "$FSTYP" ] && FSTYP="xfs"
+
+	cat <<EOF
+SECTION       -- $section
 
-cat <<EOF
 TEST: DIR=$TEST_DIR DEV=$TEST_DEV rt=[$TEST_RTDEV] log=[$TEST_LOGDEV]
 TAPE: dev=[$TAPE_DEV] rmt=[$RMT_TAPE_DEV] rmtirix=[$RMT_TAPE_USER@$RMT_IRIXTAPE_DEV]
 SCRATCH: MNT=$SCRATCH_MNT DEV=$SCRATCH_DEV rt=[$SCRATCH_RTDEV] log=[$SCRATCH_LOGDEV]
 VARIABLES: external=$USE_EXTERNAL largeblk=$USE_LBD_PATCH fstyp=$FSTYP
 	   large_scratch_dev=$LARGE_SCRATCH_DEV attrsecure=$USE_ATTR_SECURE
+--------
 EOF
+done

Signature · 6 lines

2.5.0

--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Dave Chinner

author

2016-01-21 01:44 UTC #7 lore raw

PATCH 3/3 check: add exclude sections CLI parameter

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

When we have a config file with multiple sections, we might want to exclude certain config sections from running. Rather than specifying all the section we want to run, add a "-S <section>" option to build up a list of sections to exclude.

This is useful if a given section config is known to cause a fatal failure,but you still want to run all the other config sections.

Also add support to the setup program that emits the currently configured setup for each section in the config file.

Signed-off-by Dave Chinner <[email protected]>

---
 check | 15 +++++++++++++++
 setup | 17 ++++++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

check +15 -0

index 135a9fb..3ef79fc 100755
@@ -73,6 +73,7 @@ check options
     -d			dump test output to stdout
     --large-fs		optimise scratch device for large filesystems
     -s section		run only specified section from config file
+    -S section		exclude the specified section from the config file
 
 testlist options
     -g group[,group...]	include tests from these groups

@@ -228,6 +229,7 @@ while [ $# -gt 0 ]; do
 	        fi
 		;;
 	-s)	RUN_SECTION="$RUN_SECTION $2"; shift ;;
+	-S)	EXCLUDE_SECTION="$EXCLUDE_SECTION $2"; shift ;;
 	-l)	diff="diff" ;;
 	-udiff)	diff="$diff -u" ;;

@@ -428,6 +430,19 @@ for section in $HOST_OPTIONS_SECTIONS; do
 		fi
 	fi
 
+	# Did this section get excluded?
+	if [ ! -z "$EXCLUDE_SECTION" ]; then
+		skip=false
+		for s in $EXCLUDE_SECTION; do
+			if [ $section == $s ]; then
+				skip=true
+			fi
+		done
+		if $skip; then
+			continue
+		fi
+	fi
+
 	mkdir -p $RESULT_BASE
 	if [ ! -d $RESULT_BASE ]; then
 		echo "failed to create results directory $RESULT_BASE"

setup +16 -1

index 1a41d81..6ef2293 100755
@@ -20,7 +20,8 @@ usage()
 
 	echo "Usage: $0 [options]"'
 
-	-s section		run only specified section(s) from config file
+	-s section	run only specified section(s) from config file
+	-S section	exclude the specified section from the config file
 '
 	exit 0
 }

@@ -29,6 +30,7 @@ while [ $# -gt 0 ]; do
 	case "$1" in
 	-\? | -h | --help) usage ;;
 	-s)	RUN_SECTION="$RUN_SECTION $2"; shift ;;
+	-S)	EXCLUDE_SECTION="$EXCLUDE_SECTION $2"; shift ;;
 	*)	usage ;;
 	esac
 	shift

@@ -58,6 +60,19 @@ for section in $HOST_OPTIONS_SECTIONS; do
 		fi
 	fi
 
+	# Did this section get excluded?
+	if [ ! -z "$EXCLUDE_SECTION" ]; then
+		skip=false
+		for s in $EXCLUDE_SECTION; do
+			if [ $section == $s ]; then
+				skip=true
+			fi
+		done
+		if $skip; then
+			continue
+		fi
+	fi
+
 	[ "$USE_EXTERNAL"    = yes ] || USE_EXTERNAL=no
 	[ "$USE_LBD_PATCH"   = yes ] || USE_LBD_PATCH=no
 	[ "$LARGE_SCRATCH_DEV"  = yes ] || LARGE_SCRATCH_DEV=no

Signature · 1 line

2.5.0

Dave Chinner

author

2016-02-02 03:40 UTC #8 lore raw

In reply to Dave Chinner

Quoted 9 lines · Dave Chinner

Hi folks,
This small series adds more functionality to the config file section support that I've been using locally. It adds support for external devices to the config section parsing, so we can configure external log and realtime devices for XFS tests, updates the "setup" diagnostic command to understand config sections, and adds the ability to exclude sections from the set that check will pull from the config file.

ping?

Signature · 2 lines

Dave Chinner
[email protected]

Dave Chinner

author

2016-02-04 21:07 UTC #9 lore raw

Quoted 11 lines · Dave Chinner

Quoted 9 lines · Dave Chinner
Hi folks,
This small series adds more functionality to the config file section support that I've been using locally. It adds support for external devices to the config section parsing, so we can configure external log and realtime devices for XFS tests, updates the "setup" diagnostic command to understand config sections, and adds the ability to exclude sections from the set that check will pull from the config file.

Quoted 1 line

ping?

Folks, getting reviews for my own changes is holding up the next release. Please review.

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]