-
Couldn't load subscription status.
- Fork 1.9k
zpool status -vv prints error ranges #9781
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
Conversation
61964ab to
325ce73
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9781 +/- ##
==========================================
+ Coverage 79.40% 79.66% +0.26%
==========================================
Files 385 385
Lines 121481 121533 +52
==========================================
+ Hits 96461 96821 +360
+ Misses 25020 24712 -308
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Rebasing this on master will resolve the CI failures. |
Add a '-vv' option to zpool status to print all error'd out blocks
ranges:
$ zpool status -vv
...
NAME STATE READ WRITE CKSUM
testpool ONLINE 0 0 0
loop0 ONLINE 0 0 20
errors: Permanent errors have been detected in the following files:
/var/tmp/testdir/10m_file: found 9 corrupted 128K blocks
[0x0-0x1ffff] (128K)
[0x100000-0x1fffff] (1M)
/var/tmp/testdir/1m_file: found 1 corrupted 128K block
[0x0-0x1ffff] (128K)
This is a modification of @TulsiJain's openzfs#8902 patch, with updates added
to print out the different ranges of errors, and to bring the code up to
date with master.
Signed-off-by: Tony Hutter <[email protected]>
Co-authored-by: TulsiJain <[email protected]>
325ce73 to
62b89fe
Compare
| $(top_builddir)/lib/libuutil/libuutil.la \ | ||
| $(top_builddir)/lib/libzfs/libzfs.la | ||
| $(top_builddir)/lib/libzfs/libzfs.la \ | ||
| $(top_builddir)/lib/libzpool/libzpool.la |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to avoid linking libzpool.la with any of the command line utilities. Since this was brought in solely for the btrees and range trees perhaps we can refactor those in a user space convenience library so they can be used. Alternately, we'll need to merge the bookmark_phys_t's in to ranges some other way.
| int cb_namewidth; | ||
| unsigned int cb_verbose; | ||
| boolean_t cb_allpools; | ||
| boolean_t cb_verbose; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should consider making this the default behavior and always printing the ranges.
| range_tree = range_tree_create(NULL, | ||
| RANGE_SEG64, NULL, 0, 0); | ||
| if (!range_tree) | ||
| goto fail; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of adding a label you can just break out of the while loop here.
| sizeof (*indrt_levels)); | ||
|
|
||
| /* Write our object group's objset and object */ | ||
| VERIFY0(nvlist_add_uint64(nv, ZPOOL_ERR_DATASET, zb[0].zb_objset)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the user space only code please use verify() instead.
| uint64_t zs_links; | ||
| uint64_t zs_ctime[2]; | ||
| uint64_t zs_data_block_size; | ||
| uint64_t zs_indirect_block_size; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While not needed by this patch I think it would be useful to add zs_dnode_size here as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also concur with @behlendorf's point that we shouldn't be linking in libzpool. Otherwise, it looks good to me.
|
|
||
| verify(nvlist_alloc(nverrlistp, 0, KM_SLEEP) == 0); | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: double blank line
|
@tonyhutter, I wanted to check where you are with this. Is this something that someone else could/should help to get this completed? |
|
@ahrens I'm pretty bandwidth limited right now, and it's a low priority fix for us. If someone wants to push this over the finish line, I'd be fine with that. |
|
Replaced by #17502. |
Print the byte error ranges with 'zpool status -vv'. This works with the normal zpool status formatting flags (-p, -j, --json-int). In addition: - Move range_tree/btree to common userspace/kernel code. - Modify ZFS_IOC_OBJ_TO_STATS ioctl to optionally return "extended" object stats. - Let zinject corrupt zvol data. - Add test case. This commit takes code from these PRs: openzfs#17502 openzfs#9781 openzfs#8902 Signed-off-by: Tony Hutter <[email protected]> Co-authored-by:: Alan Somers <[email protected]> Co-authored-by: TulsiJain <[email protected]>
Print the byte error ranges with 'zpool status -vv'. This works with the normal zpool status formatting flags (-p, -j, --json-int). In addition: - Move range_tree/btree to common userspace/kernel code. - Modify ZFS_IOC_OBJ_TO_STATS ioctl to optionally return "extended" object stats. - Let zinject corrupt zvol data. - Add test case. This commit takes code from these PRs: openzfs#17502 openzfs#9781 openzfs#8902 Signed-off-by: Tony Hutter <[email protected]> Co-authored-by:: Alan Somers <[email protected]> Co-authored-by: TulsiJain <[email protected]>
Motivation and Context
Print out the byte ranges of corruption in damaged files
Description
Add a
-vvoption tozpool statusto print all error'd out block ranges:This is a modification of @TulsiJain's #8902 patch, with updates added to print out the different ranges of errors, and to bring the code up to date with master.
How Has This Been Tested?
Added test case
Types of changes
Checklist:
Signed-off-by.