6298 zfs_create_008_neg and zpool_create_023_neg need to be updated f…

…or large block support Reviewed by: Matthew Ahrens <[email protected]> Reviewed by: John Kennedy <[email protected]> Approved by: Robert Mustacchi <[email protected]>
illumos · Oct 13, 2015 · e9316f7 · e9316f7
1 parent 28c4d11
commit e9316f7
Show file tree

Hide file tree

Showing 10 changed files with 87 additions and 33 deletions.
diff --git a/usr/src/cmd/zfs/zfs_main.c b/usr/src/cmd/zfs/zfs_main.c
@@ -21,7 +21,7 @@
 
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2011, 2014 by Delphix. All rights reserved.
+ * Copyright (c) 2011, 2015 by Delphix. All rights reserved.
  * Copyright 2012 Milan Jurik. All rights reserved.
  * Copyright (c) 2012, Joyent, Inc. All rights reserved.
  * Copyright (c) 2013 Steven Hartland.  All rights reserved.
@@ -809,7 +809,6 @@ zfs_do_create(int argc, char **argv)
 			goto error;
 		spa_version = zpool_get_prop_int(zpool_handle,
 		    ZPOOL_PROP_VERSION, NULL);
-		zpool_close(zpool_handle);
 		if (spa_version >= SPA_VERSION_REFRESERVATION)
 			resv_prop = ZFS_PROP_REFRESERVATION;
 		else
@@ -818,8 +817,11 @@ zfs_do_create(int argc, char **argv)
 		(void) snprintf(msg, sizeof (msg),
 		    gettext("cannot create '%s'"), argv[0]);
 		if (props && (real_props = zfs_valid_proplist(g_zfs, type,
-		    props, 0, NULL, msg)) == NULL)
+		    props, 0, NULL, zpool_handle, msg)) == NULL) {
+			zpool_close(zpool_handle);
 			goto error;
+		}
+		zpool_close(zpool_handle);
 
 		volsize = zvol_volsize_to_reservation(volsize, real_props);
 		nvlist_free(real_props);

diff --git a/usr/src/lib/libzfs/common/libzfs.h b/usr/src/lib/libzfs/common/libzfs.h
@@ -21,7 +21,7 @@
 
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2011, 2014 by Delphix. All rights reserved.
+ * Copyright (c) 2011, 2015 by Delphix. All rights reserved.
  * Copyright (c) 2012, Joyent, Inc. All rights reserved.
  * Copyright (c) 2013 Steven Hartland. All rights reserved.
  * Copyright 2013 Nexenta Systems, Inc.  All rights reserved.
@@ -421,7 +421,7 @@ extern const char *zfs_prop_column_name(zfs_prop_t);
 extern boolean_t zfs_prop_align_right(zfs_prop_t);
 
 extern nvlist_t *zfs_valid_proplist(libzfs_handle_t *, zfs_type_t,
-    nvlist_t *, uint64_t, zfs_handle_t *, const char *);
+    nvlist_t *, uint64_t, zfs_handle_t *, zpool_handle_t *, const char *);
 
 extern const char *zfs_prop_to_name(zfs_prop_t);
 extern int zfs_prop_set(zfs_handle_t *, const char *, const char *);

diff --git a/usr/src/lib/libzfs/common/libzfs_dataset.c b/usr/src/lib/libzfs/common/libzfs_dataset.c
@@ -22,7 +22,7 @@
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2013, Joyent, Inc. All rights reserved.
- * Copyright (c) 2011, 2014 by Delphix. All rights reserved.
+ * Copyright (c) 2011, 2015 by Delphix. All rights reserved.
  * Copyright (c) 2012 DEY Storage Systems, Inc.  All rights reserved.
  * Copyright (c) 2013 Martin Matuska. All rights reserved.
  * Copyright (c) 2013 Steven Hartland. All rights reserved.
