[PATCH V4 0/2] cleanup and bugfix for xfs tests related to btree format

[PATCH V4 0/2] cleanup and bugfix for xfs tests related to btree format

[Ziyang Zhang]

The first patch add a helper in common/xfs to export the inode core size
which is needed by some xfs test cases.

The second patch ensure that S_IFDIR.FMT_BTREE is in btree format while
populating dir.

V4:
- let the new helper function accept the "missing" parameter
- make _xfs_inode_core_bytes echo 176 or 96 so tests can work correctly on
  both v4 and v5
V3:
- refactor xfs tests cases using inode core size
V2:
- take Darrick's advice to cleanup code

Ziyang Zhang (2):
  common/xfs: Add a helper to export inode core size
  common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format

 common/populate | 34 +++++++++++++++++++++++++++++++++-
 common/xfs      | 24 ++++++++++++++++++++++++
 tests/xfs/335   |  3 ++-
 tests/xfs/336   |  3 ++-
 tests/xfs/337   |  3 ++-
 tests/xfs/341   |  3 ++-
 tests/xfs/342   |  3 ++-
 7 files changed, 67 insertions(+), 6 deletions(-)

-- 
2.18.4

[PATCH V4 1/2] common/xfs: Add a helper to export inode core size

[Ziyang Zhang]

Some xfs test cases need the number of bytes reserved for only the inode
record, excluding the immediate fork areas. Now the value is hard-coded
and it is not a good chioce. Add a helper in common/xfs to export the
inode core size.

Signed-off-by: Ziyang Zhang <ZiyangZhang@linux.alibaba.com>
---
 common/xfs    | 15 +++++++++++++++
 tests/xfs/335 |  3 ++-
 tests/xfs/336 |  3 ++-
 tests/xfs/337 |  3 ++-
 tests/xfs/341 |  3 ++-
 tests/xfs/342 |  3 ++-
 6 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/common/xfs b/common/xfs
index 8ac1964e..5074c350 100644
--- a/common/xfs
+++ b/common/xfs
@@ -1486,3 +1486,18 @@ _require_xfsrestore_xflag()
 	$XFSRESTORE_PROG -h 2>&1 | grep -q -e '-x' || \
 			_notrun 'xfsrestore does not support -x flag.'
 }
+
+# Number of bytes reserved for only the inode record, excluding the
+# immediate fork areas.
+_xfs_inode_core_bytes()
+{
+	local dir="$1"
+	
+	if _xfs_has_feature "$dir" crc; then
+		# v5 filesystems
+		echo 176
+	else
+		# v4 filesystems
+		echo 96
+	fi
+}
diff --git a/tests/xfs/335 b/tests/xfs/335
index ccc508e7..2fd9be54 100755
--- a/tests/xfs/335
+++ b/tests/xfs/335
@@ -31,7 +31,8 @@ blksz="$(_get_block_size $SCRATCH_MNT)"
 echo "Create a three-level rtrmapbt"
 # inode core size is at least 176 bytes; btree header is 56 bytes;
 # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes.
-i_ptrs=$(( (isize - 176) / 56 ))
+i_core_size="$(_xfs_inode_core_bytes $SCRATCH_MNT)"
+i_ptrs=$(( (isize - i_core_size) / 56 ))
 bt_ptrs=$(( (blksz - 56) / 56 ))
 bt_recs=$(( (blksz - 56) / 32 ))
 
diff --git a/tests/xfs/336 b/tests/xfs/336
index b1de8e5f..085b43ae 100755
--- a/tests/xfs/336
+++ b/tests/xfs/336
@@ -42,7 +42,8 @@ rm -rf $metadump_file
 echo "Create a three-level rtrmapbt"
 # inode core size is at least 176 bytes; btree header is 56 bytes;
 # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes.
