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

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

[Qu Wenruo]

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
 mount | grep "$TEST_DIR"
 _exit 0

And with "MOUNT_OPTIONS" set to "-o nodatasum" (one of the btrfs mount
option that affects new inodes), but "TEST_FS_MOUNT_OPTS" not set, then
running the test, the output will be:

 QA output created by 348
 /dev/mapper/test-test on /mnt/test type btrfs (rw,relatime,nodatasum,discard=async,space_cache=v2,subvolid=5,subvol=/)
 /dev/mapper/test-test on /mnt/test type btrfs (rw,relatime,discard=async,space_cache=v2,subvolid=5,subvol=/)

This changed mount option will cause several test cases to fail on btrfs
with MOUNT_OPTIONS="-o nodatasum" set:

- generic/143
- generic/154
- generic/155
- generic/463

All above test case fails because the reflink source is from the initial
test mount, which has "nodatasum" mount option.
But later after _test_cycle_mount(), no more "nodatasum" flags, causing
reflink source and destination have different "nodatasum" flags.
And btrfs will reject such reflink, causing golden output mismatch.

One way to avoid such problem is to set TEST_FS_MOUNT_OPTS to the same
as MOUNT_OPTIONS, but I'm not sure if we really want different options
for test and scratch mounts.

So here as a compromise, set TEST_FS_MOUNT_OPTS to MOUNT_OPTIONS if
the former is not set but the latter is already set by the user.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
Reason for RFC:

- Still didn't get why the initial test mount respects MOUNT_OPTIONS
  If test mount only respects TEST_FS_MOUNT_OPTS, the first mount should
  not get the MOUNT_OPTIONS.

- 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().

  But nowadays the special handling is moved to _common_mount_opts(),
  shared by both scratch and test mounts.
  Not sure if there is any fs that requires split options for test and
  scratch.

- Possible reduced coverage
  Ignoring MOUNT_OPTIONS and requiring extra TEST_FS_MOUNT_OPTS can cause
  reduced coverage, as one may expect MOUNT_OPTIONS to affect both test
  and scratch devices.
---
 common/config | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/common/config b/common/config
index d5299d5b..12e94088 100644
--- a/common/config
+++ b/common/config
@@ -419,8 +419,14 @@ _mount_opts()
 	export MOUNT_OPTIONS=$(_common_mount_opts)
 }
 
+# Must be called before _mount_opts().
 _test_mount_opts()
 {
+	# Use $MOUNT_OPTIONS if provided.
+	if [ ! -z "$MOUNT_OPTIONS" ]; then
+		export TEST_FS_MOUNT_OPTS="$MOUNT_OPTIONS"
+		return
+	fi
 	export TEST_FS_MOUNT_OPTS=$(_common_mount_opts)
 }
 
