[PATCH 1/7] btrfs/010: don't run without /sys/fs/btrfs

[PATCH 1/7] btrfs/010: don't run without /sys/fs/btrfs

[jeffm]

From: Jeff Mahoney <jeffm@suse.com>

Older kernels don't have /sys/fs/btrfs.  btrfs/010 will happily run
until it goes to check its work against sysfs and finds those
files don't exist.  This patch introduces a require check to
ensure that the sysfs files are present before running.

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 common/btrfs    | 8 ++++++++
 tests/btrfs/010 | 1 +
 2 files changed, 9 insertions(+)

diff --git a/common/btrfs b/common/btrfs
index 26dc0bb9..f6513c06 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -89,6 +89,14 @@ _require_btrfs_fs_feature()
 		_notrun "Feature $feat not supported by the available btrfs version"
 }
 
+_require_btrfs_fs_sysfs()
+{
+	modprobe btrfs > /dev/null 2>&1
+	[ -e /sys/fs/btrfs/features ] || \
+		_notrun "Sysfs not supported by the available btrfs version"
+
+}
+
 _check_btrfs_filesystem()
 {
 	device=$1
diff --git a/tests/btrfs/010 b/tests/btrfs/010
index 1a5f0146..72b2c727 100755
--- a/tests/btrfs/010
+++ b/tests/btrfs/010
@@ -33,6 +33,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_test
+_require_btrfs_fs_sysfs
 
 # Create 32k extents. All of these extents will be accounted as outstanding and
 # reserved.
-- 
2.16.4

[PATCH 2/7] filter: fix assumed whitespace in _filefrag_filter regex

[jeffm]

From: Jeff Mahoney <jeffm@suse.com>

The regex assumes there will be whitespace after the .. in the ranges
and, with larger offsets, there may not be any.

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 common/filter | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/filter b/common/filter
index ed082d24..b4443a34 100644
--- a/common/filter
+++ b/common/filter
@@ -532,7 +532,7 @@ _filter_filefrag()
 		next
 	}
 	($ext, $logical, $physical, $length) =
-		(/^\s*(\d+):\s+(\d+)..\s+\d+:\s+(\d+)..\s+\d+:\s+(\d+):/)
+		(/^\s*(\d+):\s+(\d+)\.\.\s*\d+:\s+(\d+)\.\.\s*\d+:\s+(\d+):/)
 	or next;
 	($flags) = /.*:\s*(\S*)$/;
 	print $physical * $blocksize, "#",
-- 
2.16.4

[PATCH 3/7] filter: add support for old filefrag -v

[jeffm]

From: Jeff Mahoney <jeffm@suse.com>

e2fsprogs versions prior to v1.42.7 had a different format for
filefrag -v that is incompatible with the regex in _filefrag_filter.

