ZTS: Minimize udev_wait in zvol_misc tests

The zvol_misc tests, in particular zvol_misc_volmode, make use of a common udev_wait function to wait for zvol devices in /dev to quiesce on Linux. On other platforms this function currently only sleeps for one second before returning. This is insufficient, and zvol_misc_volmode has been flaky on FreeBSD as a result. Replace udev_wait with block_device_wait, passing through the optional device parameter where possible. Rearrange a few checks to strengthen the verifications we are making and avoid unnecessarily sleeping. We must keep udev_wait in a couple places to pass in Github CI workflows. Remove zvol_misc_volmode from the maybe failing tests on FreeBSD in zts-report.py. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: John Kennedy <john.kennedy@delphix.com> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes #12583
2021-10-01 11:36:02 -04:00 · 2021-10-01 11:36:02 -04:00 · 96ad227a9d
parent ce2bdcedf5
commit 96ad227a9d
7 changed files with 35 additions and 35 deletions
--- a/tests/test-runner/bin/zts-report.py.in
+++ b/tests/test-runner/bin/zts-report.py.in
@ -283,7 +283,6 @@ if sys.platform.startswith('freebsd'):
        'delegate/zfs_allow_003_pos': ['FAIL', known_reason],
        'inheritance/inherit_001_pos': ['FAIL', '11829'],
        'resilver/resilver_restart_001': ['FAIL', known_reason],
