Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZTS: devices_001_pos and devices_002_neg #9773

Merged
merged 1 commit into from
Dec 27, 2019

Conversation

behlendorf
Copy link
Contributor

Motivation and Context

The way the devices_001_pos and devices_002_neg tests were
original written and then ported has resulted ZTS failures when running
in environment with only NVMe devices. Update the test case so they
no longer need to grub around in the /dev/ directory to determine the
major and minor numbers to be used.

Description

Update the devices_001_pos and devices_002_neg test cases such that the
special block device file created is backed by a ZFS volume. Specifying
a specific device allows the major and minor numbers to be easily
determined. Furthermore, this avoids the potentially dangerous behavior
of opening the first block device we happen to find under /dev/.

How Has This Been Tested?

Manually tested in the problematic environment as configured by the CI.
For this initial PR I've restricted the ZTS tags run to those I've observed
fail in the CI without this change. Assuming the CI confirms all is will
I'll resubmit this for a full ZTS run.

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)
  • Documentation (a change to man pages or other documentation)

Checklist:

@behlendorf behlendorf 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 Dec 26, 2019
Update the devices_001_pos and devices_002_neg test cases such that the
special block device file created is backed by a ZFS volume.  Specifying
a specific device allows the major and minor numbers to be easily
determined.  Furthermore, this avoids the potentially dangerous behavior
of opening the first block device we happen to find under /dev/.

Signed-off-by: Brian Behlendorf <[email protected]>
@behlendorf behlendorf requested a review from a user December 27, 2019 04:19
@codecov
Copy link

codecov bot commented Dec 27, 2019

Codecov Report

Merging #9773 into master will increase coverage by 13%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9773      +/-   ##
==========================================
+ Coverage      67%      80%     +13%     
==========================================
  Files         303      385      +82     
  Lines      104240   121475   +17235     
==========================================
+ Hits        69658    96888   +27230     
+ Misses      34582    24587    -9995
Flag Coverage Δ
#kernel 80% <ø> (?)
#user 67% <ø> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5c97f3...cdfb704. Read the comment docs.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The devices property is not currently supported/tested on FreeBSD (the devices tag is in linux.run). We have a special devfs for device nodes. We can drop the FreeBSD parts in this PR if you like.

@behlendorf
Copy link
Contributor Author

We can drop the FreeBSD parts in this PR if you like.

I'd like to keep it, presumably at some point we'll want to enable this test case for FreeBSD.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 27, 2019
@behlendorf behlendorf merged commit 590ff61 into openzfs:master Dec 27, 2019
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Jan 2, 2020
Update the devices_001_pos and devices_002_neg test cases such that the
special block device file created is backed by a ZFS volume.  Specifying
a specific device allows the major and minor numbers to be easily
determined.  Furthermore, this avoids the potentially dangerous behavior
of opening the first block device we happen to find under /dev/.

Reviewed-by: Ryan Moeller <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#9773
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Jan 7, 2020
Update the devices_001_pos and devices_002_neg test cases such that the
special block device file created is backed by a ZFS volume.  Specifying
a specific device allows the major and minor numbers to be easily
determined.  Furthermore, this avoids the potentially dangerous behavior
of opening the first block device we happen to find under /dev/.

Reviewed-by: Ryan Moeller <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#9773
tonyhutter pushed a commit that referenced this pull request Jan 23, 2020
Update the devices_001_pos and devices_002_neg test cases such that the
special block device file created is backed by a ZFS volume.  Specifying
a specific device allows the major and minor numbers to be easily
determined.  Furthermore, this avoids the potentially dangerous behavior
of opening the first block device we happen to find under /dev/.

Reviewed-by: Ryan Moeller <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #9773
@behlendorf behlendorf deleted the zts-devices branch April 19, 2021 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Test Suite Indicates an issue with the test framework or a test case Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant