-
Notifications
You must be signed in to change notification settings - Fork 68
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
Add syscall mount and umount #855
Conversation
631b11c
to
8c54b0a
Compare
069cd60
to
584aee3
Compare
Signed-off-by: Zhenchen Wang <[email protected]>
584aee3
to
4e10bef
Compare
afed1b5
to
0544fcd
Compare
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. Thank you for your contribution!
Tests will be added later after the |
Signed-off-by: Zhenchen Wang <[email protected]>
0544fcd
to
645ea9c
Compare
@@ -635,6 +647,14 @@ impl Dentry { | |||
self.inner.rename(old_name, &new_dir.inner, new_name) | |||
} | |||
|
|||
pub fn do_loopback(&self, recursive: bool) -> Arc<MountNode> { |
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.
There are Rust docs that explain some trivial methods of Dentry
like rename
and rmdir
, but no Rust doc for a weird method named do_loopback
having a mysterious argument named recursive
? This method is more worthy of explanation than other methods of Dentry
. I often find the lack of Rust doc is a symptom of poor design. When the design is poor, even the author finds it hard to explain it, thus omitting the doc.
I assume the method follows that of Linux's, but I doubt the wisdom of doing so. The original meaning of loopback in Linux is closely related to loopback devices, through which a Linux node can send network packages back to itself. But how can the original meaning of loopback be extended to file systems or mounts? You need to establish this loopback concept for file systems or mounts before using it. One proper place is the Rust doc of this method. If you can't convince yourself and others by writing the Rust doc, then you probably should describe this method in another way.
By the way, do_xxx
names are usually reserved for internal methods. Think twice before using it on public methods.
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.
I greatly appreciate the comprehensive review you've offered on the documentation and naming conventions.
The original naming of do_loopback
was indeed inspired by the corresponding naming conventions in Linux. However,the Linux naming is quite perplexing. In light of your feedback and to enhance clarity, I have renamed do_loopback to do_bind_mount. This new name more accurately describes the method's role in facilitating bind mount operations.
Moreover, I have restructured the method to Dentry::bind_mount_to
, which now explicitly communicates that it is intended for binding one Dentry to another. This renaming and re-encapsulation should lead to a better understanding of the method's purpose and enhance the code's readability.
Regarding the recursive parameter, it functions as follows:
do_bind_mount(
devname,
dst_dentry,
mount_flags.contains(MountFlags::MS_REC),
)?;
When the MS_REC
flag is not set, the recursive parameter is false, indicating that only the current directory is bound, and any sub-mounted directories within it are not recursively bound. Conversely, when MS_REC
is set, recursive becomes true, allowing for a recursive bind of the sub-mounted directories within the directory.
To illustrate with examples:
- A non-recursive bind mount, corresponding to the user command
mount --bind src dst
, only binds the specified source (src) to the destination (dst) without affecting sub-mounts. - A recursive bind mount, corresponding to the user command
mount --rbind src dst
, performs a recursive operation, allowing access to the contents of sub-mounted file systems within the bound directory.
@@ -558,7 +570,7 @@ impl Dentry { | |||
|
|||
/// Make this Dentry' inner to be a mountpoint, | |||
/// and set the mountpoint of the child mount to this Dentry's inner. | |||
fn set_mountpoint(&self, child_mount: Arc<MountNode>) { | |||
pub fn set_mountpoint(&self, child_mount: Arc<MountNode>) { |
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.
Why public?
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.
Given the possibility that within MountNode
we may need to reassign the mount point of a MountNode
, MountNode
could become user of set_mountpoint
. To ensure that this functionality is utilized exclusively by MountNode
, I believe opting for pub(super)
is a choice.
/// The new mount tree will replicate the structure of the original tree. | ||
/// The new tree is a separate entity rooted at the given `Dentry_`, | ||
/// and the original tree remains unchanged. | ||
pub fn copy_mount_node_tree(&self, root_dentry: Arc<Dentry_>) -> Arc<Self> { |
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.
This should be a private method.
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.
Thank you for your review and the suggestion to make copy_mount_node_tree
a private method. However, there are use cases where clone_mount_node_tree
could be utilized within Dentry, such as in the Dentry::bind_mount_to
:
pub fn bind_mount_to(&self, dst_dentry: &Arc<Self>, recursive: bool) -> Result<()> {
let src_mount = self
.mount_node
.clone_mount_node_tree(&self.inner, recursive);
src_mount.graft_mount_node_tree(dst_dentry)?;
Ok(())
}
Considering this, I believe it would be more appropriate to use pub(super) for clone_mount_node_tree
to indicate that it may be used by Dentry
.
Furthermore, in the context of the mount namespace, there may be operations involving clone_mount_node_tree where the process's root and current working directory are moved to the new mount node tree.
6099a58
to
f1a2d22
Compare
…redefined the method for bind mount. Signed-off-by: Zhenchen Wang <[email protected]>
f1a2d22
to
2f3966f
Compare
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. Thanks for the contribution!
This PR introduces incomplete implementations of the
mount
andumount
system calls. The primary changes are as follows:syscall mount
The
syscall mount
is a complex system call with various functionalities. A call to mount() performs one of several operations based on the bits specified in mountflags. The operation to perform is determined by testing the bits set in mountflags, in the following order:This PR primarily implements the
do_loopback
,do_move_mount_old
, anddo_new_mount
functions.do_loopback
This function handles the main implementation for bind mounts. A bind mount makes a file or a directory subtree visible at another point within the single directory hierarchy. By default, only the directory itself is mounted. If the MS_REC flag is specified, a recursive bind mount operation is performed, making all submounts under the source subtree (except unbindable mounts) also bind mounted at the corresponding location in the target subtree. The primary method involves copying the subtree of the directory to be bind-mounted and grafting it to another directory's subtree.
do_move_mount_old
This function moves an existing mount by detaching it from its current location in the mount tree and grafting it onto a new directory's subtree.
do_new_mount
This function mounts a new file system. Currently, I only supports mounting ext2 and exfat file systems due to limited support for file system types and device names. The implementation uses the following function to get the file system type and then perform the mount operation:
syscall umount
The
syscall umount
mainly handles the unmounting of a mounted file system using theumount
method defined in theimpl Dentry
. Further functionality requires additional features like mount namespace.VFS Layer Changes
The VFS layer includes the following new methods:
MountNode
Dentry_
, setting the parent and children manually.Dentry_
, maintaining the same structure as the original tree.Dentry_
Dentry_
is a subdirectory of another.Dentry
Testing
Tests for
syscall mount
andumount
are included inmount_test
. Both requireCAP_SYS_ADMIN
capabilities, necessitating support for capabilities to run the tests. To test, run the following commands in the shell: