From 08de8c16f5d322fb594742ea78958385d8ee5b50 Mon Sep 17 00:00:00 2001 From: LOLi Date: Thu, 17 Aug 2017 23:28:17 +0200 Subject: [PATCH] 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 Reviewed-by: Brian Behlendorf Signed-off-by: loli10K Closes #6510 Closes #6515 --- module/zfs/zfs_vfsops.c | 10 +- tests/runfiles/linux.run | 2 +- .../functional/cli_root/zfs_mount/Makefile.am | 3 +- .../cli_root/zfs_mount/zfs_mount_remount.ksh | 144 ++++++++++++++++++ 4 files changed, 156 insertions(+), 3 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_remount.ksh diff --git a/module/zfs/zfs_vfsops.c b/module/zfs/zfs_vfsops.c index b60045a95cf8..33557157b61b 100644 --- a/module/zfs/zfs_vfsops.c +++ b/module/zfs/zfs_vfsops.c @@ -1763,8 +1763,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); @@ -1774,7 +1781,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 c28710a4ab77..cb8e2841f678 100644 --- a/tests/runfiles/linux.run +++ b/tests/runfiles/linux.run @@ -119,7 +119,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_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_encrypted'] + 'zfs_mount_encrypted', '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 5b9bb937c176..85090a534c1e 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/Makefile.am +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/Makefile.am @@ -17,4 +17,5 @@ dist_pkgdata_SCRIPTS = \ zfs_mount_011_neg.ksh \ zfs_mount_012_neg.ksh \ zfs_mount_encrypted.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 index 000000000000..ab51020aa853 --- /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 . 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."