Skip to content
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.

Sign up for GitHub

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

Use MountOption enum to parse mount options defined in the spec #2937

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+194 −40
Open

Use MountOption enum to parse mount options defined in the spec #2937

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
842952c
Define MountOption enum
musaprg Sep 29, 2024
f2af633
Fix wrong test cases for mount options
musaprg Dec 30, 2024
a53a124
Add method to return known options for MountOption
musaprg Dec 30, 2024
33608e3
Revert unnecessary change
musaprg Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 44 additions & 41 deletions crates/libcontainer/src/rootfs/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use nix::sys::stat::SFlag;
use oci_spec::runtime::{LinuxDevice, LinuxDeviceBuilder, LinuxDeviceType, Mount};

use super::mount::MountError;
use crate::syscall::linux::{self, MountRecursive};
use crate::syscall::linux::{self, MountOption, MountRecursive};

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct MountOptionConfig {
Expand Down Expand Up @@ -131,51 +131,53 @@ pub fn parse_mount(m: &Mount) -> std::result::Result<MountOptionConfig, MountErr
continue;
}

if let Some((is_clear, flag)) = match option.as_str() {
"defaults" => Some((false, MsFlags::empty())),
"ro" => Some((false, MsFlags::MS_RDONLY)),
"rw" => Some((true, MsFlags::MS_RDONLY)),
"suid" => Some((true, MsFlags::MS_NOSUID)),
"nosuid" => Some((false, MsFlags::MS_NOSUID)),
"dev" => Some((true, MsFlags::MS_NODEV)),
"nodev" => Some((false, MsFlags::MS_NODEV)),
"exec" => Some((true, MsFlags::MS_NOEXEC)),
"noexec" => Some((false, MsFlags::MS_NOEXEC)),
"sync" => Some((false, MsFlags::MS_SYNCHRONOUS)),
"async" => Some((true, MsFlags::MS_SYNCHRONOUS)),
"dirsync" => Some((false, MsFlags::MS_DIRSYNC)),
"remount" => Some((false, MsFlags::MS_REMOUNT)),
"mand" => Some((false, MsFlags::MS_MANDLOCK)),
"nomand" => Some((true, MsFlags::MS_MANDLOCK)),
"atime" => Some((true, MsFlags::MS_NOATIME)),
"noatime" => Some((false, MsFlags::MS_NOATIME)),
"diratime" => Some((true, MsFlags::MS_NODIRATIME)),
"nodiratime" => Some((false, MsFlags::MS_NODIRATIME)),
"bind" => Some((false, MsFlags::MS_BIND)),
"rbind" => Some((false, MsFlags::MS_BIND | MsFlags::MS_REC)),
"unbindable" => Some((false, MsFlags::MS_UNBINDABLE)),
"runbindable" => Some((false, MsFlags::MS_UNBINDABLE | MsFlags::MS_REC)),
"private" => Some((true, MsFlags::MS_PRIVATE)),
"rprivate" => Some((true, MsFlags::MS_PRIVATE | MsFlags::MS_REC)),
"shared" => Some((true, MsFlags::MS_SHARED)),
"rshared" => Some((true, MsFlags::MS_SHARED | MsFlags::MS_REC)),
"slave" => Some((true, MsFlags::MS_SLAVE)),
"rslave" => Some((true, MsFlags::MS_SLAVE | MsFlags::MS_REC)),
"relatime" => Some((true, MsFlags::MS_RELATIME)),
"norelatime" => Some((true, MsFlags::MS_RELATIME)),
"strictatime" => Some((true, MsFlags::MS_STRICTATIME)),
"nostrictatime" => Some((true, MsFlags::MS_STRICTATIME)),
unknown => {
if let Some((is_clear, flag)) = match MountOption::from_str(option.as_ref()) {
Ok(v) => match v {
MountOption::Defaults(is_clear, flag) => Some((is_clear, flag)),
MountOption::Ro(is_clear, flag) => Some((is_clear, flag)),
MountOption::Rw(is_clear, flag) => Some((is_clear, flag)),
MountOption::Suid(is_clear, flag) => Some((is_clear, flag)),
MountOption::Nosuid(is_clear, flag) => Some((is_clear, flag)),
MountOption::Dev(is_clear, flag) => Some((is_clear, flag)),
MountOption::Nodev(is_clear, flag) => Some((is_clear, flag)),
MountOption::Exec(is_clear, flag) => Some((is_clear, flag)),
MountOption::Noexec(is_clear, flag) => Some((is_clear, flag)),
MountOption::Sync(is_clear, flag) => Some((is_clear, flag)),
MountOption::Async(is_clear, flag) => Some((is_clear, flag)),
MountOption::Dirsync(is_clear, flag) => Some((is_clear, flag)),
MountOption::Remount(is_clear, flag) => Some((is_clear, flag)),
MountOption::Mand(is_clear, flag) => Some((is_clear, flag)),
MountOption::Nomand(is_clear, flag) => Some((is_clear, flag)),
MountOption::Atime(is_clear, flag) => Some((is_clear, flag)),
MountOption::Noatime(is_clear, flag) => Some((is_clear, flag)),
MountOption::Diratime(is_clear, flag) => Some((is_clear, flag)),
MountOption::Nodiratime(is_clear, flag) => Some((is_clear, flag)),
MountOption::Bind(is_clear, flag) => Some((is_clear, flag)),
MountOption::Rbind(is_clear, flag) => Some((is_clear, flag)),
MountOption::Unbindable(is_clear, flag) => Some((is_clear, flag)),
MountOption::Runbindable(is_clear, flag) => Some((is_clear, flag)),
MountOption::Private(is_clear, flag) => Some((is_clear, flag)),
MountOption::Rprivate(is_clear, flag) => Some((is_clear, flag)),
MountOption::Shared(is_clear, flag) => Some((is_clear, flag)),
MountOption::Rshared(is_clear, flag) => Some((is_clear, flag)),
MountOption::Slave(is_clear, flag) => Some((is_clear, flag)),
MountOption::Rslave(is_clear, flag) => Some((is_clear, flag)),
MountOption::Relatime(is_clear, flag) => Some((is_clear, flag)),
MountOption::Norelatime(is_clear, flag) => Some((is_clear, flag)),
MountOption::Strictatime(is_clear, flag) => Some((is_clear, flag)),
MountOption::Nostrictatime(is_clear, flag) => Some((is_clear, flag)),
},
Err(unknown) => {
if unknown == "idmap" || unknown == "ridmap" {
return Err(MountError::UnsupportedMountOption(unknown.to_string()));
return Err(MountError::UnsupportedMountOption(unknown));
}
None
}
} {
if is_clear {
flags &= !flag;
flags.remove(flag);
} else {
flags |= flag;
flags.insert(flag);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes it easier for code readers to understand what happens! Thanks.

Copy link
Contributor Author

@musaprg musaprg Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an unnecessary change. Let me revert this.

Copy link
Contributor Author

@musaprg musaprg Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverted by 33608e3

}
continue;
}
Expand Down Expand Up @@ -243,7 +245,7 @@ mod tests {
)?;
assert_eq!(
MountOptionConfig {
flags: MsFlags::MS_NOSUID,
flags: MsFlags::MS_NOSUID | MsFlags::MS_STRICTATIME,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I ask why you changed this unit test? I just want your intention.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you fix it?
#2937 (comment)

Copy link
Contributor Author

@musaprg musaprg Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. IIUC, the original test code seemed wrong. It tried to parse the following mount options.

// ...
            &MountBuilder::default()
                .destination(PathBuf::from("/dev"))
                .typ("tmpfs")
                .source(PathBuf::from("tmpfs"))
                .options(vec![
                    "nosuid".to_string(),
                    "strictatime".to_string(),
                    "mode=755".to_string(),
                    "size=65536k".to_string(),
                ])
                .build()?,
// ...

The test case specifies both strictatime and nosuid, but the original test code only expects MsFlags::MS_NOSUID.

If I misunderstood the spec, please let me know and I'll check my code again to fix the CI.

data: "mode=755,size=65536k".to_string(),
rec_attr: None,
},
Expand Down Expand Up @@ -364,7 +366,8 @@ mod tests {
flags: MsFlags::MS_NOSUID
| MsFlags::MS_NOEXEC
| MsFlags::MS_NODEV
| MsFlags::MS_RDONLY,
| MsFlags::MS_RDONLY
| MsFlags::MS_RELATIME,
data: "".to_string(),
rec_attr: None
},
Expand Down
153 changes: 152 additions & 1 deletion crates/libcontainer/src/syscall/linux.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,145 @@ const MOUNT_ATTR_STRICTATIME: u64 = 0x00000020;
const MOUNT_ATTR_NODIRATIME: u64 = 0x00000080;
const MOUNT_ATTR_NOSYMFOLLOW: u64 = 0x00200000;

/// Constants used by mount(2).
pub enum MountOption {
Defaults(bool, MsFlags),
Ro(bool, MsFlags),
Rw(bool, MsFlags),
Suid(bool, MsFlags),
Nosuid(bool, MsFlags),
Dev(bool, MsFlags),
Nodev(bool, MsFlags),
Exec(bool, MsFlags),
Noexec(bool, MsFlags),
Sync(bool, MsFlags),
Async(bool, MsFlags),
Dirsync(bool, MsFlags),
Remount(bool, MsFlags),
Mand(bool, MsFlags),
Nomand(bool, MsFlags),
Atime(bool, MsFlags),
Noatime(bool, MsFlags),
Diratime(bool, MsFlags),
Nodiratime(bool, MsFlags),
Bind(bool, MsFlags),
Rbind(bool, MsFlags),
Unbindable(bool, MsFlags),
Runbindable(bool, MsFlags),
Private(bool, MsFlags),
Rprivate(bool, MsFlags),
Shared(bool, MsFlags),
Rshared(bool, MsFlags),
Slave(bool, MsFlags),
Rslave(bool, MsFlags),
Relatime(bool, MsFlags),
Norelatime(bool, MsFlags),
Strictatime(bool, MsFlags),
Nostrictatime(bool, MsFlags),
}

impl MountOption {
// Return all possible mount options
pub fn known_options() -> Vec<String> {
[
"defaults",
"ro",
"rw",
"suid",
"nosuid",
"dev",
"nodev",
"exec",
"noexec",
"sync",
"async",
"dirsync",
"remount",
"mand",
"nomand",
"atime",
"noatime",
"diratime",
"nodiratime",
"bind",
"rbind",
"unbindable",
"runbindable",
"private",
"rprivate",
"shared",
"rshared",
"slave",
"rslave",
"relatime",
"norelatime",
"strictatime",
"nostrictatime",
]
.iter()
.map(|s| s.to_string())
.collect()
}
}

impl FromStr for MountOption {
type Err = String;

fn from_str(option: &str) -> std::result::Result<Self, Self::Err> {
match option {
"defaults" => Ok(MountOption::Defaults(false, MsFlags::empty())),
"ro" => Ok(MountOption::Ro(false, MsFlags::MS_RDONLY)),
"rw" => Ok(MountOption::Rw(true, MsFlags::MS_RDONLY)),
"suid" => Ok(MountOption::Suid(true, MsFlags::MS_NOSUID)),
"nosuid" => Ok(MountOption::Nosuid(false, MsFlags::MS_NOSUID)),
"dev" => Ok(MountOption::Dev(true, MsFlags::MS_NODEV)),
"nodev" => Ok(MountOption::Nodev(false, MsFlags::MS_NODEV)),
"exec" => Ok(MountOption::Exec(true, MsFlags::MS_NOEXEC)),
"noexec" => Ok(MountOption::Noexec(false, MsFlags::MS_NOEXEC)),
"sync" => Ok(MountOption::Sync(false, MsFlags::MS_SYNCHRONOUS)),
"async" => Ok(MountOption::Async(true, MsFlags::MS_SYNCHRONOUS)),
"dirsync" => Ok(MountOption::Dirsync(false, MsFlags::MS_DIRSYNC)),
"remount" => Ok(MountOption::Remount(false, MsFlags::MS_REMOUNT)),
"mand" => Ok(MountOption::Mand(false, MsFlags::MS_MANDLOCK)),
"nomand" => Ok(MountOption::Nomand(true, MsFlags::MS_MANDLOCK)),
"atime" => Ok(MountOption::Atime(true, MsFlags::MS_NOATIME)),
"noatime" => Ok(MountOption::Noatime(false, MsFlags::MS_NOATIME)),
"diratime" => Ok(MountOption::Diratime(true, MsFlags::MS_NODIRATIME)),
"nodiratime" => Ok(MountOption::Nodiratime(false, MsFlags::MS_NODIRATIME)),
"bind" => Ok(MountOption::Bind(false, MsFlags::MS_BIND)),
"rbind" => Ok(MountOption::Rbind(
false,
MsFlags::MS_BIND | MsFlags::MS_REC,
)),
"unbindable" => Ok(MountOption::Unbindable(false, MsFlags::MS_UNBINDABLE)),
"runbindable" => Ok(MountOption::Runbindable(
false,
MsFlags::MS_UNBINDABLE | MsFlags::MS_REC,
)),
"private" => Ok(MountOption::Private(true, MsFlags::MS_PRIVATE)),
"rprivate" => Ok(MountOption::Rprivate(
true,
MsFlags::MS_PRIVATE | MsFlags::MS_REC,
)),
"shared" => Ok(MountOption::Shared(true, MsFlags::MS_SHARED)),
"rshared" => Ok(MountOption::Rshared(
true,
MsFlags::MS_SHARED | MsFlags::MS_REC,
)),
"slave" => Ok(MountOption::Slave(true, MsFlags::MS_SLAVE)),
"rslave" => Ok(MountOption::Rslave(
true,
MsFlags::MS_SLAVE | MsFlags::MS_REC,
)),
"relatime" => Ok(MountOption::Relatime(false, MsFlags::MS_RELATIME)),
"norelatime" => Ok(MountOption::Norelatime(true, MsFlags::MS_RELATIME)),
"strictatime" => Ok(MountOption::Strictatime(false, MsFlags::MS_STRICTATIME)),
"nostrictatime" => Ok(MountOption::Nostrictatime(true, MsFlags::MS_STRICTATIME)),
_ => Err(option.to_string()),
}
}
}

/// Constants used by mount_setattr(2).
pub enum MountRecursive {
/// Mount read-only.
Expand Down Expand Up @@ -599,12 +738,13 @@ mod tests {

use std::fs;
use std::os::unix::prelude::AsRawFd;
use std::str::FromStr;

use anyhow::{bail, Context, Result};
use nix::{fcntl, sys, unistd};
use serial_test::serial;

use super::LinuxSyscall;
use super::{LinuxSyscall, MountOption};
use crate::syscall::Syscall;

#[test]
Expand Down Expand Up @@ -666,4 +806,15 @@ mod tests {
unistd::close(fd)?;
Ok(())
}

#[test]
fn test_known_mount_options_implemented() -> Result<()> {
for option in MountOption::known_options() {
match MountOption::from_str(&option) {
Ok(_) => {}
Err(e) => bail!("failed to parse mount option: {}", e),
}
}
Ok(())
}
}
Loading