-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Return required size when encode_fh size too small #11995
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Quoting <linux/exportfs.h>: > encode_fh() should return the fileid_type on success and on error > returns 255 (if the space needed to encode fh is greater than > @max_len*4 bytes). On error @max_len contains the minimum size (in 4 > byte unit) needed to encode the file handle. ZFS was not setting max_len in the case where the handle was too small. As a result of this, the `t_name_to_handle_at.c' example in name_to_handle_at(2) did not work on ZFS. zfsctl_fid() will itself set max_len if called with a fid that is too small, so if we give zfs_fid() that behavior as well, the fix is quite easy: if the handle is too small, just use a zero-size fid instead of the handle. Tested by running t_name_to_handle_at on a normal file, a directory, a .zfs directory, and a snapshot. Thanks-to: Puck Meerburg <[email protected]> Signed-off-by: Alyssa Ross <[email protected]>
behlendorf
reviewed
May 6, 2021
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.
Good find. Thanks for opening a PR with the fix, this LGTM.
behlendorf
approved these changes
May 6, 2021
tonynguien
approved these changes
May 6, 2021
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.
LGTM.
behlendorf
pushed a commit
to behlendorf/zfs
that referenced
this pull request
May 10, 2021
Quoting <linux/exportfs.h>: > encode_fh() should return the fileid_type on success and on error > returns 255 (if the space needed to encode fh is greater than > @max_len*4 bytes). On error @max_len contains the minimum size (in 4 > byte unit) needed to encode the file handle. ZFS was not setting max_len in the case where the handle was too small. As a result of this, the `t_name_to_handle_at.c' example in name_to_handle_at(2) did not work on ZFS. zfsctl_fid() will itself set max_len if called with a fid that is too small, so if we give zfs_fid() that behavior as well, the fix is quite easy: if the handle is too small, just use a zero-size fid instead of the handle. Tested by running t_name_to_handle_at on a normal file, a directory, a .zfs directory, and a snapshot. Thanks-to: Puck Meerburg <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Tony Nguyen <[email protected]> Signed-off-by: Alyssa Ross <[email protected]> Closes openzfs#11995
sempervictus
pushed a commit
to sempervictus/zfs
that referenced
this pull request
May 31, 2021
Quoting <linux/exportfs.h>: > encode_fh() should return the fileid_type on success and on error > returns 255 (if the space needed to encode fh is greater than > @max_len*4 bytes). On error @max_len contains the minimum size (in 4 > byte unit) needed to encode the file handle. ZFS was not setting max_len in the case where the handle was too small. As a result of this, the `t_name_to_handle_at.c' example in name_to_handle_at(2) did not work on ZFS. zfsctl_fid() will itself set max_len if called with a fid that is too small, so if we give zfs_fid() that behavior as well, the fix is quite easy: if the handle is too small, just use a zero-size fid instead of the handle. Tested by running t_name_to_handle_at on a normal file, a directory, a .zfs directory, and a snapshot. Thanks-to: Puck Meerburg <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Tony Nguyen <[email protected]> Signed-off-by: Alyssa Ross <[email protected]> Closes openzfs#11995
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
I noticed that the strace test suite did not run correctly on ZFS. This was because it called
name_to_handle_at(2)
, with a zero-sized handle, expecting the kernel to change thehandle_bytes
value to the size required for a handle, so it could callname_to_handle_at()
again with a correctly-sized handle. (This behavior is documented in the man page.)I then further noticed that the t_name_to_handle_at.c example in the same man page also didn't work properly on ZFS.
Description
Quoting <linux/exportfs.h>:
ZFS was not setting
max_len
in the case where the handle was too small.zfsctl_fid()
will itself setmax_len
if called with afid
that is too small, so if we givezfs_fid()
that behavior as well, the fix is quite easy: if the handle is too small, just use a zero-sizefid
instead of the handle.How Has This Been Tested?
I ran the t_name_to_handle_at.c example from name_to_handle_at(2) on a normal file, a directory, a .zfs directory, and a snapshot.
Types of changes
Checklist:
Signed-off-by
.