Improve the handling of sharesmb,sharenfs properties

For sharesmb and sharenfs properties, the status of setting the
property is tied with whether we succeed to share the dataset or
not. In case sharing the dataset is not successful, this is
treated as overall failure of setting the property. In this case,
if we check the property after the failure, it is set to on.

This commit updates this behavior and the status of setting the
share properties is not returned as failure, when we fail to
share the dataset.

For sharenfs property, if access list is provided, the syntax
errors in access list/host adresses are not validated until after
setting the property during postfix phase while trying to
share the dataset. This is not correct, since the property has
already been set when we reach there.

Syntax errors in access list/host addresses are validated while
validating the property list, before setting the property and
failure is returned to user in this case when there are errors
in access list.

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

diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c
index 45cd2998d6e9dd72214419b8cfc41bf99064800f..76c82fd53217ffaa60c8a10c5941e335c4b4d33e 100644 (file)
--- a/cmd/zfs/zfs_main.c
+++ b/cmd/zfs/zfs_main.c
@@ -4218,7 +4218,6 @@ set_callback(zfs_handle_t *zhp, void *data)
                case EZFS_SHARENFSFAILED:
                        (void) fprintf(stderr, gettext("property may be set "
                            "but unable to reshare filesystem\n"));
-                       ret = 1;
                        break;
                }
        }

diff --git a/lib/libshare/os/freebsd/nfs.c b/lib/libshare/os/freebsd/nfs.c
index 521631c51f07261e7179995bf437e113bb93f369..d9fc661063693f62bfab03e2f34e6a34db897dbd 100644 (file)
--- a/lib/libshare/os/freebsd/nfs.c
+++ b/lib/libshare/os/freebsd/nfs.c
@@ -161,7 +161,8 @@ nfs_is_shared(sa_share_impl_t impl_share)
 static int
 nfs_validate_shareopts(const char *shareopts)
 {
-       (void) shareopts;
+       if (strlen(shareopts) == 0)
+               return (SA_SYNTAX_ERR);
        return (SA_OK);
 }
 

diff --git a/lib/libshare/os/linux/nfs.c b/lib/libshare/os/linux/nfs.c
index c27e5564c1e18ff9b040a5a97f4740cea59198c0..004946b0cfe401ece53d45c4923b665733482a52 100644 (file)
--- a/lib/libshare/os/linux/nfs.c
+++ b/lib/libshare/os/linux/nfs.c
@@ -319,12 +319,49 @@ get_linux_shareopts_cb(const char *key, const char *value, void *cookie)
            "wdelay" };
 
        char **plinux_opts = (char **)cookie;
+       char *host, *val_dup, *literal, *next;
 
-       /* host-specific options, these are taken care of elsewhere */
-       if (strcmp(key, "ro") == 0 || strcmp(key, "rw") == 0 ||
-           strcmp(key, "sec") == 0)
+       if (strcmp(key, "sec") == 0)
                return (SA_OK);
 
+       if (strcmp(key, "ro") == 0 || strcmp(key, "rw") == 0) {
+               if (value == NULL || strlen(value) == 0)
+                       return (SA_OK);
+               val_dup = strdup(value);
+               host = val_dup;
+               if (host == NULL)
+                       return (SA_NO_MEMORY);
+               do {
+                       if (*host == '[') {
+                               host++;
+                               literal = strchr(host, ']');
+                               if (literal == NULL) {
+                                       free(val_dup);
+                                       return (SA_SYNTAX_ERR);
+                               }
+                               if (literal[1] == '\0')
+                                       next = NULL;
+                               else if (literal[1] == '/') {
+                                       next = strchr(literal + 2, ':');
+                                       if (next != NULL)
+                                               ++next;
+                               } else if (literal[1] == ':')
+                                       next = literal + 2;
+                               else {
+                                       free(val_dup);
+                                       return (SA_SYNTAX_ERR);
+                               }
+                       } else {
+                               next = strchr(host, ':');
+                               if (next != NULL)
+                                       ++next;
+                       }
+                       host = next;
+               } while (host != NULL);
+               free(val_dup);
+               return (SA_OK);
+       }
+
        if (strcmp(key, "anon") == 0)
                key = "anonuid";
 
@@ -472,6 +509,10 @@ static int
 nfs_validate_shareopts(const char *shareopts)
 {
        char *linux_opts = NULL;
+
+       if (strlen(shareopts) == 0)
+               return (SA_SYNTAX_ERR);
+
        int error = get_linux_shareopts(shareopts, &linux_opts);
        if (error != SA_OK)
                return (error);

diff --git a/lib/libzfs/libzfs_changelist.c b/lib/libzfs/libzfs_changelist.c
index 4b0f6696434649bc67da7dd2575f9e24eb10c87f..efe1c0c060357adf36f4996f38b87a42f8bf35b0 100644 (file)
--- a/lib/libzfs/libzfs_changelist.c
+++ b/lib/libzfs/libzfs_changelist.c
@@ -174,7 +174,6 @@ changelist_postfix(prop_changelist_t *clp)
        prop_changenode_t *cn;
        uu_avl_walk_t *walk;
        char shareopts[ZFS_MAXPROPLEN];
-       int errors = 0;
        boolean_t commit_smb_shares = B_FALSE;
        boolean_t commit_nfs_shares = B_FALSE;
 
@@ -262,19 +261,19 @@ changelist_postfix(prop_changelist_t *clp)
                const enum sa_protocol nfs[] =
                    {SA_PROTOCOL_NFS, SA_NO_PROTOCOL};
                if (sharenfs && mounted) {
-                       errors += zfs_share(cn->cn_handle, nfs);
+                       zfs_share(cn->cn_handle, nfs);
                        commit_nfs_shares = B_TRUE;
                } else if (cn->cn_shared || clp->cl_waslegacy) {
-                       errors += zfs_unshare(cn->cn_handle, NULL, nfs);
+                       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) {
-                       errors += zfs_share(cn->cn_handle, smb);
+                       zfs_share(cn->cn_handle, smb);
                        commit_smb_shares = B_TRUE;
                } else if (cn->cn_shared || clp->cl_waslegacy) {
-                       errors += zfs_unshare(cn->cn_handle, NULL, smb);
+                       zfs_unshare(cn->cn_handle, NULL, smb);
                        commit_smb_shares = B_TRUE;
                }
        }
@@ -288,7 +287,7 @@ changelist_postfix(prop_changelist_t *clp)
        zfs_commit_shares(proto);
        uu_avl_walk_end(walk);
 
-       return (errors ? -1 : 0);
+       return (0);
 }
 
 /*