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 problems with zdb -d <pool>/<objset ID> #12944

Merged
merged 1 commit into from Jan 20, 2022
Merged

Conversation

PaulZ-98
Copy link
Contributor

@PaulZ-98 PaulZ-98 commented Jan 7, 2022

zdb -d <pool>/<objset ID> does not work when
other command line arguments are included i.e.
zdb -U <cachefile> -d <pool>/<objset ID>
This change fixes the command line parsing
to handle this situation. Also fix issue
where zdb -r <dataset> <file> does not handle
the root <dataset> of the pool. Introduce -N
option to force <objset ID> to be interpreted
as a numeric objsetID.

Fixes #12845
Signed-off-by: Paul Zuchowski pzuchowski@datto.com

Motivation and Context

zdb -d <pool>/<objset ID> has several issues.

  • fails when <pool>/<objset ID> is not argv[2]
  • fails if <objset ID> is a dataset beginning with number(s)
  • does not find <pool>/100 where 100 is the dataset name

#12845

Description

Removed the code assuming argv[2]. Added helper function to see if is numeric. Added code to assume / is a named dataset, and if that fails, silently try as an objsetID. Added -N to force to be an objset ID, not a named dataset. Augmented the zfstest for zdb -d /. Updated man page for -N.

How Has This Been Tested?

zdb section of zfstest, also
If you have tank/100 and tank/fs with an objset ID of 100:
zdb -d tank/100 displays tank/100
zdb -N tank/100 displays tank/fs
-N is the same as -d, or you can do -Nd (or -NNN converted to -ddd internally..)
If you have tank/fs1 with an objset ID of 101:
zdb -d tank/101 will try to find "tank/101" (i.e. 101 is the dataset name), and fail silently and try tank/101 as objsetID 101 and find tank/fs1
zdb -N tank/101 will display tank/fs1
If you have tank/118 with objsetID 201:
zdb -N tank/118 will fail
zdb -d tank/118 will succeed

Types of changes

  • [x ] 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)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

man/man8/zdb.8 Outdated Show resolved Hide resolved
@rincebrain
Copy link
Contributor

So, experimentally, I think -N does something that's not quite specified in the manual, at least on my system.

I would have expected, given dataset paths "workspace/1M", "workspace/0x512" and "workspace/1M/0x512" exist, zdb -d to work as expected (now) on all three, but zdb -N to fail on 1 and 3 and work IFF a dataset with id 0x512 exists, but experimentally (-d does work as expected):

$ sudo cmd/zdb/zdb -N workspace/1M
Dataset workspace/1M [ZPL], ID 60935, cr_txg 35492375, 96K, 7 objects
$ sudo cmd/zdb/zdb -N workspace/0x512
failed to hold objset 1298: No such file or directory
zdb: can't open 'workspace/0x512': No such file or directory
$ sudo cmd/zdb/zdb -N workspace/1M/0x512
Dataset workspace/1M/0x512 [ZPL], ID 60995, cr_txg 35492517, 96K, 6 objects

I don't think the behavior it has now is fundamentally wrong in any way, just that it's not what I took from the man page update and my own assumptions - I would have expected "force only objsetid interpretation", not "try objsetid interpretation if it looks like a number, otherwise just act like you passed -d".

@PaulZ-98
Copy link
Contributor Author

I opted to close the loophole with -N noted by @rincebrain.

zdb -d <pool>/<objset ID> does not work when
other command line arguments are included i.e.
zdb -U <cachefile> -d <pool>/<objset ID>
This change fixes the command line parsing
to handle this situation.  Also fix issue
where zdb -r <dataset> <file> does not handle
the root <dataset> of the pool. Introduce -N
option to force <objset ID> to be interpreted
as a numeric objsetID.

Fixes openzfs#12845
Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
Copy link
Contributor

@rincebrain rincebrain left a comment

Choose a reason for hiding this comment

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

$ sudo cmd/zdb/zdb -N workspace/1M
Supply a numeric objset ID with -N
$ sudo cmd/zdb/zdb -N workspace/0x512
failed to hold objset 1298: No such file or directory
zdb: can't open 'workspace/0x512': No such file or directory
$ sudo cmd/zdb/zdb -N workspace/1M/0x512
Supply a numeric objset ID with -N
$

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jan 14, 2022
@tonynguien tonynguien merged commit 5a4d282 into openzfs:master Jan 20, 2022
@PaulZ-98
Copy link
Contributor Author

PaulZ-98 commented Aug 2, 2022

Hello @behlendorf @tonynguien can this PR get added to the list of things to include in next 2.1.x? thanks

@behlendorf
Copy link
Contributor

@PaulZ-98 that sounds like a good idea. Would you mind opening a new PR against the zfs-2.1.6-staging branch with this change.

PaulZ-98 added a commit to datto/zfs that referenced this pull request Aug 3, 2022
zdb -d <pool>/<objset ID> does not work when
other command line arguments are included i.e.
zdb -U <cachefile> -d <pool>/<objset ID>
This change fixes the command line parsing
to handle this situation.  Also fix issue
where zdb -r <dataset> <file> does not handle
the root <dataset> of the pool. Introduce -N
option to force <objset ID> to be interpreted
as a numeric objsetID.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Rich Ercolani <rincebrain@gmail.com>
Reviewed-by: Tony Nguyen <tony.nguyen@delphix.com>
Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
Closes openzfs#12845
Closes openzfs#12944
@PaulZ-98 PaulZ-98 mentioned this pull request Aug 3, 2022
13 tasks
behlendorf pushed a commit that referenced this pull request Aug 8, 2022
zdb -d <pool>/<objset ID> does not work when
other command line arguments are included i.e.
zdb -U <cachefile> -d <pool>/<objset ID>
This change fixes the command line parsing
to handle this situation.  Also fix issue
where zdb -r <dataset> <file> does not handle
the root <dataset> of the pool. Introduce -N
option to force <objset ID> to be interpreted
as a numeric objsetID.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Rich Ercolani <rincebrain@gmail.com>
Reviewed-by: Tony Nguyen <tony.nguyen@delphix.com>
Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
Closes #12845
Closes #12944
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
zdb -d <pool>/<objset ID> does not work when
other command line arguments are included i.e.
zdb -U <cachefile> -d <pool>/<objset ID>
This change fixes the command line parsing
to handle this situation.  Also fix issue
where zdb -r <dataset> <file> does not handle
the root <dataset> of the pool. Introduce -N
option to force <objset ID> to be interpreted
as a numeric objsetID.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Rich Ercolani <rincebrain@gmail.com>
Reviewed-by: Tony Nguyen <tony.nguyen@delphix.com>
Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
Closes openzfs#12845
Closes openzfs#12944
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
zdb -d <pool>/<objset ID> does not work when
other command line arguments are included i.e.
zdb -U <cachefile> -d <pool>/<objset ID>
This change fixes the command line parsing
to handle this situation.  Also fix issue
where zdb -r <dataset> <file> does not handle
the root <dataset> of the pool. Introduce -N
option to force <objset ID> to be interpreted
as a numeric objsetID.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Rich Ercolani <rincebrain@gmail.com>
Reviewed-by: Tony Nguyen <tony.nguyen@delphix.com>
Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
Closes openzfs#12845
Closes openzfs#12944
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.

zdb gets confused by datasets starting with numbers
5 participants