Fix remounting snapshots read-write

It's not enough to preserve/restore MS_RDONLY on the superblock flags
to avoid remounting a snapshot read-write: be explicit about our
intentions to the VFS layer so the readonly bit is updated correctly
in do_remount_sb().

Reviewed-by: Chunwei Chen <tuxoko@gmail.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Closes #6510
Closes #6515

diff --git a/module/zfs/zfs_vfsops.c b/module/zfs/zfs_vfsops.c
index 0e3f37781bd4f20fbc4aba9da95ab323f069eb81..f97660f37a69466f70369a40bda3d60cd5980fa5 100644 (file)
--- a/module/zfs/zfs_vfsops.c
+++ b/module/zfs/zfs_vfsops.c
@@ -1761,8 +1761,15 @@ zfs_remount(struct super_block *sb, int *flags, zfs_mnt_t *zm)
 {
        zfsvfs_t *zfsvfs = sb->s_fs_info;
        vfs_t *vfsp;
+       boolean_t issnap = dmu_objset_is_snapshot(zfsvfs->z_os);
        int error;
 
+       if ((issnap || !spa_writeable(dmu_objset_spa(zfsvfs->z_os))) &&
+           !(*flags & MS_RDONLY)) {
+               *flags |= MS_RDONLY;
+               return (EROFS);
+       }
+
        error = zfsvfs_parse_options(zm->mnt_data, &vfsp);
        if (error)
                return (error);
@@ -1772,7 +1779,8 @@ zfs_remount(struct super_block *sb, int *flags, zfs_mnt_t *zm)
 
        vfsp->vfs_data = zfsvfs;
        zfsvfs->z_vfs = vfsp;
-       (void) zfs_register_callbacks(vfsp);
+       if (!issnap)
+               (void) zfs_register_callbacks(vfsp);
 
        return (error);
 }

diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run
index 708cb43548602fc5548debd1c518e46a754ede90..820e5604826140c5b1b4066255b59d149cef3883 100644 (file)
--- a/tests/runfiles/linux.run
+++ b/tests/runfiles/linux.run
@@ -108,7 +108,8 @@ tests = ['zfs_inherit_001_neg', 'zfs_inherit_002_neg', 'zfs_inherit_003_pos']
 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_008_pos', 'zfs_mount_009_neg', 'zfs_mount_010_neg',
-    'zfs_mount_011_neg', 'zfs_mount_012_neg', 'zfs_mount_all_001_pos']
+    'zfs_mount_011_neg', 'zfs_mount_012_neg', 'zfs_mount_all_001_pos',
+    'zfs_mount_remount']
 
 [tests/functional/cli_root/zfs_promote]
 tests = ['zfs_promote_001_pos', 'zfs_promote_002_pos', 'zfs_promote_003_pos',

diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/Makefile.am b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/Makefile.am
index f26120e2fdd3a9150d3b298a7e01cdfa4fbe4dbe..f8850406b014d7c8ebb8f9a7462579e7c706be0c 100644 (file)
--- a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/Makefile.am
+++ b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/Makefile.am
@@ -16,4 +16,5 @@ dist_pkgdata_SCRIPTS = \
        zfs_mount_010_neg.ksh \
        zfs_mount_011_neg.ksh \
        zfs_mount_012_neg.ksh \
-       zfs_mount_all_001_pos.ksh
+       zfs_mount_all_001_pos.ksh \
+       zfs_mount_remount.ksh

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
new file mode 100755 (executable)
index 0000000..ab51020
--- /dev/null
+++ b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_remount.ksh
@@ -0,0 +1,144 @@
+#!/bin/ksh -p
+#
+# 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 http://www.opensolaris.org/os/licensing.
+# 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 2017, loli10K <ezomori.nozomu@gmail.com>. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/cli_root/zfs_mount/zfs_mount.kshlib
+
+#
+# DESCRIPTION:
+# Verify remount functionality, expecially on readonly objects.
+#
+# STRATEGY:
+# 1. Prepare a filesystem and a snapshot
+# 2. Verify we can (re)mount the dataset readonly/read-write
+# 3. Verify we can mount the snapshot and it's mounted readonly
+# 4. Verify we can't remount it read-write
+# 5. Re-import the pool readonly
+# 6. Verify we can't remount its filesystem read-write
+#
+
+verify_runnable "both"
+
+function cleanup
+{
+       log_must zpool export $TESTPOOL
+       log_must zpool import $TESTPOOL
+       snapexists $TESTSNAP && log_must zfs destroy $TESTSNAP
+       [[ -d $MNTPSNAP ]] && log_must rmdir $MNTPSNAP
+       return 0
+}
+
+#
+# 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"
+
+       options="$(awk -v ds="$dataset" '$1 == ds { print $4 }' /proc/mounts)"
+       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
+
+# 1. Prepare a filesystem and a snapshot
+TESTFS=$TESTPOOL/$TESTFS
+TESTSNAP="$TESTFS@snap"
+datasetexists $TESTFS || log_must zfs create $TESTFS
+snapexists $TESTSNAP || log_must zfs snapshot $TESTSNAP
+log_must zfs set readonly=off $TESTFS
+MNTPFS="$(get_prop mountpoint $TESTFS)"
+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 -o remount,ro $TESTFS $MNTPFS
+readonlyfs $MNTPFS
+checkmount $TESTFS 'ro'
+log_must mount -o remount,rw $TESTFS $MNTPFS
+log_must touch $MNTPFS/file.dat
+checkmount $TESTFS 'rw'
+
+# 3. Verify we can (re)mount the snapshot readonly
+log_must mount -t zfs $TESTSNAP $MNTPSNAP
+readonlyfs $MNTPSNAP
+checkmount $TESTSNAP 'ro'
+log_must mount -o remount,ro $TESTSNAP $MNTPSNAP
+readonlyfs $MNTPSNAP
+checkmount $TESTSNAP 'ro'
+log_must umount $MNTPSNAP
+
+# 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 -o remount,rw $TESTSNAP $MNTPSNAP
+readonlyfs $MNTPSNAP
+checkmount $TESTSNAP 'ro'
+log_must umount $MNTPSNAP
+
+# 5. Re-import the pool readonly
+log_must zpool export $TESTPOOL
+log_must zpool import -o readonly=on $TESTPOOL
+
+# 6. Verify we can't remount its filesystem read-write
+readonlyfs $MNTPFS
+checkmount $TESTFS 'ro'
+log_mustnot mount -o remount,rw $MNTPFS
+readonlyfs $MNTPFS
+checkmount $TESTFS 'ro'
+
+log_pass "Both filesystem and snapshots can be remounted correctly."