Skip to content

Commit

Permalink
Add '-u' - nomount flag for zfs set
Browse files Browse the repository at this point in the history
This commit adds '-u' flag for zfs set operation. With this flag,
mountpoint, sharenfs and sharesmb properties can be updated
without actually mounting or sharing the dataset.

Previously, if dataset was unmounted, and mountpoint property was
updated, dataset was not mounted after the update. This behavior
is changed in previous commit. We now mount the dataset whenever
mountpoint property is updated, regardless if it's mounted or not.

To provide the user with option to keep the dataset unmounted and
still update the mountpoint without mounting the dataset, '-u'
flag can be used.

While unmounted, if sharenfs or sharesmb is set to on, this leads
to mounting the dataset. If sharenfs and sharesmb properties are
set with '-u' flag while dataset is unmounted, this will not mount
or share the dataset, but it will set the property to desired
value.

When dataset is mounted, and '-u' flag is used to update any of
mountpoint, sharenfs and sharesmb property/properties, the
property itself is updated but no action is performed. For
mountpoint property, the property is updated but dataset is still
mounted at previous mountpoint. For sharenfs and sharesmb, the
property is set but dataset in not shared/unshared.

'-u' flag would only work if any of mountpoint, sharenfs or
sharesmb is found in property list.

Signed-off-by: Umer Saleem <[email protected]>
  • Loading branch information
