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.

Sign up for GitHub

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

Jump to bottom

Fix divide-by-zero in mmp_delay_update() #7391

Merged
merged 1 commit into from Apr 6, 2018
Merged

Fix divide-by-zero in mmp_delay_update() #7391

merged 1 commit into from Apr 6, 2018

Conversation

ofaaland
Copy link
Contributor

@ofaaland ofaaland commented Apr 5, 2018 •
edited

Description

Replace vdev_count_leaves() with MAX(1,vdev_count_leaves()) in denominator of expression.

Motivation and Context

vdev_count_leaves() may return 0, resulting in divide-by-zero. Caught by Coverity.
Introduced by

  • 533ea04 Update mmp_delay on sync or skipped, failed write

How Has This Been Tested?

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.
  • All new and existing tests passed.
  • All commit messages are properly formatted and contain Signed-off-by.
  • Change has been approved by a ZFS on Linux member.

@ofaaland
Fix divide-by-zero in mmp_delay_update()
3f391d4 
vdev_count_leaves() in the denominator may return 0, caught by Coverity.
Introduced by

* 533ea04 Update mmp_delay on sync or skipped, failed write

Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
@codecov
Copy link

codecov bot commented Apr 5, 2018 •
edited

Codecov Report

Merging #7391 into master will decrease coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7391      +/-   ##
==========================================
- Coverage   76.39%   76.33%   -0.06%     
==========================================
  Files         329      329              
  Lines      104248   104248              
==========================================
- Hits        79638    79576      -62     
- Misses      24610    24672      +62
Flag Coverage Δ
#kernel 76.34% <100%> (+0.17%) ⬆️
#user 65.67% <100%> (+0.02%) ⬆️

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 533ea04...3f391d4. Read the comment docs.

@behlendorf behlendorf merged commit 0ba106e into openzfs:master Apr 6, 2018
@adilger
Copy link
Contributor

adilger commented May 26, 2018

It looks like this patch still needs to be landed onto 0.7, or is vdev_count_leaves(spa) == 0 caused by something only on master (e.g. device removal)?

@ofaaland
Copy link
Contributor Author

@tonyhutter , can you consider this for 0.7.10?

@tonyhutter
Copy link
Contributor

Yep, I'll include it

@tonyhutter tonyhutter added this to To do in 0.7.10 May 29, 2018
@tonyhutter tonyhutter moved this from To do to In progress in 0.7.10 Aug 14, 2018
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 15, 2018
@ofaaland @tonyhutter
Fix divide-by-zero in mmp_delay_update()
e60b495 
vdev_count_leaves() in the denominator may return 0, caught by Coverity.
Introduced by

* 533ea04 Update mmp_delay on sync or skipped, failed write

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: George Melikov <mail@gmelikov.ru>
Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Closes openzfs#7391
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 23, 2018
@ofaaland @tonyhutter
Fix divide-by-zero in mmp_delay_update()
ce32ed6 
vdev_count_leaves() in the denominator may return 0, caught by Coverity.
Introduced by

* 533ea04 Update mmp_delay on sync or skipped, failed write

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: George Melikov <mail@gmelikov.ru>
Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Closes openzfs#7391
@tonyhutter tonyhutter moved this from In progress to Done in 0.7.10 Aug 30, 2018
@ofaaland ofaaland deleted the b_mmp_no_div_zero branch September 5, 2018 15:53
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 5, 2018
@ofaaland @tonyhutter
Fix divide-by-zero in mmp_delay_update()
6b5cc49 
vdev_count_leaves() in the denominator may return 0, caught by Coverity.
Introduced by

* 533ea04 Update mmp_delay on sync or skipped, failed write

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: George Melikov <mail@gmelikov.ru>
Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Closes openzfs#7391
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behlendorf behlendorf behlendorf approved these changes

@gmelikov gmelikov gmelikov approved these changes

@dinatale2 dinatale2 dinatale2 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
No open projects
0.7.10
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@ofaaland @adilger @tonyhutter @behlendorf @gmelikov @dinatale2