This patch adds support for the older format to test on older releases

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 common/filter | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/common/filter b/common/filter
index b4443a34..a8ddf571 100644
--- a/common/filter
+++ b/common/filter
@@ -528,13 +528,27 @@ _filter_filefrag()
 {
 	perl -ne '
 	if (/blocks? of (\d+) bytes/) {
+		$oldfmt = 0;
 		$blocksize = $1;
-		next
+		next;
+	} elsif (/\d+ blocks, blocksize (\d+)/) {
+		$oldfmt = 1;
+		$blocksize = $1;
+		next;
+	}
+	if ($oldfmt) {
+		chomp;
+		($ext, $logical, $physical, $ignore, $length1, $length2, $flags) =
+			(/^\s*(\d+)\s+(\d+)\s+(\d+)\s+((\d+)|\d+\s+(\d+))\s+([a-z0-9,]*)$/)
+		or next;
+		$length = "$length1$length2";
+	} else {
+		($ext, $logical, $physical, $length) =
+			(/^\s*(\d+):\s+(\d+)\.\.\s*\d+:\s+(\d+)\.\.\s*\d+:\s+(\d+):/)
+		or next;
+		($flags) = /.*:\s*(\S*)$/;
 	}
-	($ext, $logical, $physical, $length) =
-		(/^\s*(\d+):\s+(\d+)\.\.\s*\d+:\s+(\d+)\.\.\s*\d+:\s+(\d+):/)
-	or next;
-	($flags) = /.*:\s*(\S*)$/;
+
 	print $physical * $blocksize, "#",
 	      $length * $blocksize, "#",
 	      $logical * $blocksize, "#",
-- 
2.16.4

[PATCH 4/7] btrfs: require feature raid56 for raid56 tests

[jeffm]

From: Jeff Mahoney <jeffm@suse.com>

btrfs/125, btrfs/148, btrfs/157, and btrfs/158 test for raid56
behavior.  We shouldn't run if the kernel doesn't have support for them.

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 tests/btrfs/125 | 1 +
 tests/btrfs/148 | 1 +
 tests/btrfs/157 | 1 +
 tests/btrfs/158 | 1 +
 4 files changed, 4 insertions(+)

diff --git a/tests/btrfs/125 b/tests/btrfs/125
index 5ac68b67..847fa62a 100755
--- a/tests/btrfs/125
+++ b/tests/btrfs/125
@@ -51,6 +51,7 @@ _supported_os Linux
 _require_scratch_dev_pool 3
 _test_unmount
 _require_loadable_fs_module "btrfs"
+_require_btrfs_fs_feature raid56
 
 _scratch_dev_pool_get 3
 
diff --git a/tests/btrfs/148 b/tests/btrfs/148
index d0e554c2..3fd8616f 100755
--- a/tests/btrfs/148
+++ b/tests/btrfs/148
@@ -30,6 +30,7 @@ _supported_os Linux
 _require_scratch
 _require_scratch_dev_pool 4
 _require_odirect
+_require_btrfs_fs_feature raid56
 
 _scratch_dev_pool_get 4
 
diff --git a/tests/btrfs/157 b/tests/btrfs/157
index d9ea4b7b..7f75c407 100755
--- a/tests/btrfs/157
+++ b/tests/btrfs/157
@@ -49,6 +49,7 @@ _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 4
 _require_btrfs_command inspect-internal dump-tree
+_require_btrfs_fs_feature raid56
 
 get_physical_stripe0()
 {
diff --git a/tests/btrfs/158 b/tests/btrfs/158
index fe2dd956..603e8bea 100755
--- a/tests/btrfs/158
+++ b/tests/btrfs/158
@@ -41,6 +41,7 @@ _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 4
 _require_btrfs_command inspect-internal dump-tree
+_require_btrfs_fs_feature raid56
 
 get_physical_stripe0()
 {
-- 
2.16.4

[PATCH 5/7] btrfs/023: skip trying to test raid56 without kernel support

[jeffm]

From: Jeff Mahoney <jeffm@suse.com>

Older kernels don't support raid56.  This test is still valid for
other profiles, so skip raid56 if the kernel doesn't support it.

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 tests/btrfs/023 | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/btrfs/023 b/tests/btrfs/023
index 6d9cc443..6c02113b 100755
--- a/tests/btrfs/023
+++ b/tests/btrfs/023
@@ -63,11 +63,13 @@ check_group_profile "RAID1"
 create_group_profile "raid10"
 check_group_profile "RAID10"
 
-create_group_profile "raid5"
-check_group_profile "RAID5"
+if [ -e "/sys/fs/btrfs/features/raid56" ]; then
+	create_group_profile "raid5"
+	check_group_profile "RAID5"
 
-create_group_profile "raid6"
-check_group_profile "RAID6"
+	create_group_profile "raid6"
+	check_group_profile "RAID6"
+fi
 
 # success, all done
 echo "Silence is golden"
-- 
2.16.4

[PATCH 6/7] btrfs/131: require support for free-space-tree

[jeffm]

From: Jeff Mahoney <jeffm@suse.com>

btrfs/131 tests the free space tree, which older kernels won't have.  We
shouldn't run there.

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 tests/btrfs/131 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/btrfs/131 b/tests/btrfs/131
index 0da1e7be..3de7eef8 100755
--- a/tests/btrfs/131
+++ b/tests/btrfs/131
@@ -34,6 +34,7 @@ _supported_fs btrfs
 _supported_os Linux
 _require_scratch
 _require_btrfs_command inspect-internal dump-super
+_require_btrfs_fs_feature free_space_tree
 
 mkfs_v1()
 {
-- 
2.16.4

[PATCH 7/7] check: move test exclusion handling to _prepare_test_list

[jeffm]

From: Jeff Mahoney <jeffm@suse.com>

In order to simplify combining excluded tests specified on the command
line vs specified via config files, it makes sense to push the handling
into _prepare_test_list.  This means we start with a fresh $tmp.xlist
and rebuild it each time _prepare_test_list is called.

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 check | 41 ++++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/check b/check
index 77a06b00..17073c4e 100755
--- a/check
+++ b/check
@@ -230,7 +230,28 @@ _prepare_test_list()
 		done
 	fi
 
-	# Specified groups to exclude
+	:> $tmp.xlist
+
+	# Per-fstype/generic/shared file of tests to exclude (-X)
+	for xfile in $XGROUP_FILES; do
+		for d in $SRC_GROUPS $FSTYP; do
+			[ -f $SRC_DIR/$d/$xfile ] || continue
+			for f in `sed "s/#.*$//" $SRC_DIR/$d/$xfile`; do
+				echo "$d/$f command line" >> $tmp.xlist
+			done
+		done
+	done
+
+	# External file of tests to exclude (-E)
+	for xfile in $EXCLUDE_FILES; do
+		if [ -f $xfile ]; then
+			sed -e "s/#.*$//" \
+			    -e "s;$; file $xfile;" "$xfile" \
+			    >> $tmp.xlist
+	        fi
+	done
+
+	# Specified groups to exclude (-x)
 	for xgroup in $XGROUP_LIST; do
 		list=$(get_group_list $xgroup)
 		if [ -z "$list" ]; then
@@ -273,13 +294,8 @@ while [ $# -gt 0 ]; do
 		XGROUP_LIST="$XGROUP_LIST ${xgroup//,/ }"
 		;;
 
-	-X)	subdir_xfile=$2; shift ;
-		;;
-	-E)	xfile=$2; shift ;
-		if [ -f $xfile ]; then
-			sed "s/#.*$//" "$xfile" >> $tmp.xlist
-	        fi
-		;;
+	-X)	XGROUP_FILES="$XGROUP_FILES $2" ; shift ;;
+	-E)	EXCLUDE_FILES="$EXCLUDE_FILES $2" ; shift ;;
 	-s)	RUN_SECTION="$RUN_SECTION $2"; shift ;;
 	-S)	EXCLUDE_SECTION="$EXCLUDE_SECTION $2"; shift ;;
 	-l)	diff="diff" ;;
