diff --git a/src/agent/src/mount.rs b/src/agent/src/mount.rs index 2f318b8b2f..94d65d6c7f 100644 --- a/src/agent/src/mount.rs +++ b/src/agent/src/mount.rs @@ -145,78 +145,53 @@ pub const STORAGE_HANDLER_LIST: &[&str] = &[ DRIVER_WATCHABLE_BIND_TYPE, ]; -#[derive(Debug, Clone)] -pub struct BareMount<'a> { - source: &'a str, - destination: &'a str, - fs_type: &'a str, +#[instrument] +pub fn baremount( + source: &str, + destination: &str, + fs_type: &str, flags: MsFlags, - options: &'a str, - logger: Logger, -} + options: &str, + logger: &Logger, +) -> Result<()> { + let logger = logger.new(o!("subsystem" => "baremount")); -// mount mounts a source in to a destination. This will do some bookkeeping: -// * evaluate all symlinks -// * ensure the source exists -impl<'a> BareMount<'a> { - #[instrument] - pub fn new( - s: &'a str, - d: &'a str, - fs_type: &'a str, - flags: MsFlags, - options: &'a str, - logger: &Logger, - ) -> Self { - BareMount { - source: s, - destination: d, - fs_type, - flags, - options, - logger: logger.new(o!("subsystem" => "baremount")), - } + if source.is_empty() { + return Err(anyhow!("need mount source")); } - #[instrument] - pub fn mount(&self) -> Result<()> { - if self.source.is_empty() { - return Err(anyhow!("need mount source")); - } + if destination.is_empty() { + return Err(anyhow!("need mount destination")); + } - if self.destination.is_empty() { - return Err(anyhow!("need mount destination")); - } + if fs_type.is_empty() { + return Err(anyhow!("need mount FS type")); + } - if self.fs_type.is_empty() { - return Err(anyhow!("need mount FS type")); - } + info!( + logger, + "mount source={:?}, dest={:?}, fs_type={:?}, options={:?}", + source, + destination, + fs_type, + options + ); - info!( - self.logger, - "mount source={:?}, dest={:?}, fs_type={:?}, options={:?}", - self.source, - self.destination, - self.fs_type, - self.options - ); - - nix::mount::mount( - Some(self.source), - self.destination, - Some(self.fs_type), - self.flags, - Some(self.options), + nix::mount::mount( + Some(source), + destination, + Some(fs_type), + flags, + Some(options), + ) + .map_err(|e| { + anyhow!( + "failed to mount {:?} to {:?}, with error: {}", + source, + destination, + e ) - .map_err(|e| { - anyhow!( - "failed to mount {:?} to {:?}, with error: {}", - self.source, - self.destination, - e - ) - }) - } + }) } #[instrument] @@ -487,16 +462,14 @@ fn mount_storage(logger: &Logger, storage: &Storage) -> Result<()> { "mount-options" => options.as_str(), ); - let bare_mount = BareMount::new( + baremount( storage.source.as_str(), storage.mount_point.as_str(), storage.fstype.as_str(), flags, options.as_str(), &logger, - ); - - bare_mount.mount() + ) } /// Looks for `mount_point` entry in the /proc/mounts. @@ -615,11 +588,9 @@ fn mount_to_rootfs(logger: &Logger, m: &InitMount) -> Result<()> { let (flags, options) = parse_mount_flags_and_options(options_vec); - let bare_mount = BareMount::new(m.src, m.dest, m.fstype, flags, options.as_str(), logger); - fs::create_dir_all(Path::new(m.dest)).context("could not create directory")?; - bare_mount.mount().or_else(|e| { + baremount(m.src, m.dest, m.fstype, flags, &options, logger).or_else(|e| { if m.src != "dev" { return Err(e); } @@ -983,7 +954,7 @@ mod tests { std::fs::create_dir_all(d).expect("failed to created directory"); } - let bare_mount = BareMount::new( + let result = baremount( &src_filename, &dest_filename, d.fs_type, @@ -992,8 +963,6 @@ mod tests { &logger, ); - let result = bare_mount.mount(); - let msg = format!("{}: result: {:?}", msg, result); if d.error_contains.is_empty() { @@ -1070,7 +1039,7 @@ mod tests { } // Create an actual mount - let bare_mount = BareMount::new( + let result = baremount( mnt_src_filename, mnt_dest_filename, "bind", @@ -1078,8 +1047,6 @@ mod tests { "", &logger, ); - - let result = bare_mount.mount(); assert!(result.is_ok(), "mount for test setup failed"); let tests = &[ diff --git a/src/agent/src/namespace.rs b/src/agent/src/namespace.rs index 200fc7c09d..8970e5f30c 100644 --- a/src/agent/src/namespace.rs +++ b/src/agent/src/namespace.rs @@ -13,7 +13,7 @@ use std::fs::File; use std::path::{Path, PathBuf}; use tracing::instrument; -use crate::mount::{BareMount, FLAGS}; +use crate::mount::{baremount, FLAGS}; use slog::Logger; const PERSISTENT_NS_DIR: &str = "/var/run/sandbox-ns"; @@ -129,8 +129,7 @@ impl Namespace { } }; - let bare_mount = BareMount::new(source, destination, "none", flags, "", &logger); - bare_mount.mount().map_err(|e| { + baremount(source, destination, "none", flags, "", &logger).map_err(|e| { anyhow!( "Failed to mount {} to {} with err:{:?}", source, diff --git a/src/agent/src/rpc.rs b/src/agent/src/rpc.rs index 4698e23339..775b119dfd 100644 --- a/src/agent/src/rpc.rs +++ b/src/agent/src/rpc.rs @@ -47,7 +47,7 @@ use rustjail::process::ProcessOperations; use crate::device::{add_devices, pcipath_to_sysfs, rescan_pci_bus, update_device_cgroup}; use crate::linux_abi::*; use crate::metrics::get_metrics; -use crate::mount::{add_storages, remove_mounts, BareMount, STORAGE_HANDLER_LIST}; +use crate::mount::{add_storages, baremount, remove_mounts, STORAGE_HANDLER_LIST}; use crate::namespace::{NSTYPEIPC, NSTYPEPID, NSTYPEUTS}; use crate::network::setup_guest_dns; use crate::random; @@ -1624,15 +1624,14 @@ fn setup_bundle(cid: &str, spec: &mut Spec) -> Result { let rootfs_path = bundle_path.join("rootfs"); fs::create_dir_all(&rootfs_path)?; - BareMount::new( + baremount( &spec_root.path, rootfs_path.to_str().unwrap(), "bind", MsFlags::MS_BIND, "", &sl!(), - ) - .mount()?; + )?; spec.root = Some(Root { path: rootfs_path.to_str().unwrap().to_owned(), readonly: spec_root.readonly, diff --git a/src/agent/src/sandbox.rs b/src/agent/src/sandbox.rs index 93a4fa0feb..6829eb2cf5 100644 --- a/src/agent/src/sandbox.rs +++ b/src/agent/src/sandbox.rs @@ -449,7 +449,7 @@ fn online_memory(logger: &Logger) -> Result<()> { #[cfg(test)] mod tests { use super::Sandbox; - use crate::{mount::BareMount, skip_if_not_root}; + use crate::{mount::baremount, skip_if_not_root}; use anyhow::Error; use nix::mount::MsFlags; use oci::{Linux, Root, Spec}; @@ -461,8 +461,7 @@ mod tests { use tempfile::Builder; fn bind_mount(src: &str, dst: &str, logger: &Logger) -> Result<(), Error> { - let baremount = BareMount::new(src, dst, "bind", MsFlags::MS_BIND, "", logger); - baremount.mount() + baremount(src, dst, "bind", MsFlags::MS_BIND, "", logger) } #[tokio::test] diff --git a/src/agent/src/watcher.rs b/src/agent/src/watcher.rs index b6ec528988..840c847dec 100644 --- a/src/agent/src/watcher.rs +++ b/src/agent/src/watcher.rs @@ -20,7 +20,7 @@ use tokio::sync::Mutex; use tokio::task; use tokio::time::{self, Duration}; -use crate::mount::BareMount; +use crate::mount::baremount; use crate::protocols::agent as protos; /// The maximum number of file system entries agent will watch for each mount. @@ -314,16 +314,14 @@ impl SandboxStorages { } } - match BareMount::new( + match baremount( entry.source_mount_point.to_str().unwrap(), entry.target_mount_point.to_str().unwrap(), "bind", MsFlags::MS_BIND, "bind", logger, - ) - .mount() - { + ) { Ok(_) => { entry.watch = false; info!(logger, "watchable mount replaced with bind mount") @@ -427,15 +425,14 @@ impl BindWatcher { async fn mount(&self, logger: &Logger) -> Result<()> { fs::create_dir_all(WATCH_MOUNT_POINT_PATH).await?; - BareMount::new( + baremount( "tmpfs", WATCH_MOUNT_POINT_PATH, "tmpfs", MsFlags::empty(), "", logger, - ) - .mount()?; + )?; Ok(()) }