usaleem-ix committed Sep 15, 2023
1 parent 3753ce4 commit 1ea8897
Show file tree
Hide file tree
Showing 10 changed files with 150 additions and 32 deletions.
48 changes: 29 additions & 19 deletions cmd/zfs/zfs_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ get_usage(zfs_help_t idx)
"\tsend [-nVvPe] -t <receive_resume_token>\n"
"\tsend [-PnVv] --saved filesystem\n"));
case HELP_SET:
return (gettext("\tset <property=value> ... "
return (gettext("\tset [-u] <property=value> ... "
"<filesystem|volume|snapshot> ...\n"));
case HELP_SHARE:
return (gettext("\tshare [-l] <-a [nfs|smb] | filesystem>\n"));
Expand Down Expand Up @@ -4197,9 +4197,9 @@ zfs_do_rollback(int argc, char **argv)
static int
set_callback(zfs_handle_t *zhp, void *data)
{
nvlist_t *props = data;
zprop_set_cbdata_t *cb = data;
int ret = 0;
ret = zfs_prop_set_list(zhp, props);
ret = zfs_prop_set_list(zhp, cb->cb_proplist, cb->cb_flags);

if (ret != 0 || libzfs_errno(g_zfs) != EZFS_SUCCESS) {
switch (libzfs_errno(g_zfs)) {
Expand All @@ -4219,25 +4219,35 @@ set_callback(zfs_handle_t *zhp, void *data)
static int
zfs_do_set(int argc, char **argv)
{
nvlist_t *props = NULL;
zprop_set_cbdata_t cb = { 0 };
int ds_start = -1; /* argv idx of first dataset arg */
int ret = 0;
int i;
int i, c;

/* check for options */
if (argc > 1 && argv[1][0] == '-') {
(void) fprintf(stderr, gettext("invalid option '%c'\n"),
argv[1][1]);
usage(B_FALSE);
/* check options */
while ((c = getopt(argc, argv, "u")) != -1) {
switch (c) {
case 'u':
cb.cb_flags |= ZFS_SET_NOMOUNT;
break;
case '?':
default:
(void) fprintf(stderr, gettext("invalid option '%c'\n"),
optopt);
usage(B_FALSE);
}
}

argc -= optind;
argv += optind;

/* check number of arguments */
if (argc < 2) {
if (argc < 1) {
(void) fprintf(stderr, gettext("missing arguments\n"));
usage(B_FALSE);
}
if (argc < 3) {
if (strchr(argv[1], '=') == NULL) {
if (argc < 2) {
if (strchr(argv[0], '=') == NULL) {
(void) fprintf(stderr, gettext("missing property=value "
"argument(s)\n"));
} else {
Expand All @@ -4248,7 +4258,7 @@ zfs_do_set(int argc, char **argv)
}

/* validate argument order: prop=val args followed by dataset args */
for (i = 1; i < argc; i++) {
for (i = 0; i < argc; i++) {
if (strchr(argv[i], '=') != NULL) {
if (ds_start > 0) {
/* out-of-order prop=val argument */
Expand All @@ -4266,20 +4276,20 @@ zfs_do_set(int argc, char **argv)
}

/* Populate a list of property settings */
if (nvlist_alloc(&props, NV_UNIQUE_NAME, 0) != 0)
if (nvlist_alloc(&cb.cb_proplist, NV_UNIQUE_NAME, 0) != 0)
nomem();
for (i = 1; i < ds_start; i++) {
if (!parseprop(props, argv[i])) {
for (i = 0; i < ds_start; i++) {
if (!parseprop(cb.cb_proplist, argv[i])) {
ret = -1;
goto error;
}
}

ret = zfs_for_each(argc - ds_start, argv + ds_start, 0,
ZFS_TYPE_DATASET, NULL, NULL, 0, set_callback, props);
ZFS_TYPE_DATASET, NULL, NULL, 0, set_callback, &cb);

error:
nvlist_free(props);
nvlist_free(cb.cb_proplist);
return (ret);
}

Expand Down
9 changes: 8 additions & 1 deletion include/libzfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ _LIBZFS_H nvlist_t *zfs_valid_proplist(libzfs_handle_t *, zfs_type_t,

_LIBZFS_H const char *zfs_prop_to_name(zfs_prop_t);
_LIBZFS_H int zfs_prop_set(zfs_handle_t *, const char *, const char *);
_LIBZFS_H int zfs_prop_set_list(zfs_handle_t *, nvlist_t *);
_LIBZFS_H int zfs_prop_set_list(zfs_handle_t *, nvlist_t *, int);
_LIBZFS_H int zfs_prop_get(zfs_handle_t *, zfs_prop_t, char *, size_t,
zprop_source_t *, char *, size_t, boolean_t);
_LIBZFS_H int zfs_prop_get_recvd(zfs_handle_t *, const char *, char *, size_t,
Expand Down Expand Up @@ -645,6 +645,13 @@ typedef struct zprop_get_cbdata {
vdev_cbdata_t cb_vdevs;
} zprop_get_cbdata_t;

#define ZFS_SET_NOMOUNT 1

typedef struct zprop_set_cbdata {
int cb_flags;
nvlist_t *cb_proplist;
} zprop_set_cbdata_t;

_LIBZFS_H void zprop_print_one_property(const char *, zprop_get_cbdata_t *,
const char *, const char *, zprop_source_t, const char *,
const char *);
Expand Down
1 change: 1 addition & 0 deletions lib/libzfs/libzfs.abi
Original file line number Diff line number Diff line change
Expand Up @@ -4422,6 +4422,7 @@
<function-decl name='zfs_prop_set_list' mangled-name='zfs_prop_set_list' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='zfs_prop_set_list'>
<parameter type-id='9200a744' name='zhp'/>
<parameter type-id='5ce45b60' name='props'/>
<parameter type-id='95e97e5e' name='flags'/>
<return type-id='95e97e5e'/>
</function-decl>
<function-decl name='zfs_prop_inherit' mangled-name='zfs_prop_inherit' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='zfs_prop_inherit'>
Expand Down
19 changes: 12 additions & 7 deletions lib/libzfs/libzfs_changelist.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ changelist_prefix(prop_changelist_t *clp)
*/
switch (clp->cl_prop) {
case ZFS_PROP_MOUNTPOINT:
if (clp->cl_gflags & CL_GATHER_DONT_UNMOUNT)
if ((clp->cl_gflags & CL_GATHER_DONT_UNMOUNT) ||
(clp->cl_gflags & CL_GATHER_DONT_MOUNT))
break;
if (zfs_unmount(cn->cn_handle, NULL,
clp->cl_mflags) != 0) {
Expand Down Expand Up @@ -188,7 +189,8 @@ changelist_postfix(prop_changelist_t *clp)
return (0);

if (clp->cl_prop == ZFS_PROP_MOUNTPOINT &&
!(clp->cl_gflags & CL_GATHER_DONT_UNMOUNT))
!(clp->cl_gflags & CL_GATHER_DONT_UNMOUNT) &&
!(clp->cl_gflags & CL_GATHER_DONT_MOUNT))
remove_mountpoint(cn->cn_handle);

/*
Expand All @@ -205,6 +207,7 @@ changelist_postfix(prop_changelist_t *clp)
boolean_t sharenfs;
boolean_t sharesmb;
boolean_t mounted;
boolean_t nomount;
boolean_t needs_key;

/*
Expand Down Expand Up @@ -242,7 +245,9 @@ changelist_postfix(prop_changelist_t *clp)
mounted = (clp->cl_gflags & CL_GATHER_DONT_UNMOUNT) ||
zfs_is_mounted(cn->cn_handle, NULL);

if (!mounted && !needs_key && (cn->cn_mounted ||
nomount = (clp->cl_gflags & CL_GATHER_DONT_MOUNT) != 0;

if (!mounted && !nomount && !needs_key && (cn->cn_mounted ||
(((clp->cl_prop == ZFS_PROP_MOUNTPOINT &&
clp->cl_prop == clp->cl_realprop) ||
sharenfs || sharesmb || clp->cl_waslegacy) &&
Expand All @@ -260,19 +265,19 @@ changelist_postfix(prop_changelist_t *clp)
*/
const enum sa_protocol nfs[] =
{SA_PROTOCOL_NFS, SA_NO_PROTOCOL};
if (sharenfs && mounted) {
if (sharenfs && mounted && !nomount) {
zfs_share(cn->cn_handle, nfs);
commit_nfs_shares = B_TRUE;
} else if (cn->cn_shared || clp->cl_waslegacy) {
} else if ((cn->cn_shared || clp->cl_waslegacy) && !nomount) {
zfs_unshare(cn->cn_handle, NULL, nfs);
commit_nfs_shares = B_TRUE;
}
const enum sa_protocol smb[] =
{SA_PROTOCOL_SMB, SA_NO_PROTOCOL};
if (sharesmb && mounted) {
if (sharesmb && mounted && !nomount) {
zfs_share(cn->cn_handle, smb);
commit_smb_shares = B_TRUE;
} else if (cn->cn_shared || clp->cl_waslegacy) {
} else if ((cn->cn_shared || clp->cl_waslegacy) && !nomount) {
zfs_unshare(cn->cn_handle, NULL, smb);
commit_smb_shares = B_TRUE;
}
Expand Down
19 changes: 16 additions & 3 deletions lib/libzfs/libzfs_dataset.c
Original file line number Diff line number Diff line change
Expand Up @@ -1764,7 +1764,7 @@ zfs_prop_set(zfs_handle_t *zhp, const char *propname, const char *propval)
goto error;
}

ret = zfs_prop_set_list(zhp, nvl);
ret = zfs_prop_set_list(zhp, nvl, 0);

error:
nvlist_free(nvl);
Expand All @@ -1778,7 +1778,7 @@ zfs_prop_set(zfs_handle_t *zhp, const char *propname, const char *propval)
* given dataset.
*/
int
zfs_prop_set_list(zfs_handle_t *zhp, nvlist_t *props)
zfs_prop_set_list(zfs_handle_t *zhp, nvlist_t *props, int flags)
{
zfs_cmd_t zc = {"\0"};
int ret = -1;
Expand All @@ -1802,6 +1802,17 @@ zfs_prop_set_list(zfs_handle_t *zhp, nvlist_t *props)
B_FALSE, errbuf)) == NULL)
goto error;

if ((flags & ZFS_SET_NOMOUNT) &&
!nvlist_exists(props, "mountpoint") &&
!nvlist_exists(props, "sharesmb") &&
!nvlist_exists(props, "sharenfs")) {
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, "-u specified "
"but mountpoint, sharenfs or sharesmb not found in "
"property list"));
ret = zfs_error(hdl, EZFS_BADPROP, errbuf);
goto error;
}

/*
* We have to check for any extra properties which need to be added
* before computing the length of the nvlist.
Expand Down Expand Up @@ -1848,7 +1859,9 @@ zfs_prop_set_list(zfs_handle_t *zhp, nvlist_t *props)
if (prop != ZFS_PROP_CANMOUNT ||
(fnvpair_value_uint64(elem) == ZFS_CANMOUNT_OFF &&
zfs_is_mounted(zhp, NULL))) {
cls[cl_idx] = changelist_gather(zhp, prop, 0, 0);
cls[cl_idx] = changelist_gather(zhp, prop,
((flags & ZFS_SET_NOMOUNT) ?
CL_GATHER_DONT_MOUNT : 0), 0);
if (cls[cl_idx] == NULL)
goto error;
}
Expand Down
4 changes: 4 additions & 0 deletions lib/libzfs/libzfs_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ extern int zprop_expand_list(libzfs_handle_t *hdl, zprop_list_t **plp,
* Use this changelist_gather() flag to prevent unmounting of file systems.
*/
#define CL_GATHER_DONT_UNMOUNT 4
/*
* Use this changelist_gather() flag to prevent mounting of file systems.
*/
#define CL_GATHER_DONT_MOUNT 8

typedef struct prop_changelist prop_changelist_t;

Expand Down
2 changes: 1 addition & 1 deletion tests/runfiles/common.run
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ tests = ['cache_001_pos', 'cache_002_neg', 'canmount_001_pos',
'user_property_004_pos', 'version_001_neg', 'zfs_set_001_neg',
'zfs_set_002_neg', 'zfs_set_003_neg', 'property_alias_001_pos',
'mountpoint_003_pos', 'ro_props_001_pos', 'zfs_set_keylocation',
'zfs_set_feature_activation']
'zfs_set_feature_activation', 'zfs_set_nomount']
tags = ['functional', 'cli_root', 'zfs_set']

[tests/functional/cli_root/zfs_share]
Expand Down
2 changes: 1 addition & 1 deletion tests/runfiles/sanity.run
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ tests = ['cache_001_pos', 'cache_002_neg', 'canmount_001_pos',
'user_property_001_pos', 'user_property_003_neg', 'readonly_001_pos',
'user_property_004_pos', 'version_001_neg',
'zfs_set_003_neg', 'property_alias_001_pos',
'zfs_set_keylocation', 'zfs_set_feature_activation']
'zfs_set_keylocation', 'zfs_set_feature_activation', 'zfs_set_nomount']
tags = ['functional', 'cli_root', 'zfs_set']

[tests/functional/cli_root/zfs_snapshot]
Expand Down
1 change: 1 addition & 0 deletions tests/zfs-tests/tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
functional/cli_root/zfs_set/zfs_set_003_neg.ksh \
functional/cli_root/zfs_set/zfs_set_feature_activation.ksh \
functional/cli_root/zfs_set/zfs_set_keylocation.ksh \
functional/cli_root/zfs_set/zfs_set_nomount.ksh \
functional/cli_root/zfs_share/cleanup.ksh \
functional/cli_root/zfs_share/setup.ksh \
functional/cli_root/zfs_share/zfs_share_001_pos.ksh \
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
#!/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 https://opensource.org/licenses/CDDL-1.0.
# 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 (c) 2023 by iXsystems, Inc. All rights reserved.
#

. $STF_SUITE/include/libtest.shlib
. $STF_SUITE/tests/functional/cli_root/zfs_set/zfs_set_common.kshlib

verify_runnable "both"

function cleanup
{
log_must zfs set sharenfs=off $TESTPOOL/$TESTFS
if is_linux; then
log_must zfs set sharesmb=off $TESTPOOL/$TESTFS
fi
rm -r $newmpt
}

log_assert "'zfs set -u' sets the mountpoint and share properties without " \
"mounting the dataset"
log_onexit cleanup

oldmpt=$(get_prop mountpoint $TESTPOOL/$TESTFS)
newmpt=$TEST_BASE_DIR/abc

# Test while dataset is mounted
log_must ismounted $TESTPOOL/$TESTFS
log_must zfs set -u mountpoint=$newmpt $TESTPOOL/$TESTFS
log_must check_user_prop $TESTPOOL/$TESTFS mountpoint $newmpt
log_must eval [ `$mountcmd | grep $TESTPOOL/$TESTFS | awk '{print $3}'` == $oldmpt ]
log_must zfs unmount $TESTPOOL/$TESTFS
log_mustnot ismounted $TESTPOOL/$TESTFS
log_must zfs mount $TESTPOOL/$TESTFS
log_must eval [ `$mountcmd | grep $TESTPOOL/$TESTFS | awk '{print $3}'` == $newmpt ]

# Test while dataset is unmounted
log_must zfs set mountpoint=$oldmpt $TESTPOOL/$TESTFS
log_must ismounted $TESTPOOL/$TESTFS
log_must zfs unmount $TESTPOOL/$TESTFS
log_must zfs set -u mountpoint=$newmpt $TESTPOOL/$TESTFS
log_mustnot ismounted $TESTPOOL/$TESTFS
log_must zfs set -u sharenfs=on $TESTPOOL/$TESTFS
log_mustnot ismounted $TESTPOOL/$TESTFS
if is_linux; then
log_must zfs set -u sharesmb=on $TESTPOOL/$TESTFS
log_mustnot ismounted $TESTPOOL/$TESTFS
fi
logmust zfs mount $TESTPOOL/$TESTFS
log_must check_user_prop $TESTPOOL/$TESTFS mountpoint $newmpt
log_must eval [ `$mountcmd | grep $TESTPOOL/$TESTFS | awk '{print $3}'` == $newmpt ]

log_must zfs set mountpoint=$oldmpt $TESTPOOL/$TESTFS
log_must ismounted $TESTPOOL/$TESTFS

log_pass "'zfs set -u' functions correctly"

0 comments on commit 1ea8897

Please sign in to comment.