@@ -320,15 +336,6 @@ if ! . ./common/rc; then
 	exit 1
 fi
 
-if [ -n "$subdir_xfile" ]; then
-	for d in $SRC_GROUPS $FSTYP; do
-		[ -f $SRC_DIR/$d/$subdir_xfile ] || continue
-		for f in `sed "s/#.*$//" $SRC_DIR/$d/$subdir_xfile`; do
-			echo $d/$f >> $tmp.xlist
-		done
-	done
-fi
-
 # Process tests from command line now.
 if $have_test_arg; then
 	while [ $# -gt 0 ]; do
-- 
2.16.4

Re: [PATCH 2/7] filter: fix assumed whitespace in _filefrag_filter regex

[Dave Chinner]

On Mon, Jan 21, 2019 at 11:33:11AM -0500, jeffm@suse.com wrote:
> From: Jeff Mahoney <jeffm@suse.com>
> 
> The regex assumes there will be whitespace after the .. in the ranges
> and, with larger offsets, there may not be any.

Does more than this, right?

> Signed-off-by: Jeff Mahoney <jeffm@suse.com>
> ---
>  common/filter | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/filter b/common/filter
> index ed082d24..b4443a34 100644
> --- a/common/filter
> +++ b/common/filter
> @@ -532,7 +532,7 @@ _filter_filefrag()
>  		next
>  	}
>  	($ext, $logical, $physical, $length) =
> -		(/^\s*(\d+):\s+(\d+)..\s+\d+:\s+(\d+)..\s+\d+:\s+(\d+):/)
> +		(/^\s*(\d+):\s+(\d+)\.\.\s*\d+:\s+(\d+)\.\.\s*\d+:\s+(\d+):/)

This is escaping "..", too, isn't it? 

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 3/7] filter: add support for old filefrag -v

[Dave Chinner]

