Kernel/VFS: Disallow umount on busy mounts#26610
Open
supercomputer7 wants to merge 9 commits intoSerenityOS:masterfrom
Open
Kernel/VFS: Disallow umount on busy mounts#26610supercomputer7 wants to merge 9 commits intoSerenityOS:masterfrom
supercomputer7 wants to merge 9 commits intoSerenityOS:masterfrom
Conversation
supercomputer7
force-pushed
the
vfs-mount-improvements
branch
from
e6460e0 to
18242d5
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! |
Member
Author
|
There are PRs to merge first, so this is stale for a reason :) |
It makes much more sense to put that there, as this method should not be a "public" API of the VirtualFileSystem namespace.
The create method which doesn't require specifying a Process is not used anymore so let's remove it.
Add a method to set the stored length of a FixedStringBuffer, and add a method to retrieve the max length of a FixedStringBuffer. It should be noted that the set_stored_length method is quite dangerous and should be used carefully to avoid providing bogus StringView's. Those methods will be used later in path resolution code, specifically in resolving an Inode as symlink more cleanly.
Nobody uses this method besides the readlink syscall and when trying to lookup what is the target of a symlink during path resolution. Therefore, add a more convenient method for that purpose as a public API and use the previous method internally.
That will be useful later on when working on the KString buffer and manipulate its content.
Add a method to canonicalize absolute path and another method to join two paths if the second is non canonical. These method will be used in the new path resolution code in the VFS.
This is probably one of the biggest changes in the VFS layer which makes it much more advanced. A summary of how the path resolution is changed - 1) Renaming CustodyBase => UnresolvedPath: CustodyBase was a beast that we used to defer path resolution to the point we needed it, but we still passed many StringView's of paths for no good reason, so delete these instances and use UnresolvedPath in those places. The idea is to make it more clear what UnresolvedPath is about - usually we provide a dirfd and a StringView of a path, or for symlinks, with Custody parent and the actual symlink path "pointer", and it helps us deciding which path is the "base" path to do path resolution upon. A Path object is standing for a resolved path, so it includes a Custody and a Mount responsible for the path. This object is now ref-counted so we can easily pass it around. 2) Add a new class called CanonicalizedPath: This class ensures that the core of the path resolution code is getting to do the work as quickly as possible with a canonicalized path string. We can also ensure we are more correct with KLexical::basename calls now as we call them on a canonicalized path string and not on the raw path. 3) In general, an evolution of a path in the kernel looks like that: A path is converted to string that can be processed by kernel, hence ensuring no unsafe pointers from userspace. It then becomes an UnresolvedPath, and is sent to the VFS. In the VFS layer, it becomes a CanonicalizedPath - we resolve the base Custody and ensure that we have a concise path, but we still don't know if it exists or what is standing behind that path. It should be noted, that both steps are completely technical - we don't care about anything besides memory & semantic correctness. Afterwards, during path resolution, it becomes a resolved path so we now create a Path object, with everything needed to perform other filesystem (or VFS) functions with it. In that sense, a Path replaces passing a Custody - this will allow us to do interesting things on a mount later on, like keeping track of usage (open files, writers) and therefore prevent unmounting while the mount is being used. The path resolution code is also more clear and correct now, as it tries to build early on a concise string of the absolute path to be resolved and only then use a lexer to parse each part in the path, which is also canonicalized to prevent going back and forth in the path lookup. It should be noted, however, that most callers don't actually care about the responsible mount for a custody. Since the old code already knew the properties of the responsible mount, there was basically no new penalty of actually **getting** the mount object during the lookup.
We use refcounting (which now is possible due to previous commits) to ensure nothing besides the mount table holds a reference to the Mount to be removed.
It helps checking permissions (RW or read-only) of a path and allows us to keep a file descriptor open on a path, hence keeping the responsible mount busy.
supercomputer7
force-pushed
the
vfs-mount-improvements
branch
from
18242d5 to
8d3b03c
Compare
Member
Author
|
Ready for review now :) |
github-actions
bot
added
the
👀 pr-needs-review
Member
|
CI is red |
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
2 participants
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.
Relies on #26609.