[PATCHSET v23.3 0/4] fstests: refactor xfs geometry computation

[PATCHSET v23.3 0/4] fstests: refactor xfs geometry computation

[Darrick J. Wong]

Hi all,

There are numerous tests that do things based on the geometry of a
mounted filesystem.  Before we start adding more tests that do this
(e.g. online fsck stress tests), refactor them into common/xfs helpers.

v23.2: refactor more number extraction grep/sed patterns
v23.3: rebase per maintainer request

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

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=refactor-xfs-geometry
---
 common/ext4     |    9 +++++
 common/populate |   24 ++++++-------
 common/rc       |    2 +
 common/xfs      |  104 ++++++++++++++++++++++++++++++++++++++++++++++++-------
 tests/xfs/097   |    2 +
 tests/xfs/099   |    2 +
 tests/xfs/100   |    2 +
 tests/xfs/101   |    2 +
 tests/xfs/102   |    2 +
 tests/xfs/105   |    2 +
 tests/xfs/112   |    2 +
 tests/xfs/113   |    2 +
 tests/xfs/146   |    2 +
 tests/xfs/147   |    2 +
 tests/xfs/151   |    3 +-
 tests/xfs/271   |    2 +
 tests/xfs/307   |    2 +
 tests/xfs/308   |    2 +
 tests/xfs/348   |    2 +
 tests/xfs/530   |    3 +-
 20 files changed, 129 insertions(+), 44 deletions(-)

[PATCH 1/4] xfs: refactor filesystem feature detection logic

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

There are a lot of places where we open-code detecting features of a
specific filesystem.  Refactor this into a couple of helpers in
preparation for adding stress tests for online repair and fuzzing.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/populate |   14 ++++++-----
 common/rc       |    2 +-
 common/xfs      |   67 ++++++++++++++++++++++++++++++++++++++++++++++---------
 tests/xfs/097   |    2 +-
 tests/xfs/151   |    3 +-
 tests/xfs/271   |    2 +-
 tests/xfs/307   |    2 +-
 tests/xfs/308   |    2 +-
 tests/xfs/348   |    2 +-
 9 files changed, 70 insertions(+), 26 deletions(-)


diff --git a/common/populate b/common/populate
index 58b07e33be..9fa1a06798 100644
--- a/common/populate
+++ b/common/populate
@@ -131,7 +131,7 @@ _populate_xfs_qmount_option()
 	fi
 
 	# Turn on all the quotas
-	if $XFS_INFO_PROG "${TEST_DIR}" | grep -q 'crc=1'; then
+	if _xfs_has_feature "$TEST_DIR" crc; then
 		# v5 filesystems can have group & project quotas
 		quota="usrquota,grpquota,prjquota"
 	else
@@ -176,7 +176,7 @@ _scratch_xfs_populate() {
 
 	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
 	dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
-	crc="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep crc= | sed -e 's/^.*crc=//g' -e 's/\([0-9]*\).*$/\1/g')"
+	crc="$(_xfs_has_feature "$SCRATCH_MNT" crc -v)"
 	if [ $crc -eq 1 ]; then
 		leaf_hdr_size=64
 	else
@@ -315,7 +315,7 @@ _scratch_xfs_populate() {
 	done
 
 	# Reverse-mapping btree
-	is_rmapbt="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'rmapbt=1')"
+	is_rmapbt="$(_xfs_has_feature "$SCRATCH_MNT" rmapbt -v)"
 	if [ $is_rmapbt -gt 0 ]; then
 		echo "+ rmapbt btree"
 		nr="$((blksz * 2 / 24))"
@@ -332,7 +332,7 @@ _scratch_xfs_populate() {
 	fi
 
 	# Reference-count btree
-	is_reflink="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'reflink=1')"
+	is_reflink="$(_xfs_has_feature "$SCRATCH_MNT" reflink -v)"
 	if [ $is_reflink -gt 0 ]; then
 		echo "+ reflink btree"
 		nr="$((blksz * 2 / 12))"
@@ -597,9 +597,9 @@ _scratch_xfs_populate_check() {
 	leaf_attr="$(__populate_find_inode "${SCRATCH_MNT}/ATTR.FMT_LEAF")"
 	node_attr="$(__populate_find_inode "${SCRATCH_MNT}/ATTR.FMT_NODE")"
 	btree_attr="$(__populate_find_inode "${SCRATCH_MNT}/ATTR.FMT_BTREE")"
-	is_finobt=$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'finobt=1')
-	is_rmapbt=$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'rmapbt=1')
-	is_reflink=$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'reflink=1')
+	is_finobt=$(_xfs_has_feature "$SCRATCH_MNT" finobt -v)
+	is_rmapbt=$(_xfs_has_feature "$SCRATCH_MNT" rmapbt -v)
+	is_reflink=$(_xfs_has_feature "$SCRATCH_MNT" reflink -v)
 
 	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
 	dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
diff --git a/common/rc b/common/rc
index f4785c17ca..8060c03b7d 100644
--- a/common/rc
+++ b/common/rc
@@ -247,7 +247,7 @@ _supports_filetype()
 	local fstyp=`$DF_PROG $dir | tail -1 | $AWK_PROG '{print $2}'`
 	case "$fstyp" in
 	xfs)
-		$XFS_INFO_PROG $dir | grep -q "ftype=1"
+		_xfs_has_feature $dir ftype
 		;;
 	ext2|ext3|ext4)
 		local dev=`$DF_PROG $dir | tail -1 | $AWK_PROG '{print $1}'`
diff --git a/common/xfs b/common/xfs
index e1c15d3d04..b2ac78de0c 100644
--- a/common/xfs
+++ b/common/xfs
@@ -413,6 +413,56 @@ _require_xfs_crc()
 	_scratch_unmount
 }
 
+# If the xfs_info output for the given XFS filesystem mount mentions the given
+# feature.  If so, return 0 for success.  If not, return 1 for failure.  If the
+# third option is -v, echo 1 for success and 0 for not.
+#
+# Starting with xfsprogs 4.17, this also works for unmounted filesystems.
+_xfs_has_feature()
+{
+	local fs="$1"
+	local feat="$2"
+	local verbose="$3"
+
+	local answer="$($XFS_INFO_PROG "$fs" 2>&1 | grep -w -c "$feat=1")"
+	if [ "$answer" -ne 0 ]; then
+		test "$verbose" = "-v" && echo 1
+		return 0
+	fi
+
+	test "$verbose" = "-v" && echo 0
+	return 1
+}
+
+# Require that the xfs_info output for the given XFS filesystem mount mentions
+# the given feature flag.  If the third argument is -u (or is empty and the
+# second argument is $SCRATCH_MNT), unmount the fs on failure.  If a fourth
+# argument is supplied, it will be used as the _notrun message.
+_require_xfs_has_feature()
+{
+	local fs="$1"
+	local feat="$2"
+	local umount="$3"
+	local message="$4"
+
+	if [ -z "$umount" ] && [ "$fs" = "$SCRATCH_MNT" ]; then
+		umount="-u"
+	fi
+
+	_xfs_has_feature "$1" "$2" && return 0
+
+	test "$umount" = "-u" && umount "$fs" &>/dev/null
+
+	test -n "$message" && _notrun "$message"
+
+	case "$fs" in
+	"$TEST_DIR"|"$TEST_DEV")	fsname="test";;
+	"$SCRATCH_MNT"|"$SCRATCH_DEV")	fsname="scratch";;
+	*)				fsname="$fs";;
+	esac
+	_notrun "$2 not supported by $fsname filesystem type: $FSTYP"
+}
+
 # this test requires the xfs kernel support crc feature on scratch device
 #
 _require_scratch_xfs_crc()
@@ -420,7 +470,8 @@ _require_scratch_xfs_crc()
 	_scratch_mkfs_xfs >/dev/null 2>&1
 	_try_scratch_mount >/dev/null 2>&1 \
 	   || _notrun "Kernel doesn't support crc feature"
-	$XFS_INFO_PROG $SCRATCH_MNT | grep -q 'crc=1' || _notrun "crc feature not supported by this filesystem"
+	_require_xfs_has_feature $SCRATCH_MNT crc -u \
+		"crc feature not supported by this filesystem"
 	_scratch_unmount
 }
 
@@ -739,10 +790,7 @@ _check_xfs_test_fs()
 _require_xfs_test_rmapbt()
 {
 	_require_test
-
-	if [ "$($XFS_INFO_PROG "$TEST_DIR" | grep -c "rmapbt=1")" -ne 1 ]; then
-		_notrun "rmapbt not supported by test filesystem type: $FSTYP"
-	fi
+	_require_xfs_has_feature "$TEST_DIR" rmapbt
 }
 
 _require_xfs_scratch_rmapbt()
@@ -751,10 +799,7 @@ _require_xfs_scratch_rmapbt()
 
 	_scratch_mkfs > /dev/null
 	_scratch_mount
-	if [ "$($XFS_INFO_PROG "$SCRATCH_MNT" | grep -c "rmapbt=1")" -ne 1 ]; then
-		_scratch_unmount
-		_notrun "rmapbt not supported by scratch filesystem type: $FSTYP"
-	fi
+	_require_xfs_has_feature "$SCRATCH_MNT" rmapbt
 	_scratch_unmount
 }
 
