[PATCH 0/5] fstests: various fixes

[PATCH 0/5] fstests: various fixes

[Darrick J. Wong]

Hi all,

Various fixes for the new tests:

- Fix get_max_lfs_filesize naming breakage
- Don't OOM the box when opening tempfiles
- More fixes to the generic/529 attribute test
- Fix incorrect btree block type usage when fuzzing
- Fix tool usage in shared/298
- Check statx attributes_mask
- Force the XFS fs population helper to create multilevel inode btrees

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=djwong-devel

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=djwong-devel

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=djwong-devel

[PATCH 1/5] common/rc: fix get_max_lfs_filesize

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Helper functions are supposed to have a leading underscore ('_') in the
function name, but this one doesn't have it.  Unfortunately, the calling
test cases (generic/349-351) /do/ have the leading underscore, so now
they're broken.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/rc |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/common/rc b/common/rc
index 8b40aabe..1c42515f 100644
--- a/common/rc
+++ b/common/rc
@@ -3834,7 +3834,7 @@ _get_max_file_size()
 }
 
 # get MAX_LFS_FILESIZE
-get_max_lfs_filesize()
+_get_max_lfs_filesize()
 {
 	case "$(getconf LONG_BIT)" in
 	"32")

[PATCH 2/5] xfs/010: use correct type for finobt corrupting

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Use 'type finobt' for corrupting the finobt.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/010 |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)


