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>
This commit is contained in:
David Gibson 2021-08-24 16:24:33 +10:00
parent 49282854f1
commit 9fa3beff4f
5 changed files with 56 additions and 95 deletions

View File

@ -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 = &[

View File

@ -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,

View File

@ -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,

View File

@ -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]

View File

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