Repository navigation
virtiofs: stop failed directory reads from leaking lookup references - #4836
Open
Pedro Henrique Penna (ppenna) wants to merge 1 commit into
Open
Pedro Henrique Penna (ppenna) wants to merge 1 commit into
Pedro Henrique Penna (ppenna) wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the virtio-fs directory-read reference-handling fix.
No open issues 🎉
What changed in this PR
This PR fixes inode lookup-reference leaks in OpenVMM’s virtio-fs directory reads, addressing #4834. It prevents plain listings from registering children and preserves references already included in partial READDIRPLUS replies.
Changes:
- Use a metadata lookup rather than a reference-taking lookup for plain
READDIR. - Return accumulated entries when a later lookup fails, including for the aggregate synthetic root.
- Add regression tests for reference ownership, error delivery on the next read, and cleanup through
FORGET.
| File | Description |
|---|---|
vm/devices/virtio/virtiofs/src/test_helpers.rs |
Adds a shared directory-reply decoder for tests. |
vm/devices/virtio/virtiofs/src/lib.rs |
Registers the test-only helper module. |
vm/devices/virtio/virtiofs/src/file.rs |
Fixes directory-read reference handling and adds regression tests. |
vm/devices/virtio/virtiofs/src/aggregate.rs |
Preserves partial synthetic-root replies after lookup errors. |
vm/devices/virtio/virtiofs/src/aggregate_tests.rs |
Tests partial replies, retry errors, and reference cleanup for the synthetic root. |
Review effort: Balanced
John Starks (jstarks)
approved these changes
Nov 1, 2026
Pedro Henrique Penna (ppenna)
marked this pull request as ready for review
November 2, 2026 18:55
Pedro Henrique Penna (ppenna)
requested review from
a team and
Anatol Belski (weltling)
as code owners
November 2, 2026 18:55
A READDIRPLUS that failed partway through discarded entries whose lookups had already taken references, so the guest could never forget them. Return the entries read so far instead, and report the error only when there are none: the guest's next read starts at the failing entry and gets the error then. Apply the same rule to the aggregate synthetic root. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pedro Henrique Penna (ppenna)
force-pushed
the
fix-virtiofs-readdir-lookup-refs
branch
from
November 2, 2026 23:19
23b9278 to
169d1ef
Compare
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
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.
A
READDIRPLUSin the virtio-fs server that failed partway through discarded entries whose lookups had already taken inode lookup references. The guest sendsFORGETonly for node IDs it received, so these references were never released: the inodes stayed in the inode map untilFUSE_DESTROY. Fixes #4834.The plain
READDIRhalf of #4834 is already fixed onmainby #3440, which removed the per-entry lookup from plainreaddir, so this PR covers onlyREADDIRPLUS.Summary
READDIRPLUSentries that took lookup references are no longer discardedREADDIRPLUSof a directory or of the aggregate root that fails partway returns the entry holding the reference, reports the error on the next read, and releases the inode on the guest'sFORGETNo host condition portably makes a later entry's lookup fail with an error other than
EACCESorENOENT, so the tests inject the failure:read_dirandread_synthetic_root_dirnow delegate to private helpers that take the per-child lookup as a closure. The same rule also covers theENOSPCthat #4594 adds to node-ID allocation.Validation
cargo clippy --all-targets -p virtiofs, on Windows x86-64 and with--target x86_64-unknown-linux-gnucargo doc --no-deps -p virtiofscargo nextest run --profile agent -p virtiofscargo xtask fmt --fixReverting either fix makes its new test fail.