@@ -1357,8 +1402,8 @@ _require_scratch_xfs_bigtime()
 		_notrun "mkfs.xfs doesn't support bigtime feature"
 	_try_scratch_mount || \
 		_notrun "kernel doesn't support xfs bigtime feature"
-	$XFS_INFO_PROG "$SCRATCH_MNT" | grep -q -w "bigtime=1" || \
-		_notrun "bigtime feature not advertised on mount?"
+	_require_xfs_has_feature $SCRATCH_MNT bigtime -u \
+		"crc feature not supported by this filesystem"
 	_scratch_unmount
 }
 
diff --git a/tests/xfs/097 b/tests/xfs/097
index 4cad7216cd..1df34eeddc 100755
--- a/tests/xfs/097
+++ b/tests/xfs/097
@@ -42,7 +42,7 @@ _scratch_mkfs_xfs -m crc=1,finobt=1 > /dev/null
 
 echo "+ mount fs image"
 _scratch_mount
-$XFS_INFO_PROG "${SCRATCH_MNT}" | grep -q "finobt=1" || _notrun "finobt not enabled"
+_require_xfs_has_feature "$SCRATCH_MNT" finobt
 blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
 
 echo "+ make some files"
diff --git a/tests/xfs/151 b/tests/xfs/151
index 66425f6710..b2fe16aefb 100755
--- a/tests/xfs/151
+++ b/tests/xfs/151
@@ -24,8 +24,7 @@ echo "Format filesystem and populate"
 _scratch_mkfs > $seqres.full
 _scratch_mount >> $seqres.full
 