@@ -823,8 +829,8 @@ get_next_config() {
 
 	parse_config_section $1
 	if [ ! -z "$OLD_FSTYP" ] && [ $OLD_FSTYP != $FSTYP ]; then
-		[ -z "$MOUNT_OPTIONS" ] && _mount_opts
 		[ -z "$TEST_FS_MOUNT_OPTS" ] && _test_mount_opts
+		[ -z "$MOUNT_OPTIONS" ] && _mount_opts
 		[ -z "$MKFS_OPTIONS" ] && _mkfs_opts
 		[ -z "$FSCK_OPTIONS" ] && _fsck_opts
 
@@ -915,8 +921,8 @@ if [ -z "$CONFIG_INCLUDED" ]; then
 	fi
 	FSTYP=${FSTYP:=xfs}
 	export FSTYP
-	[ -z "$MOUNT_OPTIONS" ] && _mount_opts
 	[ -z "$TEST_FS_MOUNT_OPTS" ] && _test_mount_opts
+	[ -z "$MOUNT_OPTIONS" ] && _mount_opts
 	[ -z "$MKFS_OPTIONS" ] && _mkfs_opts
 	[ -z "$FSCK_OPTIONS" ] && _fsck_opts
 else
-- 
2.51.2

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

[Christoph Hellwig]

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().

AFAIK TEST_FS_MOUNT_OPTS gets overriden and isn't even a public option.
It gets filled by fstype-sepcific options in _common_mount_opts.

This started to really confuse me lately when I was trying to test with
mount options on the test fs.

I've looked at bit into the area but haven't come to a conclusion yet.
The most recent change in this area is 1dd7395623cb ("common/config:
Make test and scratch devices use the same mount options"), which didn't
really make them common, adding to my confusion.

So I really think we need to fix things here, but right now I'm not sure
how much of the current situation is intentional, and what our final
goal is :(

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

[Zorro Lang]

On Mon, Jun 01, 2026 at 11:13:01PM -0700, Christoph Hellwig wrote:
> 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().
>
> AFAIK TEST_FS_MOUNT_OPTS gets overriden and isn't even a public option.
> It gets filled by fstype-sepcific options in _common_mount_opts.
> 
> This started to really confuse me lately when I was trying to test with
> mount options on the test fs.
> 
> I've looked at bit into the area but haven't come to a conclusion yet.
> The most recent change in this area is 1dd7395623cb ("common/config:
> Make test and scratch devices use the same mount options"), which didn't
> really make them common, adding to my confusion.
> 
> So I really think we need to fix things here, but right now I'm not sure
> how much of the current situation is intentional, and what our final
> goal is :(

Sorry for this terrible code logic.

AFAIK, TEST_FS_MOUNT_OPTS and MOUNT_OPTIONS serve different purposes:
TEST_FS_MOUNT_OPTS is used for TEST_DEV, while MOUNT_OPTIONS is used for
SCRATCH_DEV as well as loop or dm devices and so on. For instance, _test_mount
only applies TEST_FS_MOUNT_OPTS, whereas _scratch_mount only uses MOUNT_OPTIONS.
I used xfstests for over a decade, I typically configure both variables together
(often to the same values), so I didn't give them too much thought.

About the commit 1dd7395623cb, the subject of it would be better to be
"Make test and scratch devices use the same *default* mount options"
from ${FSTYP}_MOUNT_OPTIONS parameters. The original intent was not to merge
TEST_FS_MOUNT_OPTS and MOUNT_OPTIONS into a single variable.

About documentation, it is true that TEST_FS_MOUNT_OPTS is barely mentioned.
It is completely missing from the main README and only appears in other
sub-documents like README.config-sections. We should definitely clarify the
relationship between TEST_FS_MOUNT_OPTS and TEST_DEV, and metion that they
have no connection to MOUNT_OPTIONS.

Right now, the biggest inconsistent issue is probably with those feature-probing
helpers. They often check TEST_DIR to determine behaviors for SCRATCH_MNT. While
this works fine for features unaffected by mount options, but we might should
handle mount options related features (for TEST_DEV or SCRATCH_DEV) separately.

Thanks,
Zorro

> 

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

[Christoph Hellwig]

On Fri, Jun 05, 2026 at 12:46:13AM +0800, Zorro Lang wrote:
> Right now, the biggest inconsistent issue is probably with those feature-probing
> helpers. They often check TEST_DIR to determine behaviors for SCRATCH_MNT. While
> this works fine for features unaffected by mount options, but we might should
> handle mount options related features (for TEST_DEV or SCRATCH_DEV) separately.

Yes, I've run into a lot of issues when TEST_DIR has more feature than
SCRATCH_DEV.  The problem is just that feature testing for scratch is
a bit annoying because we have to create a file system for it first.

It might make sense to catch some of the results, but then again we have
tests that override options for the scratch side so that this won't
apply.

I've been wanting to look into splitting the scratch tests up into those
that "just" format the file systems, and those testing very specific
corners cases using special options.  Especially as we should not have
to run the latter multiple times for different configurations.

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

[Zorro Lang]

On Thu, Jun 04, 2026 at 09:13:30PM -0700, Christoph Hellwig wrote:
> On Fri, Jun 05, 2026 at 12:46:13AM +0800, Zorro Lang wrote:
> > Right now, the biggest inconsistent issue is probably with those feature-probing
> > helpers. They often check TEST_DIR to determine behaviors for SCRATCH_MNT. While
> > this works fine for features unaffected by mount options, but we might should
> > handle mount options related features (for TEST_DEV or SCRATCH_DEV) separately.
> 
> Yes, I've run into a lot of issues when TEST_DIR has more feature than
> SCRATCH_DEV.  The problem is just that feature testing for scratch is
> a bit annoying because we have to create a file system for it first.

Yeah, if we make those _require_*feature or _has_*feature helpers accept a
parameter like "local dev="${TEST_DEV:-$1}" to allow specifying $SCRATCH_DEV,
it would mean we can only call these helpers after running _scratch_mkfs or
even _scratch_mount. I haven't come up with a better idea for now ...

> 
> It might make sense to catch some of the results, but then again we have
> tests that override options for the scratch side so that this won't
> apply.
> 
> I've been wanting to look into splitting the scratch tests up into those
> that "just" format the file systems, and those testing very specific
> corners cases using special options.  Especially as we should not have
> to run the latter multiple times for different configurations.

Hopefully, specifying the same TEST_FS_MOUNT_OPTS and MOUNT_OPTIONS for XFS
can help resolve some of the issues you encountered. But that doesn't help if
your TEST_DEV and SCRATCH_DEV are different devices.

About separating "corner cases using special options", xfstests always welcome
"finer-grained" group tags to better categorize and differentiate these test
cases.

Thanks,
Zorro

> 
> 

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

[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.

>  mount | grep "$TEST_DIR"
>  _exit 0
> 
> And with "MOUNT_OPTIONS" set to "-o nodatasum" (one of the btrfs mount
> option that affects new inodes), but "TEST_FS_MOUNT_OPTS" not set, then
> running the test, the output will be:
> 
>  QA output created by 348
>  /dev/mapper/test-test on /mnt/test type btrfs (rw,relatime,nodatasum,discard=async,space_cache=v2,subvolid=5,subvol=/)
>  /dev/mapper/test-test on /mnt/test type btrfs (rw,relatime,discard=async,space_cache=v2,subvolid=5,subvol=/)
> 
> This changed mount option will cause several test cases to fail on btrfs
> with MOUNT_OPTIONS="-o nodatasum" set:
> 
> - generic/143
> - generic/154
> - generic/155
> - generic/463
> 
> All above test case fails because the reflink source is from the initial
> test mount, which has "nodatasum" mount option.
> But later after _test_cycle_mount(), no more "nodatasum" flags, causing
> reflink source and destination have different "nodatasum" flags.
> And btrfs will reject such reflink, causing golden output mismatch.
> 
> One way to avoid such problem is to set TEST_FS_MOUNT_OPTS to the same
> as MOUNT_OPTIONS, but I'm not sure if we really want different options
> for test and scratch mounts.
> 
> So here as a compromise, set TEST_FS_MOUNT_OPTS to MOUNT_OPTIONS if
> the former is not set but the latter is already set by the user.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> Reason for RFC:
> 
> - Still didn't get why the initial test mount respects MOUNT_OPTIONS
>   If test mount only respects TEST_FS_MOUNT_OPTS, the first mount should
>   not get the MOUNT_OPTIONS.
> 
> - 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).

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

> 
>   But nowadays the special handling is moved to _common_mount_opts(),
>   shared by both scratch and test mounts.
>   Not sure if there is any fs that requires split options for test and
>   scratch.
> 
> - Possible reduced coverage
>   Ignoring MOUNT_OPTIONS and requiring extra TEST_FS_MOUNT_OPTS can cause
>   reduced coverage, as one may expect MOUNT_OPTIONS to affect both test
>   and scratch devices.
> ---
>  common/config | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/common/config b/common/config
> index d5299d5b..12e94088 100644
> --- a/common/config
> +++ b/common/config
> @@ -419,8 +419,14 @@ _mount_opts()
>  	export MOUNT_OPTIONS=$(_common_mount_opts)
>  }
>  
> +# Must be called before _mount_opts().
>  _test_mount_opts()
>  {
> +	# Use $MOUNT_OPTIONS if provided.
> +	if [ ! -z "$MOUNT_OPTIONS" ]; then
> +		export TEST_FS_MOUNT_OPTS="$MOUNT_OPTIONS"
> +		return
> +	fi
>  	export TEST_FS_MOUNT_OPTS=$(_common_mount_opts)
>  }
>  
> @@ -823,8 +829,8 @@ get_next_config() {
>  
>  	parse_config_section $1
>  	if [ ! -z "$OLD_FSTYP" ] && [ $OLD_FSTYP != $FSTYP ]; then
> -		[ -z "$MOUNT_OPTIONS" ] && _mount_opts
>  		[ -z "$TEST_FS_MOUNT_OPTS" ] && _test_mount_opts
> +		[ -z "$MOUNT_OPTIONS" ] && _mount_opts
>  		[ -z "$MKFS_OPTIONS" ] && _mkfs_opts
>  		[ -z "$FSCK_OPTIONS" ] && _fsck_opts
>  
> @@ -915,8 +921,8 @@ if [ -z "$CONFIG_INCLUDED" ]; then
>  	fi
>  	FSTYP=${FSTYP:=xfs}
>  	export FSTYP
> -	[ -z "$MOUNT_OPTIONS" ] && _mount_opts
>  	[ -z "$TEST_FS_MOUNT_OPTS" ] && _test_mount_opts
> +	[ -z "$MOUNT_OPTIONS" ] && _mount_opts
>  	[ -z "$MKFS_OPTIONS" ] && _mkfs_opts
>  	[ -z "$FSCK_OPTIONS" ] && _fsck_opts
>  else
> -- 
> 2.51.2
> 
> 

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

[Qu Wenruo]

在 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