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

Fix and reenable zfs_rename tests #8088

Merged
merged 1 commit into from
Nov 8, 2018

Conversation

tcaputi
Copy link
Contributor

@tcaputi tcaputi commented Nov 2, 2018

zfs_rename_006_pos has been flaky in the past because it was
missing a call to block_device_wait to ensure the zvols it creates
are present before running dd. Whenever this this happened,
zfs_rename_009_neg would also fail because the first test would
leak a zvol clone that it did not know how to clean up. This patch
fixes the root cause and reenables the test. It also fixes some
minor grammar errors.

Signed-off-by: Tom Caputi tcaputi@datto.com

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:

zfs_rename_006_pos has been flaky in the past because it was
missing a call to block_device_wait to ensure the zvols it creates
are present before running dd. Whenever this this happened,
zfs_rename_009_neg would also fail because the first test would
leak a zvol clone that it did not know how to clean up. This patch
fixes the root cause and reenables the test. It also fixes some
minor grammar errors.

Signed-off-by: Tom Caputi <tcaputi@datto.com>
@tcaputi tcaputi mentioned this pull request Nov 2, 2018
12 tasks
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Nov 2, 2018
@codecov
Copy link

codecov bot commented Nov 3, 2018

Codecov Report

Merging #8088 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8088      +/-   ##
==========================================
- Coverage   78.46%   78.42%   -0.04%     
==========================================
  Files         377      377              
  Lines      114518   114518              
==========================================
- Hits        89851    89816      -35     
- Misses      24667    24702      +35
Flag Coverage Δ
#kernel 78.75% <ø> (-0.02%) ⬇️
#user 67.38% <ø> (-0.05%) ⬇️

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 6644e5b...afa6236. Read the comment docs.

@behlendorf behlendorf requested a review from jwk404 November 5, 2018 17:50
@behlendorf behlendorf merged commit d8244d3 into openzfs:master Nov 8, 2018
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Nov 8, 2018
GregorKopka pushed a commit to GregorKopka/zfs that referenced this pull request Jan 7, 2019
zfs_rename_006_pos has been flaky in the past because it was
missing a call to block_device_wait to ensure the zvols it creates
are present before running dd. Whenever this this happened,
zfs_rename_009_neg would also fail because the first test would
leak a zvol clone that it did not know how to clean up. This patch
fixes the root cause and reenables the test. It also fixes some
minor grammar errors.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tom Caputi <tcaputi@datto.com>
Closes openzfs#5647 
Closes openzfs#5648 
Closes openzfs#8088
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants