Skip to content

Commit

Permalink
Remove dependency on sharetab file and refactor sharing logic
Browse files Browse the repository at this point in the history
== Motivation and Context

The current implementation of 'sharenfs' and 'sharesmb' relies on
the use of the sharetab file. The use of this file is os-specific
and not required by linux or freebsd. Currently the code must
maintain updates to this file which adds complexity and presents
a significant performance impact when sharing many datasets. In
addition, concurrently running 'zfs sharenfs' command results in
missing entries in the sharetab file leading to unexpected failures.

== Description

This change removes the sharetab logic from the linux and freebsd
implementation of 'sharenfs' and 'sharesmb'. It still preserves an
os-specific library which contains the logic required for sharing
NFS or SMB. The following entry points exist in the vastly simplified
libshare library:

- sa_enable_share -- shares a dataset but may not commit the change
- sa_disable_share -- unshares a dataset but may not commit the change
- sa_is_shared -- determine if a dataset is shared
- sa_commit_share -- notify NFS/SMB subsystem to commit the shares
- sa_validate_shareopts -- determine if sharing options are valid

The sa_commit_share entry point is provided as a performance enhancement
and is not required. The sa_enable_share/sa_disable_share may commit
the share as part of the implementation. Libshare provides a framework
for both NFS and SMB but some operating systems may not fully support
these protocols or all features of the protocol.

NFS Operation:
For linux, libshare updates /etc/exports.d/zfs.exports to add
and remove shares and then commits the changes by invoking
'exportfs -r'. This file, is automatically read by the kernel NFS
implementation which makes for better integration with the NFS systemd
service. For FreeBSD, libshare updates /etc/zfs/exports to add and
remove shares and then commits the changes by sending a SIGHUP to
mountd.

SMB Operation:
For linux, libshare adds and removes files in /var/lib/samba/usershares
by calling the 'net' command directly. There is no need to commit the
changes. FreeBSD does not support SMB.

== Performance Results

To test sharing performance we created a pool with an increasing number
of datasets and invoked various zfs actions that would enable and
disable sharing. The performance testing was limited to NFS sharing.
The following tests were performed on an 8 vCPU system with 128GB and
a pool comprised of 4 50GB SSDs:

Scale testing:
- Share all filesystems in parallel -- zfs sharenfs=on <dataset> &
- Unshare all filesystems in parallel -- zfs sharenfs=off <dataset> &

Functional testing:
- share each filesystem serially -- zfs share -a
- unshare each filesystem serially -- zfs unshare -a
- reset sharenfs property and unshare -- zfs inherit -r sharenfs <pool>

For 'zfs sharenfs=on' scale testing we saw an average reduction in time
of 89.43% and for 'zfs sharenfs=off' we saw an average reduction in time
of 83.36%.

Functional testing also shows a huge improvement:
- zfs share -- 97.97% reduction in time
- zfs unshare -- 96.47% reduction in time
- zfs inhert -r sharenfs -- 99.01% reduction in time

Reviewed-by: Matt Ahrens <matt@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Ryan Moeller <ryan@ixsystems.com>
Reviewed-by: Bryant G. Ly <bryangly@gmail.com>
Signed-off-by: George Wilson <gwilson@delphix.com>
External-Issue: DLPX-68690
Closes #1603
Closes #7692
Closes #7943
Closes #10300
  • Loading branch information
grwilson authored Jul 13, 2020
1 parent e59a377 commit c15d36c
Show file tree
Hide file tree
Showing 30 changed files with 1,545 additions and 1,651 deletions.
8 changes: 8 additions & 0 deletions cmd/zfs/zfs_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,7 @@ zfs_mount_and_share(libzfs_handle_t *hdl, const char *dataset, zfs_type_t type)
"successfully created, but not shared\n"));
ret = 1;
}
zfs_commit_all_shares();
}

zfs_close(zhp);
Expand Down Expand Up @@ -6927,6 +6928,8 @@ share_mount(int op, int argc, char **argv)
zfs_foreach_mountpoint(g_zfs, cb.cb_handles, cb.cb_used,
share_mount_one_cb, &share_mount_state,
op == OP_MOUNT && !(flags & MS_CRYPT));
zfs_commit_all_shares();

ret = share_mount_state.sm_status;

for (int i = 0; i < cb.cb_used; i++)
Expand Down Expand Up @@ -6979,6 +6982,7 @@ share_mount(int op, int argc, char **argv)
} else {
ret = share_mount_one(zhp, op, flags, NULL, B_TRUE,
options);
zfs_commit_all_shares();
zfs_close(zhp);
}
}
Expand Down Expand Up @@ -7108,6 +7112,7 @@ unshare_unmount_path(int op, char *path, int flags, boolean_t is_manual)
"not currently shared\n"), path);
} else {
ret = zfs_unshareall_bypath(zhp, path);
zfs_commit_all_shares();
}
} else {
char mtpt_prop[ZFS_MAXPROPLEN];
Expand Down Expand Up @@ -7328,6 +7333,9 @@ unshare_unmount(int op, int argc, char **argv)
free(node);
}

if (op == OP_SHARE)
zfs_commit_shares(protocol);

