Merge pull request #909 from cgwalters/clarify-roots

install: Two rootfs cleanups

Author: cgwalters

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;
@@ -580,19 +580,17 @@ 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 = root_setup.rootfs.as_path();
+    let rootfs_dir = &root_setup.physical_root;
     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))
@@ -780,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")?;
 
@@ -793,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),
@@ -903,8 +902,11 @@ fn require_skopeo_with_containers_storage() -> Result<()> {
 pub(crate) struct RootSetup {
     luks_device: Option<String>,
     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<String>,
     /// True if we should skip finalizing
     skip_finalize: bool,
@@ -926,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<String>) {
-        (self.rootfs, self.luks_device)
+        (self.physical_root_path, self.luks_device)
     }
 }
 
@@ -999,24 +1001,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<Utf8Path>,
+) -> 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(())
@@ -1319,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(())
@@ -1332,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,
         )?;
     }
@@ -1419,10 +1426,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.physical_root, fs)?;
         }
     }
 
@@ -1816,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,

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::<Vec<_>>();
 
-    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,