-$XFS_INFO_PROG $SCRATCH_MNT | grep -q ftype=1 || \
-	_notrun "filesystem does not support ftype"
+_require_xfs_has_feature "$SCRATCH_MNT" ftype
 
 filter_ls() {
 	awk '
diff --git a/tests/xfs/271 b/tests/xfs/271
index 14d64cd0e5..d67ac4d6c1 100755
--- a/tests/xfs/271
+++ b/tests/xfs/271
@@ -37,7 +37,7 @@ agcount=$(_xfs_mount_agcount $SCRATCH_MNT)
 # same owner (per-AG metadata) for rmap btree blocks and blocks on the AGFL and
 # the reverse mapping index merges records, the number of per-AG extents
 # reported will vary depending on whether the refcount btree is enabled.
-$XFS_INFO_PROG $SCRATCH_MNT | grep -q reflink=1
+_require_xfs_has_feature "$SCRATCH_MNT" reflink
 has_reflink=$(( 1 - $? ))
 perag_metadata_exts=2
 test $has_reflink -gt 0 && perag_metadata_exts=$((perag_metadata_exts + 1))
diff --git a/tests/xfs/307 b/tests/xfs/307
index ba7204dd00..f3c970fadf 100755
--- a/tests/xfs/307
+++ b/tests/xfs/307
@@ -22,7 +22,7 @@ _require_scratch_reflink
 echo "Format"
 _scratch_mkfs > $seqres.full 2>&1
 _scratch_mount >> $seqres.full
-is_rmap=$($XFS_INFO_PROG $SCRATCH_MNT | grep -c "rmapbt=1")
+is_rmap=$(_xfs_has_feature $SCRATCH_MNT rmapbt -v)
 _scratch_unmount
 
 _get_agf_data() {
diff --git a/tests/xfs/308 b/tests/xfs/308
index d0f47f5038..6da6622e14 100755
--- a/tests/xfs/308
+++ b/tests/xfs/308
@@ -22,7 +22,7 @@ _require_scratch_reflink
 echo "Format"
 _scratch_mkfs > $seqres.full 2>&1
 _scratch_mount >> $seqres.full
-is_rmap=$($XFS_INFO_PROG $SCRATCH_MNT | grep -c "rmapbt=1")
+is_rmap=$(_xfs_has_feature $SCRATCH_MNT rmapbt -v)
 _scratch_xfs_unmount_dirty
 
 _get_agf_data() {
diff --git a/tests/xfs/348 b/tests/xfs/348
index faf2dca50b..d1645d9462 100755
--- a/tests/xfs/348
+++ b/tests/xfs/348
@@ -39,7 +39,7 @@ mknod $testdir/CHRDEV c 1 1
 mknod $testdir/BLKDEV b 1 1
 mknod $testdir/FIFO p
 
-$XFS_INFO_PROG $SCRATCH_MNT | grep -q "ftype=1" && FTYPE_FEATURE=1
+_xfs_has_feature $SCRATCH_MNT ftype && FTYPE_FEATURE=1
 
 # Record test dir inode for xfs_repair filter
 inode_filter=$tmp.sed

[PATCH 2/4] xfs: refactor filesystem directory block size extraction logic

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

There are a lot of places where we open-code determining the directory
block size for a specific filesystem.  Refactor this into a single
helper to clean up existing tests.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/populate |    4 ++--
 common/xfs      |    9 +++++++++
 tests/xfs/099   |    2 +-
 tests/xfs/100   |    2 +-
 tests/xfs/101   |    2 +-
 tests/xfs/102   |    2 +-
 tests/xfs/105   |    2 +-
 tests/xfs/112   |    2 +-
 tests/xfs/113   |    2 +-
 9 files changed, 18 insertions(+), 9 deletions(-)


diff --git a/common/populate b/common/populate
index 9fa1a06798..23b2fecf69 100644
--- a/common/populate
+++ b/common/populate
@@ -175,7 +175,7 @@ _scratch_xfs_populate() {
 	_xfs_force_bdev data $SCRATCH_MNT
 
 	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
-	dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
+	dblksz="$(_xfs_get_dir_blocksize "$SCRATCH_MNT")"
 	crc="$(_xfs_has_feature "$SCRATCH_MNT" crc -v)"
 	if [ $crc -eq 1 ]; then
 		leaf_hdr_size=64
@@ -602,7 +602,7 @@ _scratch_xfs_populate_check() {
 	is_reflink=$(_xfs_has_feature "$SCRATCH_MNT" reflink -v)
 
 	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
-	dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
+	dblksz="$(_xfs_get_dir_blocksize "$SCRATCH_MNT")"
 	leaf_lblk="$((32 * 1073741824 / blksz))"
 	node_lblk="$((64 * 1073741824 / blksz))"
 	umount "${SCRATCH_MNT}"
diff --git a/common/xfs b/common/xfs
index b2ac78de0c..9b6575b5f2 100644
--- a/common/xfs
+++ b/common/xfs
@@ -194,6 +194,15 @@ _xfs_get_file_block_size()
 	$XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g'
 }
 
+# Get the directory block size of a mounted filesystem.
+_xfs_get_dir_blocksize()
+{
+	local fs="$1"
+
+	$XFS_INFO_PROG "$fs" | grep 'naming.*bsize' | \
+		sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g'
+}
+
 # Set or clear the realtime status of every supplied path.  The first argument
 # is either 'data' or 'realtime'.  All other arguments should be paths to
 # existing directories or empty regular files.
diff --git a/tests/xfs/099 b/tests/xfs/099
index a7eaff6e0c..82bef8ad26 100755
--- a/tests/xfs/099
+++ b/tests/xfs/099
@@ -37,7 +37,7 @@ _scratch_mkfs_xfs > /dev/null
 
 echo "+ mount fs image"
 _scratch_mount
-dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
+dblksz=$(_xfs_get_dir_blocksize "$SCRATCH_MNT")
 nr="$((dblksz / 40))"
 blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
 leaf_lblk="$((32 * 1073741824 / blksz))"
diff --git a/tests/xfs/100 b/tests/xfs/100
index 79da8cb02c..e638b4ba17 100755
--- a/tests/xfs/100
+++ b/tests/xfs/100
@@ -37,7 +37,7 @@ _scratch_mkfs_xfs > /dev/null
 
 echo "+ mount fs image"
 _scratch_mount
-dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
+dblksz=$(_xfs_get_dir_blocksize "$SCRATCH_MNT")
 nr="$((dblksz / 12))"
 blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
 leaf_lblk="$((32 * 1073741824 / blksz))"
diff --git a/tests/xfs/101 b/tests/xfs/101
index 64f4705aca..11ed329110 100755
--- a/tests/xfs/101
+++ b/tests/xfs/101
@@ -37,7 +37,7 @@ _scratch_mkfs_xfs > /dev/null
 
 echo "+ mount fs image"
 _scratch_mount
-dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
+dblksz=$(_xfs_get_dir_blocksize "$SCRATCH_MNT")
 nr="$((dblksz / 12))"
 blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
 leaf_lblk="$((32 * 1073741824 / blksz))"
diff --git a/tests/xfs/102 b/tests/xfs/102
index 24dce43058..43f4539181 100755
--- a/tests/xfs/102
+++ b/tests/xfs/102
@@ -37,7 +37,7 @@ _scratch_mkfs_xfs > /dev/null
 
 echo "+ mount fs image"
 _scratch_mount
-dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
+dblksz=$(_xfs_get_dir_blocksize "$SCRATCH_MNT")
 nr="$((16 * dblksz / 40))"
 blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
 leaf_lblk="$((32 * 1073741824 / blksz))"
diff --git a/tests/xfs/105 b/tests/xfs/105
index 22a8bf9fb0..002a712883 100755
--- a/tests/xfs/105
+++ b/tests/xfs/105
@@ -37,7 +37,7 @@ _scratch_mkfs_xfs > /dev/null
 
 echo "+ mount fs image"
 _scratch_mount
-dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
+dblksz=$(_xfs_get_dir_blocksize "$SCRATCH_MNT")
 nr="$((16 * dblksz / 40))"
 blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
 leaf_lblk="$((32 * 1073741824 / blksz))"
diff --git a/tests/xfs/112 b/tests/xfs/112
index bc1ab62895..e2d5932da6 100755
--- a/tests/xfs/112
+++ b/tests/xfs/112
@@ -37,7 +37,7 @@ _scratch_mkfs_xfs > /dev/null
 
 echo "+ mount fs image"
 _scratch_mount
-dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
+dblksz=$(_xfs_get_dir_blocksize "$SCRATCH_MNT")
 nr="$((16 * dblksz / 40))"
 blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
 leaf_lblk="$((32 * 1073741824 / blksz))"
diff --git a/tests/xfs/113 b/tests/xfs/113
index e820ed96da..9bb2cd304b 100755
--- a/tests/xfs/113
+++ b/tests/xfs/113
@@ -37,7 +37,7 @@ _scratch_mkfs_xfs > /dev/null
 
 echo "+ mount fs image"
 _scratch_mount
-dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
+dblksz=$(_xfs_get_dir_blocksize "$SCRATCH_MNT")
 nr="$((128 * dblksz / 40))"
 blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
 leaf_lblk="$((32 * 1073741824 / blksz))"

[PATCH 3/4] xfs: refactor filesystem realtime geometry detection logic

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

There are a lot of places where we open-code detecting the realtime
extent size and extent count of a specific filesystem.  Refactor this
into a couple of helpers to clean up the code.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/populate |    2 +-
 common/xfs      |   31 +++++++++++++++++++++++++++++--
 tests/xfs/146   |    2 +-
 tests/xfs/147   |    2 +-
 tests/xfs/530   |    3 +--
 5 files changed, 33 insertions(+), 7 deletions(-)


diff --git a/common/populate b/common/populate
index 23b2fecf69..d9d4c6c300 100644
--- a/common/populate
+++ b/common/populate
@@ -323,7 +323,7 @@ _scratch_xfs_populate() {
 	fi
 
 	# Realtime Reverse-mapping btree
-	is_rt="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'rtextents=[1-9]')"
+	is_rt="$(_xfs_get_rtextents "$SCRATCH_MNT")"
 	if [ $is_rmapbt -gt 0 ] && [ $is_rt -gt 0 ]; then
 		echo "+ rtrmapbt btree"
 		nr="$((blksz * 2 / 32))"
diff --git a/common/xfs b/common/xfs
index 9b6575b5f2..a995e0b5da 100644
--- a/common/xfs
+++ b/common/xfs
@@ -174,6 +174,24 @@ _scratch_mkfs_xfs()
 	return $mkfs_status
 }
 
+# Get the number of realtime extents of a mounted filesystem.
+_xfs_get_rtextents()
+{
+	local path="$1"
+
+	$XFS_INFO_PROG "$path" | grep 'rtextents' | \
+		sed -e 's/^.*rtextents=\([0-9]*\).*$/\1/g'
+}
+
+# Get the realtime extent size of a mounted filesystem.
+_xfs_get_rtextsize()
+{
+	local path="$1"
+
+	$XFS_INFO_PROG "$path" | grep 'realtime.*extsz' | \
+		sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g'
+}
+
 # Get the size of an allocation unit of a file.  Normally this is just the
 # block size of the file, but for realtime files, this is the realtime extent
 # size.
@@ -191,7 +209,7 @@ _xfs_get_file_block_size()
 	while ! $XFS_INFO_PROG "$path" &>/dev/null && [ "$path" != "/" ]; do
 		path="$(dirname "$path")"
 	done
-	$XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g'
+	_xfs_get_rtextsize "$path"
 }
 
 # Get the directory block size of a mounted filesystem.
@@ -427,13 +445,22 @@ _require_xfs_crc()
 # third option is -v, echo 1 for success and 0 for not.
 #
 # Starting with xfsprogs 4.17, this also works for unmounted filesystems.
+# The feature 'realtime' looks for rtextents > 0.
 _xfs_has_feature()
 {
 	local fs="$1"
 	local feat="$2"
 	local verbose="$3"
+	local feat_regex="1"
 
-	local answer="$($XFS_INFO_PROG "$fs" 2>&1 | grep -w -c "$feat=1")"
+	case "$feat" in
+	"realtime")
+		feat="rtextents"
+		feat_regex="[1-9][0-9]*"
+		;;
+	esac
+
+	local answer="$($XFS_INFO_PROG "$fs" 2>&1 | grep -E -w -c "$feat=$feat_regex")"
 	if [ "$answer" -ne 0 ]; then
 		test "$verbose" = "-v" && echo 1
 		return 0
diff --git a/tests/xfs/146 b/tests/xfs/146
index 5516d396bf..123bdff59f 100755
--- a/tests/xfs/146
+++ b/tests/xfs/146
@@ -31,7 +31,7 @@ _scratch_mkfs > $seqres.full
 _scratch_mount >> $seqres.full
 
 blksz=$(_get_block_size $SCRATCH_MNT)
-rextsize=$($XFS_INFO_PROG $SCRATCH_MNT | grep realtime.*extsz | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g')
+rextsize=$(_xfs_get_rtextsize "$SCRATCH_MNT")
 rextblks=$((rextsize / blksz))
 
 echo "blksz $blksz rextsize $rextsize rextblks $rextblks" >> $seqres.full
diff --git a/tests/xfs/147 b/tests/xfs/147
index e21fdd330c..33b3c99633 100755
--- a/tests/xfs/147
+++ b/tests/xfs/147
@@ -29,7 +29,7 @@ _scratch_mkfs -r extsize=256k > $seqres.full
 _scratch_mount >> $seqres.full
 
 blksz=$(_get_block_size $SCRATCH_MNT)
-rextsize=$($XFS_INFO_PROG $SCRATCH_MNT | grep realtime.*extsz | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g')
+rextsize=$(_xfs_get_rtextsize "$SCRATCH_MNT")
 rextblks=$((rextsize / blksz))
 
 echo "blksz $blksz rextsize $rextsize rextblks $rextblks" >> $seqres.full
diff --git a/tests/xfs/530 b/tests/xfs/530
index c960738db7..56f5e7ebdb 100755
--- a/tests/xfs/530
+++ b/tests/xfs/530
@@ -73,8 +73,7 @@ _try_scratch_mount || _notrun "Couldn't mount fs with synthetic rt volume"
 formatted_blksz="$(_get_block_size $SCRATCH_MNT)"
 test "$formatted_blksz" -ne "$dbsize" && \
 	_notrun "Tried to format with $dbsize blocksize, got $formatted_blksz."
-$XFS_INFO_PROG $SCRATCH_MNT | grep -E -q 'realtime.*blocks=0' && \
-	_notrun "Filesystem should have a realtime volume"
+_require_xfs_has_feature "$SCRATCH_MNT" realtime
 
 echo "Consume free space"
 fillerdir=$SCRATCH_MNT/fillerdir

[PATCH 4/4] common: simplify grep pipe sed interactions

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Zorro pointed out that the idiom "program | grep | sed" isn't necessary
for field extraction -- sed is perfectly capable of performing a
substitution and only printing the lines that match that substitution.
Do that for the common helpers.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/ext4     |    9 +++++++++
 common/populate |    4 ++--
 common/xfs      |   11 ++++-------
 3 files changed, 15 insertions(+), 9 deletions(-)


diff --git a/common/ext4 b/common/ext4
index f4c3c4139a..4a2eaa157f 100644
--- a/common/ext4
+++ b/common/ext4
@@ -191,3 +191,12 @@ _scratch_ext4_options()
 	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
 		SCRATCH_OPTIONS="$SCRATCH_OPTIONS ${log_opt}"
 }
+
+# Get the inode flags for a particular inode number
+_ext4_get_inum_iflags() {
+	local dev="$1"
+	local inumber="$2"
+
+	debugfs -R "stat <${inumber}>" "${dev}" 2> /dev/null | \
+			sed -n 's/^.*Flags: \([0-9a-fx]*\).*$/\1/p'
+}
diff --git a/common/populate b/common/populate
index d9d4c6c300..6e00499734 100644
--- a/common/populate
+++ b/common/populate
@@ -641,7 +641,7 @@ __populate_check_ext4_dformat() {
 	extents=0
 	etree=0
 	debugfs -R "stat <${inode}>" "${dev}" 2> /dev/null | grep 'ETB[0-9]' -q && etree=1
-	iflags="$(debugfs -R "stat <${inode}>" "${dev}" 2> /dev/null | grep 'Flags:' | sed -e 's/^.*Flags: \([0-9a-fx]*\).*$/\1/g')"
+	iflags="$(_ext4_get_inum_iflags "${dev}" "${inode}")"
 	test "$(echo "${iflags}" | awk '{print and(strtonum($1), 0x80000);}')" -gt 0 && extents=1
 
 	case "${format}" in
@@ -688,7 +688,7 @@ __populate_check_ext4_dir() {
 
 	htree=0
 	inline=0
-	iflags="$(debugfs -R "stat <${inode}>" "${dev}" 2> /dev/null | grep 'Flags:' | sed -e 's/^.*Flags: \([0-9a-fx]*\).*$/\1/g')"
+	iflags="$(_ext4_get_inum_iflags "${dev}" "${inode}")"
 	test "$(echo "${iflags}" | awk '{print and(strtonum($1), 0x1000);}')" -gt 0 && htree=1
 	test "$(echo "${iflags}" | awk '{print and(strtonum($1), 0x10000000);}')" -gt 0 && inline=1
 
diff --git a/common/xfs b/common/xfs
index a995e0b5da..4f2cd46c91 100644
--- a/common/xfs
+++ b/common/xfs
@@ -179,8 +179,7 @@ _xfs_get_rtextents()
 {
 	local path="$1"
 
-	$XFS_INFO_PROG "$path" | grep 'rtextents' | \
-		sed -e 's/^.*rtextents=\([0-9]*\).*$/\1/g'
+	$XFS_INFO_PROG "$path" | sed -n "s/^.*rtextents=\([[:digit:]]*\).*/\1/p"
 }
 
 # Get the realtime extent size of a mounted filesystem.
@@ -188,8 +187,7 @@ _xfs_get_rtextsize()
 {
 	local path="$1"
 
-	$XFS_INFO_PROG "$path" | grep 'realtime.*extsz' | \
-		sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g'
+	$XFS_INFO_PROG "$path" | sed -n "s/^.*realtime.*extsz=\([[:digit:]]*\).*/\1/p"
 }
 
 # Get the size of an allocation unit of a file.  Normally this is just the
@@ -217,8 +215,7 @@ _xfs_get_dir_blocksize()
 {
 	local fs="$1"
 
-	$XFS_INFO_PROG "$fs" | grep 'naming.*bsize' | \
-		sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g'
+	$XFS_INFO_PROG "$fs" | sed -n "s/^naming.*bsize=\([[:digit:]]*\).*/\1/p"
 }
 
 # Set or clear the realtime status of every supplied path.  The first argument
@@ -1267,7 +1264,7 @@ _force_xfsv4_mount_options()
 # Find AG count of mounted filesystem
 _xfs_mount_agcount()
 {
-	$XFS_INFO_PROG "$1" | grep agcount= | sed -e 's/^.*agcount=\([0-9]*\),.*$/\1/g'
+	$XFS_INFO_PROG "$1" | sed -n "s/^.*agcount=\([[:digit:]]*\).*/\1/p"
 }
 
 # Wipe the superblock of each XFS AGs

Re: [PATCH 1/4] xfs: refactor filesystem feature detection logic

[Zorro Lang]

On Fri, Oct 28, 2022 at 10:41:53AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> There are a lot of places where we open-code detecting features of a
> specific filesystem.  Refactor this into a couple of helpers in
> preparation for adding stress tests for online repair and fuzzing.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/populate |   14 ++++++-----
>  common/rc       |    2 +-
>  common/xfs      |   67 ++++++++++++++++++++++++++++++++++++++++++++++---------
>  tests/xfs/097   |    2 +-
>  tests/xfs/151   |    3 +-
>  tests/xfs/271   |    2 +-
>  tests/xfs/307   |    2 +-
>  tests/xfs/308   |    2 +-
>  tests/xfs/348   |    2 +-
>  9 files changed, 70 insertions(+), 26 deletions(-)
> 
> 
> diff --git a/common/populate b/common/populate
> index 58b07e33be..9fa1a06798 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -131,7 +131,7 @@ _populate_xfs_qmount_option()
>  	fi
>  
>  	# Turn on all the quotas
> -	if $XFS_INFO_PROG "${TEST_DIR}" | grep -q 'crc=1'; then
> +	if _xfs_has_feature "$TEST_DIR" crc; then
>  		# v5 filesystems can have group & project quotas
>  		quota="usrquota,grpquota,prjquota"
>  	else
> @@ -176,7 +176,7 @@ _scratch_xfs_populate() {
>  
>  	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
>  	dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> -	crc="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep crc= | sed -e 's/^.*crc=//g' -e 's/\([0-9]*\).*$/\1/g')"
> +	crc="$(_xfs_has_feature "$SCRATCH_MNT" crc -v)"
>  	if [ $crc -eq 1 ]; then
>  		leaf_hdr_size=64
>  	else
> @@ -315,7 +315,7 @@ _scratch_xfs_populate() {
>  	done
>  
>  	# Reverse-mapping btree
> -	is_rmapbt="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'rmapbt=1')"
> +	is_rmapbt="$(_xfs_has_feature "$SCRATCH_MNT" rmapbt -v)"
>  	if [ $is_rmapbt -gt 0 ]; then
>  		echo "+ rmapbt btree"
>  		nr="$((blksz * 2 / 24))"
> @@ -332,7 +332,7 @@ _scratch_xfs_populate() {
>  	fi
>  
>  	# Reference-count btree
> -	is_reflink="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'reflink=1')"
> +	is_reflink="$(_xfs_has_feature "$SCRATCH_MNT" reflink -v)"
>  	if [ $is_reflink -gt 0 ]; then
>  		echo "+ reflink btree"
>  		nr="$((blksz * 2 / 12))"
> @@ -597,9 +597,9 @@ _scratch_xfs_populate_check() {
>  	leaf_attr="$(__populate_find_inode "${SCRATCH_MNT}/ATTR.FMT_LEAF")"
>  	node_attr="$(__populate_find_inode "${SCRATCH_MNT}/ATTR.FMT_NODE")"
>  	btree_attr="$(__populate_find_inode "${SCRATCH_MNT}/ATTR.FMT_BTREE")"
> -	is_finobt=$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'finobt=1')
> -	is_rmapbt=$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'rmapbt=1')
> -	is_reflink=$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'reflink=1')
> +	is_finobt=$(_xfs_has_feature "$SCRATCH_MNT" finobt -v)
> +	is_rmapbt=$(_xfs_has_feature "$SCRATCH_MNT" rmapbt -v)
> +	is_reflink=$(_xfs_has_feature "$SCRATCH_MNT" reflink -v)
>  
>  	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
>  	dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> diff --git a/common/rc b/common/rc
> index f4785c17ca..8060c03b7d 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -247,7 +247,7 @@ _supports_filetype()
>  	local fstyp=`$DF_PROG $dir | tail -1 | $AWK_PROG '{print $2}'`
>  	case "$fstyp" in
>  	xfs)
> -		$XFS_INFO_PROG $dir | grep -q "ftype=1"
> +		_xfs_has_feature $dir ftype
>  		;;
>  	ext2|ext3|ext4)
>  		local dev=`$DF_PROG $dir | tail -1 | $AWK_PROG '{print $1}'`
> diff --git a/common/xfs b/common/xfs
> index e1c15d3d04..b2ac78de0c 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -413,6 +413,56 @@ _require_xfs_crc()
>  	_scratch_unmount
>  }
>  
> +# If the xfs_info output for the given XFS filesystem mount mentions the given
> +# feature.  If so, return 0 for success.  If not, return 1 for failure.  If the
> +# third option is -v, echo 1 for success and 0 for not.
> +#
> +# Starting with xfsprogs 4.17, this also works for unmounted filesystems.
> +_xfs_has_feature()
> +{
> +	local fs="$1"
> +	local feat="$2"
> +	local verbose="$3"
> +
> +	local answer="$($XFS_INFO_PROG "$fs" 2>&1 | grep -w -c "$feat=1")"
> +	if [ "$answer" -ne 0 ]; then
> +		test "$verbose" = "-v" && echo 1
> +		return 0
> +	fi
> +
> +	test "$verbose" = "-v" && echo 0
> +	return 1
> +}
> +
> +# Require that the xfs_info output for the given XFS filesystem mount mentions
> +# the given feature flag.  If the third argument is -u (or is empty and the
> +# second argument is $SCRATCH_MNT), unmount the fs on failure.  If a fourth
> +# argument is supplied, it will be used as the _notrun message.
> +_require_xfs_has_feature()
> +{
> +	local fs="$1"
> +	local feat="$2"
> +	local umount="$3"
> +	local message="$4"
> +
> +	if [ -z "$umount" ] && [ "$fs" = "$SCRATCH_MNT" ]; then
> +		umount="-u"
> +	fi
> +
> +	_xfs_has_feature "$1" "$2" && return 0
> +
> +	test "$umount" = "-u" && umount "$fs" &>/dev/null
> +
> +	test -n "$message" && _notrun "$message"
> +
> +	case "$fs" in
> +	"$TEST_DIR"|"$TEST_DEV")	fsname="test";;
> +	"$SCRATCH_MNT"|"$SCRATCH_DEV")	fsname="scratch";;
> +	*)				fsname="$fs";;
> +	esac
> +	_notrun "$2 not supported by $fsname filesystem type: $FSTYP"
> +}
> +
>  # this test requires the xfs kernel support crc feature on scratch device
>  #
>  _require_scratch_xfs_crc()
> @@ -420,7 +470,8 @@ _require_scratch_xfs_crc()
>  	_scratch_mkfs_xfs >/dev/null 2>&1
>  	_try_scratch_mount >/dev/null 2>&1 \
>  	   || _notrun "Kernel doesn't support crc feature"
> -	$XFS_INFO_PROG $SCRATCH_MNT | grep -q 'crc=1' || _notrun "crc feature not supported by this filesystem"
> +	_require_xfs_has_feature $SCRATCH_MNT crc -u \
> +		"crc feature not supported by this filesystem"
>  	_scratch_unmount
>  }
>  
> @@ -739,10 +790,7 @@ _check_xfs_test_fs()
>  _require_xfs_test_rmapbt()
>  {
>  	_require_test
> -
> -	if [ "$($XFS_INFO_PROG "$TEST_DIR" | grep -c "rmapbt=1")" -ne 1 ]; then
> -		_notrun "rmapbt not supported by test filesystem type: $FSTYP"
> -	fi
> +	_require_xfs_has_feature "$TEST_DIR" rmapbt
>  }
>  
>  _require_xfs_scratch_rmapbt()
> @@ -751,10 +799,7 @@ _require_xfs_scratch_rmapbt()
>  
>  	_scratch_mkfs > /dev/null
>  	_scratch_mount
> -	if [ "$($XFS_INFO_PROG "$SCRATCH_MNT" | grep -c "rmapbt=1")" -ne 1 ]; then
> -		_scratch_unmount
> -		_notrun "rmapbt not supported by scratch filesystem type: $FSTYP"
> -	fi
> +	_require_xfs_has_feature "$SCRATCH_MNT" rmapbt
>  	_scratch_unmount
>  }
>  
> @@ -1357,8 +1402,8 @@ _require_scratch_xfs_bigtime()
>  		_notrun "mkfs.xfs doesn't support bigtime feature"
>  	_try_scratch_mount || \
>  		_notrun "kernel doesn't support xfs bigtime feature"
> -	$XFS_INFO_PROG "$SCRATCH_MNT" | grep -q -w "bigtime=1" || \
> -		_notrun "bigtime feature not advertised on mount?"
> +	_require_xfs_has_feature $SCRATCH_MNT bigtime -u \
> +		"crc feature not supported by this filesystem"
>  	_scratch_unmount
>  }
>  
> diff --git a/tests/xfs/097 b/tests/xfs/097
> index 4cad7216cd..1df34eeddc 100755
> --- a/tests/xfs/097
> +++ b/tests/xfs/097
> @@ -42,7 +42,7 @@ _scratch_mkfs_xfs -m crc=1,finobt=1 > /dev/null
>  
>  echo "+ mount fs image"
>  _scratch_mount
> -$XFS_INFO_PROG "${SCRATCH_MNT}" | grep -q "finobt=1" || _notrun "finobt not enabled"
> +_require_xfs_has_feature "$SCRATCH_MNT" finobt
>  blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
>  
>  echo "+ make some files"
> diff --git a/tests/xfs/151 b/tests/xfs/151
> index 66425f6710..b2fe16aefb 100755
> --- a/tests/xfs/151
> +++ b/tests/xfs/151
> @@ -24,8 +24,7 @@ echo "Format filesystem and populate"
>  _scratch_mkfs > $seqres.full
>  _scratch_mount >> $seqres.full
>  
> -$XFS_INFO_PROG $SCRATCH_MNT | grep -q ftype=1 || \
> -	_notrun "filesystem does not support ftype"
> +_require_xfs_has_feature "$SCRATCH_MNT" ftype
>  
>  filter_ls() {
>  	awk '
> diff --git a/tests/xfs/271 b/tests/xfs/271
> index 14d64cd0e5..d67ac4d6c1 100755
> --- a/tests/xfs/271
> +++ b/tests/xfs/271
> @@ -37,7 +37,7 @@ agcount=$(_xfs_mount_agcount $SCRATCH_MNT)
>  # same owner (per-AG metadata) for rmap btree blocks and blocks on the AGFL and
>  # the reverse mapping index merges records, the number of per-AG extents
>  # reported will vary depending on whether the refcount btree is enabled.
> -$XFS_INFO_PROG $SCRATCH_MNT | grep -q reflink=1
> +_require_xfs_has_feature "$SCRATCH_MNT" reflink
>  has_reflink=$(( 1 - $? ))
>  perag_metadata_exts=2
>  test $has_reflink -gt 0 && perag_metadata_exts=$((perag_metadata_exts + 1))
> diff --git a/tests/xfs/307 b/tests/xfs/307
> index ba7204dd00..f3c970fadf 100755
> --- a/tests/xfs/307
> +++ b/tests/xfs/307
> @@ -22,7 +22,7 @@ _require_scratch_reflink
>  echo "Format"
>  _scratch_mkfs > $seqres.full 2>&1
>  _scratch_mount >> $seqres.full
> -is_rmap=$($XFS_INFO_PROG $SCRATCH_MNT | grep -c "rmapbt=1")
> +is_rmap=$(_xfs_has_feature $SCRATCH_MNT rmapbt -v)
>  _scratch_unmount
>  
>  _get_agf_data() {
> diff --git a/tests/xfs/308 b/tests/xfs/308
> index d0f47f5038..6da6622e14 100755
> --- a/tests/xfs/308
> +++ b/tests/xfs/308
> @@ -22,7 +22,7 @@ _require_scratch_reflink
>  echo "Format"
>  _scratch_mkfs > $seqres.full 2>&1
>  _scratch_mount >> $seqres.full
> -is_rmap=$($XFS_INFO_PROG $SCRATCH_MNT | grep -c "rmapbt=1")
> +is_rmap=$(_xfs_has_feature $SCRATCH_MNT rmapbt -v)
>  _scratch_xfs_unmount_dirty
>  
>  _get_agf_data() {
> diff --git a/tests/xfs/348 b/tests/xfs/348
> index faf2dca50b..d1645d9462 100755
> --- a/tests/xfs/348
> +++ b/tests/xfs/348
> @@ -39,7 +39,7 @@ mknod $testdir/CHRDEV c 1 1
>  mknod $testdir/BLKDEV b 1 1
>  mknod $testdir/FIFO p
>  
> -$XFS_INFO_PROG $SCRATCH_MNT | grep -q "ftype=1" && FTYPE_FEATURE=1
> +_xfs_has_feature $SCRATCH_MNT ftype && FTYPE_FEATURE=1
>  
>  # Record test dir inode for xfs_repair filter
>  inode_filter=$tmp.sed
> 

Re: [PATCH 2/4] xfs: refactor filesystem directory block size extraction logic

[Zorro Lang]

On Fri, Oct 28, 2022 at 10:41:59AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> There are a lot of places where we open-code determining the directory
> block size for a specific filesystem.  Refactor this into a single
> helper to clean up existing tests.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/populate |    4 ++--
>  common/xfs      |    9 +++++++++
>  tests/xfs/099   |    2 +-
>  tests/xfs/100   |    2 +-
>  tests/xfs/101   |    2 +-
>  tests/xfs/102   |    2 +-
>  tests/xfs/105   |    2 +-
>  tests/xfs/112   |    2 +-
>  tests/xfs/113   |    2 +-
>  9 files changed, 18 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/common/populate b/common/populate
> index 9fa1a06798..23b2fecf69 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -175,7 +175,7 @@ _scratch_xfs_populate() {
>  	_xfs_force_bdev data $SCRATCH_MNT
>  
>  	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
> -	dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> +	dblksz="$(_xfs_get_dir_blocksize "$SCRATCH_MNT")"
>  	crc="$(_xfs_has_feature "$SCRATCH_MNT" crc -v)"
>  	if [ $crc -eq 1 ]; then
>  		leaf_hdr_size=64
> @@ -602,7 +602,7 @@ _scratch_xfs_populate_check() {
>  	is_reflink=$(_xfs_has_feature "$SCRATCH_MNT" reflink -v)
>  
>  	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
> -	dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> +	dblksz="$(_xfs_get_dir_blocksize "$SCRATCH_MNT")"
>  	leaf_lblk="$((32 * 1073741824 / blksz))"
>  	node_lblk="$((64 * 1073741824 / blksz))"
>  	umount "${SCRATCH_MNT}"
> diff --git a/common/xfs b/common/xfs
> index b2ac78de0c..9b6575b5f2 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -194,6 +194,15 @@ _xfs_get_file_block_size()
>  	$XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g'
>  }
>  
> +# Get the directory block size of a mounted filesystem.
> +_xfs_get_dir_blocksize()
> +{
> +	local fs="$1"
> +
> +	$XFS_INFO_PROG "$fs" | grep 'naming.*bsize' | \
> +		sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g'
> +}
> +
>  # Set or clear the realtime status of every supplied path.  The first argument
>  # is either 'data' or 'realtime'.  All other arguments should be paths to
>  # existing directories or empty regular files.
> diff --git a/tests/xfs/099 b/tests/xfs/099
> index a7eaff6e0c..82bef8ad26 100755
> --- a/tests/xfs/099
> +++ b/tests/xfs/099
> @@ -37,7 +37,7 @@ _scratch_mkfs_xfs > /dev/null
>  
>  echo "+ mount fs image"
>  _scratch_mount
> -dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> +dblksz=$(_xfs_get_dir_blocksize "$SCRATCH_MNT")
>  nr="$((dblksz / 40))"
>  blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
>  leaf_lblk="$((32 * 1073741824 / blksz))"
> diff --git a/tests/xfs/100 b/tests/xfs/100
> index 79da8cb02c..e638b4ba17 100755
> --- a/tests/xfs/100
> +++ b/tests/xfs/100
> @@ -37,7 +37,7 @@ _scratch_mkfs_xfs > /dev/null
>  
>  echo "+ mount fs image"
>  _scratch_mount
> -dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> +dblksz=$(_xfs_get_dir_blocksize "$SCRATCH_MNT")
>  nr="$((dblksz / 12))"
>  blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
>  leaf_lblk="$((32 * 1073741824 / blksz))"
> diff --git a/tests/xfs/101 b/tests/xfs/101
> index 64f4705aca..11ed329110 100755
> --- a/tests/xfs/101
> +++ b/tests/xfs/101
> @@ -37,7 +37,7 @@ _scratch_mkfs_xfs > /dev/null
>  
>  echo "+ mount fs image"
>  _scratch_mount
> -dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> +dblksz=$(_xfs_get_dir_blocksize "$SCRATCH_MNT")
>  nr="$((dblksz / 12))"
>  blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
>  leaf_lblk="$((32 * 1073741824 / blksz))"
> diff --git a/tests/xfs/102 b/tests/xfs/102
> index 24dce43058..43f4539181 100755
> --- a/tests/xfs/102
> +++ b/tests/xfs/102
> @@ -37,7 +37,7 @@ _scratch_mkfs_xfs > /dev/null
>  
>  echo "+ mount fs image"
>  _scratch_mount
> -dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> +dblksz=$(_xfs_get_dir_blocksize "$SCRATCH_MNT")
>  nr="$((16 * dblksz / 40))"
>  blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
>  leaf_lblk="$((32 * 1073741824 / blksz))"
> diff --git a/tests/xfs/105 b/tests/xfs/105
> index 22a8bf9fb0..002a712883 100755
> --- a/tests/xfs/105
> +++ b/tests/xfs/105
> @@ -37,7 +37,7 @@ _scratch_mkfs_xfs > /dev/null
>  
>  echo "+ mount fs image"
>  _scratch_mount
> -dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> +dblksz=$(_xfs_get_dir_blocksize "$SCRATCH_MNT")
>  nr="$((16 * dblksz / 40))"
>  blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
>  leaf_lblk="$((32 * 1073741824 / blksz))"
> diff --git a/tests/xfs/112 b/tests/xfs/112
> index bc1ab62895..e2d5932da6 100755
> --- a/tests/xfs/112
> +++ b/tests/xfs/112
> @@ -37,7 +37,7 @@ _scratch_mkfs_xfs > /dev/null
>  
>  echo "+ mount fs image"
>  _scratch_mount
> -dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> +dblksz=$(_xfs_get_dir_blocksize "$SCRATCH_MNT")
>  nr="$((16 * dblksz / 40))"
>  blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
>  leaf_lblk="$((32 * 1073741824 / blksz))"
> diff --git a/tests/xfs/113 b/tests/xfs/113
> index e820ed96da..9bb2cd304b 100755
> --- a/tests/xfs/113
> +++ b/tests/xfs/113
> @@ -37,7 +37,7 @@ _scratch_mkfs_xfs > /dev/null
>  
>  echo "+ mount fs image"
>  _scratch_mount
> -dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> +dblksz=$(_xfs_get_dir_blocksize "$SCRATCH_MNT")
>  nr="$((128 * dblksz / 40))"
>  blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
>  leaf_lblk="$((32 * 1073741824 / blksz))"
> 

Re: [PATCH 3/4] xfs: refactor filesystem realtime geometry detection logic

[Zorro Lang]

On Fri, Oct 28, 2022 at 10:42:05AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> There are a lot of places where we open-code detecting the realtime
> extent size and extent count of a specific filesystem.  Refactor this
> into a couple of helpers to clean up the code.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/populate |    2 +-
>  common/xfs      |   31 +++++++++++++++++++++++++++++--
>  tests/xfs/146   |    2 +-
>  tests/xfs/147   |    2 +-
>  tests/xfs/530   |    3 +--
>  5 files changed, 33 insertions(+), 7 deletions(-)
> 
> 
> diff --git a/common/populate b/common/populate
> index 23b2fecf69..d9d4c6c300 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -323,7 +323,7 @@ _scratch_xfs_populate() {
>  	fi
>  
>  	# Realtime Reverse-mapping btree
> -	is_rt="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'rtextents=[1-9]')"
> +	is_rt="$(_xfs_get_rtextents "$SCRATCH_MNT")"
>  	if [ $is_rmapbt -gt 0 ] && [ $is_rt -gt 0 ]; then
>  		echo "+ rtrmapbt btree"
>  		nr="$((blksz * 2 / 32))"
> diff --git a/common/xfs b/common/xfs
> index 9b6575b5f2..a995e0b5da 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -174,6 +174,24 @@ _scratch_mkfs_xfs()
>  	return $mkfs_status
>  }
>  
> +# Get the number of realtime extents of a mounted filesystem.
> +_xfs_get_rtextents()
> +{
> +	local path="$1"
> +
> +	$XFS_INFO_PROG "$path" | grep 'rtextents' | \
> +		sed -e 's/^.*rtextents=\([0-9]*\).*$/\1/g'
> +}
> +
> +# Get the realtime extent size of a mounted filesystem.
> +_xfs_get_rtextsize()
> +{
> +	local path="$1"
> +
> +	$XFS_INFO_PROG "$path" | grep 'realtime.*extsz' | \
> +		sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g'
> +}
> +
>  # Get the size of an allocation unit of a file.  Normally this is just the
>  # block size of the file, but for realtime files, this is the realtime extent
>  # size.
> @@ -191,7 +209,7 @@ _xfs_get_file_block_size()
>  	while ! $XFS_INFO_PROG "$path" &>/dev/null && [ "$path" != "/" ]; do
>  		path="$(dirname "$path")"
>  	done
> -	$XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g'
> +	_xfs_get_rtextsize "$path"
>  }
>  
>  # Get the directory block size of a mounted filesystem.
> @@ -427,13 +445,22 @@ _require_xfs_crc()
>  # third option is -v, echo 1 for success and 0 for not.
>  #
>  # Starting with xfsprogs 4.17, this also works for unmounted filesystems.
> +# The feature 'realtime' looks for rtextents > 0.
>  _xfs_has_feature()
>  {
>  	local fs="$1"
>  	local feat="$2"
>  	local verbose="$3"
> +	local feat_regex="1"
>  
> -	local answer="$($XFS_INFO_PROG "$fs" 2>&1 | grep -w -c "$feat=1")"
> +	case "$feat" in
> +	"realtime")
> +		feat="rtextents"
> +		feat_regex="[1-9][0-9]*"
> +		;;
> +	esac
> +
> +	local answer="$($XFS_INFO_PROG "$fs" 2>&1 | grep -E -w -c "$feat=$feat_regex")"
>  	if [ "$answer" -ne 0 ]; then
>  		test "$verbose" = "-v" && echo 1
>  		return 0
> diff --git a/tests/xfs/146 b/tests/xfs/146
> index 5516d396bf..123bdff59f 100755
> --- a/tests/xfs/146
> +++ b/tests/xfs/146
> @@ -31,7 +31,7 @@ _scratch_mkfs > $seqres.full
>  _scratch_mount >> $seqres.full
>  
>  blksz=$(_get_block_size $SCRATCH_MNT)
> -rextsize=$($XFS_INFO_PROG $SCRATCH_MNT | grep realtime.*extsz | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g')
> +rextsize=$(_xfs_get_rtextsize "$SCRATCH_MNT")
>  rextblks=$((rextsize / blksz))
>  
>  echo "blksz $blksz rextsize $rextsize rextblks $rextblks" >> $seqres.full
> diff --git a/tests/xfs/147 b/tests/xfs/147
> index e21fdd330c..33b3c99633 100755
> --- a/tests/xfs/147
> +++ b/tests/xfs/147
> @@ -29,7 +29,7 @@ _scratch_mkfs -r extsize=256k > $seqres.full
>  _scratch_mount >> $seqres.full
>  
>  blksz=$(_get_block_size $SCRATCH_MNT)
> -rextsize=$($XFS_INFO_PROG $SCRATCH_MNT | grep realtime.*extsz | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g')
> +rextsize=$(_xfs_get_rtextsize "$SCRATCH_MNT")
>  rextblks=$((rextsize / blksz))
>  
>  echo "blksz $blksz rextsize $rextsize rextblks $rextblks" >> $seqres.full
> diff --git a/tests/xfs/530 b/tests/xfs/530
> index c960738db7..56f5e7ebdb 100755
> --- a/tests/xfs/530
> +++ b/tests/xfs/530
> @@ -73,8 +73,7 @@ _try_scratch_mount || _notrun "Couldn't mount fs with synthetic rt volume"
>  formatted_blksz="$(_get_block_size $SCRATCH_MNT)"
>  test "$formatted_blksz" -ne "$dbsize" && \
>  	_notrun "Tried to format with $dbsize blocksize, got $formatted_blksz."
> -$XFS_INFO_PROG $SCRATCH_MNT | grep -E -q 'realtime.*blocks=0' && \
> -	_notrun "Filesystem should have a realtime volume"
> +_require_xfs_has_feature "$SCRATCH_MNT" realtime
>  
>  echo "Consume free space"
>  fillerdir=$SCRATCH_MNT/fillerdir
> 

Re: [PATCH 4/4] common: simplify grep pipe sed interactions

[Zorro Lang]

On Fri, Oct 28, 2022 at 10:42:10AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Zorro pointed out that the idiom "program | grep | sed" isn't necessary
> for field extraction -- sed is perfectly capable of performing a
> substitution and only printing the lines that match that substitution.
> Do that for the common helpers.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/ext4     |    9 +++++++++
>  common/populate |    4 ++--
>  common/xfs      |   11 ++++-------
>  3 files changed, 15 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/common/ext4 b/common/ext4
> index f4c3c4139a..4a2eaa157f 100644
> --- a/common/ext4
> +++ b/common/ext4
> @@ -191,3 +191,12 @@ _scratch_ext4_options()
>  	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
>  		SCRATCH_OPTIONS="$SCRATCH_OPTIONS ${log_opt}"
>  }
> +
> +# Get the inode flags for a particular inode number
> +_ext4_get_inum_iflags() {
> +	local dev="$1"
> +	local inumber="$2"
> +
> +	debugfs -R "stat <${inumber}>" "${dev}" 2> /dev/null | \
> +			sed -n 's/^.*Flags: \([0-9a-fx]*\).*$/\1/p'
> +}
> diff --git a/common/populate b/common/populate
> index d9d4c6c300..6e00499734 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -641,7 +641,7 @@ __populate_check_ext4_dformat() {
>  	extents=0
>  	etree=0
>  	debugfs -R "stat <${inode}>" "${dev}" 2> /dev/null | grep 'ETB[0-9]' -q && etree=1
> -	iflags="$(debugfs -R "stat <${inode}>" "${dev}" 2> /dev/null | grep 'Flags:' | sed -e 's/^.*Flags: \([0-9a-fx]*\).*$/\1/g')"
> +	iflags="$(_ext4_get_inum_iflags "${dev}" "${inode}")"
>  	test "$(echo "${iflags}" | awk '{print and(strtonum($1), 0x80000);}')" -gt 0 && extents=1
>  
>  	case "${format}" in
> @@ -688,7 +688,7 @@ __populate_check_ext4_dir() {
>  
>  	htree=0
>  	inline=0
> -	iflags="$(debugfs -R "stat <${inode}>" "${dev}" 2> /dev/null | grep 'Flags:' | sed -e 's/^.*Flags: \([0-9a-fx]*\).*$/\1/g')"
> +	iflags="$(_ext4_get_inum_iflags "${dev}" "${inode}")"
>  	test "$(echo "${iflags}" | awk '{print and(strtonum($1), 0x1000);}')" -gt 0 && htree=1
>  	test "$(echo "${iflags}" | awk '{print and(strtonum($1), 0x10000000);}')" -gt 0 && inline=1
>  
> diff --git a/common/xfs b/common/xfs
> index a995e0b5da..4f2cd46c91 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -179,8 +179,7 @@ _xfs_get_rtextents()
>  {
>  	local path="$1"
>  
> -	$XFS_INFO_PROG "$path" | grep 'rtextents' | \
> -		sed -e 's/^.*rtextents=\([0-9]*\).*$/\1/g'
> +	$XFS_INFO_PROG "$path" | sed -n "s/^.*rtextents=\([[:digit:]]*\).*/\1/p"
>  }
>  
>  # Get the realtime extent size of a mounted filesystem.
> @@ -188,8 +187,7 @@ _xfs_get_rtextsize()
>  {
>  	local path="$1"
>  
> -	$XFS_INFO_PROG "$path" | grep 'realtime.*extsz' | \
> -		sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g'
> +	$XFS_INFO_PROG "$path" | sed -n "s/^.*realtime.*extsz=\([[:digit:]]*\).*/\1/p"
>  }
>  
>  # Get the size of an allocation unit of a file.  Normally this is just the
> @@ -217,8 +215,7 @@ _xfs_get_dir_blocksize()
>  {
>  	local fs="$1"
>  
> -	$XFS_INFO_PROG "$fs" | grep 'naming.*bsize' | \
> -		sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g'
> +	$XFS_INFO_PROG "$fs" | sed -n "s/^naming.*bsize=\([[:digit:]]*\).*/\1/p"
>  }
>  
>  # Set or clear the realtime status of every supplied path.  The first argument
> @@ -1267,7 +1264,7 @@ _force_xfsv4_mount_options()
>  # Find AG count of mounted filesystem
>  _xfs_mount_agcount()
>  {
> -	$XFS_INFO_PROG "$1" | grep agcount= | sed -e 's/^.*agcount=\([0-9]*\),.*$/\1/g'
> +	$XFS_INFO_PROG "$1" | sed -n "s/^.*agcount=\([[:digit:]]*\).*/\1/p"
>  }
>  
>  # Wipe the superblock of each XFS AGs
> 

[PATCH 1/4] xfs: refactor filesystem feature detection logic

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

There are a lot of places where we open-code detecting features of a
specific filesystem.  Refactor this into a couple of helpers in
preparation for adding stress tests for online repair and fuzzing.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/populate |   14 ++++++-----
 common/rc       |    2 +-
 common/xfs      |   67 ++++++++++++++++++++++++++++++++++++++++++++++---------
 tests/xfs/097   |    2 +-
 tests/xfs/151   |    3 +-
 tests/xfs/271   |    2 +-
 tests/xfs/307   |    2 +-
 tests/xfs/308   |    2 +-
 tests/xfs/348   |    2 +-
 9 files changed, 70 insertions(+), 26 deletions(-)


diff --git a/common/populate b/common/populate
index 58b07e33be..9fa1a06798 100644
--- a/common/populate
+++ b/common/populate
@@ -131,7 +131,7 @@ _populate_xfs_qmount_option()
 	fi
 
 	# Turn on all the quotas
-	if $XFS_INFO_PROG "${TEST_DIR}" | grep -q 'crc=1'; then
+	if _xfs_has_feature "$TEST_DIR" crc; then
 		# v5 filesystems can have group & project quotas
 		quota="usrquota,grpquota,prjquota"
 	else
@@ -176,7 +176,7 @@ _scratch_xfs_populate() {
 
 	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
 	dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
-	crc="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep crc= | sed -e 's/^.*crc=//g' -e 's/\([0-9]*\).*$/\1/g')"
+	crc="$(_xfs_has_feature "$SCRATCH_MNT" crc -v)"
 	if [ $crc -eq 1 ]; then
 		leaf_hdr_size=64
 	else
@@ -315,7 +315,7 @@ _scratch_xfs_populate() {
 	done
 
 	# Reverse-mapping btree
-	is_rmapbt="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'rmapbt=1')"
+	is_rmapbt="$(_xfs_has_feature "$SCRATCH_MNT" rmapbt -v)"
 	if [ $is_rmapbt -gt 0 ]; then
 		echo "+ rmapbt btree"
 		nr="$((blksz * 2 / 24))"
@@ -332,7 +332,7 @@ _scratch_xfs_populate() {
 	fi
 
 	# Reference-count btree
-	is_reflink="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'reflink=1')"
+	is_reflink="$(_xfs_has_feature "$SCRATCH_MNT" reflink -v)"
 	if [ $is_reflink -gt 0 ]; then
 		echo "+ reflink btree"
 		nr="$((blksz * 2 / 12))"
@@ -597,9 +597,9 @@ _scratch_xfs_populate_check() {
 	leaf_attr="$(__populate_find_inode "${SCRATCH_MNT}/ATTR.FMT_LEAF")"
 	node_attr="$(__populate_find_inode "${SCRATCH_MNT}/ATTR.FMT_NODE")"
 	btree_attr="$(__populate_find_inode "${SCRATCH_MNT}/ATTR.FMT_BTREE")"
-	is_finobt=$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'finobt=1')
-	is_rmapbt=$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'rmapbt=1')
-	is_reflink=$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep -c 'reflink=1')
+	is_finobt=$(_xfs_has_feature "$SCRATCH_MNT" finobt -v)
+	is_rmapbt=$(_xfs_has_feature "$SCRATCH_MNT" rmapbt -v)
+	is_reflink=$(_xfs_has_feature "$SCRATCH_MNT" reflink -v)
 
 	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
 	dblksz="$($XFS_INFO_PROG "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
diff --git a/common/rc b/common/rc
index f4785c17ca..8060c03b7d 100644
--- a/common/rc
+++ b/common/rc
@@ -247,7 +247,7 @@ _supports_filetype()
 	local fstyp=`$DF_PROG $dir | tail -1 | $AWK_PROG '{print $2}'`
 	case "$fstyp" in
 	xfs)
-		$XFS_INFO_PROG $dir | grep -q "ftype=1"
+		_xfs_has_feature $dir ftype
 		;;
 	ext2|ext3|ext4)
 		local dev=`$DF_PROG $dir | tail -1 | $AWK_PROG '{print $1}'`
diff --git a/common/xfs b/common/xfs
index 2cd8254937..c7496bce3f 100644
--- a/common/xfs
+++ b/common/xfs
@@ -422,6 +422,56 @@ _require_xfs_crc()
 	_scratch_unmount
 }
 
+# If the xfs_info output for the given XFS filesystem mount mentions the given
+# feature.  If so, return 0 for success.  If not, return 1 for failure.  If the
+# third option is -v, echo 1 for success and 0 for not.
+#
+# Starting with xfsprogs 4.17, this also works for unmounted filesystems.
+_xfs_has_feature()
+{
+	local fs="$1"
+	local feat="$2"
+	local verbose="$3"
+
+	local answer="$($XFS_INFO_PROG "$fs" 2>&1 | grep -w -c "$feat=1")"
+	if [ "$answer" -ne 0 ]; then
+		test "$verbose" = "-v" && echo 1
+		return 0
+	fi
+
+	test "$verbose" = "-v" && echo 0
+	return 1
+}
+
+# Require that the xfs_info output for the given XFS filesystem mount mentions
+# the given feature flag.  If the third argument is -u (or is empty and the
+# second argument is $SCRATCH_MNT), unmount the fs on failure.  If a fourth
+# argument is supplied, it will be used as the _notrun message.
+_require_xfs_has_feature()
+{
+	local fs="$1"
+	local feat="$2"
+	local umount="$3"
+	local message="$4"
+
+	if [ -z "$umount" ] && [ "$fs" = "$SCRATCH_MNT" ]; then
+		umount="-u"
+	fi
+
+	_xfs_has_feature "$1" "$2" && return 0
+
+	test "$umount" = "-u" && umount "$fs" &>/dev/null
+
+	test -n "$message" && _notrun "$message"
+
+	case "$fs" in
+	"$TEST_DIR"|"$TEST_DEV")	fsname="test";;
+	"$SCRATCH_MNT"|"$SCRATCH_DEV")	fsname="scratch";;
+	*)				fsname="$fs";;
+	esac
+	_notrun "$2 not supported by $fsname filesystem type: $FSTYP"
+}
+
 # this test requires the xfs kernel support crc feature on scratch device
 #
 _require_scratch_xfs_crc()
@@ -429,7 +479,8 @@ _require_scratch_xfs_crc()
 	_scratch_mkfs_xfs >/dev/null 2>&1
 	_try_scratch_mount >/dev/null 2>&1 \
 	   || _notrun "Kernel doesn't support crc feature"
-	$XFS_INFO_PROG $SCRATCH_MNT | grep -q 'crc=1' || _notrun "crc feature not supported by this filesystem"
+	_require_xfs_has_feature $SCRATCH_MNT crc -u \
+		"crc feature not supported by this filesystem"
 	_scratch_unmount
 }
 
@@ -748,10 +799,7 @@ _check_xfs_test_fs()
 _require_xfs_test_rmapbt()
 {
 	_require_test
-
-	if [ "$($XFS_INFO_PROG "$TEST_DIR" | grep -c "rmapbt=1")" -ne 1 ]; then
-		_notrun "rmapbt not supported by test filesystem type: $FSTYP"
-	fi
+	_require_xfs_has_feature "$TEST_DIR" rmapbt
 }
 
 _require_xfs_scratch_rmapbt()
@@ -760,10 +808,7 @@ _require_xfs_scratch_rmapbt()
 
 	_scratch_mkfs > /dev/null
 	_scratch_mount
-	if [ "$($XFS_INFO_PROG "$SCRATCH_MNT" | grep -c "rmapbt=1")" -ne 1 ]; then
-		_scratch_unmount
-		_notrun "rmapbt not supported by scratch filesystem type: $FSTYP"
-	fi
+	_require_xfs_has_feature "$SCRATCH_MNT" rmapbt
 	_scratch_unmount
 }
 
@@ -1366,8 +1411,8 @@ _require_scratch_xfs_bigtime()
 		_notrun "mkfs.xfs doesn't support bigtime feature"
 	_try_scratch_mount || \
 		_notrun "kernel doesn't support xfs bigtime feature"
