| Message ID | 20220207030958.230618-8-shinichiro.kawasaki@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fstests: fix _scratch_mkfs_sized failure handling | expand |
On Mon, Feb 07, 2022 at 12:09:58PM +0900, Shin'ichiro Kawasaki wrote: > The test case generic/204 formats the scratch device to get block size > as a part of preparation. However, this preparation is required only for > xfs. To simplify preparation for other filesystems, do the preparation > only for xfs. > > Suggested-by: Naohiro Aota <naohiro.aota@wdc.com> > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> > --- > tests/generic/204 | 23 ++++++++++++++--------- > 1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/tests/generic/204 b/tests/generic/204 > index 40d524d1..ea267760 100755 > --- a/tests/generic/204 > +++ b/tests/generic/204 > @@ -16,17 +16,18 @@ _cleanup() > sync > } > > -# Import common functions. > -. ./common/filter > - > # real QA test starts here > _supported_fs generic > > _require_scratch > > -# get the block size first > -_scratch_mkfs 2> /dev/null | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null > -. $tmp.mkfs > +dbsize=4096 > +isize=256 > +if [ $FSTYP = "xfs" ]; then > + # get the block size first > + _scratch_mkfs 2> /dev/null | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null > + . $tmp.mkfs > +fi > > # 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. > @@ -37,11 +38,15 @@ _scratch_mkfs 2> /dev/null | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null > SIZE=`expr 115 \* 1024 \* 1024` > _scratch_mkfs_sized $SIZE $dbsize 2> /dev/null > $tmp.mkfs.raw \ > || _fail "mkfs failed" Getting back to why generic/204 calls mkfs twice -- The first time is to get dbsize (aka the data block size) and the inode size. AFAICT neither mkfs call add any mkfs options that would change the block size, so I wonder why we need the first call at all? Would the following work for g/204, assuming that _filter_mkfs some day provides correct output for dbsize and isize? _require_scratch # 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 time solves this problem. [ $FSTYP = "xfs" ] && MKFS_OPTIONS="$MKFS_OPTIONS -l size=16m -i maxpct=50" SIZE=`expr 115 \* 1024 \* 1024` _scratch_mkfs_sized $SIZE 2> /dev/null > $tmp.mkfs.raw || \ _fail "mkfs failed" cat $tmp.mkfs.raw | _filter_mkfs 2> $tmp.mkfs > /dev/null _scratch_mount # Source $tmp.mkfs to get geometry . $tmp.mkfs --D > -cat $tmp.mkfs.raw | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null > + > +if [ $FSTYP = "xfs" ]; then > + cat $tmp.mkfs.raw | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null > + # Source $tmp.mkfs to get geometry > + . $tmp.mkfs > +fi > + > _scratch_mount > > -# Source $tmp.mkfs to get geometry > -. $tmp.mkfs > > # fix the reserve block pool to a known size so that the enospc calculations > # work out correctly. Space usages is based 22500 files and 1024 reserved blocks > -- > 2.34.1 >
On Feb 08, 2022 / 16:57, Darrick J. Wong wrote: > On Mon, Feb 07, 2022 at 12:09:58PM +0900, Shin'ichiro Kawasaki wrote: > > The test case generic/204 formats the scratch device to get block size > > as a part of preparation. However, this preparation is required only for > > xfs. To simplify preparation for other filesystems, do the preparation > > only for xfs. > > > > Suggested-by: Naohiro Aota <naohiro.aota@wdc.com> > > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> > > --- > > tests/generic/204 | 23 ++++++++++++++--------- > > 1 file changed, 14 insertions(+), 9 deletions(-) > > > > diff --git a/tests/generic/204 b/tests/generic/204 > > index 40d524d1..ea267760 100755 > > --- a/tests/generic/204 > > +++ b/tests/generic/204 > > @@ -16,17 +16,18 @@ _cleanup() > > sync > > } > > > > -# Import common functions. > > -. ./common/filter > > - > > # real QA test starts here > > _supported_fs generic > > > > _require_scratch > > > > -# get the block size first > > -_scratch_mkfs 2> /dev/null | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null > > -. $tmp.mkfs > > +dbsize=4096 > > +isize=256 > > +if [ $FSTYP = "xfs" ]; then > > + # get the block size first > > + _scratch_mkfs 2> /dev/null | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null > > + . $tmp.mkfs > > +fi > > > > # 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. > > @@ -37,11 +38,15 @@ _scratch_mkfs 2> /dev/null | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null > > SIZE=`expr 115 \* 1024 \* 1024` > > _scratch_mkfs_sized $SIZE $dbsize 2> /dev/null > $tmp.mkfs.raw \ > > || _fail "mkfs failed" Hi Darrick, thank you for valuable review comments. > > Getting back to why generic/204 calls mkfs twice -- > > The first time is to get dbsize (aka the data block size) and the inode > size. AFAICT neither mkfs call add any mkfs options that would change > the block size, so I wonder why we need the first call at all? Oh, this question looks valid. I think both _scratch_mkfs and _scratch_mkfs_sized handle block size and i-node size options in MKFS_OPTIONS. So the first call does not look required. > > Would the following work for g/204, assuming that _filter_mkfs some day > provides correct output for dbsize and isize? Thank you for this idea. As you suggested, I removed the first _filter_mkfs call and $dbsize option for _scratch_mkfs_sized. With this change, I tried some MKFS_OPTIONS variations for mkfs.xfs (-b size=X, -i size=Y), and observed the test condition of g/204 ($files) is preserved. Will revise the series with this fix approach. > > _require_scratch > > # 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 time solves this problem. > [ $FSTYP = "xfs" ] && MKFS_OPTIONS="$MKFS_OPTIONS -l size=16m -i maxpct=50" > > SIZE=`expr 115 \* 1024 \* 1024` > _scratch_mkfs_sized $SIZE 2> /dev/null > $tmp.mkfs.raw || \ > _fail "mkfs failed" > > cat $tmp.mkfs.raw | _filter_mkfs 2> $tmp.mkfs > /dev/null > _scratch_mount > > # Source $tmp.mkfs to get geometry > . $tmp.mkfs > > --D > > > -cat $tmp.mkfs.raw | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null > > + > > +if [ $FSTYP = "xfs" ]; then > > + cat $tmp.mkfs.raw | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null > > + # Source $tmp.mkfs to get geometry > > + . $tmp.mkfs > > +fi > > + > > _scratch_mount > > > > -# Source $tmp.mkfs to get geometry > > -. $tmp.mkfs > > > > # fix the reserve block pool to a known size so that the enospc calculations > > # work out correctly. Space usages is based 22500 files and 1024 reserved blocks > > -- > > 2.34.1 > >
diff --git a/tests/generic/204 b/tests/generic/204 index 40d524d1..ea267760 100755 --- a/tests/generic/204 +++ b/tests/generic/204 @@ -16,17 +16,18 @@ _cleanup() sync } -# Import common functions. -. ./common/filter - # real QA test starts here _supported_fs generic _require_scratch -# get the block size first -_scratch_mkfs 2> /dev/null | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null -. $tmp.mkfs +dbsize=4096 +isize=256 +if [ $FSTYP = "xfs" ]; then + # get the block size first + _scratch_mkfs 2> /dev/null | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null + . $tmp.mkfs +fi # 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. @@ -37,11 +38,15 @@ _scratch_mkfs 2> /dev/null | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null SIZE=`expr 115 \* 1024 \* 1024` _scratch_mkfs_sized $SIZE $dbsize 2> /dev/null > $tmp.mkfs.raw \ || _fail "mkfs failed" -cat $tmp.mkfs.raw | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null + +if [ $FSTYP = "xfs" ]; then + cat $tmp.mkfs.raw | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null + # Source $tmp.mkfs to get geometry + . $tmp.mkfs +fi + _scratch_mount -# Source $tmp.mkfs to get geometry -. $tmp.mkfs # fix the reserve block pool to a known size so that the enospc calculations # work out correctly. Space usages is based 22500 files and 1024 reserved blocks
The test case generic/204 formats the scratch device to get block size as a part of preparation. However, this preparation is required only for xfs. To simplify preparation for other filesystems, do the preparation only for xfs. Suggested-by: Naohiro Aota <naohiro.aota@wdc.com> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> --- tests/generic/204 | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)