Skip to content

install: Two rootfs cleanups #909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 20, 2024
Merged

install: Two rootfs cleanups #909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f6c9f4e
install: Reduce usage of absolute path for rootfs
cgwalters Nov 19, 2024
31608bf
install: Rename rootfs -> physical_root
cgwalters Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 40 additions & 34 deletions lib/src/install.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");

Expand All @@ -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)?;
}

Expand All @@ -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}"))?;
Expand Down Expand Up @@ -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)));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note here we somehow ended up with two sysroot objects pointlessly, so this drops the second.

sysroot.load(cancellable)?;
let sysroot = SysrootLock::new_from_sysroot(&sysroot).await?;
Ok((Storage::new(sysroot, &temp_run)?, has_ostree))
Expand Down Expand Up @@ -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")?;

Expand All @@ -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),
Expand Down Expand Up @@ -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,
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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(())
Expand Down Expand Up @@ -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(())
Expand All @@ -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,
)?;
}
Expand Down Expand Up @@ -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)?;
}
}

Expand Down Expand Up @@ -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,
Expand Down
16 changes: 8 additions & 8 deletions lib/src/install/baseline.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down