-i_ptrs=$(( (isize - 176) / 56 ))
+i_core_size="$(_xfs_inode_core_bytes $SCRATCH_MNT)"
+i_ptrs=$(( (isize - i_core_size) / 56 ))
 bt_ptrs=$(( (blksz - 56) / 56 ))
 bt_recs=$(( (blksz - 56) / 32 ))
 
diff --git a/tests/xfs/337 b/tests/xfs/337
index a2515e36..5d6b9964 100755
--- a/tests/xfs/337
+++ b/tests/xfs/337
@@ -33,7 +33,8 @@ blksz="$(_get_block_size $SCRATCH_MNT)"
 
 # inode core size is at least 176 bytes; btree header is 56 bytes;
 # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes.
-i_ptrs=$(( (isize - 176) / 56 ))
+i_core_size="$(_xfs_inode_core_bytes $SCRATCH_MNT)"
+i_ptrs=$(( (isize - i_core_size) / 56 ))
 bt_ptrs=$(( (blksz - 56) / 56 ))
 bt_recs=$(( (blksz - 56) / 32 ))
 
diff --git a/tests/xfs/341 b/tests/xfs/341
index f026aa37..c70dec3c 100755
--- a/tests/xfs/341
+++ b/tests/xfs/341
@@ -33,7 +33,8 @@ rtextsz_blks=$((rtextsz / blksz))
 
 # inode core size is at least 176 bytes; btree header is 56 bytes;
 # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes.
-i_ptrs=$(( (isize - 176) / 56 ))
+i_core_size="$(_xfs_inode_core_bytes $SCRATCH_MNT)"
+i_ptrs=$(( (isize - i_core_size) / 56 ))
 bt_recs=$(( (blksz - 56) / 32 ))
 
 blocks=$((i_ptrs * bt_recs + 1))
diff --git a/tests/xfs/342 b/tests/xfs/342
index 1ae414eb..4855627f 100755
--- a/tests/xfs/342
+++ b/tests/xfs/342
@@ -30,7 +30,8 @@ blksz="$(_get_block_size $SCRATCH_MNT)"
 
 # inode core size is at least 176 bytes; btree header is 56 bytes;
 # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes.
-i_ptrs=$(( (isize - 176) / 56 ))
+i_core_size="$(_xfs_inode_core_bytes $SCRATCH_MNT)"
+i_ptrs=$(( (isize - i_core_size) / 56 ))
 bt_recs=$(( (blksz - 56) / 32 ))
 
 blocks=$((i_ptrs * bt_recs + 1))
-- 
2.18.4

Re: [PATCH V4 1/2] common/xfs: Add a helper to export inode core size

[Zorro Lang]

On Wed, Dec 07, 2022 at 05:31:46PM +0800, Ziyang Zhang wrote:
> Some xfs test cases need the number of bytes reserved for only the inode
> record, excluding the immediate fork areas. Now the value is hard-coded
> and it is not a good chioce. Add a helper in common/xfs to export the
> inode core size.
> 
> Signed-off-by: Ziyang Zhang <ZiyangZhang@linux.alibaba.com>
> ---

Darrick has given RVB to this patch, if you didn't change anything from V3
ou can keep it:

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