-        'zvol/zvol_misc/zvol_misc_volmode': ['FAIL', known_reason],
    })
 elif sys.platform.startswith('linux'):
    maybe.update({
--- a/tests/zfs-tests/include/blkdev.shlib
+++ b/tests/zfs-tests/include/blkdev.shlib
@ -73,7 +73,7 @@ function scan_scsi_hosts
 function block_device_wait
 {
 	if is_linux; then
-		udevadm trigger $*
+		udevadm trigger $* 2>/dev/null
 		typeset start=$SECONDS
 		udevadm settle
 		typeset elapsed=$((SECONDS - start))
--- a/tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_common.kshlib
+++ b/tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_common.kshlib
@ -37,8 +37,6 @@
 #
 function udev_wait
 {
-	sleep 1
-	is_linux || return 0
 	udevadm trigger --action=change
 	udevadm settle
 	for i in {1..3}; do
@ -58,7 +56,6 @@ function udev_wait
 #
 function udev_cleanup
 {
-	is_linux || return 0
 	log_note "Pruning broken ZVOL symlinks ..."

 	udevadm settle
@ -79,7 +76,8 @@ function blockdev_exists # device
 	# because there are other commands (zfs snap, zfs inherit, zfs destroy)
 	# that can affect device nodes
 	for i in {1..3}; do
-		udev_wait
+		is_linux && udev_wait
+		block_device_wait "$device"
 		is_disk_device "$device" && return 0
 	done
 	log_fail "$device does not exist as a block device"
@ -96,8 +94,9 @@ function blockdev_missing # device
 	# because there are other commands (zfs snap, zfs inherit, zfs destroy)
 	# that can affect device nodes
 	for i in {1..3}; do
-		udev_wait
-		[[ ! -e "$device" ]] && return 0
+		is_linux && udev_wait
+		block_device_wait
+		is_disk_device "$device" || return 0
 	done
 	log_fail "$device exists when not expected"
 }
--- a/tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_rename_inuse.ksh
+++ b/tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_rename_inuse.ksh
@ -39,7 +39,7 @@ function cleanup
 	for ds in "$SENDFS" "$ZVOL" "$ZVOL-renamed"; do
 		destroy_dataset "$ds" '-rf'
 	done
-	udev_wait
+	block_device_wait
 }

 log_assert "Verify 'zfs rename' works on a ZVOL already in use as block device"
@ -54,7 +54,7 @@ SENDFS="$TESTPOOL/sendfs.$$"
 log_must zfs create -V $VOLSIZE "$ZVOL"

 # 2. Create a filesystem on the ZVOL device and mount it
-udev_wait
+block_device_wait "$ZDEV"
 log_must eval "new_fs $ZDEV >/dev/null 2>&1"
 log_must mkdir "$MNTPFS"
 log_must mount "$ZDEV" "$MNTPFS"
--- a/tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_snapdev.ksh
+++ b/tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_snapdev.ksh
@ -47,7 +47,7 @@ function cleanup
 	datasetexists $ZVOL && log_must zfs destroy -r $ZVOL
 	log_must zfs inherit snapdev $TESTPOOL
 	block_device_wait
-	udev_cleanup
+	is_linux && udev_cleanup
 }

 log_assert "Verify that ZFS volume property 'snapdev' works as expected."
--- a/tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_volmode.ksh
+++ b/tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_volmode.ksh
@ -44,19 +44,18 @@
 # 8. Verify "volmode" behaves accordingly to zvol_inhibit_dev (Linux only)
 #
 # NOTE: changing volmode may need to remove minors, which could be open, so call
-#       udev_wait() before we "zfs set volmode=<value>".
+#       block_device_wait() before we "zfs set volmode=<value>".

 verify_runnable "global"

 function cleanup
 {
-	datasetexists $VOLFS && log_must_busy zfs destroy -r $VOLFS
-	datasetexists $ZVOL && log_must_busy zfs destroy -r $ZVOL
-	log_must zfs inherit volmode $TESTPOOL
-	udev_wait
+	datasetexists $VOLFS && destroy_dataset $VOLFS -r
+	datasetexists $ZVOL && destroy_dataset $ZVOL -r
+	zfs inherit volmode $TESTPOOL
 	sysctl_inhibit_dev 0
 	sysctl_volmode 1
-	udev_cleanup
+	is_linux && udev_cleanup
 }

 #
@ -90,8 +89,8 @@ function test_io # dev
 {
 	typeset dev=$1

-	log_must dd if=/dev/zero of=$dev count=1
 	log_must dd if=$dev of=/dev/null count=1
+	log_must dd if=/dev/zero of=$dev count=1
 }

 log_assert "Verify that ZFS volume property 'volmode' works as intended"
@ -99,14 +98,14 @@ log_onexit cleanup

 VOLFS="$TESTPOOL/volfs"
 ZVOL="$TESTPOOL/vol"
-ZDEV="${ZVOL_DEVDIR}/$ZVOL"
+ZDEV="$ZVOL_DEVDIR/$ZVOL"
 SUBZVOL="$VOLFS/subvol"
-SUBZDEV="${ZVOL_DEVDIR}/$SUBZVOL"
+SUBZDEV="$ZVOL_DEVDIR/$SUBZVOL"

+# 0. Verify basic ZVOL functionality
 log_must zfs create -o mountpoint=none $VOLFS
 log_must zfs create -V $VOLSIZE -s $SUBZVOL
 log_must zfs create -V $VOLSIZE -s $ZVOL
-udev_wait
 blockdev_exists $ZDEV
 blockdev_exists $SUBZDEV
 test_io $ZDEV
@ -123,62 +122,63 @@ done
 log_must zfs set volmode=none $ZVOL
 blockdev_missing $ZDEV
 log_must_busy zfs destroy $ZVOL
+blockdev_missing $ZDEV

 # 3. Verify "volmode=full" exposes a fully functional device
 log_must zfs create -V $VOLSIZE -s $ZVOL
-udev_wait
+blockdev_exists $ZDEV
 log_must zfs set volmode=full $ZVOL
 blockdev_exists $ZDEV
 test_io $ZDEV
 log_must verify_partition $ZDEV
-udev_wait
 # 3.1 Verify "volmode=geom" is an alias for "volmode=full"
 log_must zfs set volmode=geom $ZVOL
 blockdev_exists $ZDEV
 if [[ "$(get_prop 'volmode' $ZVOL)" != "full" ]]; then
 	log_fail " Volmode value 'geom' is not an alias for 'full'"
 fi
-udev_wait
 log_must_busy zfs destroy $ZVOL
+blockdev_missing $ZDEV

 # 4. Verify "volmode=dev" hides partition info on the device
 log_must zfs create -V $VOLSIZE -s $ZVOL
-udev_wait
+blockdev_exists $ZDEV
 log_must zfs set volmode=dev $ZVOL
 blockdev_exists $ZDEV
 test_io $ZDEV
 log_mustnot verify_partition $ZDEV
-udev_wait
 log_must_busy zfs destroy $ZVOL
+blockdev_missing $ZDEV

 # 5. Verify "volmode=default" behaves accordingly to "volmode" module parameter
 # 5.1 Verify sysctl "volmode=full"
 sysctl_volmode 1
 log_must zfs create -V $VOLSIZE -s $ZVOL
-udev_wait
+blockdev_exists $ZDEV
 log_must zfs set volmode=default $ZVOL
 blockdev_exists $ZDEV
 log_must verify_partition $ZDEV
-udev_wait
 log_must_busy zfs destroy $ZVOL
+blockdev_missing $ZDEV
 # 5.2 Verify sysctl "volmode=dev"
 sysctl_volmode 2
 log_must zfs create -V $VOLSIZE -s $ZVOL
-udev_wait
+blockdev_exists $ZDEV
 log_must zfs set volmode=default $ZVOL
 blockdev_exists $ZDEV
 log_mustnot verify_partition $ZDEV
-udev_wait
 log_must_busy zfs destroy $ZVOL
+blockdev_missing $ZDEV
 # 5.2 Verify sysctl "volmode=none"
 sysctl_volmode 3
 log_must zfs create -V $VOLSIZE -s $ZVOL
-udev_wait
+blockdev_missing $ZDEV
 log_must zfs set volmode=default $ZVOL
 blockdev_missing $ZDEV

 # 6. Verify "volmode" property is inherited correctly
 log_must zfs inherit volmode $ZVOL
+blockdev_missing $ZDEV
 # 6.1 Check volmode=full case
 log_must zfs set volmode=full $TESTPOOL
 verify_inherited 'volmode' 'full' $ZVOL $TESTPOOL
@ -198,9 +198,7 @@ verify_inherited 'volmode' 'default' $ZVOL $TESTPOOL
 blockdev_exists $ZDEV
 # 6.5 Check inheritance on multiple levels
 log_must zfs inherit volmode $SUBZVOL
-udev_wait
 log_must zfs set volmode=none $VOLFS
-udev_wait
 log_must zfs set volmode=full $TESTPOOL
 verify_inherited 'volmode' 'none' $SUBZVOL $VOLFS
 blockdev_missing $SUBZDEV
@ -215,6 +213,8 @@ blockdev_exists $ZDEV
 blockdev_missing $SUBZDEV
 log_must_busy zfs destroy $ZVOL
 log_must_busy zfs destroy $SUBZVOL
+blockdev_missing $ZDEV
+blockdev_missing $SUBZDEV

 # 8. Verify "volmode" behaves accordingly to zvol_inhibit_dev (Linux only)
 if is_linux; then
@ -226,6 +226,7 @@ if is_linux; then
 	log_must zfs set volmode=full $ZVOL
 	blockdev_missing $ZDEV
 	log_must_busy zfs destroy $ZVOL
+	blockdev_missing $ZDEV
 	# 7.1 Verify device nodes not are not created with "volmode=dev"
 	sysctl_volmode 2
 	log_must zfs create -V $VOLSIZE -s $ZVOL
@ -233,6 +234,7 @@ if is_linux; then
 	log_must zfs set volmode=dev $ZVOL
 	blockdev_missing $ZDEV
 	log_must_busy zfs destroy $ZVOL
+	blockdev_missing $ZDEV
 	# 7.1 Verify device nodes not are not created with "volmode=none"
 	sysctl_volmode 3
 	log_must zfs create -V $VOLSIZE -s $ZVOL
--- a/tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_zil.ksh
+++ b/tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_zil.ksh
@ -42,8 +42,8 @@ verify_runnable "global"

 function cleanup
 {
-	datasetexists $ZVOL && log_must_busy zfs destroy $ZVOL
-	udev_wait
+	datasetexists $ZVOL && destroy_dataset $ZVOL
+	block_device_wait
 }

 log_assert "Verify ZIL functionality on ZVOLs"