agent: Remove unnecessary BareMount structure

struct Baremount contains the information necessary to make a new mount.
As a datastructure, however, it's pointless, since every user just
constructs it, immediately calls the BareMount::mount() method then
discards the structure.

Simplify the code by making this a direct function call baremount().

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

src/agent/src/mount.rs

@@ -145,78 +145,53 @@ pub const STORAGE_HANDLER_LIST: &[&str] = &[
     DRIVER_WATCHABLE_BIND_TYPE,
 ];
 
-#[derive(Debug, Clone)]
+#[instrument]
-pub struct BareMount<'a> {
+pub fn baremount(
-    source: &'a str,
+    source: &str,
-    destination: &'a str,
+    destination: &str,
-    fs_type: &'a str,
+    fs_type: &str,
     flags: MsFlags,
-    options: &'a str,
+    options: &str,
-    logger: Logger,
-}
-
-// 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 {
+) -> Result<()> {
-        BareMount {
+    let logger = logger.new(o!("subsystem" => "baremount"));
-            source: s,
-            destination: d,
-            fs_type,
-            flags,
-            options,
-            logger: logger.new(o!("subsystem" => "baremount")),
-        }
-    }
 
-    #[instrument]
+    if source.is_empty() {
-    pub fn mount(&self) -> Result<()> {
-        if self.source.is_empty() {
         return Err(anyhow!("need mount source"));
     }
 
-        if self.destination.is_empty() {
+    if destination.is_empty() {
         return Err(anyhow!("need mount destination"));
     }
 
-        if self.fs_type.is_empty() {
+    if fs_type.is_empty() {
         return Err(anyhow!("need mount FS type"));
     }
 
     info!(
-            self.logger,
+        logger,
         "mount source={:?}, dest={:?}, fs_type={:?}, options={:?}",
-            self.source,
+        source,
-            self.destination,
+        destination,
-            self.fs_type,
+        fs_type,
-            self.options
+        options
     );
 
     nix::mount::mount(
-            Some(self.source),
+        Some(source),
-            self.destination,
+        destination,
-            Some(self.fs_type),
+        Some(fs_type),
-            self.flags,
+        flags,
-            Some(self.options),
+        Some(options),
     )
     .map_err(|e| {
         anyhow!(
             "failed to mount {:?} to {:?}, with error: {}",
-                self.source,
+            source,
-                self.destination,
+            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 = &[

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);
+                baremount(source, destination, "none", flags, "", &logger).map_err(|e| {
-                bare_mount.mount().map_err(|e| {
                     anyhow!(
                         "Failed to mount {} to {} with err:{:?}",
                         source,

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<PathBuf> {
     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,

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(src, dst, "bind", MsFlags::MS_BIND, "", logger)
-        baremount.mount()
     }
 
     #[tokio::test]

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(())
     }