uu_avl_walk_end(walk);
uu_avl_destroy(tree);
uu_avl_pool_destroy(pool);
Expand Down
6 changes: 4 additions & 2 deletions cmd/zpool/zpool_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
/*
* Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
* Copyright 2011 Nexenta Systems, Inc. All rights reserved.
* Copyright (c) 2011, 2018 by Delphix. All rights reserved.
* Copyright (c) 2011, 2020 by Delphix. All rights reserved.
* Copyright (c) 2012 by Frederik Wessels. All rights reserved.
* Copyright (c) 2012 by Cyril Plisko. All rights reserved.
* Copyright (c) 2013 by Prasad Joshi (sTec). All rights reserved.
Expand Down Expand Up @@ -1587,8 +1587,10 @@ zpool_do_create(int argc, char **argv)
zfs_handle_t *pool = zfs_open(g_zfs,
tname ? tname : poolname, ZFS_TYPE_FILESYSTEM);
if (pool != NULL) {
if (zfs_mount(pool, NULL, 0) == 0)
if (zfs_mount(pool, NULL, 0) == 0) {
ret = zfs_shareall(pool);
zfs_commit_all_shares();
}
zfs_close(pool);
}
} else if (libzfs_errno(g_zfs) == EZFS_INVALIDNAME) {
Expand Down
1 change: 0 additions & 1 deletion etc/systemd/system/zfs-share.service.in
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ PartOf=smb.service
[Service]
Type=oneshot
RemainAfterExit=yes
ExecStartPre=-/bin/rm -f /etc/dfs/sharetab
ExecStart=@sbindir@/zfs share -a

[Install]
Expand Down
4 changes: 4 additions & 0 deletions include/libzfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,10 @@ extern int zfs_unshareall_bytype(zfs_handle_t *, const char *, const char *);
extern int zfs_unshareall(zfs_handle_t *);
extern int zfs_deleg_share_nfs(libzfs_handle_t *, char *, char *, char *,
void *, void *, int, zfs_share_op_t);
extern void zfs_commit_nfs_shares(void);
extern void zfs_commit_smb_shares(void);
extern void zfs_commit_all_shares(void);
extern void zfs_commit_shares(const char *);

extern int zfs_nicestrtonum(libzfs_handle_t *, const char *, uint64_t *);

Expand Down
16 changes: 3 additions & 13 deletions include/libzfs_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ struct libzfs_handle {
int libzfs_error;
int libzfs_fd;
FILE *libzfs_mnttab;
FILE *libzfs_sharetab;
zpool_handle_t *libzfs_pool_handles;
uu_avl_pool_t *libzfs_ns_avlpool;
uu_avl_t *libzfs_ns_avl;
Expand All @@ -59,8 +58,6 @@ struct libzfs_handle {
char libzfs_desc[1024];
int libzfs_printerr;
int libzfs_storeerr; /* stuff error messages into buffer */
void *libzfs_sharehdl; /* libshare handle */
uint_t libzfs_shareflags;
boolean_t libzfs_mnttab_enable;
/*
* We need a lock to handle the case where parallel mount
Expand All @@ -76,8 +73,6 @@ struct libzfs_handle {
regex_t libzfs_urire;
};

#define ZFSSHARE_MISS 0x01 /* Didn't find entry in cache */

struct zfs_handle {
libzfs_handle_t *zfs_hdl;
zpool_handle_t *zpool_hdl;
Expand Down Expand Up @@ -206,12 +201,6 @@ int zfs_validate_name(libzfs_handle_t *hdl, const char *path, int type,

void namespace_clear(libzfs_handle_t *);

/*
* libshare (sharemgr) interfaces used internally.
*/

extern int zfs_init_libshare(libzfs_handle_t *, int);
extern void zfs_uninit_libshare(libzfs_handle_t *);
extern int zfs_parse_options(char *, zfs_share_proto_t);

extern int zfs_unshare_proto(zfs_handle_t *,
Expand Down Expand Up @@ -256,13 +245,14 @@ extern int unshare_one(libzfs_handle_t *hdl, const char *name,
const char *mountpoint, zfs_share_proto_t proto);
extern boolean_t zfs_is_mountable(zfs_handle_t *zhp, char *buf, size_t buflen,
zprop_source_t *source, int flags);
extern zfs_share_type_t is_shared_impl(libzfs_handle_t *hdl,
const char *mountpoint, zfs_share_proto_t proto);
extern zfs_share_type_t is_shared(const char *mountpoint,
zfs_share_proto_t proto);
extern int libzfs_load_module(void);
extern int zpool_relabel_disk(libzfs_handle_t *hdl, const char *path,
const char *msg);
extern int find_shares_object(differ_info_t *di);
extern void libzfs_set_pipe_max(int infd);
extern void zfs_commit_proto(zfs_share_proto_t *);

#ifdef __cplusplus
}
Expand Down
1 change: 0 additions & 1 deletion include/sys/fs/zfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -1170,7 +1170,6 @@ typedef struct ddt_histogram {
#define ZVOL_DRIVER "zvol"
#define ZFS_DRIVER "zfs"
#define ZFS_DEV "/dev/zfs"
#define ZFS_SHARETAB "/etc/dfs/sharetab"

#define ZFS_SUPER_MAGIC 0x2fc12fc1

Expand Down
16 changes: 14 additions & 2 deletions lib/libshare/Makefile.am
Original file line number Diff line number Diff line change
@@ -1,13 +1,25 @@
include $(top_srcdir)/config/Rules.am

DEFAULT_INCLUDES += -I$(srcdir)

noinst_LTLIBRARIES = libshare.la

USER_C = \
libshare_impl.h \
libshare.c \
nfs.c \
nfs.h \
smb.c \
smb.h

if BUILD_LINUX
USER_C += \
os/linux/nfs.c \
os/linux/smb.c
endif

if BUILD_FREEBSD
USER_C += \
os/freebsd/nfs.c \
os/freebsd/smb.c
endif

libshare_la_SOURCES = $(USER_C)
Loading

0 comments on commit c15d36c

Please sign in to comment.