Ignore pool ashift property during vdev attachment

Ashift can be set for a vdev only during its creation, and the
top-level vdev does not change when a vdev is attached or replaced.
The ashift property should not be used during attachment, as it
does not allow attaching/replacing a vdev if the pool's ashift
property is increased after the existing vdev was created. Instead,
we should be able to attach the vdev if the attached vdev can
satisfy the ashift requirement with its parent.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Signed-off-by: Ameer Hamza <ahamza@ixsystems.com>
Closes #15061
This commit is contained in:
Ameer Hamza 2023-07-20 21:57:16 +05:00 committed by GitHub
parent e6ea31de9f
commit 4d2dad04aa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 36 additions and 65 deletions

View File

@ -420,6 +420,7 @@ struct vdev {
boolean_t vdev_copy_uberblocks; /* post expand copy uberblocks */ boolean_t vdev_copy_uberblocks; /* post expand copy uberblocks */
boolean_t vdev_resilver_deferred; /* resilver deferred */ boolean_t vdev_resilver_deferred; /* resilver deferred */
boolean_t vdev_kobj_flag; /* kobj event record */ boolean_t vdev_kobj_flag; /* kobj event record */
boolean_t vdev_attaching; /* vdev attach ashift handling */
vdev_queue_t vdev_queue; /* I/O deadline schedule queue */ vdev_queue_t vdev_queue; /* I/O deadline schedule queue */
spa_aux_vdev_t *vdev_aux; /* for l2cache and spares vdevs */ spa_aux_vdev_t *vdev_aux; /* for l2cache and spares vdevs */
zio_t *vdev_probe_zio; /* root of current probe */ zio_t *vdev_probe_zio; /* root of current probe */

View File

@ -889,9 +889,15 @@ vdev_alloc(spa_t *spa, vdev_t **vdp, nvlist_t *nv, vdev_t *parent, uint_t id,
&vd->vdev_not_present); &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);
} else {
vd->vdev_attaching = B_TRUE;
}
/* /*
* Retrieve the vdev creation time. * Retrieve the vdev creation time.
@ -2144,9 +2150,9 @@ vdev_open(vdev_t *vd)
return (SET_ERROR(EDOM)); return (SET_ERROR(EDOM));
} }
if (vd->vdev_top == vd) { if (vd->vdev_top == vd && vd->vdev_attaching == B_FALSE)
vdev_ashift_optimize(vd); vdev_ashift_optimize(vd);
} vd->vdev_attaching = B_FALSE;
} }
if (vd->vdev_ashift != 0 && (vd->vdev_ashift < ASHIFT_MIN || if (vd->vdev_ashift != 0 && (vd->vdev_ashift < ASHIFT_MIN ||
vd->vdev_ashift > ASHIFT_MAX)) { vd->vdev_ashift > ASHIFT_MAX)) {

View File

@ -35,7 +35,7 @@
# #
# STRATEGY: # STRATEGY:
# 1. Create various pools with different ashift values. # 1. Create various pools with different ashift values.
# 2. Verify 'attach -o ashift=<n>' works only with allowed values. # 2. Verify 'attach' works.
# #
verify_runnable "global" verify_runnable "global"
@ -66,26 +66,14 @@ log_must set_tunable32 VDEV_FILE_PHYSICAL_ASHIFT 16
typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16") typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16")
for ashift in ${ashifts[@]} for ashift in ${ashifts[@]}
do do
for cmdval in ${ashifts[@]} log_must zpool create -o ashift=$ashift $TESTPOOL1 $disk1
do log_must verify_ashift $disk1 $ashift
log_must zpool create -o ashift=$ashift $TESTPOOL1 $disk1 log_must zpool attach $TESTPOOL1 $disk1 $disk2
log_must verify_ashift $disk1 $ashift log_must verify_ashift $disk2 $ashift
# clean things for the next run
# ashift_of(attached_disk) <= ashift_of(existing_vdev) log_must zpool destroy $TESTPOOL1
if [[ $cmdval -le $ashift ]] log_must zpool labelclear $disk1
then log_must zpool labelclear $disk2
log_must zpool attach -o ashift=$cmdval $TESTPOOL1 \
$disk1 $disk2
log_must verify_ashift $disk2 $ashift
else
log_mustnot zpool attach -o ashift=$cmdval $TESTPOOL1 \
$disk1 $disk2
fi
# clean things for the next run
log_must zpool destroy $TESTPOOL1
log_must zpool labelclear $disk1
log_must zpool labelclear $disk2
done
done done
typeset badvals=("off" "on" "1" "8" "17" "1b" "ff" "-") typeset badvals=("off" "on" "1" "8" "17" "1b" "ff" "-")

View File

@ -35,7 +35,7 @@
# #
# STRATEGY: # STRATEGY:
# 1. Create various pools with different ashift values. # 1. Create various pools with different ashift values.
# 2. Verify 'replace -o ashift=<n>' works only with allowed values. # 2. Verify 'replace' works.
# #
verify_runnable "global" verify_runnable "global"
@ -66,26 +66,16 @@ log_must set_tunable32 VDEV_FILE_PHYSICAL_ASHIFT 16
typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16") typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16")
for ashift in ${ashifts[@]} for ashift in ${ashifts[@]}
do do
for cmdval in ${ashifts[@]} log_must zpool create -o ashift=$ashift $TESTPOOL1 $disk1
do log_must verify_ashift $disk1 $ashift
log_must zpool create -o ashift=$ashift $TESTPOOL1 $disk1 # ashift_of(replacing_disk) <= ashift_of(existing_vdev)
log_must verify_ashift $disk1 $ashift log_must zpool replace $TESTPOOL1 $disk1 $disk2
# ashift_of(replacing_disk) <= ashift_of(existing_vdev) log_must verify_ashift $disk2 $ashift
if [[ $cmdval -le $ashift ]] wait_replacing $TESTPOOL1
then # clean things for the next run
log_must zpool replace -o ashift=$cmdval $TESTPOOL1 \ log_must zpool destroy $TESTPOOL1
$disk1 $disk2 log_must zpool labelclear $disk1
log_must verify_ashift $disk2 $ashift log_must zpool labelclear $disk2
wait_replacing $TESTPOOL1
else
log_mustnot zpool replace -o ashift=$cmdval $TESTPOOL1 \
$disk1 $disk2
fi
# clean things for the next run
log_must zpool destroy $TESTPOOL1
log_must zpool labelclear $disk1
log_must zpool labelclear $disk2
done
done done
typeset badvals=("off" "on" "1" "8" "17" "1b" "ff" "-") typeset badvals=("off" "on" "1" "8" "17" "1b" "ff" "-")

View File

@ -34,10 +34,8 @@
# #
# STRATEGY: # STRATEGY:
# 1. Create a pool with default values. # 1. Create a pool with default values.
# 2. Verify 'zpool replace' uses the ashift pool property value when # 2. Override the pool ashift property.
# replacing an existing device. # 3. Verify 'zpool replace' works.
# 3. Verify the default ashift value can still be overridden by manually
# specifying '-o ashift=<n>' from the command line.
# #
verify_runnable "global" verify_runnable "global"
@ -72,21 +70,9 @@ do
do do
log_must zpool create -o ashift=$ashift $TESTPOOL1 $disk1 log_must zpool create -o ashift=$ashift $TESTPOOL1 $disk1
log_must zpool set ashift=$pprop $TESTPOOL1 log_must zpool set ashift=$pprop $TESTPOOL1
# ashift_of(replacing_disk) <= ashift_of(existing_vdev) log_must zpool replace $TESTPOOL1 $disk1 $disk2
if [[ $pprop -le $ashift ]] wait_replacing $TESTPOOL1
then log_must verify_ashift $disk2 $ashift
log_must zpool replace $TESTPOOL1 $disk1 $disk2
wait_replacing $TESTPOOL1
log_must verify_ashift $disk2 $ashift
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
log_must verify_ashift $disk2 $ashift
fi
# clean things for the next run # clean things for the next run
log_must zpool destroy $TESTPOOL1 log_must zpool destroy $TESTPOOL1
log_must zpool labelclear $disk1 log_must zpool labelclear $disk1