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: Introduce targeted corruption in file blocks #9288

Merged
merged 1 commit into from
Sep 9, 2019
Merged

ZTS: Introduce targeted corruption in file blocks #9288

merged 1 commit into from
Sep 9, 2019

Conversation

jwk404
Copy link
Contributor

@jwk404 jwk404 commented Sep 5, 2019

filetest_001_pos verifies that various checksum algorithms detect
corruption by overwriting the underlying vdev on which a file resides.
It is possible for the overwrite to miss the blocks of a file, causing a
spurious failure. This change introduces a function to corrupt the
individual blocks of a file as determined by zdb.

Signed-off-by: John Kennedy john.kennedy@delphix.com

Motivation and Context

This change introduces a new function to create more directly targeted
file corruption, and modifies a test to use it.

Description

The change uses the output of zdb to create a file with all the required
information to corrupt only the blocks that belong to a specific file at a
given level of indirection.

How Has This Been Tested?

Tested via ZTS with real disks as well as loop devices.

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 requested a review from a user September 5, 2019 20:18
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.

Nice! I know we've wanted this exact functionality when writing other test cases. In most cases I believe we've ended up using zinject to simulate it, but it wouldn't surprise me at all if there are additional existing tests would could take advantage of this.

tests/zfs-tests/include/blkdev.shlib Outdated Show resolved Hide resolved
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Sep 5, 2019
@codecov
Copy link

codecov bot commented Sep 6, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9288      +/-   ##
==========================================
+ Coverage   79.12%   79.31%   +0.18%     
==========================================
  Files         401      401              
  Lines      122063   122063              
==========================================
+ Hits        96587    96814     +227     
+ Misses      25476    25249     -227
Flag Coverage Δ
#kernel 79.82% <ø> (+0.07%) ⬆️
#user 67.32% <ø> (+0.56%) ⬆️

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 4bbf047...3dfb475. 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.

This is a great idea, thank you! Just a few suggestions.

tests/zfs-tests/include/blkdev.shlib Outdated Show resolved Hide resolved
tests/zfs-tests/include/blkdev.shlib Outdated Show resolved Hide resolved
tests/zfs-tests/include/blkdev.shlib Outdated Show resolved Hide resolved
tests/zfs-tests/include/blkdev.shlib Outdated Show resolved Hide resolved
filetest_001_pos verifies that various checksum algorithms detect
corruption by overwriting the underlying vdev on which a file resides.
It is possible for the overwrite to miss the blocks of a file, causing a
spurious failure. This change introduces a function to corrupt the
individual blocks of a file as determined by zdb.

Signed-off-by: John Kennedy <john.kennedy@delphix.com>
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.

Thanks!

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Sep 9, 2019
@behlendorf behlendorf merged commit b63e2d8 into openzfs:master Sep 9, 2019
mattmacy pushed a commit to zfsonfreebsd/ZoF that referenced this pull request Sep 10, 2019
filetest_001_pos verifies that various checksum algorithms detect
corruption by overwriting the underlying vdev on which a file resides.
It is possible for the overwrite to miss the blocks of a file, causing a
spurious failure. This change introduces a function to corrupt the
individual blocks of a file as determined by zdb.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Ryan Moeller <ryan@ixsystems.com>
Signed-off-by: John Kennedy <john.kennedy@delphix.com>
Closes openzfs#9288
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Dec 24, 2019
filetest_001_pos verifies that various checksum algorithms detect
corruption by overwriting the underlying vdev on which a file resides.
It is possible for the overwrite to miss the blocks of a file, causing a
spurious failure. This change introduces a function to corrupt the
individual blocks of a file as determined by zdb.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Ryan Moeller <ryan@ixsystems.com>
Signed-off-by: John Kennedy <john.kennedy@delphix.com>
Closes openzfs#9288
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Dec 27, 2019
filetest_001_pos verifies that various checksum algorithms detect
corruption by overwriting the underlying vdev on which a file resides.
It is possible for the overwrite to miss the blocks of a file, causing a
spurious failure. This change introduces a function to corrupt the
individual blocks of a file as determined by zdb.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Ryan Moeller <ryan@ixsystems.com>
Signed-off-by: John Kennedy <john.kennedy@delphix.com>
Closes openzfs#9288
tonyhutter pushed a commit that referenced this pull request Jan 23, 2020
filetest_001_pos verifies that various checksum algorithms detect
corruption by overwriting the underlying vdev on which a file resides.
It is possible for the overwrite to miss the blocks of a file, causing a
spurious failure. This change introduces a function to corrupt the
individual blocks of a file as determined by zdb.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Ryan Moeller <ryan@ixsystems.com>
Signed-off-by: John Kennedy <john.kennedy@delphix.com>
Closes #9288
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