diff --git a/tests/xfs/010 b/tests/xfs/010
index ee1595c8..e220a651 100755
--- a/tests/xfs/010
+++ b/tests/xfs/010
@@ -63,14 +63,14 @@ _corrupt_finobt_records()
 			_filter_dbval`
 
 	# Corrupt a freecount value. This should never exceed 64.
-	$XFS_DB_PROG -x -c "fsb $free_root" -c "type inobt" \
+	$XFS_DB_PROG -x -c "fsb $free_root" -c "type finobt" \
 		-c "write recs[1].freecount 70" $dev
 
 	# Create a corrupted non-free record, which should never appear in the
 	# finobt.
-	$XFS_DB_PROG -x -c "fsb $free_root" -c "type inobt" \
+	$XFS_DB_PROG -x -c "fsb $free_root" -c "type finobt" \
 		 -c "write recs[2].freecount 0" $dev
-	$XFS_DB_PROG -x -c "fsb $free_root" -c "type inobt" \
+	$XFS_DB_PROG -x -c "fsb $free_root" -c "type finobt" \
 		-c "write recs[2].free 0" $dev
 }
 

[PATCH 3/5] shared/298: unmount filesystem before examining underlying storage

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

This test does some weird things with live filesystems -- it seems to be
validating the behavior of fstrim by comparing the filesystem's free
space map to holes in the file image that backs the filesystem.
However, this doesn't account for the fact that some filesystems
maintain in-core preallocations and/or can perturb the free space data
during unmount.  This causes sporadic test failures when the two become
out of sync.

Therefore, make sure we unmount the filesystem before we start running
tools against the filesystem image file to eliminate the possibility of
changes to the free space map.  This was found by running shared/298 on
xfs with a 1k block size.

cc: enwlinux@gmail.com
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/shared/298 |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)


diff --git a/tests/shared/298 b/tests/shared/298
index aafdc25f..5d6c6ccf 100755
--- a/tests/shared/298
+++ b/tests/shared/298
@@ -46,13 +46,21 @@ _cleanup()
 
 get_holes()
 {
+	# It's not a good idea to be running tools against the image file
+	# backing a live filesystem because the filesystem could be maintaining
+	# in-core state that will perturb the free space map on umount.  Stick
+	# to established convention which requires the filesystem to be
+	# unmounted while we probe the underlying file.
+	$UMOUNT_PROG $loop_mnt
 	$XFS_IO_PROG -F -c fiemap $1 | grep hole | $SED_PROG 's/.*\[\(.*\)\.\.\(.*\)\].*/\1 \2/'
+	_mount $loop_dev $loop_mnt
 }
 
 get_free_sectors()
 {
 	case $FSTYP in
 	ext4)
+	$UMOUNT_PROG $loop_mnt
 	$DUMPE2FS_PROG $img_file  2>&1 | grep " Free blocks" | cut -d ":" -f2- | \
 		tr ',' '\n' | $SED_PROG 's/^ //' | \
 		$AWK_PROG -v spb=$sectors_per_block 'BEGIN{FS="-"};
@@ -195,6 +203,16 @@ while read line; do
 		END { if(found) exit 0; else exit 1}' $merged_sectors
 	then
 		echo "Sectors $from-$to are not marked as free!"
+
+		# Dump the state to make it easier to debug this...
+		echo free_sectors >> $seqres.full
+		sort -g < $free_sectors >> $seqres.full
+		echo fiemap_ref >> $seqres.full
+		sort -g < $fiemap_ref >> $seqres.full
+		echo merged_sectors >> $seqres.full
+		sort -g < $merged_sectors >> $seqres.full
+		echo fiemap_after >> $seqres.full
+		sort -g < $fiemap_after >> $seqres.full
 		exit
 	fi
 done < $fiemap_after

[PATCH 4/5] generic: test statx attribute_mask setting

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Make sure the filesystem reports attribute_mask for the attributes it
supports.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/generic/713     |   67 +++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/713.out |    1 +
 tests/generic/group   |    1 +
 3 files changed, 69 insertions(+)
 create mode 100755 tests/generic/713
 create mode 100644 tests/generic/713.out


diff --git a/tests/generic/713 b/tests/generic/713
new file mode 100755
index 00000000..cd128d9c
--- /dev/null
+++ b/tests/generic/713
@@ -0,0 +1,67 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2019 Oracle, Inc.  All Rights Reserved.
+#
+# FS QA Test No. 713
+#
+# Regression test for a bug where XFS fails to set statx attributes_mask but
+# sets attribute flags anyway.
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.* $testfile
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_test
+
+get_attributes() {
+	$XFS_IO_PROG -r -c "statx -r" $1 | grep 'stat.attributes =' | cut -d ' ' -f 3
+}
+
+get_attributes_mask() {
+	$XFS_IO_PROG -r -c "statx -r" $1 | grep 'stat.attributes_mask =' | cut -d ' ' -f 3
+}
+
+check_statx_attributes()
+{
+	attrs=$(get_attributes $testfile)
+	mask=$(get_attributes_mask $testfile)
+
+	echo "MASK:$mask:ATTRS:$attrs:" >> $seqres.full
+
+	test -z "$mask" && _notrun "xfs_io statx command does not support attributes_mask"
+	test $(( mask )) -eq 0 && echo "attributes mask is zero"
+	test $(( attrs & ~(mask) )) -ne 0 && echo "attributes $attrs do not appear in mask $mask"
+}
+
+# Create file, check for incorrect mask
+testfile=$TEST_DIR/$$.test
+touch $testfile
+check_statx_attributes
+
+# Do it again, but this time try to turn on one of the attributes.
+if chattr +i $testfile > /dev/null 2>&1; then
+	check_statx_attributes
+	chattr -i $testfile
+fi
+if chattr +a $testfile > /dev/null 2>&1; then
+	check_statx_attributes
+	chattr -a $testfile
+fi
+
+status=0
+exit
diff --git a/tests/generic/713.out b/tests/generic/713.out
new file mode 100644
index 00000000..066598d3
--- /dev/null
+++ b/tests/generic/713.out
@@ -0,0 +1 @@
+QA output created by 713
diff --git a/tests/generic/group b/tests/generic/group
index 15227b67..d04bf39f 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -534,3 +534,4 @@
 529 auto quick attr
 530 auto quick unlink
 531 auto quick unlink
+713 auto quick

[PATCH 5/5] populate: force large finobt creation on xfs

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Teach the populate routines to create enough inodes that we end up with
multi-level inode btrees.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/populate |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)


diff --git a/common/populate b/common/populate
index d49fbc29..95953476 100644
--- a/common/populate
+++ b/common/populate
@@ -260,6 +260,21 @@ _scratch_xfs_populate() {
 	$XFS_IO_PROG -f -c "pwrite -S 0x62 0 $((blksz * nr))" "${SCRATCH_MNT}/BNOBT"
 	./src/punch-alternating "${SCRATCH_MNT}/BNOBT"
 
+	# Inode btree
+	echo "+ inobt btree"
+	local ino_per_rec=64
+	local rec_per_btblock=16
+	local nr="$(( 2 * (blksz / rec_per_btblock) * ino_per_rec ))"
+	local dir="${SCRATCH_MNT}/INOBT"
+	mkdir -p "${dir}"
+	seq 0 "${nr}" | while read f; do
+		touch "${dir}/${f}"
+	done
+
+	seq 0 "$((ino_per_rec + 1))" "${nr}" | while read f; do
+		rm -f "${dir}/${f}"
+	done
+
 	# Reverse-mapping btree
 	is_rmapbt="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'rmapbt=1')"
 	if [ $is_rmapbt -gt 0 ]; then
@@ -562,6 +577,8 @@ _scratch_xfs_populate_check() {
 	__populate_check_xfs_aformat "${btree_attr}" "btree"
 	__populate_check_xfs_agbtree_height "bno"
 	__populate_check_xfs_agbtree_height "cnt"
+	__populate_check_xfs_agbtree_height "ino"
+	test $is_finobt -ne 0 && __populate_check_xfs_agbtree_height "fino"
 	test $is_rmapbt -ne 0 && __populate_check_xfs_agbtree_height "rmap"
 	test $is_reflink -ne 0 && __populate_check_xfs_agbtree_height "refcnt"
 }

[PATCH 6/5] ext4/032: SCRATCH_DIR -> SCRATCH_MNT

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Use SCRATCH_MNT, not SCRATCH_DIR.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/ext4/032 |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/ext4/032 b/tests/ext4/032
index 8534ddd6..277ddef0 100755
--- a/tests/ext4/032
+++ b/tests/ext4/032
@@ -19,8 +19,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 BLK_SIZ=4096
 CLUSTER_SIZ=4096
 
-IMG_FILE=$SCRATCH_DIR/$seq.fs
-IMG_MNT=$SCRATCH_DIR/$seq.mnt
+IMG_FILE=$SCRATCH_MNT/$seq.fs
+IMG_MNT=$SCRATCH_MNT/$seq.mnt
 
 ## Num clusters to blocks.
 c2b()

Re: [PATCH 1/5] common/rc: fix get_max_lfs_filesize

[Eryu Guan]

On Mon, Mar 04, 2019 at 01:19:08PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Helper functions are supposed to have a leading underscore ('_') in the
> function name, but this one doesn't have it.  Unfortunately, the calling
> test cases (generic/349-351) /do/ have the leading underscore, so now
> they're broken.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  common/rc |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> diff --git a/common/rc b/common/rc
> index 8b40aabe..1c42515f 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3834,7 +3834,7 @@ _get_max_file_size()
>  }
>  
>  # get MAX_LFS_FILESIZE
> -get_max_lfs_filesize()
> +_get_max_lfs_filesize()

I introduced this bug on commit when renaming the function, I remembered
I did test it before pushing it out.. Thanks for fixing it!

Eryu
>  {
>  	case "$(getconf LONG_BIT)" in
>  	"32")
> 

Re: [PATCH 4/5] generic: test statx attribute_mask setting

[Eryu Guan]

On Mon, Mar 04, 2019 at 01:19:35PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Make sure the filesystem reports attribute_mask for the attributes it
> supports.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

A few minor issues, and I can fix them all on commit (I'm sure I do test
the changes this time :)).

> ---
>  tests/generic/713     |   67 +++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/713.out |    1 +
>  tests/generic/group   |    1 +
>  3 files changed, 69 insertions(+)
>  create mode 100755 tests/generic/713
>  create mode 100644 tests/generic/713.out
> 
> 
> diff --git a/tests/generic/713 b/tests/generic/713
> new file mode 100755
> index 00000000..cd128d9c
> --- /dev/null
> +++ b/tests/generic/713
> @@ -0,0 +1,67 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (c) 2019 Oracle, Inc.  All Rights Reserved.
> +#
> +# FS QA Test No. 713
> +#
> +# Regression test for a bug where XFS fails to set statx attributes_mask but
> +# sets attribute flags anyway.
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.* $testfile
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_test
> +
> +get_attributes() {
> +	$XFS_IO_PROG -r -c "statx -r" $1 | grep 'stat.attributes =' | cut -d ' ' -f 3
> +}
> +
> +get_attributes_mask() {
> +	$XFS_IO_PROG -r -c "statx -r" $1 | grep 'stat.attributes_mask =' | cut -d ' ' -f 3
> +}
> +
> +check_statx_attributes()
> +{
> +	attrs=$(get_attributes $testfile)
> +	mask=$(get_attributes_mask $testfile)

Declare them as 'local'.

> +
> +	echo "MASK:$mask:ATTRS:$attrs:" >> $seqres.full
> +
> +	test -z "$mask" && _notrun "xfs_io statx command does not support attributes_mask"
> +	test $(( mask )) -eq 0 && echo "attributes mask is zero"
> +	test $(( attrs & ~(mask) )) -ne 0 && echo "attributes $attrs do not appear in mask $mask"
> +}
> +

Added the below here:

rm -f $seqres.full
echo "Silence is golden"

> +# Create file, check for incorrect mask
> +testfile=$TEST_DIR/$$.test
> +touch $testfile
> +check_statx_attributes
> +
> +# Do it again, but this time try to turn on one of the attributes.
> +if chattr +i $testfile > /dev/null 2>&1; then
> +	check_statx_attributes
> +	chattr -i $testfile
> +fi
> +if chattr +a $testfile > /dev/null 2>&1; then
> +	check_statx_attributes
> +	chattr -a $testfile

Change 'chattr' to '$CHATTR_PROG'.

Thanks,
Eryu

> +fi
> +
> +status=0
> +exit
> diff --git a/tests/generic/713.out b/tests/generic/713.out
> new file mode 100644
> index 00000000..066598d3
> --- /dev/null
> +++ b/tests/generic/713.out
> @@ -0,0 +1 @@
> +QA output created by 713
> diff --git a/tests/generic/group b/tests/generic/group
> index 15227b67..d04bf39f 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -534,3 +534,4 @@
>  529 auto quick attr
>  530 auto quick unlink
>  531 auto quick unlink
> +713 auto quick
> 

Re: [PATCH 4/5] generic: test statx attribute_mask setting

[Darrick J. Wong]

On Thu, Mar 07, 2019 at 02:14:18PM +0800, Eryu Guan wrote:
> On Mon, Mar 04, 2019 at 01:19:35PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Make sure the filesystem reports attribute_mask for the attributes it
> > supports.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> A few minor issues, and I can fix them all on commit (I'm sure I do test
> the changes this time :)).
> 
> > ---
> >  tests/generic/713     |   67 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/713.out |    1 +
> >  tests/generic/group   |    1 +
> >  3 files changed, 69 insertions(+)
> >  create mode 100755 tests/generic/713
> >  create mode 100644 tests/generic/713.out
> > 
> > 
> > diff --git a/tests/generic/713 b/tests/generic/713
> > new file mode 100755
> > index 00000000..cd128d9c
> > --- /dev/null
> > +++ b/tests/generic/713
> > @@ -0,0 +1,67 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0+
> > +# Copyright (c) 2019 Oracle, Inc.  All Rights Reserved.
> > +#
> > +# FS QA Test No. 713
> > +#
> > +# Regression test for a bug where XFS fails to set statx attributes_mask but
> > +# sets attribute flags anyway.
> > +#
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +tmp=/tmp/$$
> > +status=1	# failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -f $tmp.* $testfile
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +
> > +# real QA test starts here
> > +_supported_fs generic
> > +_supported_os Linux
> > +_require_test
> > +
> > +get_attributes() {
> > +	$XFS_IO_PROG -r -c "statx -r" $1 | grep 'stat.attributes =' | cut -d ' ' -f 3
> > +}
> > +
> > +get_attributes_mask() {
> > +	$XFS_IO_PROG -r -c "statx -r" $1 | grep 'stat.attributes_mask =' | cut -d ' ' -f 3
> > +}
> > +
> > +check_statx_attributes()
> > +{
> > +	attrs=$(get_attributes $testfile)
> > +	mask=$(get_attributes_mask $testfile)
> 
> Declare them as 'local'.
> 
> > +
> > +	echo "MASK:$mask:ATTRS:$attrs:" >> $seqres.full
> > +
> > +	test -z "$mask" && _notrun "xfs_io statx command does not support attributes_mask"
> > +	test $(( mask )) -eq 0 && echo "attributes mask is zero"
> > +	test $(( attrs & ~(mask) )) -ne 0 && echo "attributes $attrs do not appear in mask $mask"
> > +}
> > +
> 
> Added the below here:
> 
> rm -f $seqres.full
> echo "Silence is golden"
> 
> > +# Create file, check for incorrect mask
> > +testfile=$TEST_DIR/$$.test
> > +touch $testfile
> > +check_statx_attributes
> > +
> > +# Do it again, but this time try to turn on one of the attributes.
> > +if chattr +i $testfile > /dev/null 2>&1; then
> > +	check_statx_attributes
> > +	chattr -i $testfile
> > +fi
> > +if chattr +a $testfile > /dev/null 2>&1; then
> > +	check_statx_attributes
> > +	chattr -a $testfile
> 
> Change 'chattr' to '$CHATTR_PROG'.

Sounds good, thank you for fixing those!

--D

> Thanks,
> Eryu
> 
> > +fi
> > +
> > +status=0
> > +exit
> > diff --git a/tests/generic/713.out b/tests/generic/713.out
> > new file mode 100644
> > index 00000000..066598d3
> > --- /dev/null
> > +++ b/tests/generic/713.out
> > @@ -0,0 +1 @@
> > +QA output created by 713
> > diff --git a/tests/generic/group b/tests/generic/group
> > index 15227b67..d04bf39f 100644
> > --- a/tests/generic/group
> > +++ b/tests/generic/group
> > @@ -534,3 +534,4 @@
> >  529 auto quick attr
> >  530 auto quick unlink
> >  531 auto quick unlink
> > +713 auto quick
> >