6446301 ZFS mountpoint reverted back from legacy but sharenfs property not honored

6506000 onnv: sharenfs & shareiscsi 'off'->'off' should not trigger mount
6579101 zfs set mountpoint has inconsistent behaviour with long path

4 files changed, 69 insertions, 8 deletions

diff --git a/usr/src/common/zfs/zfs_namecheck.c b/usr/src/common/zfs/zfs_namecheck.c
index 769a22a04e..0bfe9be296 100644
--- a/usr/src/common/zfs/zfs_namecheck.c
+++ b/usr/src/common/zfs/zfs_namecheck.c
@@ -240,6 +240,50 @@ dataset_namecheck(const char *path, namecheck_err_t *why, char *what)
 	}
 }
 
+
+/*
+ * mountpoint names must be of the following form:
+ *
+ *	/[component][/]*[component][/]
+ */
+int
+mountpoint_namecheck(const char *path, namecheck_err_t *why)
+{
+	const char *start, *end;
+
+	/*
+	 * Make sure none of the mountpoint component names are too long.
+	 * If a component name is too long then the mkdir of the mountpoint
+	 * will fail but then the mountpoint property will be set to a value
+	 * that can never be mounted.  Better to fail before setting the prop.
+	 * Extra slashes are OK, they will be tossed by the mountpoint mkdir.
+	 */
+
+	if (path == NULL || *path != '/') {
+		if (why)
+			*why = NAME_ERR_LEADING_SLASH;
+		return (-1);
+	}
+
+	/* Skip leading slash  */
+	start = &path[1];
+	do {
+		end = start;
+		while (*end != '/' && *end != '\0')
+			end++;
+
+		if (end - start >= MAXNAMELEN) {
+			if (why)
+				*why = NAME_ERR_TOOLONG;
+			return (-1);
+		}
+		start = end + 1;
+
+	} while (*end != '\0');
+
+	return (0);
+}
+
 /*
  * For pool names, we have the same set of valid characters as described in
  * dataset names, with the additional restriction that the pool name must begin
diff --git a/usr/src/common/zfs/zfs_namecheck.h b/usr/src/common/zfs/zfs_namecheck.h
index 30fe8be9a8..ec85e62f72 100644
--- a/usr/src/common/zfs/zfs_namecheck.h
+++ b/usr/src/common/zfs/zfs_namecheck.h
@@ -49,6 +49,7 @@ typedef enum {
 
 int pool_namecheck(const char *, namecheck_err_t *, char *);
 int dataset_namecheck(const char *, namecheck_err_t *, char *);
+int mountpoint_namecheck(const char *, namecheck_err_t *);
 int dataset_name_hidden(const char *);
 int snapshot_namecheck(const char *, namecheck_err_t *, char *);
 int permset_namecheck(const char *, namecheck_err_t *, char *);
diff --git a/usr/src/lib/libzfs/common/libzfs_changelist.c b/usr/src/lib/libzfs/common/libzfs_changelist.c
index 7a16d015c6..7b7eaf66b1 100644
--- a/usr/src/lib/libzfs/common/libzfs_changelist.c
+++ b/usr/src/lib/libzfs/common/libzfs_changelist.c
@@ -226,8 +226,9 @@ changelist_postfix(prop_changelist_t *clp)
 		 * We always re-share even if the filesystem is currently
 		 * shared, so that we can adopt any new options.
 		 */
-		if (cn->cn_shared ||
-		    (clp->cl_prop == ZFS_PROP_SHARENFS && clp->cl_waslegacy)) {
+		if (cn->cn_shared || (clp->cl_waslegacy &&
+		    (clp->cl_prop == ZFS_PROP_SHARENFS ||
+		    clp->cl_prop == ZFS_PROP_MOUNTPOINT))) {
 			if (zfs_prop_get(cn->cn_handle, ZFS_PROP_SHARENFS,
 			    shareopts, sizeof (shareopts), NULL, NULL, 0,
 			    B_FALSE) == 0 && strcmp(shareopts, "off") == 0) {
@@ -608,8 +609,7 @@ changelist_gather(zfs_handle_t *zhp, zfs_prop_t prop, int flags)
 	 */
 	if (zfs_prop_get(zhp, prop, property, sizeof (property),
 	    NULL, NULL, 0, B_FALSE) == 0 &&
-	    (strcmp(property, "legacy") == 0 || strcmp(property, "none") == 0 ||
-	    strcmp(property, "off") == 0))
+	    (strcmp(property, "legacy") == 0 || strcmp(property, "none") == 0))
 		clp->cl_waslegacy = B_TRUE;
 
 	return (clp);
diff --git a/usr/src/lib/libzfs/common/libzfs_dataset.c b/usr/src/lib/libzfs/common/libzfs_dataset.c
index b2f1fa37f2..383b63169e 100644
--- a/usr/src/lib/libzfs/common/libzfs_dataset.c
+++ b/usr/src/lib/libzfs/common/libzfs_dataset.c
@@ -944,17 +944,33 @@ zfs_validate_properties(libzfs_handle_t *hdl, zfs_type_t type, char *pool_name,
 			break;
 
 		case ZFS_PROP_MOUNTPOINT:
+		{
+			namecheck_err_t why;
+
 			if (strcmp(strval, ZFS_MOUNTPOINT_NONE) == 0 ||
 			    strcmp(strval, ZFS_MOUNTPOINT_LEGACY) == 0)
 				break;
 
-			if (strval[0] != '/') {
-				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
-				    "'%s' must be an absolute path, "
-				    "'none', or 'legacy'"), propname);
+			if (mountpoint_namecheck(strval, &why)) {
+				switch (why) {
+				case NAME_ERR_LEADING_SLASH:
+					zfs_error_aux(hdl,
+					    dgettext(TEXT_DOMAIN,
+					    "'%s' must be an absolute path, "
+					    "'none', or 'legacy'"), propname);
+					break;
+				case NAME_ERR_TOOLONG:
+					zfs_error_aux(hdl,
+					    dgettext(TEXT_DOMAIN,
+					    "component of '%s' is too long"),
+					    propname);
+					break;
+				}
 				(void) zfs_error(hdl, EZFS_BADPROP, errbuf);
 				goto error;
 			}
+		}
+
 			/*FALLTHRU*/
 
 		case ZFS_PROP_SHARENFS: