-
Notifications
You must be signed in to change notification settings - Fork 163
virtiofs: Fix POSIX filesystem compatibility issues #2669
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
base: main
Are you sure you want to change the base?
Conversation
|
This PR modifies files containing For more on why we check whole files, instead of just diffs, check out the Rustonomicon |
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.
Pull request overview
This PR fixes POSIX filesystem compatibility issues in the virtiofs implementation, as identified by running the zfsonlinux fstest suite. The changes address incorrect error code mappings, edge cases in setuid/setgid bit handling, and add validation for filename lengths.
Changes:
- Map Windows STATUS_CANNOT_DELETE to ENOTEMPTY instead of EIO for more accurate directory deletion errors
- Fix chown(-1, -1) behavior to properly distinguish between regular files and directories when clearing setuid/setgid bits
- Add NAME_MAX validation (255 bytes) for FUSE file name components with appropriate ENAMETOOLONG error
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| vm/devices/support/fs/lxutil/src/windows/util.rs | Updates error code mapping for STATUS_CANNOT_DELETE and fixes setuid/setgid clearing logic for chown(-1, -1) to distinguish between files and directories |
| vm/devices/support/fs/lxutil/src/windows/mod.rs | Adds additional error codes to the list that bypass the read-only file workaround during deletion |
| vm/devices/support/fs/lxutil/src/windows/fs.rs | Adds additional error codes to the list that bypass the read-only file workaround during deletion |
| vm/devices/support/fs/fuse/src/session.rs | Adds handler for new FuseOperation::Error variant to return specific parsing errors |
| vm/devices/support/fs/fuse/src/request/macros.rs | Adds Error variant to FuseOperation enum and implements Debug for it |
| vm/devices/support/fs/fuse/src/request.rs | Changes Invalid to Error variant when parsing fails with specific error, and adds NAME_MAX validation for filenames |
I ran the following POSIX Filesystem compatibility test: https://github.com/zfsonlinux/fstest against our virtiofs implementation (with the metadata option enabled) which surfaced a couple of errors:
Returning incorrect/non-specific error codes:
https://github.com/zfsonlinux/fstest/blob/master/tests/rmdir/06.t
https://github.com/zfsonlinux/fstest/blob/master/tests/open/03.t
Edge cases when handling clearing/setting uid/gid bits on files/directories
https://github.com/zfsonlinux/fstest/blob/master/tests/chown/00.t (Specifically a couple of the assertions between 64-94)
This PR fixes these issues, and with the changes the test suite passes all testcases except for 1 related to setting the file size to a large value (999 TiB) see: https://github.com/zfsonlinux/fstest/blob/master/tests/truncate/12.t