Linux Btrfs filesystem development
 help / color / mirror / Atom feed
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


      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