From 1ea88971ff35f661375207f04855c7f51b15d53f Mon Sep 17 00:00:00 2001 From: Umer Saleem Date: Mon, 11 Sep 2023 12:09:15 +0500 Subject: [PATCH] Add '-u' - nomount flag for zfs set 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 --- cmd/zfs/zfs_main.c | 48 +++++++----- include/libzfs.h | 9 ++- lib/libzfs/libzfs.abi | 1 + lib/libzfs/libzfs_changelist.c | 19 +++-- lib/libzfs/libzfs_dataset.c | 19 ++++- lib/libzfs/libzfs_impl.h | 4 + tests/runfiles/common.run | 2 +- tests/runfiles/sanity.run | 2 +- tests/zfs-tests/tests/Makefile.am | 1 + .../cli_root/zfs_set/zfs_set_nomount.ksh | 77 +++++++++++++++++++ 10 files changed, 150 insertions(+), 32 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/cli_root/zfs_set/zfs_set_nomount.ksh diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c index c8281dd6dd3d..2f843c52488b 100644 --- a/cmd/zfs/zfs_main.c +++ b/cmd/zfs/zfs_main.c @@ -337,7 +337,7 @@ get_usage(zfs_help_t idx) "\tsend [-nVvPe] -t \n" "\tsend [-PnVv] --saved filesystem\n")); case HELP_SET: - return (gettext("\tset ... " + return (gettext("\tset [-u] ... " " ...\n")); case HELP_SHARE: return (gettext("\tshare [-l] <-a [nfs|smb] | filesystem>\n")); @@ -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)) { @@ -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 { @@ -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 */ @@ -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); } diff --git a/include/libzfs.h b/include/libzfs.h index fa05b7921bb5..a2c33ed3a35e 100644 --- a/include/libzfs.h +++ b/include/libzfs.h @@ -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, @@ -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 *); diff --git a/lib/libzfs/libzfs.abi b/lib/libzfs/libzfs.abi index 6e53bcb41a87..25ae81ec7b54 100644 --- a/lib/libzfs/libzfs.abi +++ b/lib/libzfs/libzfs.abi @@ -4422,6 +4422,7 @@ + diff --git a/lib/libzfs/libzfs_changelist.c b/lib/libzfs/libzfs_changelist.c index efe1c0c06035..36a1ed46d598 100644 --- a/lib/libzfs/libzfs_changelist.c +++ b/lib/libzfs/libzfs_changelist.c @@ -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) { @@ -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); /* @@ -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; /* @@ -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) && @@ -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; } diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c index 11d3eb6a3c60..7823d79b86f2 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -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); @@ -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; @@ -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. @@ -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; } diff --git a/lib/libzfs/libzfs_impl.h b/lib/libzfs/libzfs_impl.h index ef0359f45ea0..c41c68c0599f 100644 --- a/lib/libzfs/libzfs_impl.h +++ b/lib/libzfs/libzfs_impl.h @@ -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; diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index 342f56d50d04..ef787c65c0f9 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -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] diff --git a/tests/runfiles/sanity.run b/tests/runfiles/sanity.run index 449bf1c0f56a..ab41c05b8473 100644 --- a/tests/runfiles/sanity.run +++ b/tests/runfiles/sanity.run @@ -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] diff --git a/tests/zfs-tests/tests/Makefile.am b/tests/zfs-tests/tests/Makefile.am index 0819cb6b576e..7f2ff57c4d89 100644 --- a/tests/zfs-tests/tests/Makefile.am +++ b/tests/zfs-tests/tests/Makefile.am @@ -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 \ diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_set/zfs_set_nomount.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_set/zfs_set_nomount.ksh new file mode 100755 index 000000000000..d3d3f37ec987 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_set/zfs_set_nomount.ksh @@ -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"