Re: [PATCH RFC] fstests: use MOUNT_OPTIONS to populate TEST_FS_MOUNT_OPTS if possible

在 2026/6/5 03:18, Zorro Lang 写道:
> On Tue, Jun 02, 2026 at 09:44:22AM +0930, Qu Wenruo wrote:
>> If the test config only specifies "MOUNT_OPTIONS", but not
>> "TEST_FS_MOUNT_OPTS", the mount for TEST_DIR can have inconsistent flag
>> after _test_cycle_mount().
>>
>> Here is an very simple test case to show the problem:
>>
>>   . ./common/preamble
>>   _begin_fstest auto
>>   _require_test
>>
>>   mount | grep "$TEST_DIR"
>>   _test_cycle_mount
> 
> AFAIK, TEST_FS_MOUNT_OPTS is designed to be the exact counterpart to
> MOUNT_OPTIONS. The former applies to TEST_DEV, while the latter affects
> SCRATCH_DEV and any other manually created devices (such as loop or dm
> devices). Therefore, when testing custom mount options, better to specify
> both parameters.

Thanks for the explanation, in that case, I'd say we need to improve the 
docs, to mention both MOUNT_OPTIONS and TEST_FS_MOUNT_OPTS.

And even better, rename TEST_FS_MOUNT_OPTS to something like 
TEST_MOUNT_OPTIONS.
MOUNT_OPTIONS and TEST_FS_MOUNT_OPTS really look like they come from 
completely different worlds.

[...]
>> - Not sure if TEST_FS_MOUNT_OPTS is needed
>>    If not provided, the default config for TEST_FS_MOUNT_OPTS and
>>    MOUNT_OPTIONS are the same.
>>
>>    The introduction of TEST_FS_MOUNT_OPTS is from commit 0e9141e49d4a
>>    ("common: add cifs support"), with extra handling for
>>    _test_mount_opts().
> 
> No, actually TEST_FS_MOUNT_OPTS is far more than that. For example, you can
> check commit 3839d29973, it has TEST_FS_MOUNT_OPTS in _test_mount. But at
> that time xfstests was still under its original form, there was not common/
> directory.
> 
> Then 8c4905a4 create common/ directory and common/rc, then TEST_FS_MOUNT_OPTS
> started to appear in common/rc:_test_mount().
> 
> Then commit 0e9141e4 brought in _test_mount_opts for CIFS, but it didn't change
> the logic of TEST_FS_MOUNT_OPTS for TEST_DEV mount. And we can learn about why
> CIFS need a seperated TEST_FS_MOUNT_OPTS from its wiki link:
> 
>    https://wiki.samba.org/images/9/99/Xfstests.local.config.txt
> 
> Therefore, TEST_FS_MOUNT_OPTS has been dedicated entirely to TEST_DEV by design
> since the very inception of xfstests, and this principle seems to have remained
> unchanged. The real issue now is that we run into a conflict between
> TEST_FS_MOUNT_OPTS and MOUNT_OPTIONS when the helpers try to detect features,
> particularly those that depend on mount options.
> 
> As for whether TEST_FS_MOUNT_OPTS is still necessary, I feel we need more
> discussion on this. Some upper filesystems might still need it, and the original
> purpose of keeping it separate for XFS testing probably deserves a deeper look.
> (CC'ing Dave Chinner, who might know more that that history :)
> 
> About your current Btrfs tests, I would highly recommend specifying both
> TEST_FS_MOUNT_OPTS and MOUNT_OPTIONS at the same time. (I always do that
> actually).

Sure, I'll also set TEST_FS_MOUNT_OPT in that case.

Thanks,
Qu>
> Regarding the feature detection mismatch, a quick fix could be making TEST_FS_MOUNT_OPTS
> default to MOUNT_OPTIONS when it's not set. That said, it is still worth double-checking
> the initial purpose of TEST_FS_MOUNT_OPTS, just in case I miss something.
> 
> Thanks,
> Zorro