Re: [PATCH v1 1/2] generic/054,055: add _require_v2log

[Yang Xu <xuyang2018.jy@cn.fujitsu.com>]

on 2020/02/20 10:17, Darrick J. Wong wrote:
> On Thu, Feb 20, 2020 at 10:01:30AM +0800, Yang Xu wrote:
>>
>>
>> on 2020/02/20 6:11, Darrick J. Wong wrote:
>>> On Wed, Feb 19, 2020 at 04:48:07PM +0800, Yang Xu wrote:
>>>>
>>>>
>>>> on 2020/02/19 13:06, Darrick J. Wong wrote:
>>>>> On Tue, Feb 18, 2020 at 03:24:10PM +0800, Yang Xu wrote:
>>>>>> This two case fail on my server machine with GPT disklabe type,
>>>>>> info as below:
>>>>>> mount failed: -o logbsize=32k
>>>>>> dmesg info as below:
>>>>>> [92633.871954] XFS (sda7): logbuf size must be greater than or equal to log stripe size
>>>>>>
>>>>>> The min logbuf size is 256k on my server machine. It needs to use _require_v2log to check.
>>>>>
>>>>> Works fine here.  What is sda7's storage and raid alignment
>>>>> configuration?
>>>> /dev/sda is a SAS disk, raid aligment is 256k.  When I mkfs.xfs -f
>>>> /dev/sda7(20G), its lsunit value is 64blks(block size is 4k).
>>>
>>> _require_v2log probably ought to be smarter about that.
>> maybe _require_v2log to specify logbszie for mount options?
>>>
>>> But, uh, seeing as generic/05[45] don't currently call _require_v2log,
>>> how did logbsize=32k end up in the mount options to fail those two
>>> tests?
>>>
>> But generic/05[45] call _get_log_configs. It will use _xfs_log_config
>> and _xfs_log_config calls logbszie=32k/64k/128k/256k for mount options.
>> So xfs/xfs_super.c xfs_finish_flag will report EINVAL error if logbsize <
>> logsunit.  So on my machine this case failed without calling _require_v2log.
> 
> logbsize=32k is a perfectly valid option on a fs with a v2 log, provided
> the log sunit isn't larger than 32k.  _require_v2log conveniently
> _notruns the test when mounting with logbsize=32k fails, but
> maybe the problem here is that generic/05[45] need to format the fs with
> -l su=0 so that we can exercise full control over the mount options?
Good point, this way is more wise.  -l su=0 will report EINVAL error.

I change xfs_test_config by specifying lsu to meet mount options as below:
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"
+    echo "  version=2,su=16k     logbsize=16k"
      echo "  version=2,su=4096    logbsize=32k"
-    echo "  version=2,su=32768   logbsize=32k"
-    echo "  version=2,su=32768   logbsize=64k"
-    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"
  }

Best Regards
Yang Xu

> 
> --D
> 
>> Best Regards
>> Yang Xu
>>> --D
>>>
>>>> Best Regards
>>>> Yang Xu
>>>>>
>>>>> --D
>>>>>
>>>>>> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
>>>>>> ---
>>>>>>     common/log        | 1 +
>>>>>>     tests/generic/054 | 1 +
>>>>>>     tests/generic/055 | 1 +
>>>>>>     3 files changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/common/log b/common/log
>>>>>> index c7921f50..f82919a6 100644
>>>>>> --- a/common/log
>>>>>> +++ b/common/log
>>>>>> @@ -494,6 +494,7 @@ _log_version()
>>>>>>     _require_v2log()
>>>>>>     {
>>>>>> +    [ "$FSTYP" != xfs ] && return 0
>>>>>>         # test out mkfs to see if it supports "-l version=2"
>>>>>>         export MKFS_OPTIONS="-l version=2"
>>>>>>         if ! _scratch_mkfs_xfs >>$seqres.full 2>&1; then
>>>>>> diff --git a/tests/generic/054 b/tests/generic/054
>>>>>> index 95d40c3c..b4b74615 100755
>>>>>> --- a/tests/generic/054
>>>>>> +++ b/tests/generic/054
>>>>>> @@ -30,6 +30,7 @@ rm -f $seqres.full $tmp.*
>>>>>>     _require_scratch
>>>>>>     _require_scratch_shutdown
>>>>>>     _require_logstate
>>>>>> +_require_v2log
>>>>>>     echo "*** init FS"
>>>>>>     _scratch_unmount >/dev/null 2>&1
>>>>>> diff --git a/tests/generic/055 b/tests/generic/055
>>>>>> index 9715f27b..071c4eaf 100755
>>>>>> --- a/tests/generic/055
>>>>>> +++ b/tests/generic/055
>>>>>> @@ -57,6 +57,7 @@ rm -f $seqres.full $tmp.*
>>>>>>     _require_scratch
>>>>>>     _require_scratch_shutdown
>>>>>>     _require_logstate
>>>>>> +_require_v2log
>>>>>>     QUOTA_OPTION=`_get_quota_option`
>>>>>> -- 
>>>>>> 2.18.0
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
> 
>