@@ -858,7 +858,8 @@ zfs_which_resv_prop(zfs_handle_t *zhp, zfs_prop_t *resv_prop)
  */
 nvlist_t *
 zfs_valid_proplist(libzfs_handle_t *hdl, zfs_type_t type, nvlist_t *nvl,
-    uint64_t zoned, zfs_handle_t *zhp, const char *errbuf)
+    uint64_t zoned, zfs_handle_t *zhp, zpool_handle_t *zpool_hdl,
+    const char *errbuf)
 {
 	nvpair_t *elem;
 	uint64_t intval;
@@ -1052,8 +1053,8 @@ zfs_valid_proplist(libzfs_handle_t *hdl, zfs_type_t type, nvlist_t *nvl,
 		case ZFS_PROP_RECORDSIZE:
 		{
 			int maxbs = SPA_MAXBLOCKSIZE;
-			if (zhp != NULL) {
-				maxbs = zpool_get_prop_int(zhp->zpool_hdl,
+			if (zpool_hdl != NULL) {
+				maxbs = zpool_get_prop_int(zpool_hdl,
 				    ZPOOL_PROP_MAXBLOCKSIZE, NULL);
 			}
 			/*
@@ -1551,7 +1552,8 @@ zfs_prop_set_list(zfs_handle_t *zhp, nvlist_t *props)
 	    zhp->zfs_name);
 
 	if ((nvl = zfs_valid_proplist(hdl, zhp->zfs_type, props,
-	    zfs_prop_get_int(zhp, ZFS_PROP_ZONED), zhp, errbuf)) == NULL)
+	    zfs_prop_get_int(zhp, ZFS_PROP_ZONED), zhp, zhp->zpool_hdl,
+	    errbuf)) == NULL)
 		goto error;
 
 	/*
@@ -3187,9 +3189,23 @@ zfs_create(libzfs_handle_t *hdl, const char *path, zfs_type_t type,
 	else
 		ost = DMU_OST_ZFS;
 
+	/* open zpool handle for prop validation */
+	char pool_path[MAXNAMELEN];
+	(void) strlcpy(pool_path, path, sizeof (pool_path));
+
+	/* truncate pool_path at first slash */
+	char *p = strchr(pool_path, '/');
+	if (p != NULL)
+		*p = '\0';
+
+	zpool_handle_t *zpool_handle = zpool_open(hdl, pool_path);
+
 	if (props && (props = zfs_valid_proplist(hdl, type, props,
-	    zoned, NULL, errbuf)) == 0)
+	    zoned, NULL, zpool_handle, errbuf)) == 0) {
+		zpool_close(zpool_handle);
 		return (-1);
+	}
+	zpool_close(zpool_handle);
 
 	if (type == ZFS_TYPE_VOLUME) {
 		/*
@@ -3257,13 +3273,6 @@ zfs_create(libzfs_handle_t *hdl, const char *path, zfs_type_t type,
 			    "parent '%s' is not a filesystem"), parent);
 			return (zfs_error(hdl, EZFS_BADTYPE, errbuf));
 
-		case EDOM:
-			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
-			    "volume block size must be power of 2 from "
-			    "512B to 128KB"));
-
-			return (zfs_error(hdl, EZFS_BADPROP, errbuf));
-
 		case ENOTSUP:
 			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
 			    "pool must be upgraded to set this "
@@ -3458,7 +3467,7 @@ zfs_clone(zfs_handle_t *zhp, const char *target, nvlist_t *props)
 			type = ZFS_TYPE_FILESYSTEM;
 		}
 		if ((props = zfs_valid_proplist(hdl, type, props, zoned,
-		    zhp, errbuf)) == NULL)
+		    zhp, zhp->zpool_hdl, errbuf)) == NULL)
 			return (-1);
 	}
 
@@ -3602,11 +3611,23 @@ zfs_snapshot_nvl(libzfs_handle_t *hdl, nvlist_t *snaps, nvlist_t *props)
 		}
 	}
 
+	/*
+	 * get pool handle for prop validation. assumes all snaps are in the
+	 * same pool, as does lzc_snapshot (below).
+	 */
+	char pool[MAXNAMELEN];
+	elem = nvlist_next_nvpair(snaps, NULL);
+	(void) strlcpy(pool, nvpair_name(elem), sizeof (pool));
+	pool[strcspn(pool, "/@")] = '\0';
+	zpool_handle_t *zpool_hdl = zpool_open(hdl, pool);
+
 	if (props != NULL &&
 	    (props = zfs_valid_proplist(hdl, ZFS_TYPE_SNAPSHOT,
-	    props, B_FALSE, NULL, errbuf)) == NULL) {
+	    props, B_FALSE, NULL, zpool_hdl, errbuf)) == NULL) {
+		zpool_close(zpool_hdl);
 		return (-1);
 	}
+	zpool_close(zpool_hdl);
 
 	ret = lzc_snapshot(snaps, props, &errors);
 

diff --git a/usr/src/lib/libzfs/common/libzfs_pool.c b/usr/src/lib/libzfs/common/libzfs_pool.c
@@ -1094,8 +1094,8 @@ zpool_create(libzfs_handle_t *hdl, const char *pool, nvlist_t *nvroot,
 		    zfs_prop_to_name(ZFS_PROP_ZONED), &zonestr) == 0) &&
 		    strcmp(zonestr, "on") == 0);
 