>  common/xfs    | 15 +++++++++++++++
>  tests/xfs/335 |  3 ++-
>  tests/xfs/336 |  3 ++-
>  tests/xfs/337 |  3 ++-
>  tests/xfs/341 |  3 ++-
>  tests/xfs/342 |  3 ++-
>  6 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/common/xfs b/common/xfs
> index 8ac1964e..5074c350 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -1486,3 +1486,18 @@ _require_xfsrestore_xflag()
>  	$XFSRESTORE_PROG -h 2>&1 | grep -q -e '-x' || \
>  			_notrun 'xfsrestore does not support -x flag.'
>  }
> +
> +# Number of bytes reserved for only the inode record, excluding the
> +# immediate fork areas.
> +_xfs_inode_core_bytes()
> +{
> +	local dir="$1"
> +	
> +	if _xfs_has_feature "$dir" crc; then
> +		# v5 filesystems
> +		echo 176
> +	else
> +		# v4 filesystems
> +		echo 96
> +	fi
> +}
> diff --git a/tests/xfs/335 b/tests/xfs/335
> index ccc508e7..2fd9be54 100755
> --- a/tests/xfs/335
> +++ b/tests/xfs/335
> @@ -31,7 +31,8 @@ blksz="$(_get_block_size $SCRATCH_MNT)"
>  echo "Create a three-level rtrmapbt"
>  # inode core size is at least 176 bytes; btree header is 56 bytes;
>  # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes.
> -i_ptrs=$(( (isize - 176) / 56 ))
> +i_core_size="$(_xfs_inode_core_bytes $SCRATCH_MNT)"
> +i_ptrs=$(( (isize - i_core_size) / 56 ))
>  bt_ptrs=$(( (blksz - 56) / 56 ))
>  bt_recs=$(( (blksz - 56) / 32 ))
>  
> diff --git a/tests/xfs/336 b/tests/xfs/336
> index b1de8e5f..085b43ae 100755
> --- a/tests/xfs/336
> +++ b/tests/xfs/336
> @@ -42,7 +42,8 @@ rm -rf $metadump_file
>  echo "Create a three-level rtrmapbt"
>  # inode core size is at least 176 bytes; btree header is 56 bytes;
>  # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes.
> -i_ptrs=$(( (isize - 176) / 56 ))
> +i_core_size="$(_xfs_inode_core_bytes $SCRATCH_MNT)"
> +i_ptrs=$(( (isize - i_core_size) / 56 ))
>  bt_ptrs=$(( (blksz - 56) / 56 ))
>  bt_recs=$(( (blksz - 56) / 32 ))
>  
> diff --git a/tests/xfs/337 b/tests/xfs/337
> index a2515e36..5d6b9964 100755
> --- a/tests/xfs/337
> +++ b/tests/xfs/337
> @@ -33,7 +33,8 @@ blksz="$(_get_block_size $SCRATCH_MNT)"
>  
>  # inode core size is at least 176 bytes; btree header is 56 bytes;
>  # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes.
> -i_ptrs=$(( (isize - 176) / 56 ))
> +i_core_size="$(_xfs_inode_core_bytes $SCRATCH_MNT)"
> +i_ptrs=$(( (isize - i_core_size) / 56 ))
>  bt_ptrs=$(( (blksz - 56) / 56 ))
>  bt_recs=$(( (blksz - 56) / 32 ))
>  
> diff --git a/tests/xfs/341 b/tests/xfs/341
> index f026aa37..c70dec3c 100755
> --- a/tests/xfs/341
> +++ b/tests/xfs/341
> @@ -33,7 +33,8 @@ rtextsz_blks=$((rtextsz / blksz))
>  
>  # inode core size is at least 176 bytes; btree header is 56 bytes;
>  # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes.
> -i_ptrs=$(( (isize - 176) / 56 ))
> +i_core_size="$(_xfs_inode_core_bytes $SCRATCH_MNT)"
> +i_ptrs=$(( (isize - i_core_size) / 56 ))
>  bt_recs=$(( (blksz - 56) / 32 ))
>  
>  blocks=$((i_ptrs * bt_recs + 1))
> diff --git a/tests/xfs/342 b/tests/xfs/342
> index 1ae414eb..4855627f 100755
> --- a/tests/xfs/342
> +++ b/tests/xfs/342
> @@ -30,7 +30,8 @@ blksz="$(_get_block_size $SCRATCH_MNT)"
>  
>  # inode core size is at least 176 bytes; btree header is 56 bytes;
>  # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes.
> -i_ptrs=$(( (isize - 176) / 56 ))
> +i_core_size="$(_xfs_inode_core_bytes $SCRATCH_MNT)"
> +i_ptrs=$(( (isize - i_core_size) / 56 ))
>  bt_recs=$(( (blksz - 56) / 32 ))
>  
>  blocks=$((i_ptrs * bt_recs + 1))
> -- 
> 2.18.4
> 