On Mon, Jan 21, 2019 at 11:33:12AM -0500, jeffm@suse.com wrote:
> From: Jeff Mahoney <jeffm@suse.com>
> 
> e2fsprogs versions prior to v1.42.7 had a different format for
> filefrag -v that is incompatible with the regex in _filefrag_filter.
> 
> This patch adds support for the older format to test on older releases

Can you document what the two formats are in a comment just so it is
somewhat easier to check the regexes are, indeed, doing the right
thing?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 7/7] check: move test exclusion handling to _prepare_test_list

[Dave Chinner]

On Mon, Jan 21, 2019 at 11:33:16AM -0500, jeffm@suse.com wrote:
> From: Jeff Mahoney <jeffm@suse.com>
> 
> In order to simplify combining excluded tests specified on the command
> line vs specified via config files,

You mean defining excludes in configs/<hostname>.config files,
right? i.e. in config sections?

But the section code only calls _prepare_test_list if the test dev
is recreated by each section, right? So you can't really change the
expunge list from the config files without forcing a test dev
reformat, right?

> it makes sense to push the handling
> into _prepare_test_list.  This means we start with a fresh $tmp.xlist
> and rebuild it each time _prepare_test_list is called.

The patch does more than that, right?

> Signed-off-by: Jeff Mahoney <jeffm@suse.com>
> ---
>  check | 41 ++++++++++++++++++++++++-----------------
>  1 file changed, 24 insertions(+), 17 deletions(-)
> 
> diff --git a/check b/check
> index 77a06b00..17073c4e 100755
> --- a/check
> +++ b/check
> @@ -230,7 +230,28 @@ _prepare_test_list()
>  		done
>  	fi
>  
> -	# Specified groups to exclude
> +	:> $tmp.xlist
> +
> +	# Per-fstype/generic/shared file of tests to exclude (-X)
> +	for xfile in $XGROUP_FILES; do

That adds support for multiple group exclude files (i.e. multiple -X
options), right?

> +		for d in $SRC_GROUPS $FSTYP; do
> +			[ -f $SRC_DIR/$d/$xfile ] || continue
> +			for f in `sed "s/#.*$//" $SRC_DIR/$d/$xfile`; do
> +				echo "$d/$f command line" >> $tmp.xlist
> +			done
> +		done
> +	done
> +
> +	# External file of tests to exclude (-E)
> +	for xfile in $EXCLUDE_FILES; do
> +		if [ -f $xfile ]; then
> +			sed -e "s/#.*$//" \
> +			    -e "s;$; file $xfile;" "$xfile" \

And I have no idea what problem this second expression is solving -
it wasn't in the original code that got copied here.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 1/7] btrfs/010: don't run without /sys/fs/btrfs

[Filipe Manana]

On Mon, Jan 21, 2019 at 4:33 PM <jeffm@suse.com> wrote:
>
> From: Jeff Mahoney <jeffm@suse.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Looks good. Since it's a btrfs specific change/fix, it would have been
a good idea to cc linux-btrfs.

