Skip to content

Commit

Permalink
Allow mounting datasets more than once
Browse files Browse the repository at this point in the history
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.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Tom Caputi <tcaputi@datto.com>
Signed-off-by: Alek Pinchuk <apinchuk@datto.com>
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Closes openzfs#5796
Closes openzfs#7207
  • Loading branch information
Seth Forshee authored and behlendorf committed Apr 13, 2018
1 parent 1e37dee commit 93b43af
Show file tree
Hide file tree
Showing 8 changed files with 219 additions and 29 deletions.
28 changes: 28 additions & 0 deletions config/kernel-fst-mount.m4
Original file line number Diff line number Diff line change
@@ -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 <linux/fs.h>
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)
])
])
20 changes: 0 additions & 20 deletions config/kernel-mount-nodev.m4

This file was deleted.

2 changes: 1 addition & 1 deletion config/kernel.m4
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,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
Expand Down
24 changes: 24 additions & 0 deletions include/linux/vfs_compat.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
65 changes: 59 additions & 6 deletions module/zfs/zpl_super.c
Original file line number Diff line number Diff line change
Expand Up @@ -248,14 +248,61 @@ 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;

if (zfsvfs == NULL)
return (0);

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) {
deactivate_locked_super(s);
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);
if (IS_ERR(sb))
return (ERR_CAST(sb));

return (dget(sb->s_root));
}
#else
static int
Expand All @@ -264,9 +311,15 @@ zpl_get_sb(struct file_system_type *fs_type, int flags,
{
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));

(void) simple_set_mnt(mnt, sb);

return (0);
}
#endif /* HAVE_MOUNT_NODEV */
#endif /* HAVE_FST_MOUNT */

static void
zpl_kill_sb(struct super_block *sb)
Expand Down Expand Up @@ -333,10 +386,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,
};
2 changes: 1 addition & 1 deletion tests/runfiles/linux.run
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,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_remount']
'zfs_mount_encrypted', 'zfs_mount_remount', 'zfs_multi_mount']
tags = ['functional', 'cli_root', 'zfs_mount']

[tests/functional/cli_root/zfs_program]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ 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

dist_pkgdata_DATA = \
zfs_mount.cfg \
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
#!/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.
#
# 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 prevent rename
#

verify_runnable "both"

function cleanup
{
ismounted $MNTPFS && log_must umount $MNTPFS
ismounted $MNTPFS2 && log_must umount $MNTPFS2
ismounted $MNTPFS3 && log_must umount $MNTPFS3
ismounted $MNTPFS4 && log_must umount $MNTPFS4
ismounted $RENAMEMNT && log_must umount $RENAMEMNT
datasetexists $TESTDS && log_must destroy_dataset "$TESTDS" "-f"
}
log_onexit cleanup

log_assert "Verify multiple mounts into one namespace are possible"

# 1. Create fs
TESTDS="$TESTPOOL/multi-mount-test"
log_must zfs create $TESTDS

# 2. Create and hold open file in filesystem
MNTPFS="$(get_prop mountpoint $TESTDS)"
FILENAME="$MNTPFS/file"
log_must mkfile 128k $FILENAME
log_must exec 9<> $FILENAME # open file

# 3. Lazy umount
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
if [ ! -f $FILENAME ]; then
log_fail "Lazy remount failed"
fi
log_must exec 9>&- # close fd

# 5. Verify multiple mounts of the same dataset are possible
MNTPFS2="$MNTPFS-second"
FILENAME="$MNTPFS2/file"
log_must mkdir $MNTPFS2
log_must mount -t zfs -o zfsutil $TESTDS $MNTPFS2
if [ ! -f $FILENAME ]; then
log_fail "First multi mount failed"
fi

MNTPFS3="$MNTPFS-third"
FILENAME="$MNTPFS3/file"
log_must mkdir $MNTPFS3
log_must mount -t zfs -o zfsutil $TESTDS $MNTPFS3
if [ ! -f $FILENAME ]; then
log_fail "Second multi mount failed"
fi

# 6. Verify bind mount doesn't prevent rename
RENAMEFS="$TESTDS-newname"
MNTPFS4="$MNTPFS-fourth"
log_must mkdir $MNTPFS4
log_must mount --bind $MNTPFS $MNTPFS4
log_must zfs rename $TESTDS $RENAMEFS
RENAMEMNT="$(get_prop mountpoint $RENAMEFS)"
FILENAME="$RENAMEMNT/file"
if [ ! -f $FILENAME ]; then
log_fail "Rename failed"
fi
log_must zfs rename $RENAMEFS $TESTDS

log_pass "Multiple mounts are possible"

0 comments on commit 93b43af

Please sign in to comment.