]> git.proxmox.com Git - mirror_zfs.git/commitdiff
Add '-u' - nomount flag for zfs set
authorUmer Saleem <usaleem@ixsystems.com>
Mon, 2 Oct 2023 23:58:54 +0000 (04:58 +0500)
committerGitHub <noreply@github.com>
Mon, 2 Oct 2023 23:58:54 +0000 (16:58 -0700)
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 #15240. We 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.

If any of mountpoint, sharenfs or sharesmb properties are updated
with '-u' flag, the property is set to desired value but the
operation to (re/un)mount and/or (re/un)share the dataset is not
performed and dataset remains as it was before.

Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Umer Saleem <usaleem@ixsystems.com>
Closes #15322

cmd/zfs/zfs_main.c
include/libzfs.h
lib/libzfs/libzfs.abi
lib/libzfs/libzfs_changelist.c
lib/libzfs/libzfs_dataset.c
man/man7/zfsprops.7
man/man8/zfs-set.8
tests/runfiles/common.run
tests/runfiles/sanity.run
tests/zfs-tests/tests/Makefile.am
tests/zfs-tests/tests/functional/cli_root/zfs_set/zfs_set_nomount.ksh [new file with mode: 0755]

index 76c82fd53217ffaa60c8a10c5941e335c4b4d33e..9939f206a7f221a8020202ec63de6820ba6b2f9f 100644 (file)
@@ -339,7 +339,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"));
@@ -4206,8 +4206,8 @@ out:
 static int
 set_callback(zfs_handle_t *zhp, void *data)
 {
-       nvlist_t *props = data;
-       int ret = zfs_prop_set_list(zhp, props);
+       zprop_set_cbdata_t *cb = data;
+       int ret = zfs_prop_set_list_flags(zhp, cb->cb_proplist, cb->cb_flags);
 
        if (ret != 0 || libzfs_errno(g_zfs) != EZFS_SUCCESS) {
                switch (libzfs_errno(g_zfs)) {
@@ -4227,25 +4227,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 {
@@ -4256,7 +4266,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 */
@@ -4274,20 +4284,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);
 }
 
index 0b5501bbe39f06cd205cf647e98e3221ac6d7793..4adfa38e87befc64394d22cafb0410b7146bfbe3 100644 (file)
@@ -532,6 +532,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_flags(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,
@@ -654,6 +655,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 *);
index 907b0191f75bacb5726615e4a0bbde1a0e3f98b7..c1ce3d0f67d8b7cf99b78a5b9b77fb864e6ff0a4 100644 (file)
     <elf-symbol name='zfs_prop_readonly' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='zfs_prop_set' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='zfs_prop_set_list' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+    <elf-symbol name='zfs_prop_set_list_flags' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='zfs_prop_setonce' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='zfs_prop_string_to_index' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='zfs_prop_to_name' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
       <parameter type-id='5ce45b60' name='props'/>
       <return type-id='95e97e5e'/>
     </function-decl>
+    <function-decl name='zfs_prop_set_list_flags' mangled-name='zfs_prop_set_list_flags' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='zfs_prop_set_list_flags'>
+      <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'>
       <parameter type-id='9200a744' name='zhp'/>
       <parameter type-id='80f4b756' name='propname'/>
index efe1c0c060357adf36f4996f38b87a42f8bf35b0..4db1cbce9568d598a5c0b6874442143cecb6a5bb 100644 (file)
@@ -105,6 +105,15 @@ changelist_prefix(prop_changelist_t *clp)
            clp->cl_prop != ZFS_PROP_SHARESMB)
                return (0);
 
+       /*
+        * If CL_GATHER_DONT_UNMOUNT is set, don't want to unmount/unshare and
+        * later (re)mount/(re)share the filesystem in postfix phase, so we
+        * return from here. If filesystem is mounted or unmounted, leave it
+        * as it is.
+        */
+       if (clp->cl_gflags & CL_GATHER_DONT_UNMOUNT)
+               return (0);
+
        if ((walk = uu_avl_walk_start(clp->cl_tree, UU_WALK_ROBUST)) == NULL)
                return (-1);
 
@@ -129,8 +138,6 @@ changelist_prefix(prop_changelist_t *clp)
                         */
                        switch (clp->cl_prop) {
                        case ZFS_PROP_MOUNTPOINT:
-                               if (clp->cl_gflags & CL_GATHER_DONT_UNMOUNT)
-                                       break;
                                if (zfs_unmount(cn->cn_handle, NULL,
                                    clp->cl_mflags) != 0) {
                                        ret = -1;
@@ -164,9 +171,8 @@ changelist_prefix(prop_changelist_t *clp)
  * reshare the filesystems as necessary.  In changelist_gather() we recorded
  * whether the filesystem was previously shared or mounted.  The action we take
  * depends on the previous state, and whether the value was previously 'legacy'.
- * For non-legacy properties, we only remount/reshare the filesystem if it was
- * previously mounted/shared.  Otherwise, we always remount/reshare the
- * filesystem.
+ * For non-legacy properties, we always remount/reshare the filesystem,
+ * if CL_GATHER_DONT_UNMOUNT is not set.
  */
 int
 changelist_postfix(prop_changelist_t *clp)
@@ -177,6 +183,14 @@ changelist_postfix(prop_changelist_t *clp)
        boolean_t commit_smb_shares = B_FALSE;
        boolean_t commit_nfs_shares = B_FALSE;
 
+       /*
+        * If CL_GATHER_DONT_UNMOUNT is set, it means we don't want to (un)mount
+        * or (re/un)share the filesystem, so we return from here. If filesystem
+        * is mounted or unmounted, leave it as it is.
+        */
+       if (clp->cl_gflags & CL_GATHER_DONT_UNMOUNT)
+               return (0);
+
        /*
         * If we're changing the mountpoint, attempt to destroy the underlying
         * mountpoint.  All other datasets will have inherited from this dataset
@@ -239,8 +253,7 @@ changelist_postfix(prop_changelist_t *clp)
                needs_key = (zfs_prop_get_int(cn->cn_handle,
                    ZFS_PROP_KEYSTATUS) == ZFS_KEYSTATUS_UNAVAILABLE);
 
-               mounted = (clp->cl_gflags & CL_GATHER_DONT_UNMOUNT) ||
-                   zfs_is_mounted(cn->cn_handle, NULL);
+               mounted = zfs_is_mounted(cn->cn_handle, NULL);
 
                if (!mounted && !needs_key && (cn->cn_mounted ||
                    (((clp->cl_prop == ZFS_PROP_MOUNTPOINT &&
index 11d3eb6a3c603d4f5cf387ecca5c22dde40591b3..727efc5a91ad499dabde91f7b740d8eafafc032d 100644 (file)
@@ -1771,14 +1771,24 @@ error:
        return (ret);
 }
 
-
-
 /*
  * Given an nvlist of property names and values, set the properties for the
  * given dataset.
  */
 int
 zfs_prop_set_list(zfs_handle_t *zhp, nvlist_t *props)
+{
+       return (zfs_prop_set_list_flags(zhp, props, 0));
+}
+
+/*
+ * Given an nvlist of property names, values and flags, set the properties
+ * for the given dataset. If ZFS_SET_NOMOUNT is set, it allows to update
+ * mountpoint, sharenfs and sharesmb properties without (un/re)mounting
+ * and (un/re)sharing the dataset.
+ */
+int
+zfs_prop_set_list_flags(zfs_handle_t *zhp, nvlist_t *props, int flags)
 {
        zfs_cmd_t zc = {"\0"};
        int ret = -1;
@@ -1848,7 +1858,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_UNMOUNT : 0), 0);
                        if (cls[cl_idx] == NULL)
                                goto error;
                }
index 51ddd85eb79e6d9f02ed9274a1452d3bb51e8e5d..e3674b1f8a8d7caf8e143f825c729f4c0abf2696 100644 (file)
@@ -1248,10 +1248,18 @@ Otherwise, they are automatically remounted in the new location if the property
 was previously
 .Sy legacy
 or
-.Sy none ,
-or if they were mounted before the property was changed.
+.Sy none .
 In addition, any shared file systems are unshared and shared in the new
 location.
+.Pp
+When the
+.Sy mountpoint
+property is set with
+.Nm zfs Cm set Fl u
+, the
+.Sy mountpoint
+property is updated but dataset is not mounted or unmounted and remains
+as it was before.
 .It Sy nbmand Ns = Ns Sy on Ns | Ns Sy off
 Controls whether the file system should be mounted with
 .Sy nbmand
@@ -1656,6 +1664,13 @@ by default.
 This means that any additional access control
 (disallow specific user specific access etc) must be done on the underlying file
 system.
+.Pp
+When the
+.Sy sharesmb
+property is updated with
+.Nm zfs Cm set Fl u
+, the property is set to desired value, but the operation to share, reshare
+or unshare the the dataset is not performed.
 .It Sy sharenfs Ns = Ns Sy on Ns | Ns Sy off Ns | Ns Ar opts
 Controls whether the file system is shared via NFS, and what options are to be
 used.
@@ -1699,6 +1714,13 @@ or if they were shared before the property was changed.
 If the new property is
 .Sy off ,
 the file systems are unshared.
+.Pp
+When the
+.Sy sharenfs
+property is updated with
+.Nm zfs Cm set Fl u
+, the property is set to desired value, but the operation to share, reshare
+or unshare the the dataset is not performed.
 .It Sy logbias Ns = Ns Sy latency Ns | Ns Sy throughput
 Provide a hint to ZFS about handling of synchronous requests in this dataset.
 If
index 4cabdcd8bd8309928907f0547078631dae8bcf1f..c01bcc643e5d81c7ef9389358328088cf25d4652 100644 (file)
@@ -39,6 +39,7 @@
 .Sh SYNOPSIS
 .Nm zfs
 .Cm set
+.Op Fl u
 .Ar property Ns = Ns Ar value Oo Ar property Ns = Ns Ar value Oc Ns …
 .Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot Ns …
 .Nm zfs
@@ -60,6 +61,7 @@
 .It Xo
 .Nm zfs
 .Cm set
+.Op Fl u
 .Ar property Ns = Ns Ar value Oo Ar property Ns = Ns Ar value Oc Ns …
 .Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot Ns …
 .Xc
@@ -79,6 +81,11 @@ For more information, see the
 .Em User Properties
 section of
 .Xr zfsprops 7 .
+.Bl -tag -width "-u"
+.It Fl u
+Update mountpoint, sharenfs, sharesmb property but do not mount or share the
+dataset.
+.El
 .It Xo
 .Nm zfs
 .Cm get
index 342f56d50d041b78f7994b9d545844d3d7be8fef..ef787c65c0f9f6fc1f89a1d7885ee774f53b667a 100644 (file)
@@ -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]
index 449bf1c0f56ab3487aeac9b6e5f6cff146bbc12f..ab41c05b8473e7cee24cbb22c1075bc6b1a48e7c 100644 (file)
@@ -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]
index 1a58e6f774e9574b19bf8fda18f34f63013049a2..3272a5d5816fcff69121526e5f2b5c30cd18b1d8 100644 (file)
@@ -870,6 +870,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 (executable)
index 0000000..ebf0871
--- /dev/null
@@ -0,0 +1,103 @@
+#!/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
+
+#
+# DESCRIPTION:
+# 'zfs set -u' should update the mountpoint, sharenfs and sharesmb
+# properties without mounting and sharing the dataset. Validate the
+# bevaior while dataset is mounted and unmounted.
+#
+# STRATEGY:
+# 1. Confirm dataset is currently mounted
+# 2. Update the mountpoint with -u flag
+# 3. Confirm mountpoint property is updated with new value
+# 4. Confirm dataset is still mounted at previous mountpoint
+# 5. Unmount the dataset
+# 6. Confirm dataset is unmounted
+# 7. Mount the dataset
+# 8. Confirm dataset is mounted at new mountpoint, that was set with -u flag.
+# 9. Update and mount the dataset at previous mountpoint.
+# 10. Unmount the dataset
+# 11. Update mountpoint property with zfs set -u
+# 12. Confirm dataset is not mounted
+# 13. Update sharenfs property with zfs set -u
+# 14. Confirm dataset is not mounted
+# 15. Update sharesmb property with zfs set -u
+# 16. Confirm dataset is not mounted
+# 17. Mount the dataset and confirm dataset is mounted at new mountpoint
+#
+
+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 "[[ "$(mount | 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 "[[ "$(mount | 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
+log_must zfs mount $TESTPOOL/$TESTFS
+log_must check_user_prop $TESTPOOL/$TESTFS mountpoint $newmpt
+log_must eval "[[ "$(mount | 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"