From 96ad227a9dad38dd928bda6016240f590e7968b1 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Fri, 1 Oct 2021 11:36:02 -0400 Subject: [PATCH] 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 Reviewed-by: John Kennedy Signed-off-by: Ryan Moeller Closes #12583 --- tests/test-runner/bin/zts-report.py.in | 1 - tests/zfs-tests/include/blkdev.shlib | 2 +- .../zvol/zvol_misc/zvol_misc_common.kshlib | 11 ++--- .../zvol/zvol_misc/zvol_misc_rename_inuse.ksh | 4 +- .../zvol/zvol_misc/zvol_misc_snapdev.ksh | 2 +- .../zvol/zvol_misc/zvol_misc_volmode.ksh | 46 ++++++++++--------- .../zvol/zvol_misc/zvol_misc_zil.ksh | 4 +- 7 files changed, 35 insertions(+), 35 deletions(-) diff --git a/tests/test-runner/bin/zts-report.py.in b/tests/test-runner/bin/zts-report.py.in index 1acb8b9c1b..0cf73c3c75 100755 --- 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({ diff --git a/tests/zfs-tests/include/blkdev.shlib b/tests/zfs-tests/include/blkdev.shlib index bcba8ee759..bf70952904 100644 --- 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)) diff --git 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 index 3ee09a151b..b69d2ce029 100644 --- 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" } diff --git 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 index c4ea23c48d..e9b7bbc4c4 100755 --- 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" diff --git 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 index 8d95bfa393..bbe40734f9 100755 --- 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." diff --git 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 index 322a31e07d..264ec131ad 100755 --- 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=". +# block_device_wait() before we "zfs set volmode=". 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 diff --git 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 index b8989f4787..a393606d0f 100755 --- 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"