diff --git a/include/sys/fs/zfs.h b/include/sys/fs/zfs.h index e95e8ce68..962698c2f 100644 --- a/include/sys/fs/zfs.h +++ b/include/sys/fs/zfs.h @@ -924,6 +924,8 @@ typedef struct ddt_histogram { #define ZFS_DEV "/dev/zfs" #define ZFS_SHARETAB "/etc/dfs/sharetab" +#define ZFS_SUPER_MAGIC 0x2fc12fc1 + /* general zvol path */ #define ZVOL_DIR "/dev" diff --git a/include/sys/zfs_vfsops.h b/include/sys/zfs_vfsops.h index 870eb9727..e38cadc33 100644 --- a/include/sys/zfs_vfsops.h +++ b/include/sys/zfs_vfsops.h @@ -119,8 +119,6 @@ typedef struct zfs_sb { kmutex_t *z_hold_locks; /* znode hold locks */ } zfs_sb_t; -#define ZFS_SUPER_MAGIC 0x2fc12fc1 - #define ZSB_XATTR 0x0001 /* Enable user xattrs */ /* diff --git a/lib/libzfs/libzfs_mount.c b/lib/libzfs/libzfs_mount.c index 5fc96f1ab..312724d4d 100644 --- a/lib/libzfs/libzfs_mount.c +++ b/lib/libzfs/libzfs_mount.c @@ -75,6 +75,7 @@ #include #include #include +#include #include @@ -170,13 +171,32 @@ is_shared(libzfs_handle_t *hdl, const char *mountpoint, zfs_share_proto_t proto) return (SHARED_NOT_SHARED); } -/* - * Returns true if the specified directory is empty. If we can't open the - * directory at all, return true so that the mount can fail with a more - * informative error message. - */ static boolean_t -dir_is_empty(const char *dirname) +dir_is_empty_stat(const char *dirname) +{ + struct stat st; + + /* + * We only want to return false if the given path is a non empty + * directory, all other errors are handled elsewhere. + */ + if (stat(dirname, &st) < 0 || !S_ISDIR(st.st_mode)) { + return (B_TRUE); + } + + /* + * An empty directory will still have two entries in it, one + * entry for each of "." and "..". + */ + if (st.st_size > 2) { + return (B_FALSE); + } + + return (B_TRUE); +} + +static boolean_t +dir_is_empty_readdir(const char *dirname) { DIR *dirp; struct dirent64 *dp; @@ -205,6 +225,42 @@ dir_is_empty(const char *dirname) return (B_TRUE); } +/* + * Returns true if the specified directory is empty. If we can't open the + * directory at all, return true so that the mount can fail with a more + * informative error message. + */ +static boolean_t +dir_is_empty(const char *dirname) +{ + struct statfs64 st; + + /* + * If the statvfs call fails or the filesystem is not a ZFS + * filesystem, fall back to the slow path which uses readdir. + */ + if ((statfs64(dirname, &st) != 0) || + (st.f_type != ZFS_SUPER_MAGIC)) { + return (dir_is_empty_readdir(dirname)); + } + + /* + * At this point, we know the provided path is on a ZFS + * filesystem, so we can use stat instead of readdir to + * determine if the directory is empty or not. We try to avoid + * using readdir because that requires opening "dirname"; this + * open file descriptor can potentially end up in a child + * process if there's a concurrent fork, thus preventing the + * zfs_mount() from otherwise succeeding (the open file + * descriptor inherited by the child process will cause the + * parent's mount to fail with EBUSY). The performance + * implications of replacing the open, read, and close with a + * single stat is nice; but is not the main motivation for the + * added complexity. + */ + return (dir_is_empty_stat(dirname)); +} + /* * Checks to see if the mount is active. If the filesystem is mounted, we fill * in 'where' with the current mountpoint, and return 1. Otherwise, we return diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run index f7477981c..f09001d24 100644 --- a/tests/runfiles/linux.run +++ b/tests/runfiles/linux.run @@ -131,7 +131,7 @@ tests = ['zfs_inherit_001_neg', 'zfs_inherit_002_neg'] [tests/functional/cli_root/zfs_mount] tests = ['zfs_mount_001_pos', 'zfs_mount_002_pos', 'zfs_mount_003_pos', 'zfs_mount_004_pos', 'zfs_mount_005_pos', 'zfs_mount_008_pos', - 'zfs_mount_010_neg', 'zfs_mount_011_neg'] + 'zfs_mount_010_neg', 'zfs_mount_011_neg', 'zfs_mount_012_neg'] [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 c6ae99c6b..f26120e2f 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 @@ -15,4 +15,5 @@ dist_pkgdata_SCRIPTS = \ zfs_mount_009_neg.ksh \ zfs_mount_010_neg.ksh \ zfs_mount_011_neg.ksh \ + zfs_mount_012_neg.ksh \ zfs_mount_all_001_pos.ksh diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_012_neg.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_012_neg.ksh new file mode 100755 index 000000000..ec1ff433a --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_012_neg.ksh @@ -0,0 +1,50 @@ +#!/bin/ksh -p +# +# 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 of the CDDL is also available via the Internet at +# http://www.illumos.org/license/CDDL. +# + +# +# Copyright (c) 2015 by Delphix. All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib + +# +# DESCRIPTION: +# Verify that zfs mount should fail with a non-empty directory +# +# STRATEGY: +# 1. Unmount the dataset +# 2. Create a new empty directory +# 3. Set the dataset's mountpoint +# 4. Attempt to mount the dataset +# 5. Verify the mount succeeds +# 6. Unmount the dataset +# 7. Create a file in the directory created in step 2 +# 8. Attempt to mount the dataset +# 9. Verify the mount fails +# + +verify_runnable "both" + +log_assert "zfs mount fails with non-empty directory" + +fs=$TESTPOOL/$TESTFS + +log_must $ZFS umount $fs +log_must mkdir -p $TESTDIR +log_must $ZFS set mountpoint=$TESTDIR $fs +log_must $ZFS mount $fs +log_must $ZFS umount $fs +log_must touch $TESTDIR/testfile.$$ +log_mustnot $ZFS mount $fs +log_must rm -rf $TESTDIR + +log_pass "zfs mount fails non-empty directory as expected."