Merge pull request #1288 from cgwalters/mount-crate

Split mount code into separate helper crate

Author: cgwalters

Cargo.lock

@@ -3,6 +3,7 @@ members = [
     "cli",
     "system-reinstall-bootc",
     "lib",
+    "mount",
     "ostree-ext",
     "utils",
     "blockdev",

Cargo.toml

@@ -18,6 +18,7 @@ anstyle = "1.0.6"
 anyhow = { workspace = true }
 bootc-utils = { path = "../utils" }
 bootc-blockdev = { path = "../blockdev" }
+bootc-mount = { path = "../mount" }
 bootc-tmpfiles = { path = "../tmpfiles" }
 bootc-sysusers = { path = "../sysusers" }
 camino = { workspace = true, features = ["serde1"] }

lib/Cargo.toml

@@ -4,6 +4,7 @@ use fn_error_context::context;
 
 use crate::task::Task;
 use bootc_blockdev::PartitionTable;
+use bootc_mount as mount;
 
 /// The name of the mountpoint for efi (as a subdirectory of /boot, or at the toplevel)
 pub(crate) const EFI_DIR: &str = "efi";
@@ -33,7 +34,7 @@ pub(crate) fn install_via_bootupd(
 #[context("Installing bootloader using zipl")]
 pub(crate) fn install_via_zipl(device: &PartitionTable, boot_uuid: &str) -> Result<()> {
     // Identify the target boot partition from UUID
-    let fs = crate::mount::inspect_filesystem_by_uuid(boot_uuid)?;
+    let fs = mount::inspect_filesystem_by_uuid(boot_uuid)?;
     let boot_dir = Utf8Path::new(&fs.target);
     let maj_min = fs.maj_min;
 

lib/src/bootloader.rs

@@ -55,12 +55,12 @@ use crate::boundimage::{BoundImage, ResolvedBoundImage};
 use crate::containerenv::ContainerExecutionInfo;
 use crate::deploy::{prepare_for_pull, pull_from_prepared, PreparedImportMeta, PreparedPullResult};
 use crate::lsm;
-use crate::mount::Filesystem;
 use crate::progress_jsonl::ProgressWriter;
 use crate::spec::ImageReference;
 use crate::store::Storage;
 use crate::task::Task;
 use crate::utils::sigpolicy_from_opt;
+use bootc_mount::Filesystem;
 
 /// The toplevel boot directory
 const BOOT: &str = "boot";
@@ -1287,8 +1287,8 @@ async fn prepare_install(
     tracing::debug!("Target image reference: {target_imgref}");
 
     // A bit of basic global state setup
-    crate::mount::ensure_mirrored_host_mount("/dev")?;
-    crate::mount::ensure_mirrored_host_mount("/var/lib/containers")?;
+    bootc_mount::ensure_mirrored_host_mount("/dev")?;
+    bootc_mount::ensure_mirrored_host_mount("/var/lib/containers")?;
     ensure_var()?;
     setup_tmp_mounts()?;
     // Allocate a temporary directory we can use in various places to avoid
@@ -1763,8 +1763,8 @@ pub(crate) async fn install_to_filesystem(
     {
         tracing::debug!("Mounting host / to {ALONGSIDE_ROOT_MOUNT}");
         std::fs::create_dir(ALONGSIDE_ROOT_MOUNT)?;
-        crate::mount::bind_mount_from_pidns(
-            crate::mount::PID1,
+        bootc_mount::bind_mount_from_pidns(
+            bootc_mount::PID1,
             "/".into(),
             ALONGSIDE_ROOT_MOUNT.into(),
             true,
@@ -1829,7 +1829,7 @@ pub(crate) async fn install_to_filesystem(
     }
 
     // Gather data about the root filesystem
-    let inspect = crate::mount::inspect_filesystem(&fsopts.root_path)?;
+    let inspect = bootc_mount::inspect_filesystem(&fsopts.root_path)?;
 
     // We support overriding the mount specification for root (i.e. LABEL vs UUID versus
     // raw paths).
@@ -1881,7 +1881,7 @@ pub(crate) async fn install_to_filesystem(
     // Find the UUID of /boot because we need it for GRUB.
     let boot_uuid = if boot_is_mount {
         let boot_path = fsopts.root_path.join(BOOT);
-        let u = crate::mount::inspect_filesystem(&boot_path)
+        let u = bootc_mount::inspect_filesystem(&boot_path)
             .context("Inspecting /{BOOT}")?
             .uuid
             .ok_or_else(|| anyhow!("No UUID found for /{BOOT}"))?;

lib/src/install.rs

@@ -27,10 +27,9 @@ use super::RootSetup;
 use super::State;
 use super::RUN_BOOTC;
 use super::RW_KARG;
-use crate::mount;
-#[cfg(feature = "install-to-disk")]
-use crate::mount::is_mounted_in_pid1_mountns;
 use crate::task::Task;
+#[cfg(feature = "install-to-disk")]
+use bootc_mount::is_mounted_in_pid1_mountns;
 
 // This ensures we end up under 512 to be small-sized.
 pub(crate) const BOOTPN_SIZE_MB: u32 = 510;
@@ -421,15 +420,15 @@ pub(crate) fn install_create_rootfs(
         .chain(bootarg)
         .collect::<Vec<_>>();
 
-    mount::mount(&rootdev, &physical_root_path)?;
+    bootc_mount::mount(&rootdev, &physical_root_path)?;
     let target_rootfs = Dir::open_ambient_dir(&physical_root_path, cap_std::ambient_authority())?;
     crate::lsm::ensure_dir_labeled(&target_rootfs, "", Some("/".into()), 0o755.into(), sepolicy)?;
     let physical_root = Dir::open_ambient_dir(&physical_root_path, cap_std::ambient_authority())?;
     let bootfs = physical_root_path.join("boot");
     // Create the underlying mount point directory, which should be labeled
     crate::lsm::ensure_dir_labeled(&target_rootfs, "boot", None, 0o755.into(), sepolicy)?;
     if let Some(bootdev) = bootdev {
-        mount::mount(bootdev.node.as_str(), &bootfs)?;
+        bootc_mount::mount(bootdev.node.as_str(), &bootfs)?;
     }
     // And we want to label the root mount of /boot
     crate::lsm::ensure_dir_labeled(&target_rootfs, "boot", None, 0o755.into(), sepolicy)?;

lib/src/install/baseline.rs

@@ -35,7 +35,6 @@ mod bootloader;
 mod containerenv;
 mod install;
 mod kernel;
-pub(crate) mod mount;
 
 #[cfg(feature = "rhsm")]
 mod rhsm;

lib/src/lib.rs

@@ -0,0 +1,25 @@
+[package]
+description = "Internal mount code"
+# Should never be published to crates.io
+publish = false
+edition = "2021"
+license = "MIT OR Apache-2.0"
+name = "bootc-mount"
+repository = "https://github.com/containers/bootc"
+version = "0.0.0"
+
+[dependencies]
+anyhow = { workspace = true }
+bootc-utils = { path = "../utils" }
+camino = { workspace = true, features = ["serde1"] }
+fn-error-context = { workspace = true }
+rustix = { workspace = true }
+libc = {workspace = true}
+serde = { workspace = true, features = ["derive"] }
+tracing = { workspace = true }
+
+[dev-dependencies]
+indoc = "2.0.5"
+
+[lib]
+path = "src/mount.rs"

mount/Cargo.toml

@@ -22,11 +22,8 @@ use rustix::{
 };
 use serde::Deserialize;
 
-#[cfg(feature = "install-to-disk")]
-use crate::task::Task;
-
 /// Well known identifier for pid 1
-pub(crate) const PID1: Pid = const {
+pub const PID1: Pid = const {
     match Pid::from_raw(1) {
         Some(v) => v,
         None => panic!("Expected to parse pid1"),
@@ -36,21 +33,21 @@ pub(crate) const PID1: Pid = const {
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "kebab-case")]
 #[allow(dead_code)]
-pub(crate) struct Filesystem {
+pub struct Filesystem {
     // Note if you add an entry to this list, you need to change the --output invocation below too
-    pub(crate) source: String,
-    pub(crate) target: String,
+    pub source: String,
+    pub target: String,
     #[serde(rename = "maj:min")]
-    pub(crate) maj_min: String,
-    pub(crate) fstype: String,
-    pub(crate) options: String,
-    pub(crate) uuid: Option<String>,
-    pub(crate) children: Option<Vec<Filesystem>>,
+    pub maj_min: String,
+    pub fstype: String,
+    pub options: String,
+    pub uuid: Option<String>,
+    pub children: Option<Vec<Filesystem>>,
 }
 
 #[derive(Deserialize, Debug)]
-pub(crate) struct Findmnt {
-    pub(crate) filesystems: Vec<Filesystem>,
+pub struct Findmnt {
+    pub filesystems: Vec<Filesystem>,
 }
 
 fn run_findmnt(args: &[&str], path: &str) -> Result<Findmnt> {
@@ -80,20 +77,19 @@ fn findmnt_filesystem(args: &[&str], path: &str) -> Result<Filesystem> {
 #[context("Inspecting filesystem {path}")]
 /// Inspect a target which must be a mountpoint root - it is an error
 /// if the target is not the mount root.
-pub(crate) fn inspect_filesystem(path: &Utf8Path) -> Result<Filesystem> {
+pub fn inspect_filesystem(path: &Utf8Path) -> Result<Filesystem> {
     findmnt_filesystem(&["--mountpoint"], path.as_str())
 }
 
 #[context("Inspecting filesystem by UUID {uuid}")]
 /// Inspect a filesystem by partition UUID
-pub(crate) fn inspect_filesystem_by_uuid(uuid: &str) -> Result<Filesystem> {
+pub fn inspect_filesystem_by_uuid(uuid: &str) -> Result<Filesystem> {
     findmnt_filesystem(&["--source"], &(format!("UUID={uuid}")))
 }
 
 // Check if a specified device contains an already mounted filesystem
 // in the root mount namespace
-#[cfg(feature = "install-to-disk")]
-pub(crate) fn is_mounted_in_pid1_mountns(path: &str) -> Result<bool> {
+pub fn is_mounted_in_pid1_mountns(path: &str) -> Result<bool> {
     let o = run_findmnt(&["-N"], "1")?;
 
     let mounted = o.filesystems.iter().any(|fs| is_source_mounted(path, fs));
@@ -102,8 +98,7 @@ pub(crate) fn is_mounted_in_pid1_mountns(path: &str) -> Result<bool> {
 }
 
 // Recursively check a given filesystem to see if it contains an already mounted source
-#[cfg(feature = "install-to-disk")]
-pub(crate) fn is_source_mounted(path: &str, mounted_fs: &Filesystem) -> bool {
+pub fn is_source_mounted(path: &str, mounted_fs: &Filesystem) -> bool {
     if mounted_fs.source.contains(path) {
         return true;
     }
@@ -120,21 +115,18 @@ pub(crate) fn is_source_mounted(path: &str, mounted_fs: &Filesystem) -> bool {
 }
 
 /// Mount a device to the target path.
-#[cfg(feature = "install-to-disk")]
-pub(crate) fn mount(dev: &str, target: &Utf8Path) -> Result<()> {
-    Task::new_and_run(
-        format!("Mounting {target}"),
-        "mount",
-        [dev, target.as_str()],
-    )
+pub fn mount(dev: &str, target: &Utf8Path) -> Result<()> {
+    Command::new("mount")
+        .args([dev, target.as_str()])
+        .run_with_cmd_context()
 }
 
 /// If the fsid of the passed path matches the fsid of the same path rooted
 /// at /proc/1/root, it is assumed that these are indeed the same mounted
 /// filesystem between container and host.
 /// Path should be absolute.
 #[context("Comparing filesystems at {path} and /proc/1/root/{path}")]
-pub(crate) fn is_same_as_host(path: &Utf8Path) -> Result<bool> {
+pub fn is_same_as_host(path: &Utf8Path) -> Result<bool> {
     // Add a leading '/' in case a relative path is passed
     let path = Utf8Path::new("/").join(path);
 
@@ -155,7 +147,7 @@ pub(crate) fn is_same_as_host(path: &Utf8Path) -> Result<bool> {
 /// for a mount from that namespace.
 #[allow(unsafe_code)]
 #[context("Opening mount tree from pid")]
-pub(crate) fn open_tree_from_pidns(
+pub fn open_tree_from_pidns(
     pid: rustix::process::Pid,
     path: &Utf8Path,
     recursive: bool,
@@ -259,7 +251,7 @@ pub(crate) fn open_tree_from_pidns(
 
 /// Create a bind mount from the mount namespace of the target pid
 /// into our mount namespace.
-pub(crate) fn bind_mount_from_pidns(
+pub fn bind_mount_from_pidns(
     pid: Pid,
     src: &Utf8Path,
     target: &Utf8Path,
@@ -279,7 +271,7 @@ pub(crate) fn bind_mount_from_pidns(
 
 // If the target path is not already mirrored from the host (e.g. via -v /dev:/dev)
 // then recursively mount it.
-pub(crate) fn ensure_mirrored_host_mount(path: impl AsRef<Utf8Path>) -> Result<()> {
+pub fn ensure_mirrored_host_mount(path: impl AsRef<Utf8Path>) -> Result<()> {
     let path = path.as_ref();
     // If we didn't have this in our filesystem already (e.g. for /var/lib/containers)
     // then create it now.