From: Qu Wenruo <wqu@suse.com>
To: fstests@vger.kernel.org, linux-btrfs@vger.kernel.org,
Dave Chinner <david@fromorbit.com>
Subject: Re: [PATCH RFC] fstests: use MOUNT_OPTIONS to populate TEST_FS_MOUNT_OPTS if possible
Date: Sat, 6 Jun 2026 18:52:36 +0930 [thread overview]
Message-ID: <8d63ffbd-4e34-459b-9613-81f4b5210d15@suse.com> (raw)
In-Reply-To: <aiGsok9xtmdM-q9c@zlang-mailbox>
在 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
prev parent reply other threads:[~2026-06-06 9:22 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-02 0:14 [PATCH RFC] fstests: use MOUNT_OPTIONS to populate TEST_FS_MOUNT_OPTS if possible Qu Wenruo
2026-06-02 6:13 ` Christoph Hellwig
2026-06-04 16:46 ` Zorro Lang
2026-06-05 4:13 ` Christoph Hellwig
2026-06-06 19:33 ` Zorro Lang
2026-06-04 17:48 ` Zorro Lang
2026-06-06 9:22 ` Qu Wenruo [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8d63ffbd-4e34-459b-9613-81f4b5210d15@suse.com \
--to=wqu@suse.com \
--cc=david@fromorbit.com \
--cc=fstests@vger.kernel.org \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox