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: Test the correct filesystem_limits behavior #10082

Merged
merged 1 commit into from
Mar 4, 2020

Conversation

ghost
Copy link

@ghost ghost commented Feb 29, 2020

Motivation and Context

See issue #8226: Property filesystem_limit does not work as documented

There have been previous attempts to fix the behavior on Linux, but so
far the issue is still open. See PRs #8228, #8280.

The existing tests pass for the incorrect behavior. This is a problem
on FreeBSD; we are failing the tests because we implement the feature
correctly.

Description

I have adapted the tests based on the work by @loli10K in #8280 and
extended the changes to fix the snapshot_limit test as well.

Linux now fails these tests, so entries linking to the issue have been
added to the "maybe" group in zts-report.py.

How Has This Been Tested?

ZTS passes limits on FreeBSD now.

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:

  • My code follows the ZFS on Linux 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.

@ghost 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 Feb 29, 2020
See issue openzfs#8226: Property filesystem_limit does not work as documented

There have been previous attempts to fix the behavior on Linux, but so
far the issue is still open.  See PRs openzfs#8228, openzfs#8280.

The existing tests pass for the incorrect behavior.  This is a problem
on FreeBSD; we are failing the tests because we implement the feature
correctly.

I have adapted the tests based on the work by @loli10K in openzfs#8280 and
extended the changes to fix the snapshot_limit test as well.

Linux now fails these tests, so entries linking to the issue have been
added to the "maybe" group in zts-report.py.

Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
@ghost ghost force-pushed the zts-limits branch from 42f451a to 56785fb Compare February 29, 2020 05:46
@ghost
Copy link
Author

ghost commented Feb 29, 2020

  • Fixed spelling in zts-report.py

@codecov-io
Copy link

codecov-io commented Feb 29, 2020

Codecov Report

Merging #10082 into master will increase coverage by <1%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #10082    +/-   ##
========================================
+ Coverage      79%      80%   +<1%     
========================================
  Files         385      385            
  Lines      122306   122306            
========================================
+ Hits        96999    97393   +394     
+ Misses      25307    24913   -394
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 d32eff3...56785fb. Read the comment docs.

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Getting the test case in order makes good sense, even if it's still expected to fail on Linux. @loli10K would you mind also looking at this since the ZTS changes are largely from your original PR?

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Mar 3, 2020
@behlendorf behlendorf merged commit d16c029 into openzfs:master Mar 4, 2020
@ghost ghost deleted the zts-limits branch March 4, 2020 23:49
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
See issue openzfs#8226: Property filesystem_limit does not work as documented

There have been previous attempts to fix the behavior on Linux, but so
far the issue is still open.  See PRs openzfs#8228, openzfs#8280.

The existing tests pass for the incorrect behavior.  This is a problem
on FreeBSD; we are failing the tests because we implement the feature
correctly.

I have adapted the tests based on the work by @loli10K in openzfs#8280 and
extended the changes to fix the snapshot_limit test as well.

Linux now fails these tests, so entries linking to the issue have been
added to the "maybe" group in zts-report.py.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes openzfs#10082
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.

2 participants