>
> Older kernels don't have /sys/fs/btrfs.  btrfs/010 will happily run
> until it goes to check its work against sysfs and finds those
> files don't exist.  This patch introduces a require check to
> ensure that the sysfs files are present before running.
>
> Signed-off-by: Jeff Mahoney <jeffm@suse.com>
> ---
>  common/btrfs    | 8 ++++++++
>  tests/btrfs/010 | 1 +
>  2 files changed, 9 insertions(+)
>
> diff --git a/common/btrfs b/common/btrfs
> index 26dc0bb9..f6513c06 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -89,6 +89,14 @@ _require_btrfs_fs_feature()
>                 _notrun "Feature $feat not supported by the available btrfs version"
>  }
>
> +_require_btrfs_fs_sysfs()
> +{
> +       modprobe btrfs > /dev/null 2>&1
> +       [ -e /sys/fs/btrfs/features ] || \
> +               _notrun "Sysfs not supported by the available btrfs version"
> +
> +}
> +
>  _check_btrfs_filesystem()
>  {
>         device=$1
> diff --git a/tests/btrfs/010 b/tests/btrfs/010
> index 1a5f0146..72b2c727 100755
> --- a/tests/btrfs/010
> +++ b/tests/btrfs/010
> @@ -33,6 +33,7 @@ rm -f $seqres.full
>  _supported_fs btrfs
>  _supported_os Linux
>  _require_test
> +_require_btrfs_fs_sysfs
>
>  # Create 32k extents. All of these extents will be accounted as outstanding and
>  # reserved.
> --
> 2.16.4
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

Re: [PATCH 4/7] btrfs: require feature raid56 for raid56 tests

[Filipe Manana]

On Mon, Jan 21, 2019 at 4:33 PM <jeffm@suse.com> wrote:
>
> From: Jeff Mahoney <jeffm@suse.com>
>
> btrfs/125, btrfs/148, btrfs/157, and btrfs/158 test for raid56
> behavior.  We shouldn't run if the kernel doesn't have support for them.
>
> Signed-off-by: Jeff Mahoney <jeffm@suse.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Looks good. Since it's a btrfs specific change/fix, it would have been
a good idea to cc linux-btrfs.

> ---
>  tests/btrfs/125 | 1 +
>  tests/btrfs/148 | 1 +
>  tests/btrfs/157 | 1 +
>  tests/btrfs/158 | 1 +
>  4 files changed, 4 insertions(+)
>
> diff --git a/tests/btrfs/125 b/tests/btrfs/125
> index 5ac68b67..847fa62a 100755
> --- a/tests/btrfs/125
> +++ b/tests/btrfs/125
> @@ -51,6 +51,7 @@ _supported_os Linux
>  _require_scratch_dev_pool 3
>  _test_unmount
>  _require_loadable_fs_module "btrfs"
> +_require_btrfs_fs_feature raid56
>
>  _scratch_dev_pool_get 3
>
> diff --git a/tests/btrfs/148 b/tests/btrfs/148
> index d0e554c2..3fd8616f 100755
> --- a/tests/btrfs/148
> +++ b/tests/btrfs/148
> @@ -30,6 +30,7 @@ _supported_os Linux
>  _require_scratch
>  _require_scratch_dev_pool 4
>  _require_odirect
> +_require_btrfs_fs_feature raid56
>
>  _scratch_dev_pool_get 4
>
> diff --git a/tests/btrfs/157 b/tests/btrfs/157
> index d9ea4b7b..7f75c407 100755
> --- a/tests/btrfs/157
> +++ b/tests/btrfs/157
> @@ -49,6 +49,7 @@ _supported_fs btrfs
>  _supported_os Linux
>  _require_scratch_dev_pool 4
>  _require_btrfs_command inspect-internal dump-tree
> +_require_btrfs_fs_feature raid56
>
>  get_physical_stripe0()
>  {
> diff --git a/tests/btrfs/158 b/tests/btrfs/158
> index fe2dd956..603e8bea 100755
> --- a/tests/btrfs/158
> +++ b/tests/btrfs/158
> @@ -41,6 +41,7 @@ _supported_fs btrfs
>  _supported_os Linux
>  _require_scratch_dev_pool 4
>  _require_btrfs_command inspect-internal dump-tree
> +_require_btrfs_fs_feature raid56
>
>  get_physical_stripe0()
>  {
> --
> 2.16.4
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

Re: [PATCH 5/7] btrfs/023: skip trying to test raid56 without kernel support

[Filipe Manana]

On Mon, Jan 21, 2019 at 4:33 PM <jeffm@suse.com> wrote:
>
> From: Jeff Mahoney <jeffm@suse.com>
>
> Older kernels don't support raid56.  This test is still valid for
> other profiles, so skip raid56 if the kernel doesn't support it.
>
> Signed-off-by: Jeff Mahoney <jeffm@suse.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Looks good. Since it's a btrfs specific change/fix, it would have been
a good idea to cc linux-btrfs.

> ---
>  tests/btrfs/023 | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/tests/btrfs/023 b/tests/btrfs/023
> index 6d9cc443..6c02113b 100755
> --- a/tests/btrfs/023
> +++ b/tests/btrfs/023
> @@ -63,11 +63,13 @@ check_group_profile "RAID1"
>  create_group_profile "raid10"
>  check_group_profile "RAID10"
>
> -create_group_profile "raid5"
> -check_group_profile "RAID5"
> +if [ -e "/sys/fs/btrfs/features/raid56" ]; then
> +       create_group_profile "raid5"
> +       check_group_profile "RAID5"
>
> -create_group_profile "raid6"
> -check_group_profile "RAID6"
> +       create_group_profile "raid6"
> +       check_group_profile "RAID6"
> +fi
>
>  # success, all done
>  echo "Silence is golden"
> --
> 2.16.4
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

Re: [PATCH 6/7] btrfs/131: require support for free-space-tree

[Filipe Manana]

On Mon, Jan 21, 2019 at 4:33 PM <jeffm@suse.com> wrote:
>
> From: Jeff Mahoney <jeffm@suse.com>
>
> btrfs/131 tests the free space tree, which older kernels won't have.  We
> shouldn't run there.
>
> Signed-off-by: Jeff Mahoney <jeffm@suse.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Looks good. Since it's a btrfs specific change/fix, it would have been
a good idea to cc linux-btrfs.

> ---
>  tests/btrfs/131 | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tests/btrfs/131 b/tests/btrfs/131
> index 0da1e7be..3de7eef8 100755
> --- a/tests/btrfs/131
> +++ b/tests/btrfs/131
> @@ -34,6 +34,7 @@ _supported_fs btrfs
>  _supported_os Linux
>  _require_scratch
>  _require_btrfs_command inspect-internal dump-super
> +_require_btrfs_fs_feature free_space_tree
>
>  mkfs_v1()
>  {
> --
> 2.16.4
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

Re: [PATCH 2/7] filter: fix assumed whitespace in _filefrag_filter regex

[Jeff Mahoney]

On 1/21/19 5:51 PM, Dave Chinner wrote:
> On Mon, Jan 21, 2019 at 11:33:11AM -0500, jeffm@suse.com wrote:
>> From: Jeff Mahoney <jeffm@suse.com>
>>
>> The regex assumes there will be whitespace after the .. in the ranges
>> and, with larger offsets, there may not be any.
> 
> Does more than this, right?

I suppose.  It converts the interpreted .. into a literal .., which I
believe was the original intent.

>> Signed-off-by: Jeff Mahoney <jeffm@suse.com>
>> ---
>>  common/filter | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/common/filter b/common/filter
>> index ed082d24..b4443a34 100644
>> --- a/common/filter
>> +++ b/common/filter
>> @@ -532,7 +532,7 @@ _filter_filefrag()
>>  		next
>>  	}
>>  	($ext, $logical, $physical, $length) =
>> -		(/^\s*(\d+):\s+(\d+)..\s+\d+:\s+(\d+)..\s+\d+:\s+(\d+):/)
>> +		(/^\s*(\d+):\s+(\d+)\.\.\s*\d+:\s+(\d+)\.\.\s*\d+:\s+(\d+):/)
> 
> This is escaping "..", too, isn't it? 

Yes, but what else could the .. in the original regex have been
referring to?  I don't recall seeing any other filefrag format that
would match otherwise.

I don't have a problem documenting it further, though.

-Jeff

-- 
Jeff Mahoney
SUSE Labs

Re: [PATCH 3/7] filter: add support for old filefrag -v

[Jeff Mahoney]

On 1/21/19 5:53 PM, Dave Chinner wrote:
> On Mon, Jan 21, 2019 at 11:33:12AM -0500, jeffm@suse.com wrote:
>> From: Jeff Mahoney <jeffm@suse.com>
>>
>> e2fsprogs versions prior to v1.42.7 had a different format for
>> filefrag -v that is incompatible with the regex in _filefrag_filter.
>>
>> This patch adds support for the older format to test on older releases
> 
> Can you document what the two formats are in a comment just so it is
> somewhat easier to check the regexes are, indeed, doing the right
> thing?

Sure.  That's easy enough.

-Jeff

-- 
Jeff Mahoney
SUSE Labs

Re: [PATCH 7/7] check: move test exclusion handling to _prepare_test_list

[Jeff Mahoney]

On 1/21/19 6:09 PM, Dave Chinner wrote:
> On Mon, Jan 21, 2019 at 11:33:16AM -0500, jeffm@suse.com wrote:
>> From: Jeff Mahoney <jeffm@suse.com>
>>
>> In order to simplify combining excluded tests specified on the command
>> line vs specified via config files,
> 
> You mean defining excludes in configs/<hostname>.config files,
> right? i.e. in config sections?

Yes.

> But the section code only calls _prepare_test_list if the test dev
> is recreated by each section, right? So you can't really change the
> expunge list from the config files without forcing a test dev
> reformat, right?

Right.  In my testing I didn't hit this situation, but it's there.  I
can fix that.

>> it makes sense to push the handling
>> into _prepare_test_list.  This means we start with a fresh $tmp.xlist
>> and rebuild it each time _prepare_test_list is called.
> 
> The patch does more than that, right?
> 
>> Signed-off-by: Jeff Mahoney <jeffm@suse.com>
>> ---
>>  check | 41 ++++++++++++++++++++++++-----------------
>>  1 file changed, 24 insertions(+), 17 deletions(-)
>>
>> diff --git a/check b/check
>> index 77a06b00..17073c4e 100755
>> --- a/check
>> +++ b/check
>> @@ -230,7 +230,28 @@ _prepare_test_list()
>>  		done
>>  	fi
>>  
>> -	# Specified groups to exclude
>> +	:> $tmp.xlist
>> +
>> +	# Per-fstype/generic/shared file of tests to exclude (-X)
>> +	for xfile in $XGROUP_FILES; do
> 
> That adds support for multiple group exclude files (i.e. multiple -X
> options), right?

Yes.  I'll document that in the commit message.

>> +		for d in $SRC_GROUPS $FSTYP; do
>> +			[ -f $SRC_DIR/$d/$xfile ] || continue
>> +			for f in `sed "s/#.*$//" $SRC_DIR/$d/$xfile`; do
>> +				echo "$d/$f command line" >> $tmp.xlist
>> +			done
>> +		done
>> +	done
>> +
>> +	# External file of tests to exclude (-E)
>> +	for xfile in $EXCLUDE_FILES; do
>> +		if [ -f $xfile ]; then
>> +			sed -e "s/#.*$//" \
>> +			    -e "s;$; file $xfile;" "$xfile" \
> 
> And I have no idea what problem this second expression is solving -
> it wasn't in the original code that got copied here.

This series is part of a larger set I've been using for a while.  I
reordered them and pulled out the least controversial.  This was
originally after a patch that adds reporting for why a test was expunged
to the check output.  We can ignore this part for now.

Thanks for the review,

-Jeff

-- 
Jeff Mahoney
SUSE Labs

Re: [PATCH 2/7] filter: fix assumed whitespace in _filefrag_filter regex

[Dave Chinner]

On Tue, Jan 22, 2019 at 08:38:17PM -0500, Jeff Mahoney wrote:
> On 1/21/19 5:51 PM, Dave Chinner wrote:
> > On Mon, Jan 21, 2019 at 11:33:11AM -0500, jeffm@suse.com wrote:
> >> From: Jeff Mahoney <jeffm@suse.com>
> >>
> >> The regex assumes there will be whitespace after the .. in the ranges
> >> and, with larger offsets, there may not be any.
> > 
> > Does more than this, right?
> 
> I suppose.  It converts the interpreted .. into a literal .., which I
> believe was the original intent.
> 
> >> Signed-off-by: Jeff Mahoney <jeffm@suse.com>
> >> ---
> >>  common/filter | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/common/filter b/common/filter
> >> index ed082d24..b4443a34 100644
> >> --- a/common/filter
> >> +++ b/common/filter
> >> @@ -532,7 +532,7 @@ _filter_filefrag()
> >>  		next
> >>  	}
> >>  	($ext, $logical, $physical, $length) =
> >> -		(/^\s*(\d+):\s+(\d+)..\s+\d+:\s+(\d+)..\s+\d+:\s+(\d+):/)
> >> +		(/^\s*(\d+):\s+(\d+)\.\.\s*\d+:\s+(\d+)\.\.\s*\d+:\s+(\d+):/)
> > 
> > This is escaping "..", too, isn't it? 
> 
> Yes, but what else could the .. in the original regex have been
> referring to?  I don't recall seeing any other filefrag format that
> would match otherwise.

Neither do I. My point was it was a change that was not documented
in the commit message and so there might be something I was missing.

> I don't have a problem documenting it further, though.

That'd be great, thanks!

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com