From dbbef111623ff38efdb0fabcb26848c7a7c08789 Mon Sep 17 00:00:00 2001 From: Rob Norris Date: Tue, 19 May 2026 12:50:39 +1000 Subject: [PATCH 1/3] ZTS/zfs_mount: lift & update helpers from zfs_mount_remount zfs_mount_remount has some nice helpers for checking the claimed and actual read-only/read-write state of a mount. I wanted to use them for another test but they weren't exactly what I wanted. This adds separate functions for the different kinds of mounts the zfs_mount_remount test wants to use, mostly to avoid the assymetry of sometimes calling a helper function and sometimes doing it direct. It also separates the code to get the current ro/rw mount option from actually asserting it. Test has been updated to use the new functions, but the logic and structure has not changed. Sponsored-by: TrueNAS Signed-off-by: Rob Norris --- .../cli_root/zfs_mount/zfs_mount.kshlib | 128 ++++++++++++++++++ .../cli_root/zfs_mount/zfs_mount_remount.ksh | 103 ++++---------- 2 files changed, 156 insertions(+), 75 deletions(-) diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount.kshlib b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount.kshlib index 08795a7ea257..5d7ceb971123 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount.kshlib +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount.kshlib @@ -27,6 +27,8 @@ # # Copyright (c) 2017 by Delphix. All rights reserved. +# Copyright 2017, loli10K . All rights reserved. +# Copyright (c) 2026, TrueNAS. # . $STF_SUITE/include/libtest.shlib @@ -131,3 +133,129 @@ function verify_mount_display done return 0 } + +# Helper functions to call the system mount(8) with various options +function mount_default # +{ + typeset opts= + if is_freebsd; then + opts="-t zfs" + else + opts="-t zfs" + fi + + mount $opts "$@" + return $? +} + +function mount_ro # +{ + typeset opts= + if is_freebsd; then + opts="-t zfs -r" + else + opts="-t zfs -o ro" + fi + + mount $opts "$@" + return $? +} + +function mount_rw # +{ + typeset opts= + if is_freebsd; then + opts="-t zfs -w" + else + opts="-t zfs -o rw" + fi + + mount $opts "$@" + return $? +} + +function remount_ro # +{ + typeset opts= + if is_freebsd; then + opts="-t zfs -ur" + else + opts="-o remount,ro" + fi + + mount $opts "$@" + return $? +} + +function remount_rw # +{ + typeset opts= + if is_freebsd; then + opts="-t zfs -uw" + else + opts="-o remount,rw" + fi + + mount $opts "$@" + return $? +} + +# +# Verify that $mountpoint is mounted readonly +# This is preferred over "log_mustnot touch $fs" because we actually want to +# verify the error returned is EROFS +# +function mount_is_ro # mountpoint +{ + typeset mountpoint="$1" + + file_write -o create -f $mountpoint/file.dat + ret=$? + if [[ $ret != 30 ]]; then + log_fail "Writing to $mountpoint did not return EROFS ($ret)." + fi +} + +function mount_is_rw # mountpoint +{ + typeset mountpoint="$1" + log_must touch $mountpoint/file.dat +} + +# Get the read-only/read-write option for $mountpoint +# Prints either "ro" or "rw", or nothing if $mountpoint is not in the mount +# table, or is not a ZFS mount. +function mount_get_ro_rw # mountpoint +{ + typeset mountpoint="$1" + + if is_freebsd; then + # tank/hello /tank/hello zfs rw,nfsv4acls 0 0 + mount -p | \ + awk -v mountpoint="$mountpoint" ' + $2 != mountpoint || $3 != "zfs" { next } + $4 ~ /(^|,)ro(,|$)/ { print "ro" } + $4 ~ /(^|,)rw(,|$)/ { print "rw" }' + else + # tank/hello /tank/hello zfs rw,relatime,xattr,noacl,casesensitive 0 0 + awk -v mountpoint="$mountpoint" ' + $2 != mountpoint || $3 != "zfs" { next } + $4 ~ /(^|,)ro(,|$)/ { print "ro" } + $4 ~ /(^|,)rw(,|$)/ { print "rw" }' /proc/mounts + fi +} + +# Verify that $mountpoint is mounted with a "read-only" option +function mount_has_ro_option # mountpoint +{ + typeset ropt=$(mount_get_ro_rw "$1") + log_must test $ropt == "ro" +} + +# Verify that $mountpoint is mounted with a "read-write" option +function mount_has_rw_option # mountpoint +{ + typeset ropt=$(mount_get_ro_rw "$1") + log_must test $ropt == "rw" +} + diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_remount.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_remount.ksh index c54128f7b9e7..a16d17a12291 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_remount.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_remount.ksh @@ -23,6 +23,7 @@ # # Copyright 2017, loli10K . All rights reserved. +# Copyright (c) 2026, TrueNAS. # . $STF_SUITE/include/libtest.shlib @@ -54,54 +55,6 @@ function cleanup return 0 } -if is_freebsd; then - typeset RO="-t zfs -ur" - typeset RW="-t zfs -uw" -else - typeset RO="-o remount,ro" - typeset RW="-o remount,rw" -fi - -# -# Verify the $filesystem is mounted readonly -# This is preferred over "log_mustnot touch $fs" because we actually want to -# verify the error returned is EROFS -# -function readonlyfs # filesystem -{ - typeset filesystem="$1" - - file_write -o create -f $filesystem/file.dat - ret=$? - if [[ $ret != 30 ]]; then - log_fail "Writing to $filesystem did not return EROFS ($ret)." - fi -} - -# -# Verify $dataset is mounted with $option -# -function checkmount # dataset option -{ - typeset dataset="$1" - typeset option="$2" - typeset options="" - - if is_freebsd; then - options=$(mount -p | awk -v ds="$dataset" '$1 == ds { print $4 }') - else - options=$(awk -v ds="$dataset" '$1 == ds { print $4 }' /proc/mounts) - fi - if [[ "$options" == '' ]]; then - log_fail "Dataset $dataset is not mounted" - elif [[ ! -z "${options##*$option*}" ]]; then - log_fail "Dataset $dataset is not mounted with expected "\ - "option $option ($options)" - else - log_note "Dataset $dataset is mounted with option $option" - fi -} - log_assert "Verify remount functionality on both filesystem and snapshots" log_onexit cleanup @@ -117,35 +70,35 @@ MNTPSNAP="$TESTDIR/zfs_snap_mount" log_must mkdir -p $MNTPSNAP # 2. Verify we can (re)mount the dataset readonly/read-write -log_must touch $MNTPFS/file.dat -checkmount $TESTFS 'rw' -log_must mount $RO $TESTFS $MNTPFS -readonlyfs $MNTPFS -checkmount $TESTFS 'ro' -log_must mount $RW $TESTFS $MNTPFS -log_must touch $MNTPFS/file.dat -checkmount $TESTFS 'rw' +mount_is_rw $MNTPFS +mount_has_rw_option $MNTPFS +log_must remount_ro $TESTFS $MNTPFS +mount_is_ro $MNTPFS +mount_has_ro_option $MNTPFS +log_must remount_rw $TESTFS $MNTPFS +mount_is_rw $MNTPFS +mount_has_rw_option $MNTPFS if is_linux; then # 3. Verify we can (re)mount the snapshot readonly - log_must mount -t zfs $TESTSNAP $MNTPSNAP - readonlyfs $MNTPSNAP - checkmount $TESTSNAP 'ro' - log_must mount $RO $TESTSNAP $MNTPSNAP - readonlyfs $MNTPSNAP - checkmount $TESTSNAP 'ro' + log_must mount_default $TESTSNAP $MNTPSNAP + mount_is_ro $MNTPSNAP + mount_has_ro_option $MNTPSNAP + log_must remount_ro $TESTSNAP $MNTPSNAP + mount_is_ro $MNTPSNAP + mount_has_ro_option $MNTPSNAP log_must umount $MNTPSNAP fi # 4. Verify we can't remount a snapshot read-write # The "mount -o rw" command will succeed but the snapshot is mounted readonly. # The "mount -o remount,rw" command must fail with an explicit error. -log_must mount -t zfs -o rw $TESTSNAP $MNTPSNAP -readonlyfs $MNTPSNAP -checkmount $TESTSNAP 'ro' -log_mustnot mount $RW $TESTSNAP $MNTPSNAP -readonlyfs $MNTPSNAP -checkmount $TESTSNAP 'ro' +log_must mount_rw $TESTSNAP $MNTPSNAP +mount_is_ro $MNTPSNAP +mount_has_ro_option $MNTPSNAP +log_mustnot remount_rw $TESTSNAP $MNTPSNAP +mount_is_ro $MNTPSNAP +mount_has_ro_option $MNTPSNAP log_must umount $MNTPSNAP # 5. Verify we can remount a dataset readonly and unmount it with @@ -153,8 +106,8 @@ log_must umount $MNTPSNAP log_must eval "echo 'password' | zfs create -o sync=disabled \ -o encryption=on -o keyformat=passphrase $TESTFS/crypt" CRYPT_MNTPFS="$(get_prop mountpoint $TESTFS/crypt)" -log_must touch $CRYPT_MNTPFS/file.dat -log_must mount $RO $TESTFS/crypt $CRYPT_MNTPFS +mount_is_rw $CRYPT_MNTPFS +log_must remount_ro $TESTFS/crypt $CRYPT_MNTPFS log_must umount -f $CRYPT_MNTPFS sync_pool $TESTPOOL @@ -163,10 +116,10 @@ log_must zpool export $TESTPOOL log_must zpool import -o readonly=on $TESTPOOL # 7. Verify we can't remount its filesystem read-write -readonlyfs $MNTPFS -checkmount $TESTFS 'ro' -log_mustnot mount $RW $MNTPFS -readonlyfs $MNTPFS -checkmount $TESTFS 'ro' +mount_is_ro $MNTPFS +mount_has_ro_option $MNTPFS +log_mustnot remount_rw $MNTPFS +mount_is_ro $MNTPFS +mount_has_ro_option $MNTPFS log_pass "Both filesystem and snapshots can be remounted correctly." From b8216fb774a831042c321d13ce748d455df82936 Mon Sep 17 00:00:00 2001 From: Rob Norris Date: Tue, 19 May 2026 14:27:12 +1000 Subject: [PATCH 2/3] ZTS/zfs_mount: test that ro/rw mount methods remain consistent Whether a mount ends up as read-only or read-write depends on a combination of platform, readonly= filesystem property, mount method (system mount(8) or zfs-mount(8)) and mount option provided (ro, rw or none). This tests all combinations, and ensures they match what has traditionally been expected on this platform, so we'll know if we accidentally changed it. Sponsored-by: TrueNAS Signed-off-by: Rob Norris --- tests/runfiles/common.run | 4 +- tests/runfiles/sanity.run | 2 +- tests/zfs-tests/tests/Makefile.am | 1 + .../cli_root/zfs_mount/zfs_mount_ro_rw.ksh | 130 ++++++++++++++++++ 4 files changed, 134 insertions(+), 3 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_ro_rw.ksh diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index 82a2d1e815e4..6e62b552a0db 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -271,8 +271,8 @@ tests = ['zfs_mount_001_pos', 'zfs_mount_002_pos', 'zfs_mount_003_pos', 'zfs_mount_004_pos', 'zfs_mount_005_pos', 'zfs_mount_007_pos', 'zfs_mount_009_neg', 'zfs_mount_010_neg', 'zfs_mount_011_neg', 'zfs_mount_012_pos', 'zfs_mount_all_001_pos', 'zfs_mount_encrypted', - 'zfs_mount_remount', 'zfs_mount_all_fail', 'zfs_mount_all_mountpoints', - 'zfs_mount_test_race', 'zfs_mount_recursive'] + 'zfs_mount_remount', 'zfs_mount_ro_rw', 'zfs_mount_all_fail', + 'zfs_mount_all_mountpoints', 'zfs_mount_test_race', 'zfs_mount_recursive'] tags = ['functional', 'cli_root', 'zfs_mount'] [tests/functional/cli_root/zfs_program] diff --git a/tests/runfiles/sanity.run b/tests/runfiles/sanity.run index d62f8f3fb161..0deaa038a31f 100644 --- a/tests/runfiles/sanity.run +++ b/tests/runfiles/sanity.run @@ -156,7 +156,7 @@ tests = ['zfs_mount_001_pos', 'zfs_mount_002_pos', 'zfs_mount_003_pos', 'zfs_mount_004_pos', 'zfs_mount_005_pos', 'zfs_mount_007_pos', 'zfs_mount_009_neg', 'zfs_mount_010_neg', 'zfs_mount_011_neg', 'zfs_mount_012_pos', 'zfs_mount_encrypted', 'zfs_mount_remount', - 'zfs_mount_all_fail', 'zfs_mount_all_mountpoints', + 'zfs_mount_ro_rw', 'zfs_mount_all_fail', 'zfs_mount_all_mountpoints', 'zfs_mount_test_race', 'zfs_mount_recursive'] tags = ['functional', 'cli_root', 'zfs_mount'] diff --git a/tests/zfs-tests/tests/Makefile.am b/tests/zfs-tests/tests/Makefile.am index 85f00f28b0f9..98f392538821 100644 --- a/tests/zfs-tests/tests/Makefile.am +++ b/tests/zfs-tests/tests/Makefile.am @@ -814,6 +814,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \ functional/cli_root/zfs_mount/zfs_mount_encrypted.ksh \ functional/cli_root/zfs_mount/zfs_mount_recursive.ksh \ functional/cli_root/zfs_mount/zfs_mount_remount.ksh \ + functional/cli_root/zfs_mount/zfs_mount_ro_rw.ksh \ functional/cli_root/zfs_mount/zfs_mount_test_race.ksh \ functional/cli_root/zfs_mount/zfs_multi_mount.ksh \ functional/cli_root/zfs_program/cleanup.ksh \ diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_ro_rw.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_ro_rw.ksh new file mode 100755 index 000000000000..15e78e6fd88c --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_ro_rw.ksh @@ -0,0 +1,130 @@ +#!/bin/ksh -p +# SPDX-License-Identifier: CDDL-1.0 +# +# CDDL HEADER START +# +# The contents of this file are subject to the terms of the +# Common Development and Distribution License (the "License"). +# You may not use this file except in compliance with the License. +# +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE +# or https://opensource.org/licenses/CDDL-1.0. +# See the License for the specific language governing permissions +# and limitations under the License. +# +# When distributing Covered Code, include this CDDL HEADER in each +# file and include the License file at usr/src/OPENSOLARIS.LICENSE. +# If applicable, add the following below this CDDL HEADER, with the +# fields enclosed by brackets "[]" replaced with your own identifying +# information: Portions Copyright [yyyy] [name of copyright owner] +# +# CDDL HEADER END +# + +# +# Copyright (c) 2026, TrueNAS. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/cli_root/zfs_mount/zfs_mount.kshlib + +# +# we set up and mount multiple times, with these combinations: +# - readonly property: on, off +# - mount method: mount(8) (mountpoint=legacy), zfs-mount(8) (mountpoint=path) +# - mount option: [none], ro, rw +# +# after each mount, we check whether we ended up mounting read-only or +# read-write, and note the result. once we've done them all, we compare the +# result set to the "correct" set for this platform (by observation). the +# test passes if they match, fail if they don't +# +# readonly | on | off | +# mount method | legacy | path | legacy | path | +# mount option | -- ro rw | -- ro rw | -- ro rw | -- ro rw | +typeset -a rs_linux=( rw ro rw ro ro rw rw ro rw rw ro rw ) +typeset -a rs_freebsd=( ro ro ro ro ro rw rw ro rw rw ro rw ) + +if is_linux ; then + typeset -n rs_wanted=rs_linux +elif is_freebsd ; then + typeset -n rs_wanted=rs_freebsd +else + log_unsupported "no result set defined for this platform" +fi + +verify_runnable "both" + +testfs=$TESTPOOL/$TESTFS +testmnt=$TESTDIR/mountpoint + +function cleanup +{ + log_must zfs inherit -S canmount $testfs + log_must zfs inherit readonly $testfs + log_must zfs inherit mountpoint $testfs + log_must rm -rf $testmnt +} + +log_assert "Verify combinations of readonly/readwrite produce correct mount." + +log_onexit cleanup + + +# setup +log_must datasetexists $testfs +log_must zfs set canmount=noauto $testfs +umount $testfs + + +typeset -a rs=() + +for readonly in on off ; do + for method in legacy path ; do + for option in default ro rw ; do + + log_must zfs set readonly=$readonly $testfs + + if [[ $method == 'legacy' ]] ; then + log_must zfs set mountpoint=legacy $testfs + else + log_must zfs set mountpoint=$testmnt $testfs + fi + + # recreate the mountpoint. even if it wasn't mounted, + # changing the mountpoint property can remove it + log_must mkdir -p $testmnt + + # issue the mount with the wanted method and option + case $method in + legacy) + case $option in + default) log_must mount_default $testfs $testmnt ;; + ro) log_must mount_ro $testfs $testmnt ;; + rw) log_must mount_rw $testfs $testmnt ;; + esac + ;; + path) + case $option in + default) log_must zfs mount $testfs ;; + ro) log_must zfs mount -o ro $testfs ;; + rw) log_must zfs mount -o rw $testfs ;; + esac + ;; + esac + + result=$(mount_get_ro_rw $testmnt) + rs+=($result) + log_note "result: $result" + + log_must umount $testfs + done + done +done + +log_note "results: ${rs[@]}" +log_note "wanted: ${rs_wanted[@]}" + +log_must test "${rs[*]}" == "${rs_wanted[*]}" + +log_pass "All mounts correct for this platform." From 2ecaacff41d6cc9c08c65530d3d7401500a9c817 Mon Sep 17 00:00:00 2001 From: Rob Norris Date: Wed, 20 May 2026 09:42:19 +1000 Subject: [PATCH 3/3] linux/super: properly apply ro/rw mount option to superblock f5a9e3a622 changed how SB_RDONLY was applied to the new mount in a way that was too simplistic - it only sets readonly on the filesystem if the mount was 'ro', but it never clears it if the mount was 'rw'. This causes the 'rw' option to effectively be ignored, and so the readonly= property wins out. This fixes it by doing it the right way: checking the flags mask to see if it was actually provided as an option at all, and then setting or clearing it as appropriate. Sponsored-by: TrueNAS Signed-off-by: Rob Norris --- module/os/linux/zfs/zpl_super.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/module/os/linux/zfs/zpl_super.c b/module/os/linux/zfs/zpl_super.c index c1460edd16d0..e1fa0f8e88e8 100644 --- a/module/os/linux/zfs/zpl_super.c +++ b/module/os/linux/zfs/zpl_super.c @@ -883,9 +883,14 @@ zpl_get_tree(struct fs_context *fc) if (sb->s_root == NULL) { vfs_t *vfs = fc->fs_private; - /* Apply readonly flag as mount option */ - if (fc->sb_flags & SB_RDONLY) { - vfs->vfs_readonly = B_TRUE; + /* + * If SB_RDONLY was set/cleared from mount options, update + * them in the options struct so we set up the filesystem + * in the proper state. + */ + if (fc->sb_flags_mask & SB_RDONLY) { + vfs->vfs_readonly = + (fc->sb_flags & SB_RDONLY) ? B_TRUE : B_FALSE; vfs->vfs_do_readonly = B_TRUE; }