[PATCH V4 2/2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format

[Ziyang Zhang]

Sometimes "$((128 * dblksz / 40))" dirents cannot make sure that
S_IFDIR.FMT_BTREE could become btree format for its DATA fork.

Actually we just observed it can fail after apply our inode
extent-to-btree workaround. The root cause is that the kernel may be
too good at allocating consecutive blocks so that the data fork is
still in extents format.

Therefore instead of using a fixed number, let's make sure the number
of extents is large enough than (inode size - inode core size) /
sizeof(xfs_bmbt_rec_t).

Suggested-by: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
Signed-off-by: Ziyang Zhang <ZiyangZhang@linux.alibaba.com>
---
 common/populate | 34 +++++++++++++++++++++++++++++++++-
 common/xfs      |  9 +++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/common/populate b/common/populate
index 6e004997..95cf56de 100644
--- a/common/populate
+++ b/common/populate
@@ -71,6 +71,37 @@ __populate_create_dir() {
 	done
 }
 
+# Create a large directory and ensure that it's a btree format
+__populate_xfs_create_btree_dir() {
+	local name="$1"
+	local isize="$2"
+	local missing="$3"
+	local icore_size="$(_xfs_inode_core_bytes)"
+	# We need enough extents to guarantee that the data fork is in
+	# btree format.  Cycling the mount to use xfs_db is too slow, so
+	# watch for when the extent count exceeds the space after the
+	# inode core.
+	local max_nextents="$(((isize - icore_size) / 16))"
+
+	mkdir -p "${name}"
+	nr=0
+	while true; do
+		creat=mkdir
+		test "$((nr % 20))" -eq 0 && creat=touch
+		$creat "${name}/$(printf "%.08d" "$nr")"
+		if [ "$((nr % 40))" -eq 0 ]; then
+			nextents="$(_xfs_get_fsxattr nextents $name)"
+			[ $nextents -gt $max_nextents ] && break
+		fi
+		nr=$((nr+1))
+	done
+
+	test -z "${missing}" && return
+	seq 1 2 "${nr}" | while read d; do
+		rm -rf "${name}/$(printf "%.08d" "$d")"
+	done
+}
+
 # Add a bunch of attrs to a file
 __populate_create_attr() {
 	name="$1"
@@ -176,6 +207,7 @@ _scratch_xfs_populate() {
 
 	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
 	dblksz="$(_xfs_get_dir_blocksize "$SCRATCH_MNT")"
+	isize="$(_xfs_inode_size "$SCRATCH_MNT")"
 	crc="$(_xfs_has_feature "$SCRATCH_MNT" crc -v)"
 	if [ $crc -eq 1 ]; then
 		leaf_hdr_size=64
@@ -226,7 +258,7 @@ _scratch_xfs_populate() {
 
 	# - BTREE
 	echo "+ btree dir"
-	__populate_create_dir "${SCRATCH_MNT}/S_IFDIR.FMT_BTREE" "$((128 * dblksz / 40))" true
+	__populate_xfs_create_btree_dir "${SCRATCH_MNT}/S_IFDIR.FMT_BTREE" "$isize" true
 
 	# Symlinks
 	# - FMT_LOCAL
diff --git a/common/xfs b/common/xfs
index 5074c350..3bfe8566 100644
--- a/common/xfs
+++ b/common/xfs
@@ -1487,6 +1487,15 @@ _require_xfsrestore_xflag()
 			_notrun 'xfsrestore does not support -x flag.'
 }
 
+# Number of bytes reserved for a full inode record, which includes the
+# immediate fork areas.
+_xfs_inode_size()
+{
+	local mntpoint="$1"
+
+	$XFS_INFO_PROG "$mntpoint" | grep 'meta-data=.*isize' | sed -e 's/^.*isize=\([0-9]*\).*$/\1/g'
+}
+
 # Number of bytes reserved for only the inode record, excluding the
 # immediate fork areas.
 _xfs_inode_core_bytes()
-- 
2.18.4

Re: [PATCH V4 2/2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format

[Zorro Lang]

On Wed, Dec 07, 2022 at 05:31:47PM +0800, Ziyang Zhang wrote:
> Sometimes "$((128 * dblksz / 40))" dirents cannot make sure that
> S_IFDIR.FMT_BTREE could become btree format for its DATA fork.
> 
> Actually we just observed it can fail after apply our inode
> extent-to-btree workaround. The root cause is that the kernel may be
> too good at allocating consecutive blocks so that the data fork is
> still in extents format.
> 
> Therefore instead of using a fixed number, let's make sure the number
> of extents is large enough than (inode size - inode core size) /
> sizeof(xfs_bmbt_rec_t).
> 
> Suggested-by: "Darrick J. Wong" <djwong@kernel.org>
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> Signed-off-by: Ziyang Zhang <ZiyangZhang@linux.alibaba.com>
> ---
>  common/populate | 34 +++++++++++++++++++++++++++++++++-
>  common/xfs      |  9 +++++++++
>  2 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/common/populate b/common/populate
> index 6e004997..95cf56de 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -71,6 +71,37 @@ __populate_create_dir() {
>  	done
>  }
>  
> +# Create a large directory and ensure that it's a btree format
> +__populate_xfs_create_btree_dir() {
> +	local name="$1"
> +	local isize="$2"
> +	local missing="$3"
> +	local icore_size="$(_xfs_inode_core_bytes)"
> +	# We need enough extents to guarantee that the data fork is in
> +	# btree format.  Cycling the mount to use xfs_db is too slow, so
> +	# watch for when the extent count exceeds the space after the
> +	# inode core.
> +	local max_nextents="$(((isize - icore_size) / 16))"
> +
> +	mkdir -p "${name}"
> +	nr=0
> +	while true; do
> +		creat=mkdir
> +		test "$((nr % 20))" -eq 0 && creat=touch
> +		$creat "${name}/$(printf "%.08d" "$nr")"
> +		if [ "$((nr % 40))" -eq 0 ]; then
> +			nextents="$(_xfs_get_fsxattr nextents $name)"
> +			[ $nextents -gt $max_nextents ] && break
> +		fi
> +		nr=$((nr+1))
> +	done
> +
> +	test -z "${missing}" && return
> +	seq 1 2 "${nr}" | while read d; do
> +		rm -rf "${name}/$(printf "%.08d" "$d")"
> +	done

Oh, you've done this change in V4, sorry I just reviewed an old version. A
little picky review points as below:

This function makes sense to me, just the "local" key word is used so randomly,
some variables have, some doesn't :)

> +}
> +
>  # Add a bunch of attrs to a file
>  __populate_create_attr() {
>  	name="$1"
> @@ -176,6 +207,7 @@ _scratch_xfs_populate() {
>  
>  	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
>  	dblksz="$(_xfs_get_dir_blocksize "$SCRATCH_MNT")"
> +	isize="$(_xfs_inode_size "$SCRATCH_MNT")"
>  	crc="$(_xfs_has_feature "$SCRATCH_MNT" crc -v)"
>  	if [ $crc -eq 1 ]; then
>  		leaf_hdr_size=64
> @@ -226,7 +258,7 @@ _scratch_xfs_populate() {
>  
>  	# - BTREE
>  	echo "+ btree dir"
> -	__populate_create_dir "${SCRATCH_MNT}/S_IFDIR.FMT_BTREE" "$((128 * dblksz / 40))" true
> +	__populate_xfs_create_btree_dir "${SCRATCH_MNT}/S_IFDIR.FMT_BTREE" "$isize" true
>  
>  	# Symlinks
>  	# - FMT_LOCAL
> diff --git a/common/xfs b/common/xfs
> index 5074c350..3bfe8566 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -1487,6 +1487,15 @@ _require_xfsrestore_xflag()
>  			_notrun 'xfsrestore does not support -x flag.'
>  }
>  
> +# Number of bytes reserved for a full inode record, which includes the
> +# immediate fork areas.
> +_xfs_inode_size()

Generally common/xfs names this kind of helpers as _xfs_get_xxxx(), likes
_xfs_get_rtextents()
_xfs_get_rtextsize()
_xfs_get_dir_blocksize()
...

> +{
> +	local mntpoint="$1"
> +
> +	$XFS_INFO_PROG "$mntpoint" | grep 'meta-data=.*isize' | sed -e 's/^.*isize=\([0-9]*\).*$/\1/g'

It can be done with one pipe:
$XFS_INFO_PROG "$mntpoint" | sed -n '/meta-data=.*isize/s/^.*isize=\([0-9]*\).*$/\1/p'

With above changes you can have:
Reviewed-by: Zorro Lang <zlang@redhat.com>

> +}
> +
>  # Number of bytes reserved for only the inode record, excluding the
>  # immediate fork areas.
>  _xfs_inode_core_bytes()
> -- 
> 2.18.4
> 

Re: [PATCH V4 2/2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format

[Darrick J. Wong]

On Thu, Dec 08, 2022 at 02:28:50AM +0800, Zorro Lang wrote:
> On Wed, Dec 07, 2022 at 05:31:47PM +0800, Ziyang Zhang wrote:
> > Sometimes "$((128 * dblksz / 40))" dirents cannot make sure that
> > S_IFDIR.FMT_BTREE could become btree format for its DATA fork.
> > 
> > Actually we just observed it can fail after apply our inode
> > extent-to-btree workaround. The root cause is that the kernel may be
> > too good at allocating consecutive blocks so that the data fork is
> > still in extents format.
> > 
> > Therefore instead of using a fixed number, let's make sure the number
> > of extents is large enough than (inode size - inode core size) /
> > sizeof(xfs_bmbt_rec_t).
> > 
> > Suggested-by: "Darrick J. Wong" <djwong@kernel.org>
> > Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> > Signed-off-by: Ziyang Zhang <ZiyangZhang@linux.alibaba.com>
> > ---
> >  common/populate | 34 +++++++++++++++++++++++++++++++++-
> >  common/xfs      |  9 +++++++++
> >  2 files changed, 42 insertions(+), 1 deletion(-)
> > 
> > diff --git a/common/populate b/common/populate
> > index 6e004997..95cf56de 100644
> > --- a/common/populate
> > +++ b/common/populate
> > @@ -71,6 +71,37 @@ __populate_create_dir() {
> >  	done
> >  }
> >  
> > +# Create a large directory and ensure that it's a btree format
> > +__populate_xfs_create_btree_dir() {
> > +	local name="$1"
> > +	local isize="$2"
> > +	local missing="$3"
> > +	local icore_size="$(_xfs_inode_core_bytes)"
> > +	# We need enough extents to guarantee that the data fork is in
> > +	# btree format.  Cycling the mount to use xfs_db is too slow, so
> > +	# watch for when the extent count exceeds the space after the
> > +	# inode core.
> > +	local max_nextents="$(((isize - icore_size) / 16))"
> > +
> > +	mkdir -p "${name}"
> > +	nr=0
> > +	while true; do
> > +		creat=mkdir
> > +		test "$((nr % 20))" -eq 0 && creat=touch
> > +		$creat "${name}/$(printf "%.08d" "$nr")"
> > +		if [ "$((nr % 40))" -eq 0 ]; then
> > +			nextents="$(_xfs_get_fsxattr nextents $name)"
> > +			[ $nextents -gt $max_nextents ] && break
> > +		fi
> > +		nr=$((nr+1))
> > +	done
> > +
> > +	test -z "${missing}" && return
> > +	seq 1 2 "${nr}" | while read d; do
> > +		rm -rf "${name}/$(printf "%.08d" "$d")"
> > +	done
> 
> Oh, you've done this change in V4, sorry I just reviewed an old version. A
> little picky review points as below:
> 
> This function makes sense to me, just the "local" key word is used so randomly,
> some variables have, some doesn't :)

All variables inside a helper function *should* be using 'local', unless
the goal is to set variables in an ancestor scope.  Note that this can
mean variables in the top level namespace, or a different function
further up in the call stack.

Unfortunately, I didn't know about this bashism when I started writing
fstests, which is why it's inconsistent all over the place. :(

This little script:

#!/bin/bash

moo=5

bar() {
	moo=$((moo + 1))
}

fubar() {
	bar
}

cow() {
	local moo=7
	bar
	echo "cow $moo"
}

grud() {
	local moo=11
	fubar
	echo "grud $moo"
}

cow
bar
echo "global $moo"
grud
echo "global $moo"
fubar
echo "global $moo"

Prints this on output:

$ ./demo.sh
global 5
cow 8
global 5
global 6
grud 12
global 6
global 7

--D

> > +}
> > +
> >  # Add a bunch of attrs to a file
> >  __populate_create_attr() {
> >  	name="$1"
> > @@ -176,6 +207,7 @@ _scratch_xfs_populate() {
> >  
> >  	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
> >  	dblksz="$(_xfs_get_dir_blocksize "$SCRATCH_MNT")"
> > +	isize="$(_xfs_inode_size "$SCRATCH_MNT")"
> >  	crc="$(_xfs_has_feature "$SCRATCH_MNT" crc -v)"
> >  	if [ $crc -eq 1 ]; then
> >  		leaf_hdr_size=64
> > @@ -226,7 +258,7 @@ _scratch_xfs_populate() {
> >  
> >  	# - BTREE
> >  	echo "+ btree dir"
> > -	__populate_create_dir "${SCRATCH_MNT}/S_IFDIR.FMT_BTREE" "$((128 * dblksz / 40))" true
> > +	__populate_xfs_create_btree_dir "${SCRATCH_MNT}/S_IFDIR.FMT_BTREE" "$isize" true
> >  
> >  	# Symlinks
> >  	# - FMT_LOCAL
> > diff --git a/common/xfs b/common/xfs
> > index 5074c350..3bfe8566 100644
> > --- a/common/xfs
> > +++ b/common/xfs
> > @@ -1487,6 +1487,15 @@ _require_xfsrestore_xflag()
> >  			_notrun 'xfsrestore does not support -x flag.'
> >  }
> >  
> > +# Number of bytes reserved for a full inode record, which includes the
> > +# immediate fork areas.
> > +_xfs_inode_size()
> 
> Generally common/xfs names this kind of helpers as _xfs_get_xxxx(), likes
> _xfs_get_rtextents()
> _xfs_get_rtextsize()
> _xfs_get_dir_blocksize()
> ...
> 
> > +{
> > +	local mntpoint="$1"
> > +
> > +	$XFS_INFO_PROG "$mntpoint" | grep 'meta-data=.*isize' | sed -e 's/^.*isize=\([0-9]*\).*$/\1/g'
> 
> It can be done with one pipe:
> $XFS_INFO_PROG "$mntpoint" | sed -n '/meta-data=.*isize/s/^.*isize=\([0-9]*\).*$/\1/p'
> 
> With above changes you can have:
> Reviewed-by: Zorro Lang <zlang@redhat.com>
> 
> > +}
> > +
> >  # Number of bytes reserved for only the inode record, excluding the
> >  # immediate fork areas.
> >  _xfs_inode_core_bytes()
> > -- 
> > 2.18.4
> > 
>