The test case generic/204 calls _scratch_mkfs to get data block size and
i-node size of the filesystem and obtained data block size is passed to
the following _scratch_mfks_sized call as an option. However, the
_scratch_mkfs call is unnecessary since the sizes can be obtained by
_scratch_mkfs_sized call without the data block size option.
Also the _scratch_mkfs call is harmful when the _scratch_mkfs succeeds
and the _scratch_mkfs_sized fails. In this case, the _scratch_mkfs
leaves valid working filesystem on scratch device then following mount
and IO operations can not detect the failure of _scratch_mkfs_sized.
This results in the test case run with unexpected test condition.
Hence, remove the _scratch_mkfs call and the data block size option for
_scratch_mkfs_sized call.
Suggested-by: Darrick J. Wong <[email protected]>
Signed-off-by: Shin'ichiro Kawasaki <[email protected]>
---
tests/generic/204 | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/tests/generic/204 b/tests/generic/204
index a3dabb71..a33a090f 100755
--- a/tests/generic/204
+++ b/tests/generic/204
@@ -24,10 +24,6 @@ _supported_fs generic
_require_scratch
-# get the block size first
-_scratch_mkfs 2> /dev/null | _filter_mkfs 2> $tmp.mkfs > /dev/null
-. $tmp.mkfs
-
# For xfs, we need to handle the different default log sizes that different
# versions of mkfs create. All should be valid with a 16MB log, so use that.
# And v4/512 v5/1k xfs don't have enough free inodes, set imaxpct=50 at mkfs
@@ -35,7 +31,7 @@ _scratch_mkfs 2> /dev/null | _filter_mkfs 2> $tmp.mkfs > /dev/null
[ $FSTYP = "xfs" ] && MKFS_OPTIONS="$MKFS_OPTIONS -l size=16m -i maxpct=50"
SIZE=`expr 115 \* 1024 \* 1024`
-_scratch_mkfs_sized $SIZE $dbsize 2> /dev/null > $tmp.mkfs.raw
+_scratch_mkfs_sized $SIZE 2> /dev/null > $tmp.mkfs.raw
cat $tmp.mkfs.raw | _filter_mkfs 2> $tmp.mkfs > /dev/null
_scratch_mount
--
2.34.1
On Wed, Feb 09, 2022 at 09:33:01PM +0900, Shin'ichiro Kawasaki wrote:
> The test case generic/204 calls _scratch_mkfs to get data block size and
> i-node size of the filesystem and obtained data block size is passed to
> the following _scratch_mfks_sized call as an option. However, the
> _scratch_mkfs call is unnecessary since the sizes can be obtained by
> _scratch_mkfs_sized call without the data block size option.
>
> Also the _scratch_mkfs call is harmful when the _scratch_mkfs succeeds
> and the _scratch_mkfs_sized fails. In this case, the _scratch_mkfs
> leaves valid working filesystem on scratch device then following mount
> and IO operations can not detect the failure of _scratch_mkfs_sized.
> This results in the test case run with unexpected test condition.
>
> Hence, remove the _scratch_mkfs call and the data block size option for
> _scratch_mkfs_sized call.
>
> Suggested-by: Darrick J. Wong <[email protected]>
> Signed-off-by: Shin'ichiro Kawasaki <[email protected]>
Looks ok, assuming you've verified that fstests with FSTYP=xfs doesn't
regress...
Reviewed-by: Darrick J. Wong <[email protected]>
--D
> ---
> tests/generic/204 | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/tests/generic/204 b/tests/generic/204
> index a3dabb71..a33a090f 100755
> --- a/tests/generic/204
> +++ b/tests/generic/204
> @@ -24,10 +24,6 @@ _supported_fs generic
>
> _require_scratch
>
> -# get the block size first
> -_scratch_mkfs 2> /dev/null | _filter_mkfs 2> $tmp.mkfs > /dev/null
> -. $tmp.mkfs
> -
> # For xfs, we need to handle the different default log sizes that different
> # versions of mkfs create. All should be valid with a 16MB log, so use that.
> # And v4/512 v5/1k xfs don't have enough free inodes, set imaxpct=50 at mkfs
> @@ -35,7 +31,7 @@ _scratch_mkfs 2> /dev/null | _filter_mkfs 2> $tmp.mkfs > /dev/null
> [ $FSTYP = "xfs" ] && MKFS_OPTIONS="$MKFS_OPTIONS -l size=16m -i maxpct=50"
>
> SIZE=`expr 115 \* 1024 \* 1024`
> -_scratch_mkfs_sized $SIZE $dbsize 2> /dev/null > $tmp.mkfs.raw
> +_scratch_mkfs_sized $SIZE 2> /dev/null > $tmp.mkfs.raw
> cat $tmp.mkfs.raw | _filter_mkfs 2> $tmp.mkfs > /dev/null
> _scratch_mount
>
> --
> 2.34.1
>
On Feb 09, 2022 / 14:31, Darrick J. Wong wrote:
> On Wed, Feb 09, 2022 at 09:33:01PM +0900, Shin'ichiro Kawasaki wrote:
> > The test case generic/204 calls _scratch_mkfs to get data block size and
> > i-node size of the filesystem and obtained data block size is passed to
> > the following _scratch_mfks_sized call as an option. However, the
> > _scratch_mkfs call is unnecessary since the sizes can be obtained by
> > _scratch_mkfs_sized call without the data block size option.
> >
> > Also the _scratch_mkfs call is harmful when the _scratch_mkfs succeeds
> > and the _scratch_mkfs_sized fails. In this case, the _scratch_mkfs
> > leaves valid working filesystem on scratch device then following mount
> > and IO operations can not detect the failure of _scratch_mkfs_sized.
> > This results in the test case run with unexpected test condition.
> >
> > Hence, remove the _scratch_mkfs call and the data block size option for
> > _scratch_mkfs_sized call.
> >
> > Suggested-by: Darrick J. Wong <[email protected]>
> > Signed-off-by: Shin'ichiro Kawasaki <[email protected]>
>
> Looks ok, assuming you've verified that fstests with FSTYP=xfs doesn't
> regress...
>
> Reviewed-by: Darrick J. Wong <[email protected]>
Thanks for reviewing. I tested the test case with FSTYP=xfs on a few devices and
3 variety of MKFS_OPTIONS (no option, "-b size=1024 -i size=512" and
"-b size=4096 -i size=2048") and all passed. Also I ran whole fstests with
FSTYP=xfs, and confirmed that this change does not cause additional failure.
--
Best Regards,
Shin'ichiro Kawasaki