For vdev attach, ignore pool ashift, and check disk sector size

Signed-off-by: Ameer Hamza <[email protected]>
truenas · Jul 11, 2023 · 267d5df · 267d5df
1 parent e50c124
commit 267d5df
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 66 deletions.
diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c
@@ -854,9 +854,13 @@ vdev_alloc(spa_t *spa, vdev_t **vdp, nvlist_t *nv, vdev_t *parent, uint_t id,
 	    &vd->vdev_not_present);
 
 	/*
-	 * Get the alignment requirement.
+	 * Get the alignment requirement. Ignore pool ashift for vdev
+	 * attach case.
 	 */
-	(void) nvlist_lookup_uint64(nv, ZPOOL_CONFIG_ASHIFT, &vd->vdev_ashift);
+	if (alloctype != VDEV_ALLOC_ATTACH) {
+		(void) nvlist_lookup_uint64(nv, ZPOOL_CONFIG_ASHIFT,
+		    &vd->vdev_ashift);
+	}
 
 	/*
 	 * Retrieve the vdev creation time.

diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_attach/attach-o_ashift.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_attach/attach-o_ashift.ksh
@@ -35,7 +35,7 @@
 #
 # STRATEGY:
 #	1. Create various pools with different ashift values.
-#	2. Verify 'attach -o ashift=<n>' works only with allowed values.
+#	2. Verify 'attach -o ashift=<n>' works.
 #
 
 verify_runnable "global"
@@ -69,27 +69,10 @@ do
 	for cmdval in ${ashifts[@]}
 	do
 		log_must zpool create -o ashift=$ashift $TESTPOOL1 $disk1
-		verify_ashift $disk1 $ashift
-		if [[ $? -ne 0 ]]
-		then
-			log_fail "Pool was created without setting ashift " \
-			    "value to $ashift"
-		fi
-		# ashift_of(attached_disk) <= ashift_of(existing_vdev)
-		if [[ $cmdval -le $ashift ]]
-		then
-			log_must zpool attach -o ashift=$cmdval $TESTPOOL1 \
-			    $disk1 $disk2
-			verify_ashift $disk2 $ashift
-			if [[ $? -ne 0 ]]
-			then
-				log_fail "Device was attached without " \
-				    "setting ashift value to $ashift"
-			fi
-		else
-			log_mustnot zpool attach -o ashift=$cmdval $TESTPOOL1 \
-			    $disk1 $disk2
-		fi
+		log_must verify_ashift $disk1 $ashift
+		log_must zpool attach -o ashift=$cmdval $TESTPOOL1 \
+		    $disk1 $disk2
+		log_must verify_ashift $disk2 $ashift
 		# clean things for the next run
 		log_must zpool destroy $TESTPOOL1
 		log_must zpool labelclear $disk1

diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_replace/replace-o_ashift.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_replace/replace-o_ashift.ksh
@@ -35,7 +35,7 @@
 #
 # STRATEGY:
 #	1. Create various pools with different ashift values.
-#	2. Verify 'replace -o ashift=<n>' works only with allowed values.
+#	2. Verify 'replace -o ashift=<n>' works.
 #
 
 verify_runnable "global"
@@ -76,21 +76,10 @@ do
 			    "value to $ashift"
 		fi
 		# ashift_of(replacing_disk) <= ashift_of(existing_vdev)
-		if [[ $cmdval -le $ashift ]]
-		then
-			log_must zpool replace -o ashift=$cmdval $TESTPOOL1 \
-			    $disk1 $disk2
-			verify_ashift $disk2 $ashift
-			if [[ $? -ne 0 ]]
-			then
-				log_fail "Device was replaced without " \
-				    "setting ashift value to $ashift"
-			fi
-			wait_replacing $TESTPOOL1
-		else
-			log_mustnot zpool replace -o ashift=$cmdval $TESTPOOL1 \
-			    $disk1 $disk2
-		fi
+		log_must zpool replace -o ashift=$cmdval $TESTPOOL1 \
+		    $disk1 $disk2
+		log_must verify_ashift $disk2 $ashift
+		wait_replacing $TESTPOOL1
 		# clean things for the next run
 		log_must zpool destroy $TESTPOOL1
 		log_must zpool labelclear $disk1

diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_replace/replace_prop_ashift.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_replace/replace_prop_ashift.ksh
@@ -36,8 +36,6 @@
 #	1. Create a pool with default values.
 #	2. Verify 'zpool replace' uses the ashift pool property value when
 #	   replacing an existing device.
-#	3. Verify the default ashift value can still be overridden by manually
-#	   specifying '-o ashift=<n>' from the command line.
 #
 
 verify_runnable "global"
@@ -73,30 +71,9 @@ do
 		log_must zpool create -o ashift=$ashift $TESTPOOL1 $disk1
 		log_must zpool set ashift=$pprop $TESTPOOL1
 		# ashift_of(replacing_disk) <= ashift_of(existing_vdev)
-		if [[ $pprop -le $ashift ]]
-		then
-			log_must zpool replace $TESTPOOL1 $disk1 $disk2
-			wait_replacing $TESTPOOL1
-			verify_ashift $disk2 $ashift
-			if [[ $? -ne 0 ]]
-			then
-				log_fail "Device was replaced without " \
-				    "setting ashift value to $ashift"
-			fi
-		else
-			# cannot replace if pool prop ashift > vdev ashift
-			log_mustnot zpool replace $TESTPOOL1 $disk1 $disk2
-			# verify we can override the pool prop value manually
-			log_must zpool replace -o ashift=$ashift $TESTPOOL1 \
-			    $disk1 $disk2
-			wait_replacing $TESTPOOL1
-			verify_ashift $disk2 $ashift
-			if [[ $? -ne 0 ]]
-			then
-				log_fail "Device was replaced without " \
-				    "setting ashift value to $ashift"
-			fi
-		fi
+		log_must zpool replace $TESTPOOL1 $disk1 $disk2
+		wait_replacing $TESTPOOL1
+		log_must verify_ashift $disk2 $ashift
 		# clean things for the next run
 		log_must zpool destroy $TESTPOOL1
 		log_must zpool labelclear $disk1