on 2020/02/24 0:37, Darrick J. Wong wrote:
On Sun, Feb 23, 2020 at 10:22:10PM +0800, Eryu Guan wrote:
Hi Darrick,
On Thu, Feb 20, 2020 at 01:38:01PM +0800, Yang Xu wrote:
Currently, if we don't specify -l sunit or -l su option, mkfs.xfs
will get the stripe size from underlying device.
It works file on most situations. But on some machine, the size of
underlying device greater than logbsize of mount options, it will
report error like "logbuf size must be greater than or equal to log
stripe size". We can specify -l su=4096 to meet this requirement and
case can still run normally.
Also, from xfs manpage, version 2 also supports 16k log buf size for
mount option and case passed(only generic/054,055 used this api) on
my machine. So delete 32k and 64k with different sunit to be consistented
I don't understand why there's a need to be consistent, which means I
don't understand why we'd "delete 32k and 64k with different sunit".
ext4 tests should not be invoking _xfs_log_config()
Sorry for the confused message.
_get_log_configs{
xfs)
_xfs_log_config
;;
f2fs)
_f2fs_log_config
;;
ext4)
_ext4_log_config
}
xfs ,f2fs, and ext4 all have test1~test10.
So for a generic case, it should be consistent.
And only generic/054 055 uses _get_log_configs api and no test case
used _xfs_log_config api. I think we should keep this(test1~test10,
054.out only 10 times mkfs and mount output)
with ext4 test num(10) and we can test all logbuf size.
Hm? ext4/010 is an inode bitmap fuzz test.
No. I mean _ext4_log_config has test1~test10.
Best Regards
Yang Xu
Signed-off-by: Yang Xu <xuyang2018.jy@xxxxxxxxxxxxxx>
Would you like to review this v2 patch again? I feel more confident if
xfs maintainer could ack it :)
Thanks,
Eryu
---
common/log | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/common/log b/common/log
index c7921f50..9b5a2f6d 100644
--- a/common/log
+++ b/common/log
@@ -546,15 +546,15 @@ _xfs_log_config()
{
echo "# mkfs-opt mount-opt"
echo "# ------------------------------"
- echo " version=2 logbsize=32k"
+ echo " version=2,su=4096 logbsize=16k"
The straight substitutions look fine to me, but then...
+ echo " version=2,su=16k logbsize=16k"
...I can't tell why we're adding this extra case here, or why this
has to be here and not in a separate patch justifying the addition?
echo " version=2,su=4096 logbsize=32k"
- echo " version=2,su=32768 logbsize=32k"
- echo " version=2,su=32768 logbsize=64k"
...I also don't think it's a good idea to reduce test coverage, and
definitely not buried in something that sounds like a fix patch.
--D
- echo " version=2 logbsize=64k"
+ echo " version=2,su=32k logbsize=32k"
+ echo " version=2,su=4096 logbsize=64k"
echo " version=2,su=64k logbsize=64k"
- echo " version=2 logbsize=128k"
+ echo " version=2,su=4096 logbsize=128k"
echo " version=2,su=128k logbsize=128k"
- echo " version=2 logbsize=256k"
+ echo " version=2,su=4096 logbsize=256k"
echo " version=2,su=256k logbsize=256k"
}
--
2.18.0