-	$XFS_INFO_PROG "$SCRATCH_MNT" | grep -q -w "bigtime=1" || \
-		_notrun "bigtime feature not advertised on mount?"
+	_require_xfs_has_feature $SCRATCH_MNT bigtime -u \
+		"crc feature not supported by this filesystem"
 	_scratch_unmount
 }
 
diff --git a/tests/xfs/097 b/tests/xfs/097
index 4cad7216cd..1df34eeddc 100755
--- a/tests/xfs/097
+++ b/tests/xfs/097
@@ -42,7 +42,7 @@ _scratch_mkfs_xfs -m crc=1,finobt=1 > /dev/null
 
 echo "+ mount fs image"
 _scratch_mount
-$XFS_INFO_PROG "${SCRATCH_MNT}" | grep -q "finobt=1" || _notrun "finobt not enabled"
+_require_xfs_has_feature "$SCRATCH_MNT" finobt
 blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
 
 echo "+ make some files"
diff --git a/tests/xfs/151 b/tests/xfs/151
index 66425f6710..b2fe16aefb 100755
--- a/tests/xfs/151
+++ b/tests/xfs/151
@@ -24,8 +24,7 @@ echo "Format filesystem and populate"
 _scratch_mkfs > $seqres.full
 _scratch_mount >> $seqres.full
 
-$XFS_INFO_PROG $SCRATCH_MNT | grep -q ftype=1 || \
-	_notrun "filesystem does not support ftype"
+_require_xfs_has_feature "$SCRATCH_MNT" ftype
 
 filter_ls() {
 	awk '
diff --git a/tests/xfs/271 b/tests/xfs/271
index 14d64cd0e5..d67ac4d6c1 100755
--- a/tests/xfs/271
+++ b/tests/xfs/271
@@ -37,7 +37,7 @@ agcount=$(_xfs_mount_agcount $SCRATCH_MNT)
 # same owner (per-AG metadata) for rmap btree blocks and blocks on the AGFL and
 # the reverse mapping index merges records, the number of per-AG extents
 # reported will vary depending on whether the refcount btree is enabled.
-$XFS_INFO_PROG $SCRATCH_MNT | grep -q reflink=1
+_require_xfs_has_feature "$SCRATCH_MNT" reflink
 has_reflink=$(( 1 - $? ))
 perag_metadata_exts=2
 test $has_reflink -gt 0 && perag_metadata_exts=$((perag_metadata_exts + 1))
diff --git a/tests/xfs/307 b/tests/xfs/307
index ba7204dd00..f3c970fadf 100755
--- a/tests/xfs/307
+++ b/tests/xfs/307
@@ -22,7 +22,7 @@ _require_scratch_reflink
 echo "Format"
 _scratch_mkfs > $seqres.full 2>&1
 _scratch_mount >> $seqres.full
-is_rmap=$($XFS_INFO_PROG $SCRATCH_MNT | grep -c "rmapbt=1")
+is_rmap=$(_xfs_has_feature $SCRATCH_MNT rmapbt -v)
 _scratch_unmount
 
 _get_agf_data() {
diff --git a/tests/xfs/308 b/tests/xfs/308
index d0f47f5038..6da6622e14 100755
--- a/tests/xfs/308
+++ b/tests/xfs/308
@@ -22,7 +22,7 @@ _require_scratch_reflink
 echo "Format"
 _scratch_mkfs > $seqres.full 2>&1
 _scratch_mount >> $seqres.full
-is_rmap=$($XFS_INFO_PROG $SCRATCH_MNT | grep -c "rmapbt=1")
+is_rmap=$(_xfs_has_feature $SCRATCH_MNT rmapbt -v)
 _scratch_xfs_unmount_dirty
 
 _get_agf_data() {
diff --git a/tests/xfs/348 b/tests/xfs/348
index faf2dca50b..d1645d9462 100755
--- a/tests/xfs/348
+++ b/tests/xfs/348
@@ -39,7 +39,7 @@ mknod $testdir/CHRDEV c 1 1
 mknod $testdir/BLKDEV b 1 1
 mknod $testdir/FIFO p
 
-$XFS_INFO_PROG $SCRATCH_MNT | grep -q "ftype=1" && FTYPE_FEATURE=1
+_xfs_has_feature $SCRATCH_MNT ftype && FTYPE_FEATURE=1
 
 # Record test dir inode for xfs_repair filter
 inode_filter=$tmp.sed