-		if ((zc_fsprops = zfs_valid_proplist(hdl,
-		    ZFS_TYPE_FILESYSTEM, fsprops, zoned, NULL, msg)) == NULL) {
+		if ((zc_fsprops = zfs_valid_proplist(hdl, ZFS_TYPE_FILESYSTEM,
+		    fsprops, zoned, NULL, NULL, msg)) == NULL) {
 			goto create_failed;
 		}
 		if (!zc_props &&
@@ -1131,6 +1131,21 @@ zpool_create(libzfs_handle_t *hdl, const char *pool, nvlist_t *nvroot,
 			    "one or more vdevs refer to the same device"));
 			return (zfs_error(hdl, EZFS_BADDEV, msg));
 
+		case ERANGE:
+			/*
+			 * This happens if the record size is smaller or larger
+			 * than the allowed size range, or not a power of 2.
+			 *
+			 * NOTE: although zfs_valid_proplist is called earlier,
+			 * this case may have slipped through since the
+			 * pool does not exist yet and it is therefore
+			 * impossible to read properties e.g. max blocksize
+			 * from the pool.
+			 */
+			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+			    "record size invalid"));
+			return (zfs_error(hdl, EZFS_BADPROP, msg));
+
 		case EOVERFLOW:
 			/*
 			 * This occurs when one of the devices is below

diff --git a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_clone/zfs_clone_001_neg.ksh b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_clone/zfs_clone_001_neg.ksh
@@ -26,7 +26,7 @@
 #
 
 #
-# Copyright (c) 2012 by Delphix. All rights reserved.
+# Copyright (c) 2012, 2015 by Delphix. All rights reserved.
 #
 
 . $STF_SUITE/include/libtest.shlib
@@ -45,6 +45,7 @@
 #		* The target clone already exists.
 #		* Null target clone argument.
 #		* Too many arguments.
+#               * Invalid record sizes.
 #
 # STRATEGY:
 #	1. Create an array of parameters
@@ -66,7 +67,9 @@ set -A args "" \
 	"$SNAPFS $TESTPOOL//$TESTFS1" "$SNAPFS1 $TESTPOOL//$TESTFS1" \
 	"$SNAPFS $NONEXISTPOOLNAME/$TESTFS" "$SNAPFS1 $NONEXISTPOOLNAME/$TESTFS" \
 	"$SNAPFS" "$SNAPFS1" \
-	"$SNAPFS $target1 $target2" "$SNAPFS1 $target1 $target2"
+	"$SNAPFS $target1 $target2" "$SNAPFS1 $target1 $target2" \
+        "-o recordsize=2M $SNAPFS1 $target1" \
+        "-o recordsize=128B $SNAPFS1 $target1"
 typeset -i argsnum=${#args[*]}
 typeset -i j=0
 while (( j < argsnum )); do

diff --git a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_create/properties.kshlib b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_create/properties.kshlib
@@ -25,12 +25,12 @@
 #
 
 #
-# Copyright (c) 2012 by Delphix. All rights reserved.
+# Copyright (c) 2012, 2015 by Delphix. All rights reserved.
 #
 
 set -A RW_FS_PROP "quota=512M" \
 		  "reservation=512M" \
-		  "recordsize=64K" \
+		  "recordsize=256K" \
 		  "mountpoint=/tmp/mnt$$" \
 		  "checksum=fletcher2" \
 		  "compression=lzjb" \

diff --git a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_create/zfs_create_007_pos.ksh b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_create/zfs_create_007_pos.ksh
@@ -26,7 +26,7 @@
 #
 
 #
-# Copyright (c) 2012 by Delphix. All rights reserved.
+# Copyright (c) 2012, 2015 by Delphix. All rights reserved.
 #
 
 . $STF_SUITE/include/libtest.shlib
@@ -49,6 +49,8 @@ verify_runnable "global"
 
 function cleanup
 {
+        datasetexists $TESTPOOL/$TESTVOL && \
+                log_must $ZFS destroy -f $TESTPOOL/$TESTVOL
 	datasetexists $TESTPOOL/$TESTVOL1 && \
 		log_must $ZFS destroy -f $TESTPOOL/$TESTVOL1
 }

diff --git a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_create/zfs_create_008_neg.ksh b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_create/zfs_create_008_neg.ksh
@@ -26,7 +26,7 @@
 #
 
 #
-# Copyright (c) 2012 by Delphix. All rights reserved.
+# Copyright (c) 2012, 2015 by Delphix. All rights reserved.
 #
 
 . $STF_SUITE/include/libtest.shlib
@@ -57,8 +57,8 @@ set -A args "ab" "-?" "-cV" "-Vc" "-c -V" "c" "V" "--c" "-e" "-s" \
     "-blah" "-cV 12k" "-s -cV 1P" "-sc" "-Vs 5g" "-o" "--o" "-O" "--O" \
     "-o QuOta=none" "-o quota=non" "-o quota=abcd" "-o quota=0" "-o quota=" \
     "-o ResErVaTi0n=none" "-o reserV=none" "-o reservation=abcd" "-o reserv=" \
-    "-o recorDSize=64k" "-o recordsize=256K" "-o recordsize=256" \
-    "-o recsize=" "-o recsize=zero" "-o recordsize=0" \
+    "-o recorDSize=64k" "-o recordsize=2048K" "-o recordsize=2M" \
+    "-o recordsize=256" "-o recsize=" "-o recsize=zero" "-o recordsize=0" \
     "-o mountPoint=/tmp/tmpfile$$" "-o mountpoint=non0" "-o mountpoint=" \
     "-o mountpoint=LEGACY" "-o mounpoint=none" \
     "-o sharenfs=ON" "-o ShareNFS=off" "-o sharenfs=sss" \

diff --git a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_set/zfs_set_001_neg.ksh b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_set/zfs_set_001_neg.ksh
@@ -25,13 +25,17 @@
 # Use is subject to license terms.
 #
 
+#
+# Copyright (c) 2015 by Delphix. All rights reserved.
+#
+
 . $STF_SUITE/include/libtest.shlib
 . $STF_SUITE/tests/functional/cli_root/zfs_set/zfs_set_common.kshlib
 
 #
 # DESCRIPTION:
 # Setting invalid value to mountpoint, checksum, atime, readonly, setuid,
-# zoned or canmount on a file system, volume. It should be failed.
+# zoned, recordsize, or canmount on a file system, volume. It should be failed.
 #
 # STRATEGY:
 # 1. Create pool, then create file system & volume within it.
@@ -68,6 +72,9 @@ while (( i < ${#dataset[@]} )); do
 		done
 		(( j += 1 ))
 	done
+        # Additional recordsize
+        set_n_check_prop "recordsize" "2048K" "${dataset[i]}" false
+        set_n_check_prop "recordsize" "128B" "${dataset[i]}" false
 	(( i += 1 ))
 done
 

diff --git a/usr/src/test/zfs-tests/tests/functional/cli_root/zpool_create/zpool_create_023_neg.ksh b/usr/src/test/zfs-tests/tests/functional/cli_root/zpool_create/zpool_create_023_neg.ksh
@@ -25,6 +25,10 @@
 # Use is subject to license terms.
 #
 
+#
+# Copyright (c) 2015 by Delphix. All rights reserved.
+#
+
 . $STF_SUITE/include/libtest.shlib
 
 #
@@ -48,8 +52,8 @@ log_onexit cleanup
 
 set -A args "QuOta=none" "quota=non" "quota=abcd" "quota=0" "quota=" \
     "ResErVaTi0n=none" "reserV=none" "reservation=abcd" "reserv=" \
-    "recorDSize=64k" "recordsize=256K" "recordsize=256" \
-    "recsize=" "recsize=zero" "recordsize=0" \
+    "recorDSize=64k" "recordsize=2M" "recordsize=2048K" \
+    "recordsize=256" "recsize=" "recsize=zero" "recordsize=0" \
     "mountPoint=/tmp/tmpfile$$" "mountpoint=non0" "mountpoint=" \
     "mountpoint=LEGACY" "mounpoint=none" \
     "sharenfs=ON" "ShareNFS=off" "sharenfs=sss" \