Skip to content

Commit

Permalink
Improve the handling of sharesmb,sharenfs properties
Browse files Browse the repository at this point in the history
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.

Signed-off-by: Umer Saleem <[email protected]>
  • Loading branch information
usaleem-ix committed Sep 6, 2023
1 parent 574232c commit 2cf84fd
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 10 deletions.
1 change: 0 additions & 1 deletion cmd/zfs/zfs_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -4213,7 +4213,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;
}
}
Expand Down
47 changes: 44 additions & 3 deletions lib/libshare/os/linux/nfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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);
Expand Down
11 changes: 5 additions & 6 deletions lib/libzfs/libzfs_changelist.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
}
Expand All @@ -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);
}

/*
Expand Down

0 comments on commit 2cf84fd

Please sign in to comment.