From 058ed293e074360c7f26d32d818505cd38e92b69 Mon Sep 17 00:00:00 2001 From: Seth Forshee Date: Wed, 21 Feb 2018 20:41:43 -0500 Subject: [PATCH] Allow mounting datasets more than once Currently mounting an already mounted zfs dataset results in an error, whereas it is typically allowed with other filesystems. This causes some bad interactions with mount namespaces. Take this sequence for example: - Create a dataset - Create a snapshot of the dataset - Create a clone of the snapshot - Create a new mount namespace - Rename the original dataset The rename results in unmounting and remounting the clone in the original mount namespace, however the remount fails because the dataset is still mounted in the new mount namespace. (Note that this means the mount in the new mount namespace is never being unmounted, so perhaps the unmount/remount of the clone isn't actually necessary.) The problem here is a result of the way mounting is implemented in the kernel module. Since it is not mounting block devices it uses mount_nodev() instead of the usual mount_bdev(). However, mount_nodev() is written for filesystems for which each mount is a new instance (i.e. a new super block), and zfs should be able to detect when a mount request can be satisfied using an existing super block. Change zpl_mount() to call sget() directly with it's own test callback. Passing the objset_t object as the fs data allows checking if a superblock already exists for the dataset, and in that case we just need to return a new reference for the sb's root dentry. Signed-off-by: Seth Forshee Closes #5796 --- config/kernel-fst-mount.m4 | 28 ++++++ config/kernel-mount-nodev.m4 | 20 ----- config/kernel.m4 | 2 +- include/linux/vfs_compat.h | 24 +++++ module/zfs/zpl_super.c | 57 ++++++++++-- .../functional/cli_root/zfs_mount/Makefile.am | 3 +- .../cli_root/zfs_mount/zfs_multi_mount.ksh | 88 +++++++++++++++++++ 7 files changed, 192 insertions(+), 30 deletions(-) create mode 100644 config/kernel-fst-mount.m4 delete mode 100644 config/kernel-mount-nodev.m4 create mode 100755 tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_multi_mount.ksh diff --git a/config/kernel-fst-mount.m4 b/config/kernel-fst-mount.m4 new file mode 100644 index 000000000000..a8ac50bdd5d9 --- /dev/null +++ b/config/kernel-fst-mount.m4 @@ -0,0 +1,28 @@ +dnl # +dnl # 2.6.38 API change +dnl # The .get_sb callback has been replaced by a .mount callback +dnl # in the file_system_type structure. +dnl # +AC_DEFUN([ZFS_AC_KERNEL_FST_MOUNT], [ + AC_MSG_CHECKING([whether fst->mount() exists]) + ZFS_LINUX_TRY_COMPILE([ + #include + + static struct dentry * + mount(struct file_system_type *fs_type, int flags, + const char *osname, void *data) { + struct dentry *d = NULL; + return (d); + } + + static struct file_system_type fst __attribute__ ((unused)) = { + .mount = mount, + }; + ],[ + ],[ + AC_MSG_RESULT(yes) + AC_DEFINE(HAVE_FST_MOUNT, 1, [fst->mount() exists]) + ],[ + AC_MSG_RESULT(no) + ]) +]) diff --git a/config/kernel-mount-nodev.m4 b/config/kernel-mount-nodev.m4 deleted file mode 100644 index 28a45157e4d0..000000000000 --- a/config/kernel-mount-nodev.m4 +++ /dev/null @@ -1,20 +0,0 @@ -dnl # -dnl # 2.6.39 API change -dnl # The .get_sb callback has been replaced by a .mount callback -dnl # in the file_system_type structure. When using the new -dnl # interface the caller must now use the mount_nodev() helper. -dnl # This updated callback and helper no longer pass the vfsmount. -dnl # -AC_DEFUN([ZFS_AC_KERNEL_MOUNT_NODEV], - [AC_MSG_CHECKING([whether mount_nodev() is available]) - ZFS_LINUX_TRY_COMPILE_SYMBOL([ - #include - ], [ - mount_nodev(NULL, 0, NULL, NULL); - ], [mount_nodev], [fs/super.c], [ - AC_MSG_RESULT(yes) - AC_DEFINE(HAVE_MOUNT_NODEV, 1, [mount_nodev() is available]) - ], [ - AC_MSG_RESULT(no) - ]) -]) diff --git a/config/kernel.m4 b/config/kernel.m4 index f8d81cdc0fb5..fb7a1bfa15a0 100644 --- a/config/kernel.m4 +++ b/config/kernel.m4 @@ -98,7 +98,7 @@ AC_DEFUN([ZFS_AC_CONFIG_KERNEL], [ ZFS_AC_KERNEL_TRUNCATE_SETSIZE ZFS_AC_KERNEL_6ARGS_SECURITY_INODE_INIT_SECURITY ZFS_AC_KERNEL_CALLBACK_SECURITY_INODE_INIT_SECURITY - ZFS_AC_KERNEL_MOUNT_NODEV + ZFS_AC_KERNEL_FST_MOUNT ZFS_AC_KERNEL_SHRINK ZFS_AC_KERNEL_SHRINK_CONTROL_HAS_NID ZFS_AC_KERNEL_S_INSTANCES_LIST_HEAD diff --git a/include/linux/vfs_compat.h b/include/linux/vfs_compat.h index 6111f0afca1d..ae77af841cb5 100644 --- a/include/linux/vfs_compat.h +++ b/include/linux/vfs_compat.h @@ -182,6 +182,30 @@ zpl_bdi_destroy(struct super_block *sb) } #endif +/* + * 4.14 adds SB_* flag definitions, define them to MS_* equivalents + * if not set. + */ +#ifndef SB_RDONLY +#define SB_RDONLY MS_RDONLY +#endif + +#ifndef SB_SILENT +#define SB_SILENT MS_SILENT +#endif + +#ifndef SB_ACTIVE +#define SB_ACTIVE MS_ACTIVE +#endif + +#ifndef SB_POSIXACL +#define SB_POSIXACL MS_POSIXACL +#endif + +#ifndef SB_MANDLOCK +#define SB_MANDLOCK MS_MANDLOCK +#endif + /* * 2.6.38 API change, * LOOKUP_RCU flag introduced to distinguish rcu-walk from ref-walk cases. diff --git a/module/zfs/zpl_super.c b/module/zfs/zpl_super.c index b6ef60277664..5418f840d4dd 100644 --- a/module/zfs/zpl_super.c +++ b/module/zfs/zpl_super.c @@ -248,14 +248,53 @@ zpl_fill_super(struct super_block *sb, void *data, int silent) return (error); } -#ifdef HAVE_MOUNT_NODEV +static int +zpl_test_super(struct super_block *s, void *data) +{ + zfsvfs_t *zfsvfs = s->s_fs_info; + objset_t *os = data; + + return (os == zfsvfs->z_os); +} + +static struct super_block * +zpl_mount_impl(struct file_system_type *fs_type, int flags, zfs_mnt_t *zm) +{ + struct super_block *s; + objset_t *os; + int err; + + err = dmu_objset_hold(zm.mnt_osname, FTAG, &os); + if (err) + return (ERR_PTR(-err)); + + s = zpl_sget(fs_type, zpl_test_super, set_anon_super, flags, os); + dmu_objset_rele(os, FTAG); + if (IS_ERR(s)) + return (ERR_CAST(s)); + + if (s->s_root == NULL) { + err = zpl_fill_super(s, &zm, flags & SB_SILENT ? 1 : 0); + if (err) { + deactivate_locked_super(s); + return (ERR_PTR(err)); + } + s->s_flags |= SB_ACTIVE; + } else if ((flags ^ s->s_flags) & SB_RDONLY) { + return (ERR_PTR(-EBUSY)); + } + + return (s); +} + +#ifdef HAVE_FST_MOUNT static struct dentry * zpl_mount(struct file_system_type *fs_type, int flags, const char *osname, void *data) { zfs_mnt_t zm = { .mnt_osname = osname, .mnt_data = data }; - - return (mount_nodev(fs_type, flags, &zm, zpl_fill_super)); + struct super_block *sb = zpl_mount_impl(fs_type, flags, &zm); + return (dget(sb->s_root)); } #else static int @@ -263,10 +302,12 @@ zpl_get_sb(struct file_system_type *fs_type, int flags, const char *osname, void *data, struct vfsmount *mnt) { zfs_mnt_t zm = { .mnt_osname = osname, .mnt_data = data }; - - return (get_sb_nodev(fs_type, flags, &zm, zpl_fill_super, mnt)); + struct super_block *sb = zpl_mount_impl(fs_type, flags, &zm); + if (IS_ERR(sb)) + return (PTR_ERR(sb)); + return (simple_set_mnt(mnt, sb)); } -#endif /* HAVE_MOUNT_NODEV */ +#endif /* HAVE_FST_MOUNT */ static void zpl_kill_sb(struct super_block *sb) @@ -333,10 +374,10 @@ const struct super_operations zpl_super_operations = { struct file_system_type zpl_fs_type = { .owner = THIS_MODULE, .name = ZFS_DRIVER, -#ifdef HAVE_MOUNT_NODEV +#ifdef HAVE_FST_MOUNT .mount = zpl_mount, #else .get_sb = zpl_get_sb, -#endif /* HAVE_MOUNT_NODEV */ +#endif /* HAVE_FST_MOUNT */ .kill_sb = zpl_kill_sb, }; 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 85090a534c1e..939a0ac973c1 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 @@ -18,4 +18,5 @@ dist_pkgdata_SCRIPTS = \ zfs_mount_012_neg.ksh \ zfs_mount_encrypted.ksh \ zfs_mount_all_001_pos.ksh \ - zfs_mount_remount.ksh + zfs_mount_remount.ksh \ + zfs_multi_mount.ksh diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_multi_mount.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_multi_mount.ksh new file mode 100755 index 000000000000..d5af47ca8356 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_multi_mount.ksh @@ -0,0 +1,88 @@ +#!/bin/ksh -p +# +# CDDL HEADER START +# +# This file and its contents are supplied under the terms of the +# Common Development and Distribution License ("CDDL"), version 1.0. +# You may only use this file in accordance with the terms of version +# 1.0 of the CDDL. +# +# A full copy of the text of the CDDL should have accompanied this +# source. A copy is of the CDDL is also available via the Internet +# at http://www.illumos.org/license/CDDL. +# source. A copy of the CDDL is also available via the Internet at +# http://www.illumos.org/license/CDDL. +# +# CDDL HEADER END +# + +# +# Copyright(c) 2018 Datto Inc. +# + +. $STF_SUITE/include/libtest.shlib + +# +# DESCRIPTION: +# Verify multi mount functionality +# +# STRATEGY: +# 1. Create fs +# 2. Create and hold open file in filesystem +# 3. Lazy unmount +# 4. Verify remounting fs that was lazily unmounted is possible +# 5. Verify multiple mounts of the same dataset are possible +# 6. Verify bind mount doesn't block rename +# + +verify_runnable "both" + +function cleanup +{ + log_must pkill -P $TAILPPID tail + return 0 +} + +log_assert "Verify multiple mounts into one namespace are possible" + +log_onexit cleanup + +# 1. Create fs +TESTDS="$TESTPOOL/multi-mount-test" +log_must zfs create $TESTDS +MNTPFS="$(get_prop mountpoint $TESTDS)" + +# 2. Create and hold open file in filesystem +FILENAME="$MNTPFS/file" +log_must mkfile 128k $FILENAME +log_must tail -f $FILENAME 1>/dev/null 2>/dev/null & +TAILPPID=$! + +# 3. Lazy umount +MNTPFS2="$MNTPFS-second" +MNTPFS3="$MNTPFS-third" +log_must umount -l $MNTPFS +if [ -f $FILENAME ]; then + log_fail "Lazy unmount failed" +fi + +# 4. Verify remounting fs that was lazily unmounted is possible +log_must zfs mount $TESTDS + +# 5. Verify multiple mounts of the same dataset are possible +log_must mkdir $MNTPFS2 +log_must mount -t zfs -o zfsutil $TESTDS $MNTPFS2 +log_must mkdir $MNTPFS3 +log_must mount -t zfs -o zfsutil $TESTDS $MNTPFS3 + +# 6. Verify bind mount doesn't block rename +CLONEFS="$TESTDS-clone" +RENAMEFS="$TESTDS-newname" +TESTSNAP="$TESTDS@snap" +MNTPFS4="$MNTPFS-fourth" +log_must mkdir $MNTPFS4 +log_must mount --bind $MNTPFS $MNTPFS4 +log_must zfs rename $TESTDS $RENAMEFS +log_must zfs rename $RENAMEFS $TESTDS + +log_pass "Multiple mounts are possible"