ZTS: Eliminate udev_wait in zvol_misc tests #12583

ghost · 2021-09-23T13:43:42Z

Motivation and Context

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.

Description

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.
Remove zvol_misc_volmode from the maybe failing tests on FreeBSD in
zts-report.py.

How Has This Been Tested?

Tested across all versions of FreeBSD several times and on Debian to ensure no unexpected breakage on Linux.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the OpenZFS code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
I have run the ZFS Test Suite with this change applied.
All commit messages are properly formatted and contain Signed-off-by.

behlendorf

Thanks!

behlendorf · 2021-09-23T16:37:22Z

tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_common.kshlib

@@ -37,8 +37,11 @@
 #
 function udev_wait
 {
+	if ! is_linux; then


It may be possible to use block_device_wait on all platforms now that devices can be passed to it. I'd think it would now correctly retry in the cases where the device was open, which would be more reliably then just checking if zvol_id or blkid are running. It's probably worth trying.

Ok I'll give that a shot. That was my original plan until I saw what was going on in here then I started having second thoughts about touching something so purpose-built. 😅

behlendorf

Thanks!

behlendorf

Looks good assuming the CI confirms all is well.

tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_common.kshlib

jwk404 · 2021-09-24T21:05:29Z

@freqlabs the Ubuntu 20.04 ZTS run had a failure in zvol_misc_volmode that looks at first glance to be related. Could you take a look?

ghost · 2021-09-27T16:02:37Z

That may be a legitimate bug, it seems a device node was created in spite of zvol_inhibit_dev=1 being set. I've changed blockdev_missing to use is_disk_device instead of merely checking whether a file of any type exists in case it might catch some IO test having written out a regular file there by mistake.

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. Signed-off-by: Ryan Moeller <ryan@iXsystems.com>

ghost · 2021-09-29T17:55:33Z

Swapped the order of dd commands in test_io so we attempt to read before writing
Brought back udev_wait since tests seem to be flaky on Linux without it, but used only in blockdev_exists and blockdev_missing
Pulled is_linux out of udev_wait and udev_cleanup to better telegraph that they are only for Linux at the call site
Silence udevadm trigger error for devices that don't exist yet

behlendorf · 2021-09-30T20:26:26Z

Thanks for updating this again, I think we can move forward with this as is.

jwk404 · 2021-09-30T21:15:59Z

@freqlabs could you please add zvol_misc_volmode to the linux section in zts_report.py, and we can get this committed.

ghost · 2021-09-30T21:41:02Z

@freqlabs could you please add zvol_misc_volmode to the linux section in zts_report.py, and we can get this committed.

It was only in there for FreeBSD before, so ideally it shouldn't be needed for Linux there now. The intent of this PR is to make the tests less flaky so it can be removed from zts_report.py.

jwk404 · 2021-10-01T00:02:36Z

It was only in there for FreeBSD before, so ideally it shouldn't be needed for Linux there now. The intent of this PR is to make the tests less flaky so it can be removed from zts_report.py.

Understood. However on the previous 20.04 run, zvol_misc_volmode failed in the same place. Let's see how the one currently in progress goes.

behlendorf · 2021-10-01T00:09:43Z

Right, it may be that this test occasionally fails on Linux as well. Although, looking through the last 2 weeks of logs I wasn't able to find any examples so I'm not sure. Let's just try and confirm we haven't made it more likely by running it through a couple of times.

jwk404 · 2021-10-01T15:35:17Z

This one passed. I'll merge this now, and if the test proves to be a problem on Linux, I can update the script later.

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 openzfs#12583

ghost added Component: Test Suite Indicates an issue with the test framework or a test case Status: Code Review Needed Ready for review and testing labels Sep 23, 2021

ghost requested a review from behlendorf September 23, 2021 13:43

jwk404 self-assigned this Sep 23, 2021

behlendorf reviewed Sep 23, 2021

View reviewed changes

ghost force-pushed the zts-udev_wait branch from b0e121c to 82d4324 Compare September 23, 2021 17:59

ghost changed the title ~~ZTS: Improve udev_wait in zvol_misc tests~~ ZTS: Eliminate udev_wait in zvol_misc tests Sep 23, 2021

behlendorf approved these changes Sep 23, 2021

View reviewed changes

tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_common.kshlib Show resolved Hide resolved

jwk404 approved these changes Sep 24, 2021

View reviewed changes

ghost force-pushed the zts-udev_wait branch from 82d4324 to c688917 Compare September 27, 2021 16:01

ghost force-pushed the zts-udev_wait branch from c688917 to 81207f6 Compare September 29, 2021 17:46

behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Sep 30, 2021

jwk404 merged commit 96ad227 into openzfs:master Oct 1, 2021

ghost deleted the zts-udev_wait branch October 1, 2021 16:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ZTS: Eliminate udev_wait in zvol_misc tests #12583

ZTS: Eliminate udev_wait in zvol_misc tests #12583

ghost commented Sep 23, 2021 •

edited by ghost

Loading

behlendorf left a comment

behlendorf Sep 23, 2021

ghost Sep 23, 2021

behlendorf left a comment

behlendorf left a comment

jwk404 commented Sep 24, 2021

ghost commented Sep 27, 2021

ghost commented Sep 29, 2021

behlendorf commented Sep 30, 2021

jwk404 commented Sep 30, 2021

ghost commented Sep 30, 2021 •

edited by ghost

Loading

jwk404 commented Oct 1, 2021

behlendorf commented Oct 1, 2021

jwk404 commented Oct 1, 2021

ZTS: Eliminate udev_wait in zvol_misc tests #12583

ZTS: Eliminate udev_wait in zvol_misc tests #12583

Conversation

ghost commented Sep 23, 2021 • edited by ghost Loading

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

behlendorf left a comment

Choose a reason for hiding this comment

behlendorf Sep 23, 2021

Choose a reason for hiding this comment

ghost Sep 23, 2021

Choose a reason for hiding this comment

behlendorf left a comment

Choose a reason for hiding this comment

behlendorf left a comment

Choose a reason for hiding this comment

jwk404 commented Sep 24, 2021

ghost commented Sep 27, 2021

ghost commented Sep 29, 2021

behlendorf commented Sep 30, 2021

jwk404 commented Sep 30, 2021

ghost commented Sep 30, 2021 • edited by ghost Loading

jwk404 commented Oct 1, 2021

behlendorf commented Oct 1, 2021

jwk404 commented Oct 1, 2021

ghost commented Sep 23, 2021 •

edited by ghost

Loading

ghost commented Sep 30, 2021 •

edited by ghost

Loading