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 zdb failure with EREMOTEIO when examining live pool #7801

Merged
merged 1 commit into from Aug 20, 2018

Conversation

ofaaland
Copy link
Contributor

@ofaaland ofaaland commented Aug 18, 2018

Motivation and Context

Fixes #7797

Description

Within zdb, sets flag ZFS_IMPORT_SKIP_MMP in spa->spa_import_flags before spa_open() and related calls. The pools are opened read-only anyway, so in the worst case zdb crashes if the pool structure is changing at the wrong time.

How Has This Been Tested?

Added zdb tests to mmp test suite.

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 ofaaland added the Status: Work in Progress Not yet ready for general review label Aug 18, 2018
#

#
# Copyright (c) 2018 by Nutanix. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

Update the copyright.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Since zdb opens the pools read-only, it cannot damage the pool in the
event the pool is already imported either on the same host or on
another one.

If the pool vdev structure is changing while zdb is importing the
pool, it may cause zdb to crash.  However this is unlikely, and in any
case it's a user space process and can simply be run again.

For this reason, zdb should disable the multihost activity test on
import that is normally run.

This commit fixes a few zdb code paths where that had been overlooked.
It also adds tests to ensure that several common use cases handle this
properly in the future.

Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
@ofaaland ofaaland force-pushed the b-zdb-remoteio-imported-pool branch from 3d6338d to 1d17765 Compare August 19, 2018 00:26
@ofaaland ofaaland changed the title [WIP] Fix zdb EREMOTEIO when examining live pool Fix zdb failure with EREMOTEIO when examining live pool Aug 19, 2018
@ofaaland ofaaland removed the Status: Work in Progress Not yet ready for general review label Aug 19, 2018
@codecov
Copy link

codecov bot commented Aug 19, 2018

Codecov Report

Merging #7801 into master will increase coverage by 0.15%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7801      +/-   ##
==========================================
+ Coverage   78.38%   78.54%   +0.15%     
==========================================
  Files         373      373              
  Lines      112791   112795       +4     
==========================================
+ Hits        88413    88596     +183     
+ Misses      24378    24199     -179
Flag Coverage Δ
#kernel 78.62% <ø> (+0.02%) ⬆️
#user 67.88% <100%> (+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 a9d6270...1d17765. Read the comment docs.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) Reviewed labels Aug 20, 2018
@behlendorf behlendorf merged commit 34fe773 into openzfs:master Aug 20, 2018
@ofaaland ofaaland deleted the b-zdb-remoteio-imported-pool branch August 20, 2018 17:09
@tonyhutter tonyhutter added this to To do in 0.7.11 Sep 13, 2018
@tonyhutter tonyhutter added this to To do in 0.7.12 Sep 14, 2018
@tonyhutter tonyhutter moved this from To do to In progress in 0.7.12 Oct 26, 2018
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Oct 31, 2018
Since zdb opens the pools read-only, it cannot damage the pool in the
event the pool is already imported either on the same host or on
another one.

If the pool vdev structure is changing while zdb is importing the
pool, it may cause zdb to crash.  However this is unlikely, and in any
case it's a user space process and can simply be run again.

For this reason, zdb should disable the multihost activity test on
import that is normally run.

This commit fixes a few zdb code paths where that had been overlooked.
It also adds tests to ensure that several common use cases handle this
properly in the future.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Gu Zheng <guzheng2331314@163.com>
Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Closes openzfs#7797
Closes openzfs#7801
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Nov 5, 2018
Since zdb opens the pools read-only, it cannot damage the pool in the
event the pool is already imported either on the same host or on
another one.

If the pool vdev structure is changing while zdb is importing the
pool, it may cause zdb to crash.  However this is unlikely, and in any
case it's a user space process and can simply be run again.

For this reason, zdb should disable the multihost activity test on
import that is normally run.

This commit fixes a few zdb code paths where that had been overlooked.
It also adds tests to ensure that several common use cases handle this
properly in the future.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Gu Zheng <guzheng2331314@163.com>
Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Closes openzfs#7797
Closes openzfs#7801
tonyhutter pushed a commit that referenced this pull request Nov 13, 2018
Since zdb opens the pools read-only, it cannot damage the pool in the
event the pool is already imported either on the same host or on
another one.

If the pool vdev structure is changing while zdb is importing the
pool, it may cause zdb to crash.  However this is unlikely, and in any
case it's a user space process and can simply be run again.

For this reason, zdb should disable the multihost activity test on
import that is normally run.

This commit fixes a few zdb code paths where that had been overlooked.
It also adds tests to ensure that several common use cases handle this
properly in the future.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Gu Zheng <guzheng2331314@163.com>
Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Closes #7797
Closes #7801
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
No open projects
0.7.11
  
To do
0.7.12
  
In progress
Development

Successfully merging this pull request may close these issues.

zdb <dataset> fails with Remote I/O error when Multihost is enabled
3 participants