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

[Qu Wenruo <wqu@suse.com>]

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