From f6c9f4e99acd54e669c299cfa3afbc7ede680285 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 19 Nov 2024 11:03:00 -0500 Subject: [PATCH 1/2] install: Reduce usage of absolute path for rootfs In the install flow we have both `rootfs` and `rootfs_fd` which hold the physical root. Using fd-relative accesses where we can provides a lot of advantages, so switch most uses over to the file descriptor. Signed-off-by: Colin Walters --- lib/src/install.rs | 51 ++++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/lib/src/install.rs b/lib/src/install.rs index 83dd7752..b1ec987d 100644 --- a/lib/src/install.rs +++ b/lib/src/install.rs @@ -12,7 +12,7 @@ mod osbuild; pub(crate) mod osconfig; use std::io::Write; -use std::os::fd::AsFd; +use std::os::fd::{AsFd, AsRawFd}; use std::os::unix::process::CommandExt; use std::path::Path; use std::process::Command; @@ -581,18 +581,16 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result let sepolicy = sepolicy.as_ref(); // Load a fd for the mounted target physical root let rootfs_dir = &root_setup.rootfs_fd; - let rootfs = root_setup.rootfs.as_path(); let cancellable = gio::Cancellable::NONE; let stateroot = state.stateroot(); let has_ostree = rootfs_dir.try_exists("ostree/repo")?; if !has_ostree { - Task::new_and_run( - "Initializing ostree layout", - "ostree", - ["admin", "init-fs", "--modern", rootfs.as_str()], - )?; + Task::new("Initializing ostree layout", "ostree") + .args(["admin", "init-fs", "--modern", "."]) + .cwd(rootfs_dir)? + .run()?; } else { println!("Reusing extant ostree layout"); @@ -607,8 +605,7 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result crate::lsm::ensure_dir_labeled(rootfs_dir, "", Some("/".into()), 0o755.into(), sepolicy)?; // And also label /boot AKA xbootldr, if it exists - let bootdir = rootfs.join("boot"); - if bootdir.try_exists()? { + if rootfs_dir.try_exists("boot")? { crate::lsm::ensure_dir_labeled(rootfs_dir, "boot", None, 0o755.into(), sepolicy)?; } @@ -626,7 +623,10 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result .run()?; } - let sysroot = ostree::Sysroot::new(Some(&gio::File::for_path(rootfs))); + let sysroot = { + let path = format!("/proc/self/fd/{}", rootfs_dir.as_fd().as_raw_fd()); + ostree::Sysroot::new(Some(&gio::File::for_path(path))) + }; sysroot.load(cancellable)?; let stateroot_exists = rootfs_dir.try_exists(format!("ostree/deploy/{stateroot}"))?; @@ -661,7 +661,6 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result )?; } - let sysroot = ostree::Sysroot::new(Some(&gio::File::for_path(rootfs))); sysroot.load(cancellable)?; let sysroot = SysrootLock::new_from_sysroot(&sysroot).await?; Ok((Storage::new(sysroot, &temp_run)?, has_ostree)) @@ -999,24 +998,29 @@ pub(crate) fn reexecute_self_for_selinux_if_needed( /// Trim, flush outstanding writes, and freeze/thaw the target mounted filesystem; /// these steps prepare the filesystem for its first booted use. -pub(crate) fn finalize_filesystem(fs: &Utf8Path) -> Result<()> { - let fsname = fs.file_name().unwrap(); +pub(crate) fn finalize_filesystem( + fsname: &str, + root: &Dir, + path: impl AsRef, +) -> Result<()> { + let path = path.as_ref(); // fstrim ensures the underlying block device knows about unused space - Task::new_and_run( - format!("Trimming {fsname}"), - "fstrim", - ["--quiet-unsupported", "-v", fs.as_str()], - )?; + Task::new(format!("Trimming {fsname}"), "fstrim") + .args(["--quiet-unsupported", "-v", path.as_str()]) + .cwd(root)? + .run()?; // Remounting readonly will flush outstanding writes and ensure we error out if there were background // writeback problems. Task::new(format!("Finalizing filesystem {fsname}"), "mount") - .args(["-o", "remount,ro", fs.as_str()]) + .cwd(root)? + .args(["-o", "remount,ro", path.as_str()]) .run()?; // Finally, freezing (and thawing) the filesystem will flush the journal, which means the next boot is clean. for a in ["-f", "-u"] { Task::new("Flushing filesystem journal", "fsfreeze") .quiet() - .args([a, fs.as_str()]) + .cwd(root)? + .args([a, path.as_str()]) .run()?; } Ok(()) @@ -1419,10 +1423,9 @@ async fn install_to_filesystem_impl(state: &State, rootfs: &mut RootSetup) -> Re // Finalize mounted filesystems if !rootfs.skip_finalize { - let bootfs = rootfs.boot.as_ref().map(|_| rootfs.rootfs.join("boot")); - let bootfs = bootfs.as_ref().map(|p| p.as_path()); - for fs in std::iter::once(rootfs.rootfs.as_path()).chain(bootfs) { - finalize_filesystem(fs)?; + let bootfs = rootfs.boot.as_ref().map(|_| ("boot", "boot")); + for (fsname, fs) in std::iter::once(("root", ".")).chain(bootfs) { + finalize_filesystem(fsname, &rootfs.rootfs_fd, fs)?; } } From 31608bf13e5e53ce04148180c8888ce62b783eb1 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 19 Nov 2024 11:07:10 -0500 Subject: [PATCH 2/2] install: Rename rootfs -> physical_root In the install flow we juggle *three* file systems in general: - The container/host root - The physical root - The deployment root "rootfs" in theory could be any of those three. In the install code it's the physical (target) root, so rename the variable to clarify. Signed-off-by: Colin Walters --- lib/src/install.rs | 25 ++++++++++++++----------- lib/src/install/baseline.rs | 16 ++++++++-------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/lib/src/install.rs b/lib/src/install.rs index b1ec987d..eff015c3 100644 --- a/lib/src/install.rs +++ b/lib/src/install.rs @@ -580,7 +580,7 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result let sepolicy = state.load_policy()?; let sepolicy = sepolicy.as_ref(); // Load a fd for the mounted target physical root - let rootfs_dir = &root_setup.rootfs_fd; + let rootfs_dir = &root_setup.physical_root; let cancellable = gio::Cancellable::NONE; let stateroot = state.stateroot(); @@ -779,7 +779,7 @@ async fn install_container( // SAFETY: There must be a path let path = sysroot.deployment_dirpath(&deployment); let root = root_setup - .rootfs_fd + .physical_root .open_dir(path.as_str()) .context("Opening deployment dir")?; @@ -792,7 +792,7 @@ async fn install_container( for d in ["ostree", "boot"] { let mut pathbuf = Utf8PathBuf::from(d); crate::lsm::ensure_dir_labeled_recurse( - &root_setup.rootfs_fd, + &root_setup.physical_root, &mut pathbuf, policy, Some(deployment_root_devino), @@ -902,8 +902,11 @@ fn require_skopeo_with_containers_storage() -> Result<()> { pub(crate) struct RootSetup { luks_device: Option, device_info: crate::blockdev::PartitionTable, - rootfs: Utf8PathBuf, - rootfs_fd: Dir, + /// Absolute path to the location where we've mounted the physical + /// root filesystem for the system we're installing. + physical_root_path: Utf8PathBuf, + /// Directory file descriptor for the above physical root. + physical_root: Dir, rootfs_uuid: Option, /// True if we should skip finalizing skip_finalize: bool, @@ -925,7 +928,7 @@ impl RootSetup { // Drop any open file descriptors and return just the mount path and backing luks device, if any fn into_storage(self) -> (Utf8PathBuf, Option) { - (self.rootfs, self.luks_device) + (self.physical_root_path, self.luks_device) } } @@ -1323,7 +1326,7 @@ async fn install_with_sysroot( let (_deployment, aleph) = install_container(state, rootfs, &sysroot, has_ostree).await?; // Write the aleph data that captures the system state at the time of provisioning for aid in future debugging. rootfs - .rootfs_fd + .physical_root .atomic_replace_with(BOOTC_ALEPH_PATH, |f| { serde_json::to_writer(f, &aleph)?; anyhow::Ok(()) @@ -1336,7 +1339,7 @@ async fn install_with_sysroot( } else { crate::bootloader::install_via_bootupd( &rootfs.device_info, - &rootfs.rootfs, + &rootfs.physical_root_path, &state.config_opts, )?; } @@ -1425,7 +1428,7 @@ async fn install_to_filesystem_impl(state: &State, rootfs: &mut RootSetup) -> Re if !rootfs.skip_finalize { let bootfs = rootfs.boot.as_ref().map(|_| ("boot", "boot")); for (fsname, fs) in std::iter::once(("root", ".")).chain(bootfs) { - finalize_filesystem(fsname, &rootfs.rootfs_fd, fs)?; + finalize_filesystem(fsname, &rootfs.physical_root, fs)?; } } @@ -1819,8 +1822,8 @@ pub(crate) async fn install_to_filesystem( let mut rootfs = RootSetup { luks_device: None, device_info, - rootfs: fsopts.root_path, - rootfs_fd, + physical_root_path: fsopts.root_path, + physical_root: rootfs_fd, rootfs_uuid: inspect.uuid.clone(), boot, kargs, diff --git a/lib/src/install/baseline.rs b/lib/src/install/baseline.rs index 50ef8d36..5db2e221 100644 --- a/lib/src/install/baseline.rs +++ b/lib/src/install/baseline.rs @@ -223,8 +223,8 @@ pub(crate) fn install_create_rootfs( // Create a temporary directory to use for mount points. Note that we're // in a mount namespace, so these should not be visible on the host. - let rootfs = mntdir.join("rootfs"); - std::fs::create_dir_all(&rootfs)?; + let physical_root_path = mntdir.join("rootfs"); + std::fs::create_dir_all(&physical_root_path)?; let bootfs = mntdir.join("boot"); std::fs::create_dir_all(bootfs)?; @@ -389,11 +389,11 @@ pub(crate) fn install_create_rootfs( .chain(bootarg) .collect::>(); - mount::mount(&rootdev, &rootfs)?; - let target_rootfs = Dir::open_ambient_dir(&rootfs, cap_std::ambient_authority())?; + 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 rootfs_fd = Dir::open_ambient_dir(&rootfs, cap_std::ambient_authority())?; - let bootfs = rootfs.join("boot"); + 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 { @@ -422,8 +422,8 @@ pub(crate) fn install_create_rootfs( Ok(RootSetup { luks_device, device_info, - rootfs, - rootfs_fd, + physical_root_path, + physical_root, rootfs_uuid: Some(root_uuid.to_string()), boot, kargs,