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 gets confused by datasets starting with numbers #12845

Closed
rincebrain opened this issue Dec 13, 2021 · 3 comments · Fixed by #12944
Closed

zdb gets confused by datasets starting with numbers #12845

rincebrain opened this issue Dec 13, 2021 · 3 comments · Fixed by #12944
Labels
Type: Defect Incorrect behavior (e.g. crash, hang)

Comments

@rincebrain
Copy link
Contributor

System information

Type Version/Name
Distribution Name Debian
Distribution Version 11
Kernel Version 5.10.0-9-amd64
Architecture x86_64
OpenZFS Version c9d62d1 or so

Describe the problem you're observing

(I'll go figure out a "correct" handling for this later, I just wanted to mention it in case I forgot after I ran into it...)

I tried to examine a child dataset under a dataset named "1M", at the top of a pool, "lz4newtestpool".

# sudo zdb -dbdbdbdbdbdb lz4newtestpool/1M/largefiles 256
failed to hold objset 1: Invalid argument
zdb: can't open 'lz4newtestpool/1M/largefiles': Invalid argument

But if I rename 1M to sigh1M, it works as expected.

I would presume this is due to:

zfs/cmd/zdb/zdb.c

Lines 8525 to 8539 in 510885a

char *endptr;
errno = 0;
objset_id = strtoull(objset_str, &endptr, 0);
/* dataset 0 is the same as opening the pool */
if (errno == 0 && endptr != objset_str &&
objset_id != 0) {
target_is_spa = B_FALSE;
dataset_lookup = B_TRUE;
} else if (objset_id != 0) {
printf("failed to open objset %s "
"%llu %s", objset_str,
(u_longlong_t)objset_id,
strerror(errno));
exit(1);
}

since patching the second check to not exit(1) and reset objset_id to -1 allows us to proceed, albeit having printed the error.

Describe how to reproduce the problem

sudo zdb -d pool/1zzz

Include any warning/errors/backtraces from the system logs

GOTO 10

@rincebrain rincebrain added the Type: Defect Incorrect behavior (e.g. crash, hang) label Dec 13, 2021
@PaulZ-98
Copy link
Contributor

PaulZ-98 commented Jan 3, 2022

It turns out that the -d /dataset | objsetID is not going to be a good zdb command line interface for using the numeric objset id, for the reason you're seeing. stroull will convert 1M to 1 and then not find it. Also tank/100 is a valid dataset name so it's ambiguous if you want it to be an objsetID or dataset name string.

I had originally done

zdb tank -N 236:2   (objset:object)

but we didn't go with that. Now we have objectID ranges to consider if we adopt a ":" separator. Ideas welcome.

@rincebrain
Copy link
Contributor Author

I'd probably opt for a variant of that, really - something like zdb tank -N 236 [object or range[,...]], since I don't think it's ever currently legal to specify multiple datasets no matter how you phrase it, and if it were, then we could (ew) go for positional-based parsing where it affects later arguments?

I'd still prefer it if the existing logic did something like "try as path name" => "if it fails, try as objsetid", and then you can unambiguously specify it if you need to? (Mostly in that order because I think people are more likely to run into the case of "I have a dataset named 100 and want to reference it when there's an objsetid 100" than "I have an objsetid 100 and there's a dataset named 100 i wanted to reference instead".)

@PaulZ-98
Copy link
Contributor

PaulZ-98 commented Jan 3, 2022

I'll work on getting the "try as dataset name" => "if it fails and (if numeric looking), try as objsetID" working. Also the way it uses args[2] is wrong and won't work if you have other args so I'm already fixing that.

PaulZ-98 added a commit to datto/zfs that referenced this issue 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 openzfs#12845
Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
PaulZ-98 added a commit to datto/zfs that referenced this issue 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 openzfs#12845
Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
PaulZ-98 added a commit to datto/zfs that referenced this issue Jan 10, 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 openzfs#12845
Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
PaulZ-98 added a commit to datto/zfs that referenced this issue Jan 10, 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 openzfs#12845
Signed-off-by: Paul Zuchowski <pzuchowski@datto.com>
tonynguien pushed a commit that referenced this issue Jan 20, 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
PaulZ-98 added a commit to datto/zfs that referenced this issue 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 issue Aug 3, 2022
13 tasks
behlendorf pushed a commit that referenced this issue 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 issue 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 issue 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
Type: Defect Incorrect behavior (e.g. crash, hang)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants