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

zdb long options #8846

Closed
wants to merge 2 commits into from
Closed

Conversation

richardelling
Copy link
Contributor

Add long options to zdb command.
Includes updates to ZTS tests and man page.
Includes fix for known failures of zdb_006_pos

Motivation and Context

Resolves #8739

Description

Adds long option names for existing zdb options.
Adds --help
Includes support for future long options that will be needed for the
draid project.

How Has This Been Tested?

ZTS tests included and pass

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:

@codecov
Copy link

codecov bot commented Jun 2, 2019

Codecov Report

Merging #8846 into master will decrease coverage by 0.11%.
The diff coverage is 86.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8846      +/-   ##
==========================================
- Coverage   78.77%   78.65%   -0.12%     
==========================================
  Files         382      382              
  Lines      117770   117754      -16     
==========================================
- Hits        92775    92625     -150     
- Misses      24995    25129     +134
Flag Coverage Δ
#kernel 79.28% <ø> (-0.06%) ⬇️
#user 67.38% <86.66%> (+0.28%) ⬆️

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 2531ce3...7dc9d7d. Read the comment docs.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jun 5, 2019
@behlendorf behlendorf self-requested a review June 5, 2019 00:00
"\t\t<poolname> <vdev>:<offset>:<size>[:<flags>]\n"
"\t%s -E [-A] word0:word1:...:word15\n"
"\t%s -S [-AP] [-e [-V] [-p <path> ...]] [-U <cache>] "
"\t%s -S [-AP] [-e [-V] [-p <path> ...]] [-U <cachefile>] "
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look like this help output is yet over the 80 column limit. But this would be a good opportunity to extend commit 2696e86 to cover the zdb help output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea, I'll add a test case for this

"Options to control amount of output:\n"
" -b, --block block information\n"
" -c, --checksum checksum all metadata "
"(-cc for all data) blocks\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there are some tabs vs spaces white space formatting issues for this block.

@@ -6042,12 +6118,27 @@ main(int argc, char **argv)
argc -= optind;
argv += optind;

if (argc < 2 && dump_opt['R'])
if (getenv("ZFS_OPTIONS_DEBUG") != NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you either document ZFS_OPTIONS_DEBUG in the zdb man page, or if it's intended to only be used for internal ZTS testing add a comment to that effect here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is for testing only, I'll add references to make it clearer

@behlendorf behlendorf added Status: Revision Needed Changes are required for the PR to be accepted and removed Status: Code Review Needed Ready for review and testing labels Jun 12, 2019
@behlendorf behlendorf added Status: Work in Progress Not yet ready for general review and removed Status: Revision Needed Changes are required for the PR to be accepted labels Jul 17, 2019
@behlendorf
Copy link
Contributor

@richardelling how would you like to move forward with this. If you could address the handful of review comments and get it rebased we can get it merged.

@richardelling
Copy link
Contributor Author

I've got too many deadlines this week, was planning to rebase and address the other comments this weekend.

@behlendorf
Copy link
Contributor

@richardelling it'd be great if you could dust this off, rebase, and address the comments.

@ahrens ahrens added Status: Inactive Not being actively updated and removed Status: Work in Progress Not yet ready for general review labels Nov 15, 2019
@ahrens
Copy link
Member

ahrens commented Nov 15, 2019

@richardelling When do you think you'll have time to get back to this?

@richardelling
Copy link
Contributor Author

after supercomputing conference

@behlendorf
Copy link
Contributor

Closing. @richardelling if you get a chance to get back to this please feel free to open a new PR.

@behlendorf behlendorf closed this Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Inactive Not being actively updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zdb needs long options
3 participants