From e62bfab9218d16124122a689a26d2c9a300231f6 Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Mon, 17 Aug 2026 10:13:54 -0500 Subject: [PATCH 1/5] install: Add /var and /etc migration options Preserve existing-root data while providing a clear, reliable /etc merge path for migration. Assisted-by: AI Signed-off-by: ckyrouac --- crates/etc-merge/src/lib.rs | 172 +++++++- crates/lib/src/cli.rs | 5 + crates/lib/src/install.rs | 136 +++++- crates/lib/src/install/config.rs | 7 + crates/lib/src/install/migrate.rs | 395 ++++++++++++++++++ docs/src/bootc-installation.7.md | 20 +- .../man/bootc-install-to-existing-root.8.md | 43 +- 7 files changed, 740 insertions(+), 38 deletions(-) create mode 100644 crates/lib/src/install/migrate.rs diff --git a/crates/etc-merge/src/lib.rs b/crates/etc-merge/src/lib.rs index 66d74ad1be..daa87770f9 100644 --- a/crates/etc-merge/src/lib.rs +++ b/crates/etc-merge/src/lib.rs @@ -470,6 +470,28 @@ pub fn compute_diff( pristine_etc_files: &FileSystem, current_etc_files: &FileSystem, new_etc_files: &FileSystem, +) -> anyhow::Result { + compute_diff_impl(pristine_etc_files, current_etc_files, new_etc_files, true) +} + +/// Computes differences without treating paths absent from current `/etc` as deletions. +/// +/// This is intended for package-mode to image-mode migration, where the current +/// system may predate defaults in the new image. In that case, absence does not +/// imply that an administrator deleted the path. +pub fn compute_diff_without_deletions( + pristine_etc_files: &FileSystem, + current_etc_files: &FileSystem, + new_etc_files: &FileSystem, +) -> anyhow::Result { + compute_diff_impl(pristine_etc_files, current_etc_files, new_etc_files, false) +} + +fn compute_diff_impl( + pristine_etc_files: &FileSystem, + current_etc_files: &FileSystem, + new_etc_files: &FileSystem, + detect_deletions: bool, ) -> anyhow::Result { let mut diff = Diff { added: vec![], @@ -488,12 +510,14 @@ pub fn compute_diff( &mut diff, )?; - get_deletions( - &pristine_etc_files.root, - ¤t_etc_files.root, - PathBuf::new(), - &mut diff, - )?; + if detect_deletions { + get_deletions( + &pristine_etc_files.root, + ¤t_etc_files.root, + PathBuf::new(), + &mut diff, + )?; + } Ok(diff) } @@ -788,7 +812,10 @@ fn merge_leaf( }; if matches!(new_inode, Some(Inode::Directory(..))) { - anyhow::bail!("Modified config file {file:?} newly defaults to directory. Cannot merge") + anyhow::bail!( + "Cannot merge host path {file:?}: the new image contains a directory at this path; \ + resolve the conflict manually before retrying" + ); }; // If a new file with the same path exists, we delete it @@ -802,7 +829,13 @@ fn merge_leaf( } else { current_etc_fd .copy(&file, new_etc_fd, &file) - .with_context(|| format!("Copying file {file:?}"))?; + .with_context(|| { + format!( + "Merge conflict for host /etc path {file:?}: unable to copy it into the new /etc; \ + inspect and resolve conflicting path components (especially absolute symlinks \ + that escape /etc) so the host file can be copied, then retry" + ) + })?; }; rustix::fs::chownat( @@ -1049,6 +1082,54 @@ mod tests { Ok(()) } + #[test] + fn migration_diff_preserves_image_only_default() -> anyhow::Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + + tempdir.create_dir("pristine_etc")?; + tempdir.create_dir("current_etc")?; + tempdir.create_dir("new_etc")?; + + let pristine = tempdir.open_dir("pristine_etc")?; + let current = tempdir.open_dir("current_etc")?; + let new = tempdir.open_dir("new_etc")?; + + pristine.write("image-only.conf", b"image default")?; + new.write("image-only.conf", b"image default")?; + + let (pristine_tree, current_tree, new_tree) = + traverse_etc(&pristine, ¤t, Some(&new))?; + let new_tree = new_tree.unwrap(); + let diff = compute_diff_without_deletions(&pristine_tree, ¤t_tree, &new_tree)?; + + merge(¤t, ¤t_tree, &new, &new_tree, &diff)?; + + assert_eq!(new.read("image-only.conf")?, b"image default"); + Ok(()) + } + + #[test] + fn compute_diff_still_detects_deletions() -> anyhow::Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + + tempdir.create_dir("pristine_etc")?; + tempdir.create_dir("current_etc")?; + tempdir.create_dir("new_etc")?; + + let pristine = tempdir.open_dir("pristine_etc")?; + let current = tempdir.open_dir("current_etc")?; + let new = tempdir.open_dir("new_etc")?; + + pristine.write("deleted.conf", b"old default")?; + + let (pristine_tree, current_tree, new_tree) = + traverse_etc(&pristine, ¤t, Some(&new))?; + let diff = compute_diff(&pristine_tree, ¤t_tree, &new_tree.as_ref().unwrap())?; + + assert_eq!(diff.removed, [PathBuf::from("deleted.conf")]); + Ok(()) + } + fn compare_meta(meta1: Metadata, meta2: Metadata) -> bool { return meta1.is_file() == meta2.is_file() && meta1.is_dir() == meta2.is_dir() @@ -1291,12 +1372,79 @@ mod tests { let merge_res = merge(&c, ¤t_etc_files, &n, &new_etc_files.unwrap(), &diff); - assert!(merge_res.is_err()); - assert_eq!( - merge_res.unwrap_err().root_cause().to_string(), - "Modified config file \"file-to-dir\" newly defaults to directory. Cannot merge" + // The directory should still exist in new_etc (image's directory wins) + let error = merge_res.expect_err("file-to-directory conflict must fail migration"); + let error = format!("{error:#}"); + assert!(error.contains("Cannot merge host path")); + assert!(error.contains("resolve the conflict manually before retrying")); + + Ok(()) + } + + #[test] + fn regular_file_copy_conflict_returns_actionable_error() -> anyhow::Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + + tempdir.create_dir("pristine_etc")?; + tempdir.create_dir("current_etc")?; + tempdir.create_dir("new_etc")?; + + let p = tempdir.open_dir("pristine_etc")?; + let c = tempdir.open_dir("current_etc")?; + let n = tempdir.open_dir("new_etc")?; + + p.write("conflict", "pristine contents")?; + c.write("conflict", "host contents")?; + n.write("conflict", "image contents")?; + + let (pristine_tree, current_tree, new_tree) = traverse_etc(&p, &c, Some(&n))?; + let new_tree = new_tree.unwrap(); + let diff = compute_diff(&pristine_tree, ¤t_tree, &new_tree)?; + + // Make the source path escape the /etc sandbox after traversal so the + // snapshot still identifies it as a modified regular host file. + c.remove_file("conflict")?; + symlinkat("/outside-etc", &c, "conflict")?; + + let error = merge(&c, ¤t_tree, &n, &new_tree, &diff) + .expect_err("copy through an absolute symlink outside current_etc must fail"); + let error = format!("{error:#}"); + assert!(error.contains("Merge conflict"), "{error}"); + assert!(error.contains("host /etc path \"conflict\""), "{error}"); + assert!(error.contains("absolute symlinks"), "{error}"); + assert!( + error.contains("resolve conflicting path components"), + "{error}" ); Ok(()) } + + #[test] + fn regular_host_file_replaces_new_image_symlink() -> anyhow::Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + + tempdir.create_dir("pristine_etc")?; + tempdir.create_dir("current_etc")?; + tempdir.create_dir("new_etc")?; + + let p = tempdir.open_dir("pristine_etc")?; + let c = tempdir.open_dir("current_etc")?; + let n = tempdir.open_dir("new_etc")?; + + p.write("conflict", "pristine contents")?; + c.write("conflict", "host contents")?; + symlinkat("/outside-etc", &n, "conflict")?; + + let (pristine_tree, current_tree, new_tree) = traverse_etc(&p, &c, Some(&n))?; + let new_tree = new_tree.unwrap(); + let diff = compute_diff(&pristine_tree, ¤t_tree, &new_tree)?; + + merge(&c, ¤t_tree, &n, &new_tree, &diff)?; + + assert_eq!(n.read("conflict")?, b"host contents"); + assert!(n.metadata("conflict")?.is_file()); + + Ok(()) + } } diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 0a7c0e5987..fc0bef4482 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -346,6 +346,10 @@ pub(crate) enum InstallOpts { /// the running host root filesystem. Currently, the host root filesystem's `/boot` partition /// will be wiped, but the content of the existing root will otherwise be retained, and will /// need to be cleaned up if desired when rebooted into the new root. + /// + /// When migrating from a package-mode system, use `--preserve-var` to copy `/var` data + /// into the new deployment and write a GRUB rollback entry, and `--merge-etc` to carry + /// forward `/etc` customisations via a 3-way merge. ToExistingRoot(crate::install::InstallToExistingRootOpts), /// Nondestructively create a fresh installation state inside an existing bootc system. /// @@ -2382,6 +2386,7 @@ async fn run_from_opt(opt: Opt) -> Result { InstallOpts::ToFilesystem(opts) => { crate::install::install_to_filesystem(opts, false, crate::install::Cleanup::Skip) .await + .map(|_| ()) } InstallOpts::ToExistingRoot(opts) => { crate::install::install_to_existing_root(opts).await diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 03eeb22172..6889b8f3ec 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -144,6 +144,7 @@ mod aleph; pub(crate) mod baseline; pub(crate) mod completion; pub(crate) mod config; +pub(crate) mod migrate; mod osbuild; pub(crate) mod osconfig; @@ -620,6 +621,52 @@ pub(crate) struct InstallToExistingRootOpts { #[clap(flatten)] pub(crate) composefs_opts: InstallComposefsOpts, + + /// Preserve the running system's `/var` data into the new bootc deployment. + /// + /// After a plain `bootc install to-existing-root`, the new deployment's + /// `/var` is initially empty (ostree bind-mounts it from a fresh directory). + /// Passing this flag performs the following additional steps **after** the + /// core install completes: + /// + /// 1. `/var` content is copied into the new deployment: + /// - **Reflink copy** (btrfs / XFS): `cp --reflink=always` performs an + /// instantaneous copy-on-write clone — no extra disk space consumed. + /// - **Full copy** (filesystems without reflinks): data is copied into + /// the deployment's writable `/var` directory. + /// 2. Paths listed with `--preserve-var-skip` (or in install configuration) + /// are excluded. Rollback preservation is intentionally independent. + /// + /// The running system's `root_path` must be mounted (e.g. `-v /:/target`). + #[clap(long)] + pub(crate) preserve_var: bool, + + /// Relative paths below `/var` to leave out while preserving `/var`. + #[clap(long = "preserve-var-skip", value_name = "PATH")] + pub(crate) preserve_var_skip: Vec, + + /// Merge the running system's `/etc` customisations into the new deployment. + /// + /// Plain `bootc install to-existing-root` populates the new deployment's + /// `/etc` directly from the image. The running admin's customisations + /// (NIC profiles, SSH host keys, secrets, custom CA certificates, etc.) + /// remain at `/etc` but are not applied to the new deployment. + /// + /// Passing this flag runs a 3-way merge using the `etc-merge` algorithm + /// after the core install completes: + /// + /// A (pristine baseline) = `/usr/etc` — image's shipped defaults + /// B (current live) = `/etc` — running system's `/etc` + /// C (new deployment) = `/etc` — deploy target + /// + /// The diff A→B captures every file the admin changed relative to the image + /// defaults and applies those changes onto C. This is the same algorithm + /// bootc uses during `bootc upgrade`, applied at install time rather than + /// only at upgrade time. + /// + /// The running system's `root_path` must be mounted (e.g. `-v /:/target`). + #[clap(long)] + pub(crate) merge_etc: bool, } #[derive(Debug, clap::Parser, PartialEq, Eq)] @@ -2021,7 +2068,7 @@ async fn install_with_sysroot( boot_uuid: &str, bound_images: BoundImages, has_ostree: bool, -) -> Result<()> { +) -> Result { let ostree = storage.get_ostree()?; let c_storage = storage.get_ensure_imgstore()?; @@ -2088,7 +2135,7 @@ async fn install_with_sysroot( } } - Ok(()) + Ok(camino::Utf8PathBuf::from(deployment_path.to_string())) } enum BoundImages { @@ -2127,7 +2174,11 @@ impl BoundImages { } } -async fn ostree_install(state: &State, rootfs: &RootSetup, cleanup: Cleanup) -> Result<()> { +async fn ostree_install( + state: &State, + rootfs: &RootSetup, + cleanup: Cleanup, +) -> Result { // We verify this upfront because it's currently required by bootupd let boot_uuid = rootfs .get_boot_uuid()? @@ -2139,10 +2190,10 @@ async fn ostree_install(state: &State, rootfs: &RootSetup, cleanup: Cleanup) -> // Initialize the ostree sysroot (repo, stateroot, etc.) - { + let deployment_path = { let (sysroot, has_ostree) = initialize_ostree_root(state, rootfs).await?; - install_with_sysroot( + let deployment_path = install_with_sysroot( state, rootfs, &sysroot, @@ -2165,19 +2216,20 @@ async fn ostree_install(state: &State, rootfs: &RootSetup, cleanup: Cleanup) -> // We must drop the sysroot here in order to close any open file // descriptors. + deployment_path }; // Run this on every install as the penultimate step install_finalize(&rootfs.physical_root_path).await?; - Ok(()) + Ok(deployment_path) } async fn install_to_filesystem_impl( state: &State, rootfs: &mut RootSetup, cleanup: Cleanup, -) -> Result<()> { +) -> Result> { if matches!(state.selinux_state, SELinuxFinalState::ForceTargetDisabled) { rootfs.kargs.extend(&Cmdline::from("selinux=0")); } @@ -2199,7 +2251,7 @@ async fn install_to_filesystem_impl( } } - if state.composefs_options.composefs_backend { + let deployment_path = if state.composefs_options.composefs_backend { let fetch_ref = state.source.composefs_fetch_reference(); let manifest = get_container_manifest_and_config(&fetch_ref).await?; // A capable filesystem gets a strict provisional repository. The @@ -2269,8 +2321,9 @@ async fn install_to_filesystem_impl( ) .context("SELinux labeling of composefs objects")?; } + None } else { - ostree_install(state, rootfs, cleanup).await?; + let deployment_path = ostree_install(state, rootfs, cleanup).await?; // For s390x, we set zipl as the bootloader // this needs to be done after the ostree commit is deployed, @@ -2289,7 +2342,8 @@ async fn install_to_filesystem_impl( .run_capture_stderr() .context("Setting bootloader config to zipl")?; } - } + Some(deployment_path) + }; // As the very last step before filesystem finalization, do a full SELinux // relabel of the physical root filesystem. Any files that are already @@ -2311,7 +2365,7 @@ async fn install_to_filesystem_impl( } } - Ok(()) + Ok(deployment_path) } fn installation_complete() { @@ -2668,7 +2722,7 @@ pub(crate) async fn install_to_filesystem( opts: InstallToFilesystemOpts, targeting_host_root: bool, cleanup: Cleanup, -) -> Result<()> { +) -> Result> { // Log the installation operation to systemd journal const INSTALL_FILESYSTEM_JOURNAL_ID: &str = "9a8b7c6d5e4f3a2b1c0d9e8f7a6b5c4d3"; let source_image = opts @@ -2939,14 +2993,14 @@ pub(crate) async fn install_to_filesystem( skip_finalize, }; - install_to_filesystem_impl(&state, &mut rootfs, cleanup).await?; + let deployment_path = install_to_filesystem_impl(&state, &mut rootfs, cleanup).await?; // Drop all data about the root except the path to ensure any file descriptors etc. are closed. drop(rootfs); installation_complete(); - Ok(()) + Ok(deployment_path) } pub(crate) async fn install_to_existing_root(opts: InstallToExistingRootOpts) -> Result<()> { @@ -2979,7 +3033,22 @@ pub(crate) async fn install_to_existing_root(opts: InstallToExistingRootOpts) -> false => Cleanup::Skip, }; - let opts = InstallToFilesystemOpts { + // Extract migration flags before opts is consumed. + let preserve_var = opts.preserve_var; + let merge_etc = opts.merge_etc; + let root_path = std::path::PathBuf::from(opts.root_path.as_str()); + let mut preserve_var_skip = opts.preserve_var_skip; + if let Some(config) = config::load_config()? { + if let Some(config_skip) = config.preserve_var_skip { + preserve_var_skip.extend(config_skip); + } + } + let preserve_var_skip = migrate::validate_exclusions(&preserve_var_skip)?; + if (preserve_var || merge_etc) && opts.composefs_opts.composefs_backend { + anyhow::bail!("--preserve-var and --merge-etc require the ostree backend"); + } + + let fs_opts = InstallToFilesystemOpts { filesystem_opts: InstallTargetFilesystemOpts { root_path: opts.root_path, root_mount_spec: None, @@ -2994,7 +3063,42 @@ pub(crate) async fn install_to_existing_root(opts: InstallToExistingRootOpts) -> composefs_opts: opts.composefs_opts, }; - install_to_filesystem(opts, true, cleanup).await + let deployment_path = install_to_filesystem(fs_opts, true, cleanup).await?; + + // Post-install migration steps (run after the ostree deploy is complete). + if preserve_var { + println!(); + println!("Preserving /var..."); + let deployment_path = deployment_path + .as_ref() + .context("Install did not produce an ostree deployment")?; + let physical_root = if root_path.join("sysroot/ostree").exists() { + root_path.join("sysroot") + } else { + root_path.clone() + }; + let deploy_dir = physical_root.join(deployment_path); + let new_var = migrate::deployment_var_path(&deploy_dir)?; + migrate::preserve_var(&root_path.join("var"), &new_var, &preserve_var_skip) + .context("Post-install /var preservation")?; + } + + if merge_etc { + println!(); + println!("Merging running /etc into new deployment..."); + let deployment_path = deployment_path + .as_ref() + .context("Install did not produce an ostree deployment")?; + let physical_root = if root_path.join("sysroot/ostree").exists() { + root_path.join("sysroot") + } else { + root_path.clone() + }; + migrate::merge_etc_into_deployment(&root_path, &physical_root.join(deployment_path)) + .context("Post-install /etc merge")?; + } + + Ok(()) } /// Read the /boot entry from /etc/fstab, if it exists diff --git a/crates/lib/src/install/config.rs b/crates/lib/src/install/config.rs index 55fdd42f98..93da528344 100644 --- a/crates/lib/src/install/config.rs +++ b/crates/lib/src/install/config.rs @@ -132,6 +132,8 @@ pub(crate) struct InstallConfiguration { /// Enforce that the containers-storage stack has a non-default /// (i.e. not `insecureAcceptAnything`) container image signature policy. pub(crate) enforce_container_sigpolicy: Option, + /// Paths below `/var` which should not be copied by `--preserve-var`. + pub(crate) preserve_var_skip: Option>, } fn merge_basic(s: &mut Option, o: Option, _env: &EnvProperties) { @@ -226,6 +228,11 @@ impl Mergeable for InstallConfiguration { other.enforce_container_sigpolicy, env, ); + if let Some(other_skip) = other.preserve_var_skip { + self.preserve_var_skip + .get_or_insert_with(Default::default) + .extend(other_skip) + } if let Some(other_kargs) = other.kargs { self.kargs .get_or_insert_with(Default::default) diff --git a/crates/lib/src/install/migrate.rs b/crates/lib/src/install/migrate.rs new file mode 100644 index 0000000000..771da598ad --- /dev/null +++ b/crates/lib/src/install/migrate.rs @@ -0,0 +1,395 @@ +//! # Package-mode to image-mode migration helpers +//! +//! This module implements the post-install state-preservation steps that make +//! `bootc install to-existing-root` useful for converting a live package-mode +//! (RPM/DEB) system to a bootc image-mode deployment without losing data. +//! +//! These steps are triggered by passing `--preserve-var` and/or `--merge-etc` +//! to `bootc install to-existing-root`. They run **after** the core install +//! (ostree deploy + bootupd) completes but **before** the first reboot, while +//! the package-mode environment is still the running OS. +//! +//! ## `/var` preservation (`--preserve-var`) +//! +//! After a plain `bootc install to-existing-root`, the new deployment's `/var` +//! is bound from `/ostree/deploy//var/` — an initially-empty +//! directory. The old package-mode `/var` is stranded at `/var` +//! (inside the container, the host root is mounted at `root_path`). +//! +//! Two strategies are tried in order: +//! +//! - **Strategy C — reflink copy** (btrfs / XFS with reflinks): `cp --reflink=always` +//! performs an instantaneous copy-on-write clone. No extra disk space is used +//! until data diverges. +//! +//! - **Strategy D — plain copy** (ext4 and other non-reflink filesystems): +//! `cp -a` copies each non-ephemeral subdirectory of `/var` into the new +//! deployment's stateroot `var/`. This is correct but slow for large `/var` +//! trees, and **unsafe for live databases** (see the known-limitation comment +//! on `preserve_var_copy`). +//! +//! Exclusions such as `tmp`, `log/journal`, `lib/containers`, or `lib/rpm` +//! can be supplied through configuration or repeated `--preserve-var-skip` +//! options. +//! +//! ## `/etc` merge (`--merge-etc`) +//! +//! A plain `bootc install to-existing-root` populates the new deployment's `/etc` +//! from the image. The running system's admin customisations (NIC profiles, SSH +//! host keys, secrets, etc.) end up at `/etc` but are not applied to +//! the new `/etc`. +//! +//! `--merge-etc` runs the 3-way merge from the `etc-merge` crate at install time: +//! +//! | Input | Source | +//! |-------|--------| +//! | A — pristine baseline | `/usr/etc` (image's shipped defaults) | +//! | B — current live | `/etc` (running admin customisations) | +//! | C — new deployment | `/etc` (deploy target, written by image) | +//! +//! The diff A→B captures everything the admin changed relative to the image +//! defaults and applies those changes onto C. Machine-specific files (SSH host +//! keys, machine-id, NIC profiles) are included because they are precisely what +//! needs to be transferred to make the migrated system functional. +//! +//! Rollback preservation is intentionally independent from `/var` migration. + +use std::path::{Component, Path}; +use std::process::Stdio; + +use anyhow::{Context, Result}; +use cap_std_ext::cap_std::ambient_authority; +use cap_std_ext::cap_std::fs::Dir as CapStdDir; +use composefs_ctl::composefs::generic_tree::{FileSystem, Stat}; +use etc_merge::{compute_diff_without_deletions, merge, traverse_etc}; +use fn_error_context::context; + +// ── Top-level entry points ──────────────────────────────────────────────────── + +/// Run the 3-way `/etc` merge on the new deployment. +/// +/// Inputs: +/// - **A (pristine)** = `/usr/etc` +/// - **B (current)** = `/etc` +/// - **C (new)** = `/etc` +/// +/// `root_path` is the host root as seen from inside the install container. +#[context("Running 3-way /etc merge into new deployment")] +pub(crate) fn merge_etc_into_deployment(root_path: &Path, deploy_dir: &Path) -> Result<()> { + let deploy_usr_etc = deploy_dir.join("usr/etc"); + let deploy_etc = deploy_dir.join("etc"); + let host_etc = root_path.join("etc"); + + merge_etc(&host_etc, &deploy_usr_etc, &deploy_etc) +} + +// ── Internal helpers ────────────────────────────────────────────────────────── + +/// Preserve the running `/var` into the new deployment's `var/` directory. +/// +/// `src_var` is the running system's `/var` (at `/var`). +/// `new_var` is the new deployment's empty `var/` directory. +pub(crate) fn preserve_var(src_var: &Path, new_var: &Path, exclusions: &[String]) -> Result<()> { + std::fs::create_dir_all(new_var).with_context(|| format!("Creating {}", new_var.display()))?; + + let mut effective_exclusions = ["tmp", "cache", "log/journal", "lib/containers"] + .into_iter() + .map(String::from) + .collect::>(); + effective_exclusions.extend(exclusions.iter().cloned()); + + if reflinks_supported(src_var, new_var) { + println!(" Filesystem supports reflinks — using copy-on-write clone (Strategy C)"); + preserve_var_reflink(src_var, new_var, &effective_exclusions) + } else { + println!(" Filesystem does not support reflinks — falling back to full copy (Strategy D)"); + preserve_var_copy(src_var, new_var, &effective_exclusions) + } +} + +pub(crate) fn deployment_var_path(deploy_dir: &Path) -> Result { + let deploy_parent = deploy_dir + .parent() + .context("Deployment path has no parent")?; + let stateroot = deploy_parent + .parent() + .context("Deployment path has no stateroot")?; + Ok(stateroot.join("var")) +} + +/// Returns true if the filesystem hosting `new_var` supports reflinks. +/// +/// Probes by attempting a zero-byte reflink from `src_var` into `new_var`. +fn reflinks_supported(src_var: &Path, new_var: &Path) -> bool { + let probe_src = src_var.join(".bootc-reflink-probe-src"); + let probe_dst = new_var.join(".bootc-reflink-probe"); + + let _ = std::fs::write(&probe_src, b"probe"); + let result = std::process::Command::new("cp") + .args(["--reflink=always", "-a"]) + .arg(&probe_src) + .arg(&probe_dst) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .map(|s| s.success()) + .unwrap_or(false); + + let _ = std::fs::remove_file(&probe_src); + let _ = std::fs::remove_file(&probe_dst); + result +} + +/// Strategy C: reflink-copy each top-level entry under `src_var` into `new_var`. +/// +/// Skips well-known ephemeral subdirectories. +#[context("Reflink-copying /var into new deployment (Strategy C)")] +fn preserve_var_reflink(src_var: &Path, new_var: &Path, exclusions: &[String]) -> Result<()> { + let entries = + std::fs::read_dir(src_var).with_context(|| format!("Reading {}", src_var.display()))?; + + for entry in entries { + let entry = entry.with_context(|| format!("Reading entry in {}", src_var.display()))?; + let name = entry.file_name(); + let name_str = name.to_string_lossy(); + + if exclusions.iter().any(|s| s == name_str.as_ref()) { + println!( + " Skipping {} (ephemeral)", + src_var.join(name_str.as_ref()).display() + ); + continue; + } + + let src_entry = src_var.join(name_str.as_ref()); + let dst_entry = new_var.join(name_str.as_ref()); + + if let Some(skip) = exclusions.iter().find_map(|path| { + path.strip_prefix(&format!("{name_str}/")) + .filter(|rest| !rest.contains('/')) + }) { + copy_dir_skip_subdir(&src_entry, &dst_entry, skip, true)?; + continue; + } + + println!( + " Reflink-copying {} → {}", + src_entry.display(), + dst_entry.display() + ); + let status = std::process::Command::new("cp") + .args(["--reflink=always", "-a", "--no-clobber"]) + .arg(&src_entry) + .arg(new_var) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .status() + .with_context(|| format!("cp --reflink=always {}", src_entry.display()))?; + + anyhow::ensure!( + status.success(), + "cp --reflink=always failed for {}", + src_entry.display() + ); + } + + println!(" /var reflink copy complete."); + Ok(()) +} + +/// Copy a directory recursively, skipping one named subdirectory. +/// +/// Used by both Strategy C and Strategy D to copy `var/log/` while excluding +/// `var/log/journal/`. `reflink` selects whether `cp --reflink=always` or +/// plain `cp -a` is used. +fn copy_dir_skip_subdir(src: &Path, dst: &Path, skip_name: &str, reflink: bool) -> Result<()> { + std::fs::create_dir_all(dst).with_context(|| format!("Creating {}", dst.display()))?; + + let entries = std::fs::read_dir(src).with_context(|| format!("Reading {}", src.display()))?; + + for entry in entries { + let entry = entry.with_context(|| format!("Reading entry in {}", src.display()))?; + let name = entry.file_name(); + let name_str = name.to_string_lossy(); + + if name_str == skip_name { + println!(" Skipping {}/{} (ephemeral)", src.display(), name_str); + continue; + } + + let src_entry = src.join(name_str.as_ref()); + let mut cmd = std::process::Command::new("cp"); + if reflink { + cmd.args(["--reflink=always", "-a", "--no-clobber"]); + } else { + cmd.args(["-a", "--no-clobber"]); + } + let status = cmd + .arg(&src_entry) + .arg(dst) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .status() + .with_context(|| format!("cp -a {}", src_entry.display()))?; + + anyhow::ensure!(status.success(), "cp failed for {}", src_entry.display()); + } + + Ok(()) +} + +/// Strategy D: plain recursive copy of `/var` for filesystems without reflink support. +/// +/// # Known limitation +/// +/// This performs a full `cp -a` of every included subdirectory of the running +/// `/var` into the new deployment's ostree stateroot `var/`. For most workloads +/// this is fine, but it is **unsafe for databases and other applications that +/// keep open write handles into `/var`** (e.g. PostgreSQL in `/var/lib/pgsql`, +/// MySQL/MariaDB in `/var/lib/mysql`, SQLite databases under `/var/lib/*`). +/// Copying a live database with `cp -a` will almost certainly produce a +/// corrupted copy. +/// +/// The correct fix is to run this migration only after stopping all stateful +/// services that write to `/var`, or — better — to migrate the filesystem to +/// btrfs or XFS (which support reflinks, Strategy C) so that the copy is +/// instantaneous and atomic from the kernel's perspective. +/// +/// A future improvement would be to accept a user-supplied exclusion list so +/// that specific high-risk directories (e.g. `/var/lib/pgsql`) can be skipped +/// and migrated manually. For now, operators are responsible for stopping +/// affected services before running `bootc install to-existing-root --preserve-var` +/// on ext4 (or other non-reflink) filesystems. +/// +/// See: +#[context("Copying /var into new deployment (Strategy D)")] +fn preserve_var_copy(src_var: &Path, new_var: &Path, exclusions: &[String]) -> Result<()> { + let entries = + std::fs::read_dir(src_var).with_context(|| format!("Reading {}", src_var.display()))?; + + for entry in entries { + let entry = entry.with_context(|| format!("Reading entry in {}", src_var.display()))?; + let name = entry.file_name(); + let name_str = name.to_string_lossy(); + + if exclusions.iter().any(|s| s == name_str.as_ref()) { + println!( + " Skipping {} (ephemeral)", + src_var.join(name_str.as_ref()).display() + ); + continue; + } + + let src_entry = src_var.join(name_str.as_ref()); + let dst_entry = new_var.join(name_str.as_ref()); + + if let Some(skip) = exclusions.iter().find_map(|path| { + path.strip_prefix(&format!("{name_str}/")) + .filter(|rest| !rest.contains('/')) + }) { + copy_dir_skip_subdir(&src_entry, &dst_entry, skip, false)?; + continue; + } + + println!( + " Copying {} → {}", + src_entry.display(), + dst_entry.display() + ); + let status = std::process::Command::new("cp") + .args(["-a", "--no-clobber"]) + .arg(&src_entry) + .arg(new_var) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .status() + .with_context(|| format!("cp -a {}", src_entry.display()))?; + + anyhow::ensure!(status.success(), "cp -a failed for {}", src_entry.display()); + } + + println!(" /var copy complete."); + Ok(()) +} + +/// Validate exclusions before touching either tree. Paths are relative to `/var`. +pub(crate) fn validate_exclusions(exclusions: &[String]) -> Result> { + exclusions + .iter() + .map(|path| { + let path = path.trim_end_matches('/'); + anyhow::ensure!(!path.is_empty(), "empty /var exclusion"); + anyhow::ensure!( + Path::new(path) + .components() + .all(|component| matches!(component, Component::Normal(_))), + "invalid /var exclusion {path:?}; expected a relative path" + ); + anyhow::ensure!( + Path::new(path).components().count() <= 2, + "unsupported nested /var exclusion {path:?}; use at most one child path" + ); + Ok(path.to_string()) + }) + .collect() +} + +/// 3-way `/etc` merge. +/// +/// - `host_etc` = `/etc` (running system, input B) +/// - `deploy_usr_etc` = `/usr/etc` (image defaults, input A) +/// - `deploy_etc` = `/etc` (new deploy target, input C) +#[context("Merging running /etc into new deployment")] +fn merge_etc(host_etc: &Path, deploy_usr_etc: &Path, deploy_etc: &Path) -> Result<()> { + let pristine_fd = CapStdDir::open_ambient_dir(deploy_usr_etc, ambient_authority()) + .with_context(|| format!("Opening pristine etc: {}", deploy_usr_etc.display()))?; + let current_fd = CapStdDir::open_ambient_dir(host_etc, ambient_authority()) + .with_context(|| format!("Opening running etc: {}", host_etc.display()))?; + let new_fd = CapStdDir::open_ambient_dir(deploy_etc, ambient_authority()) + .with_context(|| format!("Opening deploy etc: {}", deploy_etc.display()))?; + + let (pristine_tree, current_tree, new_tree_opt) = + traverse_etc(&pristine_fd, ¤t_fd, Some(&new_fd)) + .context("Traversing /etc trees for 3-way merge")?; + + let new_tree = new_tree_opt.unwrap_or_else(|| FileSystem::new(Stat::uninitialized())); + + let diff = compute_diff_without_deletions(&pristine_tree, ¤t_tree, &new_tree) + .context("Computing /etc diff")?; + + println!(" /etc diff (changes being applied from running system):"); + etc_merge::print_diff(&diff, &mut std::io::stdout()); + + merge(¤t_fd, ¤t_tree, &new_fd, &new_tree, &diff) + .context("Applying /etc 3-way merge")?; + + println!(" /etc merge complete."); + Ok(()) +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn exclusions_are_relative_and_normalized() { + assert_eq!( + validate_exclusions(&["lib/rpm/".into()]).unwrap(), + ["lib/rpm"] + ); + assert!(validate_exclusions(&["../etc".into()]).is_err()); + assert!(validate_exclusions(&["/etc".into()]).is_err()); + assert!(validate_exclusions(&["lib/containers/storage".into()]).is_err()); + } + + #[test] + fn deployment_var_is_stateroot_var() { + let deploy = Path::new("/sysroot/ostree/deploy/default/deploy/checksum.0"); + assert_eq!( + deployment_var_path(deploy).unwrap(), + Path::new("/sysroot/ostree/deploy/default/var") + ); + } +} diff --git a/docs/src/bootc-installation.7.md b/docs/src/bootc-installation.7.md index 22f086adda..de243fdc95 100644 --- a/docs/src/bootc-installation.7.md +++ b/docs/src/bootc-installation.7.md @@ -294,6 +294,17 @@ and data from the old system (now accessible at `/sysroot`) to the new system. T useful for preserving network settings, user accounts, or application data from the previous installation. +By default, the old `/var` and `/etc` are not migrated into the new deployment. With +the ostree backend, `--preserve-var` opts into copying the old `/var` into the new +stateroot before reboot; this uses reflinks when supported and otherwise makes a full +copy, which can be unsafe while databases or other stateful services are running. +`/var/tmp`, `/var/cache`, `/var/log/journal`, and `/var/lib/containers` are always +excluded. Repeat `--preserve-var-skip PATH` to add further exclusions using paths +relative to `/var` (a path or one direct child path, such as `lib/rpm`). The +ostree-only `--merge-etc` option instead performs a three-way merge of the +host's `/etc` customizations into the new deployment before the first reboot. See the +[to-existing-root man page](man/bootc-install-to-existing-root.8.md) for details. + ##### Before reboot: Injecting new configuration After running `bootc install to-existing-root`, you may want to inject @@ -342,11 +353,10 @@ After rebooting into the new bootc system, the previous root filesystem is mounted at `/sysroot`. You can then migrate configuration and data from the old system to the new one. -**Important:** Any data from `/etc` that you want to use in the new system must be -manually copied from `/sysroot/etc` to `/etc` after rebooting into the new system. -There is currently no automated mechanism for migrating this configuration data. -This applies to network configurations, user accounts, application settings, and other -system configuration stored in `/etc`. +Without `--merge-etc`, configuration data from the old `/etc` remains at +`/sysroot/etc` and is not applied to the new system. Manually copy or merge any data +you need after rebooting. This applies to network configurations, user accounts, +application settings, and other system configuration stored in `/etc`. For example, after rebooting: diff --git a/docs/src/man/bootc-install-to-existing-root.8.md b/docs/src/man/bootc-install-to-existing-root.8.md index 3ff11166ed..508395ea98 100644 --- a/docs/src/man/bootc-install-to-existing-root.8.md +++ b/docs/src/man/bootc-install-to-existing-root.8.md @@ -24,6 +24,25 @@ configuration files: 1. **Before rebooting**: Injecting new configuration into the newly installed system 2. **After rebooting**: Migrating configuration from the old system to the new system +By default, installation does not migrate the running system's `/var` data or +customizations in `/etc`. The opt-in `--preserve-var` and `--merge-etc` options +provide these migrations on the **ostree backend only**. They run after the +installation completes and before the first reboot. + +`--preserve-var` copies the old `/var` into the new stateroot. Where supported, +the copy uses filesystem reflinks; otherwise it performs a full copy. A full copy +can be unsafe while databases or other stateful services are running, so stop +those services before installation when a full copy will be used. The paths +`tmp`, `cache`, `log/journal`, and `lib/containers` are always excluded. Use the +repeatable `--preserve-var-skip` option to add exclusions relative to `/var`; +each path may be one component or a direct child path (for example, `tmp` or +`lib/rpm`). + +`--merge-etc` performs a three-way merge that applies the running host's `/etc` +customizations to the new deployment before its first reboot. Without this +option, the new deployment's `/etc` comes from the image; there is no automatic +`/etc` migration by default. + ### Before reboot: Injecting new configuration If you need to inject new configuration files (such as custom `/etc/fstab` entries, @@ -76,9 +95,10 @@ After rebooting into the new bootc system, the previous root filesystem data is accessible at `/sysroot` (the "physical root"). This allows you to migrate data from the old system to the new one. -**Important:** Any configuration data from `/etc` that you want to use in the -new system must be **manually copied** from `/sysroot/etc` to `/etc` after -rebooting. There is currently no automated mechanism for migrating this data. +With the default installation behavior, configuration data from the old `/etc` +remains at `/sysroot/etc` and is not applied to the new system. Copy or merge +any needed configuration manually after reboot, unless you used `--merge-etc` +to apply the old host's customizations to the new deployment before rebooting. For example, to migrate configuration after rebooting: @@ -99,8 +119,9 @@ vipw # Carefully review and merge users from /sysroot/etc/passwd ``` This applies to network configurations, user accounts, application settings, -and other system configuration stored in `/etc`. Review files in `/sysroot/etc` -and manually copy or merge what you need into `/etc`. +and other system configuration stored in `/etc` when `--merge-etc` was not used. +Review files in `/sysroot/etc` and manually copy or merge what you need into +`/etc`. **Note:** A generated `/etc/resolv.conf` should not normally be copied or edited directly. It may be a dangling symlink after reboot or contain only a @@ -231,6 +252,18 @@ of migrating the fstab entries. See the "Injecting kernel arguments" section abo Name of the UKI addons to install without the ".efi.addon" suffix. This option can be provided multiple times if multiple addons are to be installed (composefs backend only) +**--preserve-var** + + Preserve the running system's `/var` data into the new bootc deployment + +**--preserve-var-skip**=*PATH* + + Relative paths below `/var` to leave out while preserving `/var` + +**--merge-etc** + + Merge the running system's `/etc` customisations into the new deployment + # VERSION From d638a137751313099b1a05adb2156505b39e3d6b Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Thu, 1 Oct 2026 14:05:49 -0500 Subject: [PATCH 2/5] install: Optimize /var migration across directory capabilities Use directory-capability checks to preserve /var efficiently across filesystems and document migration exclusions. Assisted-by: AI Signed-off-by: ckyrouac --- crates/lib/src/install/migrate.rs | 542 ++++++++++++++---- crates/lib/src/store/mod.rs | 14 +- crates/lib/src/store/reflink.rs | 279 +++++++++ docs/src/bootc-installation.7.md | 8 +- .../man/bootc-install-to-existing-root.8.md | 14 +- 5 files changed, 721 insertions(+), 136 deletions(-) create mode 100644 crates/lib/src/store/reflink.rs diff --git a/crates/lib/src/install/migrate.rs b/crates/lib/src/install/migrate.rs index 771da598ad..8c40a0aae7 100644 --- a/crates/lib/src/install/migrate.rs +++ b/crates/lib/src/install/migrate.rs @@ -19,8 +19,9 @@ //! Two strategies are tried in order: //! //! - **Strategy C — reflink copy** (btrfs / XFS with reflinks): `cp --reflink=always` -//! performs an instantaneous copy-on-write clone. No extra disk space is used -//! until data diverges. +//! clones files copy-on-write, avoiding an immediate second copy of their data. +//! Files are cloned individually, so this is not an atomic snapshot of the +//! `/var` tree or of a live database. //! //! - **Strategy D — plain copy** (ext4 and other non-reflink filesystems): //! `cp -a` copies each non-ephemeral subdirectory of `/var` into the new @@ -55,15 +56,25 @@ //! Rollback preservation is intentionally independent from `/var` migration. use std::path::{Component, Path}; -use std::process::Stdio; +use std::{ + ffi::OsStr, + os::fd::AsFd, + path::PathBuf, + process::{Command, Stdio}, + sync::Arc, +}; use anyhow::{Context, Result}; use cap_std_ext::cap_std::ambient_authority; use cap_std_ext::cap_std::fs::Dir as CapStdDir; +use cap_std_ext::cap_std::io_lifetimes::AsFilelike; +use cap_std_ext::cmdext::{CapStdExtCommandExt, CmdFds}; use composefs_ctl::composefs::generic_tree::{FileSystem, Stat}; use etc_merge::{compute_diff_without_deletions, merge, traverse_etc}; use fn_error_context::context; +use crate::store::reflink::supports_reflink; + // ── Top-level entry points ──────────────────────────────────────────────────── /// Run the 3-way `/etc` merge on the new deployment. @@ -90,7 +101,9 @@ pub(crate) fn merge_etc_into_deployment(root_path: &Path, deploy_dir: &Path) -> /// `src_var` is the running system's `/var` (at `/var`). /// `new_var` is the new deployment's empty `var/` directory. pub(crate) fn preserve_var(src_var: &Path, new_var: &Path, exclusions: &[String]) -> Result<()> { - std::fs::create_dir_all(new_var).with_context(|| format!("Creating {}", new_var.display()))?; + let src_var = CapStdDir::open_ambient_dir(src_var, ambient_authority()) + .with_context(|| format!("Opening source {}", src_var.display()))?; + let new_var = open_or_create_var_dir(new_var)?; let mut effective_exclusions = ["tmp", "cache", "log/journal", "lib/containers"] .into_iter() @@ -98,13 +111,39 @@ pub(crate) fn preserve_var(src_var: &Path, new_var: &Path, exclusions: &[String] .collect::>(); effective_exclusions.extend(exclusions.iter().cloned()); - if reflinks_supported(src_var, new_var) { + if supports_reflink(&src_var, &new_var)? { println!(" Filesystem supports reflinks — using copy-on-write clone (Strategy C)"); - preserve_var_reflink(src_var, new_var, &effective_exclusions) + preserve_var_reflink(&src_var, &new_var, &effective_exclusions) } else { println!(" Filesystem does not support reflinks — falling back to full copy (Strategy D)"); - preserve_var_copy(src_var, new_var, &effective_exclusions) + preserve_var_copy(&src_var, &new_var, &effective_exclusions) + } +} + +/// Open the destination through its parent capability, creating just the +/// final `/var` directory if needed. The caller-supplied parent is the initial +/// ambient authority; all subsequent traversal is relative to opened handles. +fn open_or_create_var_dir(path: &Path) -> Result { + let parent = path.parent().context("Destination /var has no parent")?; + let name = path + .file_name() + .context("Destination /var has no final path component")?; + let parent = CapStdDir::open_ambient_dir(parent, ambient_authority()) + .with_context(|| format!("Opening destination parent {}", parent.display()))?; + match parent.create_dir(name) { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { + anyhow::ensure!( + parent.symlink_metadata(name)?.is_dir(), + "Destination {} exists and is not a directory", + path.display() + ); + } + Err(e) => return Err(e).with_context(|| format!("Creating {}", path.display())), } + parent + .open_dir(name) + .with_context(|| format!("Opening destination {}", path.display())) } pub(crate) fn deployment_var_path(deploy_dir: &Path) -> Result { @@ -117,79 +156,102 @@ pub(crate) fn deployment_var_path(deploy_dir: &Path) -> Result bool { - let probe_src = src_var.join(".bootc-reflink-probe-src"); - let probe_dst = new_var.join(".bootc-reflink-probe"); - - let _ = std::fs::write(&probe_src, b"probe"); - let result = std::process::Command::new("cp") - .args(["--reflink=always", "-a"]) - .arg(&probe_src) - .arg(&probe_dst) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .status() - .map(|s| s.success()) - .unwrap_or(false); +/// Run GNU cp with directory handles inherited as fixed descriptors. Each +/// operand is `/proc/self/fd/N[/entry]`; cp's top-level paths therefore remain +/// rooted in the opened capabilities even if the original paths are renamed. +/// GNU cp still performs its own recursive traversal beneath each operand, so +/// this is not race-safe against a concurrent writer replacing entries while +/// the copy is in progress. +fn run_cp( + src_root: &CapStdDir, + src_names: &[&OsStr], + dst_root: &CapStdDir, + dst_name: Option<&OsStr>, + options: &[&str], + stdout: Stdio, + stderr: Stdio, +) -> Result { + let src_fd = Arc::new( + src_root + .as_filelike_view::() + .as_fd() + .try_clone_to_owned()?, + ); + let dst_fd = Arc::new( + dst_root + .as_filelike_view::() + .as_fd() + .try_clone_to_owned()?, + ); + let mut fds = CmdFds::new(); + fds.take_fd_n(src_fd, 3); + fds.take_fd_n(dst_fd, 4); + + let src_paths = src_names.iter().map(|name| { + let mut path = PathBuf::from("/proc/self/fd/3"); + path.push(name); + path + }); + let mut dst_path = PathBuf::from("/proc/self/fd/4"); + if let Some(name) = dst_name { + dst_path.push(name); + } - let _ = std::fs::remove_file(&probe_src); - let _ = std::fs::remove_file(&probe_dst); - result + let status = Command::new("cp") + .args(options) + .arg("--") + .args(src_paths) + .arg(dst_path) + .stdout(stdout) + .stderr(stderr) + .take_fds(fds) + .status() + .context("Running cp")?; + Ok(status) } /// Strategy C: reflink-copy each top-level entry under `src_var` into `new_var`. /// /// Skips well-known ephemeral subdirectories. #[context("Reflink-copying /var into new deployment (Strategy C)")] -fn preserve_var_reflink(src_var: &Path, new_var: &Path, exclusions: &[String]) -> Result<()> { - let entries = - std::fs::read_dir(src_var).with_context(|| format!("Reading {}", src_var.display()))?; +fn preserve_var_reflink( + src_var: &CapStdDir, + new_var: &CapStdDir, + exclusions: &[String], +) -> Result<()> { + let entries = src_var.read_dir(".").context("Reading source /var")?; for entry in entries { - let entry = entry.with_context(|| format!("Reading entry in {}", src_var.display()))?; + let entry = entry.context("Reading entry in source /var")?; let name = entry.file_name(); let name_str = name.to_string_lossy(); - if exclusions.iter().any(|s| s == name_str.as_ref()) { - println!( - " Skipping {} (ephemeral)", - src_var.join(name_str.as_ref()).display() - ); + if exclusions.iter().any(|s| name.to_str() == Some(s.as_str())) { + println!(" Skipping {} (ephemeral)", name_str); continue; } - let src_entry = src_var.join(name_str.as_ref()); - let dst_entry = new_var.join(name_str.as_ref()); - - if let Some(skip) = exclusions.iter().find_map(|path| { - path.strip_prefix(&format!("{name_str}/")) - .filter(|rest| !rest.contains('/')) - }) { - copy_dir_skip_subdir(&src_entry, &dst_entry, skip, true)?; + let skips = nested_exclusions(&name, exclusions); + if !skips.is_empty() && src_var.symlink_metadata(&name)?.is_dir() { + copy_dir_skip_subdir(src_var, new_var, &name, &skips, true)?; continue; } - println!( - " Reflink-copying {} → {}", - src_entry.display(), - dst_entry.display() - ); - let status = std::process::Command::new("cp") - .args(["--reflink=always", "-a", "--no-clobber"]) - .arg(&src_entry) - .arg(new_var) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .status() - .with_context(|| format!("cp --reflink=always {}", src_entry.display()))?; + println!(" Reflink-copying {} → {}", name_str, name_str); + let status = run_cp( + src_var, + &[&name], + new_var, + None, + &["--reflink=always", "-a", "--no-clobber"], + Stdio::inherit(), + Stdio::inherit(), + )?; anyhow::ensure!( status.success(), "cp --reflink=always failed for {}", - src_entry.display() + name_str ); } @@ -197,42 +259,70 @@ fn preserve_var_reflink(src_var: &Path, new_var: &Path, exclusions: &[String]) - Ok(()) } -/// Copy a directory recursively, skipping one named subdirectory. +/// Copy a directory recursively, skipping selected child directories. /// /// Used by both Strategy C and Strategy D to copy `var/log/` while excluding /// `var/log/journal/`. `reflink` selects whether `cp --reflink=always` or -/// plain `cp -a` is used. -fn copy_dir_skip_subdir(src: &Path, dst: &Path, skip_name: &str, reflink: bool) -> Result<()> { - std::fs::create_dir_all(dst).with_context(|| format!("Creating {}", dst.display()))?; - - let entries = std::fs::read_dir(src).with_context(|| format!("Reading {}", src.display()))?; - +/// plain `cp -a` is used. Included sibling entries are passed in one cp process +/// so hardlinks between them remain linked. As in the previous implementation, +/// this filtered path does not copy metadata from the containing directory +/// itself; ordinary (unfiltered) entries are copied wholly by cp -a. +fn copy_dir_skip_subdir( + src_root: &CapStdDir, + dst_root: &CapStdDir, + dir_name: &OsStr, + skip_names: &[&str], + reflink: bool, +) -> Result<()> { + let src = open_child_dir_nofollow(src_root, dir_name) + .with_context(|| format!("Opening /var/{}", dir_name.to_string_lossy()))?; + let dst = open_or_create_child_dir(dst_root, dir_name)?; + let entries = src + .read_dir(".") + .context("Reading excluded-copy source directory")?; + + let mut included_entries = Vec::new(); for entry in entries { - let entry = entry.with_context(|| format!("Reading entry in {}", src.display()))?; + let entry = entry.context("Reading entry in source child directory")?; let name = entry.file_name(); let name_str = name.to_string_lossy(); - if name_str == skip_name { - println!(" Skipping {}/{} (ephemeral)", src.display(), name_str); + if skip_names.iter().any(|skip| name == OsStr::new(skip)) { + println!( + " Skipping {}/{} (ephemeral)", + dir_name.to_string_lossy(), + name_str + ); continue; } + included_entries.push(name); + } - let src_entry = src.join(name_str.as_ref()); - let mut cmd = std::process::Command::new("cp"); - if reflink { - cmd.args(["--reflink=always", "-a", "--no-clobber"]); + if !included_entries.is_empty() { + let names = included_entries + .iter() + .map(|name| name.as_os_str()) + .collect::>(); + let options = if reflink { + &["--reflink=always", "-a", "--no-clobber"][..] } else { - cmd.args(["-a", "--no-clobber"]); - } - let status = cmd - .arg(&src_entry) - .arg(dst) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .status() - .with_context(|| format!("cp -a {}", src_entry.display()))?; - - anyhow::ensure!(status.success(), "cp failed for {}", src_entry.display()); + &["-a", "--no-clobber"][..] + }; + let status = run_cp( + &src, + &names, + &dst, + None, + options, + Stdio::inherit(), + Stdio::inherit(), + )?; + + anyhow::ensure!( + status.success(), + "cp failed while copying {}", + dir_name.to_string_lossy() + ); } Ok(()) @@ -252,66 +342,108 @@ fn copy_dir_skip_subdir(src: &Path, dst: &Path, skip_name: &str, reflink: bool) /// /// The correct fix is to run this migration only after stopping all stateful /// services that write to `/var`, or — better — to migrate the filesystem to -/// btrfs or XFS (which support reflinks, Strategy C) so that the copy is -/// instantaneous and atomic from the kernel's perspective. +/// btrfs or XFS (which support reflinks, Strategy C) to avoid copying every +/// file's data immediately. Reflinks clone files individually; they do not +/// provide an atomic snapshot of the `/var` tree or of a live database. /// -/// A future improvement would be to accept a user-supplied exclusion list so -/// that specific high-risk directories (e.g. `/var/lib/pgsql`) can be skipped -/// and migrated manually. For now, operators are responsible for stopping -/// affected services before running `bootc install to-existing-root --preserve-var` -/// on ext4 (or other non-reflink) filesystems. +/// Exclusions can be configured with install configuration or repeated +/// `--preserve-var-skip` options. Paths are relative to `/var` and may be one +/// component (such as `lib`) or exactly one child (such as `lib/pgsql`). `/var` +/// itself and deeper paths are not accepted. Excluded state is not migrated by +/// this command and must be handled separately. Exclusions do not make a live database copy +/// consistent: any database state that is copied while it is being written can +/// still be corrupted. Stop stateful services before copying their data, or use +/// a consistent snapshot. /// /// See: #[context("Copying /var into new deployment (Strategy D)")] -fn preserve_var_copy(src_var: &Path, new_var: &Path, exclusions: &[String]) -> Result<()> { - let entries = - std::fs::read_dir(src_var).with_context(|| format!("Reading {}", src_var.display()))?; +fn preserve_var_copy( + src_var: &CapStdDir, + new_var: &CapStdDir, + exclusions: &[String], +) -> Result<()> { + let entries = src_var.read_dir(".").context("Reading source /var")?; for entry in entries { - let entry = entry.with_context(|| format!("Reading entry in {}", src_var.display()))?; + let entry = entry.context("Reading entry in source /var")?; let name = entry.file_name(); let name_str = name.to_string_lossy(); - if exclusions.iter().any(|s| s == name_str.as_ref()) { - println!( - " Skipping {} (ephemeral)", - src_var.join(name_str.as_ref()).display() - ); + if exclusions.iter().any(|s| name.to_str() == Some(s.as_str())) { + println!(" Skipping {} (ephemeral)", name_str); continue; } - let src_entry = src_var.join(name_str.as_ref()); - let dst_entry = new_var.join(name_str.as_ref()); - - if let Some(skip) = exclusions.iter().find_map(|path| { - path.strip_prefix(&format!("{name_str}/")) - .filter(|rest| !rest.contains('/')) - }) { - copy_dir_skip_subdir(&src_entry, &dst_entry, skip, false)?; + let skips = nested_exclusions(&name, exclusions); + if !skips.is_empty() && src_var.symlink_metadata(&name)?.is_dir() { + copy_dir_skip_subdir(src_var, new_var, &name, &skips, false)?; continue; } - println!( - " Copying {} → {}", - src_entry.display(), - dst_entry.display() - ); - let status = std::process::Command::new("cp") - .args(["-a", "--no-clobber"]) - .arg(&src_entry) - .arg(new_var) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .status() - .with_context(|| format!("cp -a {}", src_entry.display()))?; - - anyhow::ensure!(status.success(), "cp -a failed for {}", src_entry.display()); + println!(" Copying {} → {}", name_str, name_str); + let status = run_cp( + src_var, + &[&name], + new_var, + None, + &["-a", "--no-clobber"], + Stdio::inherit(), + Stdio::inherit(), + )?; + + anyhow::ensure!(status.success(), "cp -a failed for {}", name_str); } println!(" /var copy complete."); Ok(()) } +fn nested_exclusions<'a>(name: &OsStr, exclusions: &'a [String]) -> Vec<&'a str> { + let Some(name) = name.to_str() else { + return Vec::new(); + }; + let prefix = format!("{name}/"); + exclusions + .iter() + .filter_map(|path| { + path.strip_prefix(&prefix) + .filter(|rest| !rest.contains('/')) + }) + .collect() +} + +/// Open a single source child directory without following a symlink swapped +/// into place after the caller's metadata check. The entry name comes directly +/// from `read_dir`, so this is one path component beneath the capability root. +fn open_child_dir_nofollow(parent: &CapStdDir, name: &OsStr) -> std::io::Result { + use rustix::fs::{Mode, OFlags}; + + let fd = rustix::fs::openat( + parent.as_filelike_view::().as_fd(), + name, + OFlags::CLOEXEC | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::RDONLY, + Mode::empty(), + )?; + Ok(CapStdDir::from_std_file(std::fs::File::from(fd))) +} + +fn open_or_create_child_dir(parent: &CapStdDir, name: &OsStr) -> Result { + match parent.create_dir(name) { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { + anyhow::ensure!( + parent.symlink_metadata(name)?.is_dir(), + "Destination child {} exists and is not a directory", + name.to_string_lossy() + ); + } + Err(e) => return Err(e).context("Creating destination child directory"), + } + parent + .open_dir(name) + .context("Opening destination child directory") +} + /// Validate exclusions before touching either tree. Paths are relative to `/var`. pub(crate) fn validate_exclusions(exclusions: &[String]) -> Result> { exclusions @@ -372,6 +504,16 @@ fn merge_etc(host_etc: &Path, deploy_usr_etc: &Path, deploy_etc: &Path) -> Resul #[cfg(test)] mod tests { use super::*; + use cap_std_ext::dirext::CapStdExtDirExt; + use std::{ + fs, + os::unix::fs::{MetadataExt, PermissionsExt, symlink}, + time::{Duration, SystemTime}, + }; + + fn cap_dir(path: &Path) -> CapStdDir { + CapStdDir::open_ambient_dir(path, ambient_authority()).unwrap() + } #[test] fn exclusions_are_relative_and_normalized() { @@ -392,4 +534,160 @@ mod tests { Path::new("/sysroot/ostree/deploy/default/var") ); } + + #[test] + fn plain_copy_skips_multiple_children_under_parent() -> Result<()> { + let exclusions = ["tmp".into(), "log/journal".into(), "log/private".into()]; + assert_eq!( + nested_exclusions(OsStr::new("log"), &exclusions), + ["journal", "private"] + ); + + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir_all(src_path.join("tmp"))?; + fs::create_dir_all(src_path.join("log/journal"))?; + fs::create_dir_all(src_path.join("log/private"))?; + fs::create_dir_all(src_path.join("log/keep"))?; + fs::write(src_path.join("tmp/ignored"), b"tmp")?; + fs::write(src_path.join("log/journal/ignored"), b"journal")?; + fs::write(src_path.join("log/private/ignored"), b"private")?; + fs::write(src_path.join("log/keep/preserved"), b"keep")?; + fs::hard_link( + src_path.join("log/keep/preserved"), + src_path.join("log/keep/preserved-hardlink"), + )?; + fs::create_dir(&dst_path)?; + + preserve_var_copy(&cap_dir(&src_path), &cap_dir(&dst_path), &exclusions)?; + + assert!(!dst_path.join("tmp").exists()); + assert!(!dst_path.join("log/journal").exists()); + assert!(!dst_path.join("log/private").exists()); + assert_eq!(fs::read(dst_path.join("log/keep/preserved"))?, b"keep"); + assert_eq!( + fs::metadata(dst_path.join("log/keep/preserved"))?.ino(), + fs::metadata(dst_path.join("log/keep/preserved-hardlink"))?.ino() + ); + Ok(()) + } + + #[test] + fn reflink_strategy_skips_multiple_children_under_parent() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir_all(src_path.join("log/journal"))?; + fs::create_dir_all(src_path.join("log/private"))?; + fs::create_dir_all(src_path.join("log/keep"))?; + fs::write(src_path.join("log/journal/ignored"), b"journal")?; + fs::write(src_path.join("log/private/ignored"), b"private")?; + fs::write(src_path.join("log/keep/preserved"), b"keep")?; + fs::create_dir(&dst_path)?; + let src = cap_dir(&src_path); + let dst = cap_dir(&dst_path); + let supports_reflink = supports_reflink(&src, &dst)?; + if !supports_reflink { + fs::remove_file(src_path.join("log/keep/preserved"))?; + fs::remove_dir(src_path.join("log/keep"))?; + } + + // Exercise Strategy C's exclusion routing on every filesystem and, if + // available, its reflink copy of a retained sibling. + preserve_var_reflink(&src, &dst, &["log/journal".into(), "log/private".into()])?; + + assert!(!dst_path.join("log/journal").exists()); + assert!(!dst_path.join("log/private").exists()); + assert_eq!( + dst_path.join("log/keep/preserved").exists(), + supports_reflink + ); + Ok(()) + } + + #[test] + fn copy_preserves_symlink_without_traversing_target() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + let outside = temp.path().join("outside"); + fs::create_dir(&src_path)?; + fs::create_dir(&dst_path)?; + fs::create_dir(&outside)?; + fs::write(outside.join("sentinel"), b"outside")?; + symlink(&outside, src_path.join("escape"))?; + + preserve_var_copy( + &cap_dir(&src_path), + &cap_dir(&dst_path), + &["escape/sentinel".into()], + )?; + + let copied = fs::symlink_metadata(dst_path.join("escape"))?; + assert!(copied.file_type().is_symlink()); + assert_eq!(fs::read_link(dst_path.join("escape"))?, outside); + // The target remains outside the destination tree; it was not copied. + assert!(!dst_path.join("sentinel").exists()); + Ok(()) + } + + #[test] + fn copy_preserves_archive_metadata_and_hardlinks() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + let src_state = src_path.join("state"); + fs::create_dir_all(&src_state)?; + fs::create_dir(&dst_path)?; + fs::write(src_state.join("one"), b"contents")?; + fs::hard_link(src_state.join("one"), src_state.join("two"))?; + fs::set_permissions(&src_state, fs::Permissions::from_mode(0o751))?; + fs::set_permissions(src_state.join("one"), fs::Permissions::from_mode(0o640))?; + let src_cap = cap_dir(&src_path); + let dst_cap = cap_dir(&dst_path); + let xattr_supported = src_cap + .setxattr("state/one", "user.bootc_migrate_test", b"xattr") + .is_ok(); + let expected_mtime = SystemTime::UNIX_EPOCH + Duration::from_secs(1_700_000_000); + fs::File::open(&src_state)?.set_times(fs::FileTimes::new().set_modified(expected_mtime))?; + + preserve_var_copy(&src_cap, &dst_cap, &[])?; + + let copied_dir = fs::metadata(dst_path.join("state"))?; + let copied_one = fs::metadata(dst_path.join("state/one"))?; + let copied_two = fs::metadata(dst_path.join("state/two"))?; + assert_eq!(copied_dir.permissions().mode() & 0o7777, 0o751); + assert_eq!(copied_one.permissions().mode() & 0o7777, 0o640); + assert_eq!(copied_dir.modified()?, expected_mtime); + assert_eq!(copied_one.ino(), copied_two.ino()); + assert_eq!(copied_one.nlink(), 2); + assert_eq!(copied_one.uid(), fs::metadata(src_state.join("one"))?.uid()); + assert_eq!(copied_one.gid(), fs::metadata(src_state.join("one"))?.gid()); + if xattr_supported { + assert_eq!( + dst_cap.getxattr("state/one", "user.bootc_migrate_test")?, + Some(b"xattr".to_vec()) + ); + } + Ok(()) + } + + #[test] + fn copy_keeps_existing_collisions_untouched() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir_all(src_path.join("state"))?; + fs::create_dir_all(dst_path.join("state"))?; + fs::write(src_path.join("state/existing"), b"source")?; + fs::write(src_path.join("state/new"), b"new")?; + fs::write(dst_path.join("state/existing"), b"destination")?; + + preserve_var_copy(&cap_dir(&src_path), &cap_dir(&dst_path), &[])?; + + assert_eq!(fs::read(dst_path.join("state/existing"))?, b"destination"); + assert_eq!(fs::read(dst_path.join("state/new"))?, b"new"); + Ok(()) + } } diff --git a/crates/lib/src/store/mod.rs b/crates/lib/src/store/mod.rs index e517bc9637..c2d945a1a3 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -32,7 +32,7 @@ //! The composefs → ostree step (arrow 2) was proven by the `composefs-to-ostree` //! spike branch. The planned implementation for the ostree backend will: //! -//! 1. Perform a lazy cached probe (`reflinks_supported`) at install time. +//! 1. Probe the relevant source/destination filesystem pair as needed. //! 2. Pull into containers-storage first (Stage 1). //! 3. Use `composefs_oci::pull` with `LocalFetchOpt::ZeroCopy` to populate composefs (Stage 2). //! 4. Finally, synthesize the ostree commit by walking the composefs tree, @@ -66,12 +66,10 @@ //! //! ## Reflink probe //! -//! The reflink probe is performed lazily and cached. It creates -//! two anonymous temporary files (via `O_TMPFILE`, no -//! cleanup needed), writes one byte to the source, and attempts -//! `ioctl(FICLONE)`. Returns `true` on success, `false` on `EOPNOTSUPP` or -//! `EXDEV`. The probe directory is `composefs/objects` if it already exists, -//! otherwise the physical root itself. +//! [`reflink::supports_reflink`] probes a specific source/destination directory +//! pair using temporary files and descriptor-level `ioctl(FICLONE)`. The +//! result is not cached: reflink support can differ across filesystems, so the +//! source/destination pair is the unit of the probe. //! //! # OSTree //! @@ -126,6 +124,8 @@ use crate::podstorage::CStorage; use crate::spec::{BootloaderKind, ImageStatus}; use crate::utils::{EfiError, deployment_fd, open_dir_remount_rw, read_uefi_var}; +pub(crate) mod reflink; + /// See pub type ComposefsRepository = composefs::repository::Repository; diff --git a/crates/lib/src/store/reflink.rs b/crates/lib/src/store/reflink.rs new file mode 100644 index 0000000000..148a40153b --- /dev/null +++ b/crates/lib/src/store/reflink.rs @@ -0,0 +1,279 @@ +//! Pairwise reflink capability probing for storage directories. + +use std::io::Write as _; + +use anyhow::{Context, Result}; +use cap_std_ext::cap_std::fs::{Dir as CapStdDir, File, OpenOptions}; +use cap_std_ext::cap_std::io_lifetimes::AsFilelike; +use rustix::fd::AsFd as _; + +/// Returns whether a file in `src` can be reflinked into `dst`. +/// +/// The result is scoped to this source/destination pair; it is deliberately +/// not cached. Temporary files are created exclusively beneath the supplied +/// directory capabilities and removed before returning. +pub(crate) fn supports_reflink(src: &CapStdDir, dst: &CapStdDir) -> Result { + probe_with(src, dst, |destination, source| { + let source_view = source.as_filelike_view::(); + let destination_view = destination.as_filelike_view::(); + let source_fd = source_view.as_fd(); + let destination_fd = destination_view.as_fd(); + rustix::fs::ioctl_ficlone(destination_fd, source_fd) + }) +} + +fn probe_with( + src: &CapStdDir, + dst: &CapStdDir, + clone: impl FnOnce(&File, &File) -> rustix::io::Result<()>, +) -> Result { + let mut source = match create_probe_file(src) { + Ok(source) => source, + Err(error) => return source_probe_failure(error), + }; + let mut destination = match create_probe_file(dst) { + Ok(destination) => destination, + Err(error) => { + source + .cleanup() + .context("Removing reflink probe source after destination creation failed")?; + return Err(error); + } + }; + + let probe_result = match source + .file + .as_mut() + .expect("probe source file is open") + .write_all(b"bootc reflink probe") + { + Err(error) => { + source_probe_failure(anyhow::Error::from(error).context("Writing reflink probe source")) + } + Ok(()) => match clone( + destination + .file + .as_ref() + .expect("probe destination file is open"), + source.file.as_ref().expect("probe source file is open"), + ) { + Ok(()) => Ok(true), + Err(error) if reflink_unsupported(error) => Ok(false), + Err(error) => Err(anyhow::Error::from(error).context("Probing FICLONE support")), + }, + }; + + // Attempt both removals even if one fails. Drop provides a best-effort + // fallback if the operation above returned early (for example, a write + // error). + let source_cleanup = source.cleanup(); + let destination_cleanup = destination.cleanup(); + source_cleanup.context("Removing reflink probe source")?; + destination_cleanup.context("Removing reflink probe destination")?; + probe_result +} + +fn reflink_unsupported(error: rustix::io::Errno) -> bool { + // Both files are newly created regular files, so EINVAL here indicates + // that FICLONE is unsupported rather than an invalid source type. + matches!( + error, + rustix::io::Errno::OPNOTSUPP | rustix::io::Errno::XDEV | rustix::io::Errno::INVAL + ) +} + +fn source_probe_access_error(error: &anyhow::Error) -> bool { + error.chain().any(|cause| { + cause.downcast_ref::().is_some_and(|error| { + matches!( + error.kind(), + std::io::ErrorKind::PermissionDenied | std::io::ErrorKind::ReadOnlyFilesystem + ) + }) + }) +} + +fn source_probe_failure(error: anyhow::Error) -> Result { + if source_probe_access_error(&error) { + tracing::debug!("Cannot prepare reflink probe source, using full copy: {error:#}"); + Ok(false) + } else { + Err(error) + } +} + +struct ProbeFile<'a> { + dir: &'a CapStdDir, + name: String, + file: Option, + removed: bool, +} + +impl ProbeFile<'_> { + fn cleanup(&mut self) -> std::io::Result<()> { + self.cleanup_with(|dir, name| dir.remove_file(name)) + } + + fn cleanup_with( + &mut self, + remove_file: impl FnOnce(&CapStdDir, &str) -> std::io::Result<()>, + ) -> std::io::Result<()> { + drop(self.file.take()); + match remove_file(self.dir, &self.name) { + Ok(()) => { + self.removed = true; + Ok(()) + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + self.removed = true; + Ok(()) + } + Err(error) => Err(error), + } + } +} + +impl Drop for ProbeFile<'_> { + fn drop(&mut self) { + if !self.removed { + let _ = self.cleanup(); + } + } +} + +fn create_probe_file(dir: &CapStdDir) -> Result> { + loop { + let name = format!(".bootc-reflink-probe-{}", uuid::Uuid::new_v4()); + match create_probe_file_with_name(dir, &name) { + Ok(file) => return Ok(file), + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(error) => return Err(error).context("Creating reflink probe file"), + } + } +} + +fn create_probe_file_with_name<'a>( + dir: &'a CapStdDir, + name: &str, +) -> std::io::Result> { + let file = dir.open_with( + name, + OpenOptions::new().read(true).write(true).create_new(true), + )?; + Ok(ProbeFile { + dir, + name: name.to_owned(), + file: Some(file), + removed: false, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use cap_std_ext::cap_std::ambient_authority; + use std::fs; + + fn cap_dir(path: &std::path::Path) -> CapStdDir { + CapStdDir::open_ambient_dir(path, ambient_authority()).unwrap() + } + + #[test] + fn probe_reports_success_and_cleans_up() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir(&src_path)?; + fs::create_dir(&dst_path)?; + + let supported = probe_with(&cap_dir(&src_path), &cap_dir(&dst_path), |_, _| Ok(()))?; + assert!(supported); + assert!(fs::read_dir(&src_path)?.next().is_none()); + assert!(fs::read_dir(&dst_path)?.next().is_none()); + Ok(()) + } + + #[test] + fn probe_classifies_unsupported_and_propagates_unexpected_errors() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir(&src_path)?; + fs::create_dir(&dst_path)?; + let src = cap_dir(&src_path); + let dst = cap_dir(&dst_path); + + assert!(!probe_with(&src, &dst, |_, _| { + Err(rustix::io::Errno::OPNOTSUPP) + })?); + assert!(!probe_with(&src, &dst, |_, _| { + Err(rustix::io::Errno::INVAL) + })?); + assert!(fs::read_dir(&src_path)?.next().is_none()); + assert!(fs::read_dir(&dst_path)?.next().is_none()); + + assert!(probe_with(&src, &dst, |_, _| Err(rustix::io::Errno::IO)).is_err()); + assert!(fs::read_dir(&src_path)?.next().is_none()); + assert!(fs::read_dir(&dst_path)?.next().is_none()); + Ok(()) + } + + #[test] + fn classifies_only_known_unsupported_errors() { + assert!(reflink_unsupported(rustix::io::Errno::OPNOTSUPP)); + assert!(reflink_unsupported(rustix::io::Errno::XDEV)); + assert!(reflink_unsupported(rustix::io::Errno::INVAL)); + assert!(!reflink_unsupported(rustix::io::Errno::IO)); + } + + #[test] + fn source_permission_and_read_only_errors_select_full_copy() { + for error in [ + std::io::Error::from(std::io::ErrorKind::PermissionDenied), + std::io::Error::from(std::io::ErrorKind::ReadOnlyFilesystem), + ] { + let error = anyhow::Error::from(error).context("Creating reflink probe file"); + assert!(source_probe_access_error(&error)); + assert!(!source_probe_failure(error).unwrap()); + } + assert!(source_probe_failure(anyhow::anyhow!("unrelated error")).is_err()); + } + + #[test] + fn cleanup_retries_unlink_on_drop_after_failure() -> Result<()> { + let temp = tempfile::tempdir()?; + let path = temp.path().join("src"); + fs::create_dir(&path)?; + let dir = cap_dir(&path); + let name = ".bootc-reflink-probe-retry"; + let mut probe = create_probe_file_with_name(&dir, name)?; + + let error = probe + .cleanup_with(|_, _| Err(std::io::Error::from(std::io::ErrorKind::PermissionDenied))) + .unwrap_err(); + assert_eq!(error.kind(), std::io::ErrorKind::PermissionDenied); + assert!(path.join(name).exists()); + + drop(probe); + assert!(!path.join(name).exists()); + Ok(()) + } + + #[test] + fn probe_file_creation_never_overwrites_existing_files() -> Result<()> { + let temp = tempfile::tempdir()?; + let path = temp.path().join("src"); + fs::create_dir(&path)?; + let existing = path.join(".bootc-reflink-probe-collision"); + fs::write(&existing, b"leave this alone")?; + + let error = + match create_probe_file_with_name(&cap_dir(&path), ".bootc-reflink-probe-collision") { + Ok(_) => panic!("create_new unexpectedly replaced an existing probe file"), + Err(error) => error, + }; + assert_eq!(error.kind(), std::io::ErrorKind::AlreadyExists); + assert_eq!(fs::read(existing)?, b"leave this alone"); + Ok(()) + } +} diff --git a/docs/src/bootc-installation.7.md b/docs/src/bootc-installation.7.md index de243fdc95..6a2cad0502 100644 --- a/docs/src/bootc-installation.7.md +++ b/docs/src/bootc-installation.7.md @@ -297,10 +297,14 @@ previous installation. By default, the old `/var` and `/etc` are not migrated into the new deployment. With the ostree backend, `--preserve-var` opts into copying the old `/var` into the new stateroot before reboot; this uses reflinks when supported and otherwise makes a full -copy, which can be unsafe while databases or other stateful services are running. +copy of non-excluded data. Reflinks clone individual files, but the directory-tree copy is not an atomic +database snapshot; concurrent changes can still result in inconsistent data. Stop +databases and other stateful services before copying. `/var/tmp`, `/var/cache`, `/var/log/journal`, and `/var/lib/containers` are always excluded. Repeat `--preserve-var-skip PATH` to add further exclusions using paths -relative to `/var` (a path or one direct child path, such as `lib/rpm`). The +relative to `/var`: each exclusion must be either one component (such as `lib`) or +exactly one child (such as `lib/pgsql`). `/var` itself and deeper paths are not +accepted. Excluded data is not migrated and must be handled separately. The ostree-only `--merge-etc` option instead performs a three-way merge of the host's `/etc` customizations into the new deployment before the first reboot. See the [to-existing-root man page](man/bootc-install-to-existing-root.8.md) for details. diff --git a/docs/src/man/bootc-install-to-existing-root.8.md b/docs/src/man/bootc-install-to-existing-root.8.md index 508395ea98..5290bd6a9e 100644 --- a/docs/src/man/bootc-install-to-existing-root.8.md +++ b/docs/src/man/bootc-install-to-existing-root.8.md @@ -30,13 +30,17 @@ provide these migrations on the **ostree backend only**. They run after the installation completes and before the first reboot. `--preserve-var` copies the old `/var` into the new stateroot. Where supported, -the copy uses filesystem reflinks; otherwise it performs a full copy. A full copy -can be unsafe while databases or other stateful services are running, so stop -those services before installation when a full copy will be used. The paths +the copy uses filesystem reflinks; otherwise it performs a regular copy of the +non-excluded contents. A regular copy +can be unsafe while databases or other stateful services are running. Reflinks +clone individual files, but the directory-tree copy is not an atomic database +snapshot; concurrent changes can still result in inconsistent data. Stop those +services before copying. The paths `tmp`, `cache`, `log/journal`, and `lib/containers` are always excluded. Use the repeatable `--preserve-var-skip` option to add exclusions relative to `/var`; -each path may be one component or a direct child path (for example, `tmp` or -`lib/rpm`). +each path must be either one component (for example, `lib`) or exactly one child +(for example, `lib/pgsql`). `/var` itself and deeper paths such as `lib/pgsql/data` +are not accepted. Excluded data is not migrated and must be handled separately. `--merge-etc` performs a three-way merge that applies the running host's `/etc` customizations to the new deployment before its first reboot. Without this From 074d241b5e3085c026800eb58aaeaa918c52f424 Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Mon, 12 Oct 2026 17:11:01 -0400 Subject: [PATCH 3/5] tmt: Add single-guest package-mode migration test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidate package-mode TMT coverage and its framework changes into one commit while preserving the branch’s existing final tree. Assisted-by: AI Signed-off-by: ckyrouac --- .packit.yaml | 44 +++++ Justfile | 35 +++- hack/provision-packit.sh | 8 +- tmt/plans/package-mode.fmf | 155 ++++++++++++++++++ ...to-existing-root-preserve-var-merge-etc.sh | 66 ++++++++ ...o-existing-root-preserve-var-merge-etc.fmf | 10 ++ 6 files changed, 315 insertions(+), 3 deletions(-) create mode 100644 tmt/plans/package-mode.fmf create mode 100644 tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh create mode 100644 tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf diff --git a/.packit.yaml b/.packit.yaml index e2f7430571..d00a5c05c6 100644 --- a/.packit.yaml +++ b/.packit.yaml @@ -102,6 +102,28 @@ jobs: context: running_env: "packit" + # This plan runs the package-mode migration on one guest. Testing Farm's + # default pipeline provisions a single Artemis guest for this plan. + - job: tests + trigger: pull_request + identifier: package-mode-tier-1 + require: + label: + present: + - ci/tier-1 + targets: + - centos-stream-10-x86_64 + - centos-stream-10-aarch64 + - fedora-latest-stable-x86_64 + - fedora-latest-stable-aarch64 + tmt_plan: /tmt/plans/package-mode + tf_extra_params: + environments: + - tmt: + context: + running_env: "packit" + package_source: "artifacts" + # ci/merge: full target set - job: tests trigger: pull_request @@ -124,6 +146,28 @@ jobs: context: running_env: "packit" + - job: tests + trigger: pull_request + identifier: package-mode-full + require: + label: + present: + - ci/merge + targets: + - centos-stream-9-x86_64 + - centos-stream-9-aarch64 + - centos-stream-10-x86_64 + - centos-stream-10-aarch64 + - fedora-all-x86_64 + - fedora-all-aarch64 + tmt_plan: /tmt/plans/package-mode + tf_extra_params: + environments: + - tmt: + context: + running_env: "packit" + package_source: "artifacts" + - job: propose_downstream trigger: release dist_git_branches: diff --git a/Justfile b/Justfile index afcb3a1984..8f77f3cd4e 100644 --- a/Justfile +++ b/Justfile @@ -48,7 +48,8 @@ erofs_version := env("BOOTC_erofs_version", "v1") # Baseconfigs to inject into the image for testing (e.g. "etc-transient" or "root-transient") baseconfigs := env("BOOTC_baseconfigs", "") # Base container image to build from -base := env("BOOTC_base", "quay.io/centos-bootc/centos-bootc:stream10") +default_base := "quay.io/centos-bootc/centos-bootc:stream10" +base := env("BOOTC_base", default_base) # Buildroot base image buildroot_base := env("BOOTC_buildroot_base", "quay.io/centos/centos:stream10") # Optional: path to extra source (e.g. composefs-rs) for local development @@ -192,6 +193,38 @@ test-tmt *ARGS: build @just _build-upgrade-image @just test-tmt-nobuild {{ARGS}} +# The VM cannot share controller Podman storage, so the package-mode plan +# transfers a local OCI archive through TMT's workdir sync. +# Run the plan using one TMT-provisioned guest. +[group('core')] +test-tmt-package-mode: + #!/bin/bash + set -euo pipefail + # Build and tag the local image, then archive it for TMT's guest workdir sync. + just base={{base}} build + if [[ "{{base_img}}" != localhost/bootc ]]; then + podman tag {{base_img}} localhost/bootc + fi + archive_dir=$(mktemp -d -- tmt/plans/.package-mode-local.XXXXXX) + cleanup() { + rm -rf -- "$archive_dir" + } + trap cleanup EXIT + trap 'cleanup; exit 130' INT + trap 'cleanup; exit 143' TERM + archive_path="$archive_dir/bootc.oci" + if [[ -e "$archive_path" ]]; then + echo "Refusing to overwrite existing archive: $archive_path" >&2 + exit 1 + fi + podman save --format oci-archive --output "$archive_path" localhost/bootc + tmt \ + --context=running_env=packit \ + --context=package_source=local \ + --context="test_disk_image=${test_disk_image:-centos-stream-10}" \ + run --environment "PACKAGE_MODE_IMAGE_ARCHIVE=$archive_path" \ + plans --name '^/tmt/plans/package-mode$' + # Split out from `test-container` because, unlike the container integration tests, # unit tests don't depend on variant/filesystem/bootloader/boot_type/seal_state, so # CI runs this once per OS (in the `package` job) instead of once per test-integration diff --git a/hack/provision-packit.sh b/hack/provision-packit.sh index 9ec9a144ae..cbe176e83b 100755 --- a/hack/provision-packit.sh +++ b/hack/provision-packit.sh @@ -15,6 +15,7 @@ trap 'rm -rf -- "$BOOTC_TEMPDIR"' EXIT cp -a . "$BOOTC_TEMPDIR" # Keep testing farm run folder +mkdir -p /var/ARTIFACTS cp -r /var/ARTIFACTS "$BOOTC_TEMPDIR" # Copy bootc repo @@ -91,6 +92,9 @@ podman build --jobs=4 --from "$BASE" -v "$BOOTC_TEMPDIR":/bootc-test:z -t localh # Keep these in sync with what's used in hack/lbi podman pull -q --retry 5 --retry-delay 5s quay.io/curl/curl:latest quay.io/curl/curl-base:latest registry.access.redhat.com/ubi9/podman:latest -# Run system-reinstall-bootc # TODO make it more scriptable instead of expect + send -./system-reinstall-bootc.exp +if [[ ${BOOTC_PROVISION_PACKIT_SKIP_REINSTALL:-0} == 1 ]]; then + echo "BOOTC_PROVISION_PACKIT_SKIP_REINSTALL=1: skipping system-reinstall-bootc conversion and reboot" +else + ./system-reinstall-bootc.exp +fi diff --git a/tmt/plans/package-mode.fmf b/tmt/plans/package-mode.fmf new file mode 100644 index 0000000000..8bf30936a2 --- /dev/null +++ b/tmt/plans/package-mode.fmf @@ -0,0 +1,155 @@ +summary: Test package-mode to-existing-root migration flags +environment: + PACKAGE_SOURCE: $@{package_source} +provision: + how: virtual + image: $@{test_disk_image} + user: root +prepare: + # The VM cannot share controller Podman storage; TMT syncs the OCI archive + # through its workdir before prepare runs. + - how: install + order: 93 + package: + - podman + - jq + - bootc + when: package_source == local + # Packit build-only preparation needs the full integration toolchain. + - how: install + order: 93 + package: + - podman + - skopeo + - jq + - bootc + - system-reinstall-bootc + - zstd + - cpio + - rpm-build + - expect + - ansible-core + - parted + - lvm2 + - dosfstools + - e2fsprogs + when: package_source == artifacts + # Reject missing/unknown sources before any mode-specific setup runs. + - how: shell + order: 95 + script: | + case "${PACKAGE_SOURCE:-}" in + artifacts|local) ;; + *) + printf 'package_source must be artifacts or local (got %q)\n' "${PACKAGE_SOURCE:-}" >&2 + exit 1 + ;; + esac + # Load the TMT-synced archive into rootful guest Podman, then clean its copy. + - how: shell + order: 96 + script: | + set -eu + archive_relative=${PACKAGE_MODE_IMAGE_ARCHIVE:-} + case "$archive_relative" in + tmt/plans/.package-mode-local.*/bootc.oci) ;; + *) + printf 'Unexpected PACKAGE_MODE_IMAGE_ARCHIVE path: %s\n' "$archive_relative" >&2 + exit 1 + ;; + esac + archive_directory_relative=${archive_relative%/*} + temporary_name=${archive_directory_relative##*/} + case "$temporary_name" in + .package-mode-local.*) ;; + *) + printf 'Unexpected package-mode archive directory: %s\n' "$archive_directory_relative" >&2 + exit 1 + ;; + esac + if [ "$archive_directory_relative" != "tmt/plans/$temporary_name" ]; then + printf 'Package-mode archive must be directly under tmt/plans: %s\n' "$archive_relative" >&2 + exit 1 + fi + + tree_root=$(realpath -e -- "${TMT_TREE:?TMT_TREE is required}") + if [ ! -d "$tree_root" ]; then + printf 'TMT_TREE is not a directory: %s\n' "$tree_root" >&2 + exit 1 + fi + plans_root=$(realpath -e -- "$tree_root/tmt/plans") + plans_root_prefix=${tree_root%/}/ + if [ "$plans_root" = "$tree_root" ]; then + printf 'TMT plans directory resolves to the worktree root: %s\n' "$plans_root" >&2 + exit 1 + fi + case "$plans_root" in + "$plans_root_prefix"*) ;; + *) + printf 'TMT plans directory resolves outside the worktree: %s\n' "$plans_root" >&2 + exit 1 + ;; + esac + archive_directory=$(realpath -m -- "$tree_root/$archive_directory_relative") + if [ "$archive_directory" != "$plans_root/$temporary_name" ]; then + printf 'Package-mode archive directory resolves outside tmt/plans: %s\n' "$archive_relative" >&2 + exit 1 + fi + cleanup() { + rm -rf -- "$archive_directory" + } + trap cleanup EXIT + trap 'exit 130' INT + trap 'exit 143' TERM + + archive_path=$(realpath -e -- "$archive_directory/bootc.oci") + if [ "$archive_path" != "$archive_directory/bootc.oci" ]; then + printf 'Package-mode archive must not resolve outside its temporary directory: %s\n' "$archive_relative" >&2 + exit 1 + fi + if [ ! -f "$archive_path" ]; then + printf 'Package-mode archive is not a regular file: %s\n' "$archive_relative" >&2 + exit 1 + fi + podman --remote=false load --input "$archive_path" + podman --remote=false image exists localhost/bootc + when: package_source == local + - how: shell + order: 98 + script: + - mkdir -p bootc && cp /var/share/test-artifacts/*.src.rpm bootc + # Packit artifacts are already staged in the guest before extraction. + - cd bootc && rpm2cpio *.src.rpm > pkg.cpio && cpio -idmv < pkg.cpio && rm pkg.cpio && rm -f *-vendor.tar.zstd && zstd -d *.tar.zstd && tar -xvf *.tar -C . --strip-components=1 && ls -al + - cd bootc/hack && BOOTC_PROVISION_PACKIT_SKIP_REINSTALL=1 ./provision-packit.sh + when: package_source == artifacts + # Both sources now provide localhost/bootc in the guest's rootful storage. + # Pull the LBI references and build the marker image in that same storage. + - how: shell + order: 99 + script: | + podman --remote=false image exists localhost/bootc + podman --remote=false pull quay.io/curl/curl:latest + podman --remote=false pull quay.io/curl/curl-base:latest + podman --remote=false pull registry.access.redhat.com/ubi9/podman:latest + workdir=$(mktemp -d) + trap 'rm -rf -- "$workdir"' EXIT + cat > "$workdir/Containerfile" <<'EOF' + FROM localhost/bootc + # Fedora Cloud and fedora-bootc have different /etc/ssl/certs baselines. + RUN rm -rf /etc/ssl/certs && \ + mkdir -p /etc/bootc-tmt-merge && \ + printf '%s\n' image-default > /etc/bootc-tmt-merge/default.conf && \ + printf '%s\n' image-only > /etc/bootc-tmt-merge/image-only.conf + EOF + podman --remote=false build \ + -t localhost/bootc-to-existing-root-preserve-var-merge-etc:latest "$workdir" + podman --remote=false image exists \ + localhost/bootc-to-existing-root-preserve-var-merge-etc:latest + when: package_source == artifacts or package_source == local +execute: + how: tmt +discover: + how: fmf + prune: true + test: + - /tmt/tests/test-50-to-existing-root-preserve-var-merge-etc diff --git a/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh b/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh new file mode 100644 index 0000000000..1587b91cb3 --- /dev/null +++ b/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh @@ -0,0 +1,66 @@ +#!/bin/bash +# Both local and Packit runs prepare one package-mode guest, then convert and +# reboot that same guest; local loads the checkout image, Packit builds it. +set -xeuo pipefail + +target_image=localhost/bootc-to-existing-root-preserve-var-merge-etc:latest +var_marker=/var/lib/bootc-tmt-preserve-var/sentinel +etc_dir=/etc/bootc-tmt-merge + +case ${TMT_REBOOT_COUNT:-0} in + 0) + podman --remote=false image exists "$target_image" + + # The guest must still be the original package-mode system. + test ! -e /run/ostree-booted + + # Exercise both an admin-created file and an admin modification of an + # image default. The derived image below supplies the pristine values. + mkdir -p "$etc_dir" "${var_marker%/*}" + + # TMT connects as root. Package-mode hosts keep root's home at /root, + # while bootc images link /root to /var/roothome, so preserve its key + # under /var for TMT to reconnect after the migration reboot. + install -d -m 0700 /var/roothome/.ssh + install -m 0600 /root/.ssh/authorized_keys /var/roothome/.ssh/authorized_keys + restorecon -RF /var/roothome/.ssh + + printf '%s\n' admin-modified | tee "$etc_dir/default.conf" + printf '%s\n' admin-only | tee "$etc_dir/admin-only.conf" + printf '%s\n' preserved-var | tee "$var_marker" + + # The flags require the OSTree backend. Run the current build-under-test + # as a privileged installer, with the package-mode root mounted at /target. + podman --remote=false run --rm --privileged --pid=host --user=root:root \ + -v /dev:/dev \ + -v /:/target \ + -v /boot:/target/boot \ + -v /var/lib/containers:/var/lib/containers \ + --security-opt label=type:unconfined_t \ + "$target_image" \ + bootc install to-existing-root \ + --preserve-var \ + --merge-etc \ + --acknowledge-destructive + + bootc status + # Ask the guest to reboot itself; this is more reliable than + # testcloud's ACPI soft-poweroff for a full bootc deployment reboot. + tmt-reboot -c 'systemctl reboot' + ;; + 1) + test -e /run/ostree-booted + + booted_image=$(bootc status --json | jq -r '.status.booted.image.image.image') + test "$booted_image" = "$target_image" + + test "$(<"$var_marker")" = preserved-var + test "$(<"$etc_dir/default.conf")" = admin-modified + test "$(<"$etc_dir/admin-only.conf")" = admin-only + test "$(<"$etc_dir/image-only.conf")" = image-only + ;; + *) + printf 'Unexpected TMT_REBOOT_COUNT=%s\n' "$TMT_REBOOT_COUNT" >&2 + exit 1 + ;; +esac diff --git a/tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf b/tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf new file mode 100644 index 0000000000..e65b6073a1 --- /dev/null +++ b/tmt/tests/test-50-to-existing-root-preserve-var-merge-etc.fmf @@ -0,0 +1,10 @@ +# The VM cannot share controller Podman storage, so TMT syncs the local OCI +# archive to the single guest before migration. It reboots once, then verifies +# preserved /var data and merged /etc configuration. +summary: Package-mode /var preservation and /etc merge +duration: 45m +adjust: + - when: running_env == image_mode + enabled: false + because: this migration test requires a package-mode target +test: bash booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh From f00fef713307917cc476fd6328a4ce229ab0f86d Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Tue, 13 Oct 2026 14:35:14 -0400 Subject: [PATCH 4/5] install: Skip nested mounts during var migration Preserve mountpoint directories without copying mounted contents so --preserve-var handles nested filesystems such as rpc_pipefs. Copy into the shared stateroot var per entry to avoid leaving partial entries on failure. Keep the package-mode test's TMT run directory under preserved /var across the migration reboot so TMT can resume the test and collect results. Assisted-by: AI Signed-off-by: ckyrouac --- crates/lib/src/install.rs | 13 +- crates/lib/src/install/migrate.rs | 1665 +++++++++++++++-- docs/src/bootc-installation.7.md | 16 + .../man/bootc-install-to-existing-root.8.md | 35 +- ...to-existing-root-preserve-var-merge-etc.sh | 228 ++- 5 files changed, 1809 insertions(+), 148 deletions(-) diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 6889b8f3ec..b8cc0a21e6 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -629,13 +629,18 @@ pub(crate) struct InstallToExistingRootOpts { /// Passing this flag performs the following additional steps **after** the /// core install completes: /// - /// 1. `/var` content is copied into the new deployment: - /// - **Reflink copy** (btrfs / XFS): `cp --reflink=always` performs an - /// instantaneous copy-on-write clone — no extra disk space consumed. + /// 1. Non-excluded `/var` content is copied into the new deployment: + /// - **Reflink copy** (btrfs / XFS): reflinks avoid immediate data + /// duplication, but are created per file and are not a snapshot. /// - **Full copy** (filesystems without reflinks): data is copied into /// the deployment's writable `/var` directory. /// 2. Paths listed with `--preserve-var-skip` (or in install configuration) - /// are excluded. Rollback preservation is intentionally independent. + /// are excluded, including any matching nested mountpoint; exclusions + /// take precedence over creating mountpoint placeholders. Other nested + /// mountpoints get empty placeholders, not copied contents. `/etc` + /// configuration merged with `--merge-etc` can restore configured + /// mounts at boot; runtime-only mounts need separate configuration. + /// Rollback preservation is intentionally independent. /// /// The running system's `root_path` must be mounted (e.g. `-v /:/target`). #[clap(long)] diff --git a/crates/lib/src/install/migrate.rs b/crates/lib/src/install/migrate.rs index 8c40a0aae7..1a13192769 100644 --- a/crates/lib/src/install/migrate.rs +++ b/crates/lib/src/install/migrate.rs @@ -29,6 +29,15 @@ //! trees, and **unsafe for live databases** (see the known-limitation comment //! on `preserve_var_copy`). //! +//! Nested mountpoints are selected from one `/proc/self/mountinfo` snapshot +//! before the copy. This is a point-in-time, best-effort snapshot, not a +//! race-free guard: mounts added or removed beneath `/var` during copying can +//! race with the copy, just like concurrent file changes. +//! Each missing entry is copied to a unique sibling staging directory and +//! published atomically without replacing an existing destination. A failed +//! entry leaves no partial final entry and can be retried; this is not a global +//! transaction, so entries completely published before a later failure remain. +//! //! Exclusions such as `tmp`, `log/journal`, `lib/containers`, or `lib/rpm` //! can be supplied through configuration or repeated `--preserve-var-skip` //! options. @@ -57,16 +66,21 @@ use std::path::{Component, Path}; use std::{ + collections::BTreeSet, ffi::OsStr, os::fd::AsFd, + os::unix::ffi::OsStringExt, path::PathBuf, process::{Command, Stdio}, - sync::Arc, + sync::{ + Arc, + atomic::{AtomicU64, Ordering}, + }, }; use anyhow::{Context, Result}; use cap_std_ext::cap_std::ambient_authority; -use cap_std_ext::cap_std::fs::Dir as CapStdDir; +use cap_std_ext::cap_std::fs::{Dir as CapStdDir, Metadata, MetadataExt}; use cap_std_ext::cap_std::io_lifetimes::AsFilelike; use cap_std_ext::cmdext::{CapStdExtCommandExt, CmdFds}; use composefs_ctl::composefs::generic_tree::{FileSystem, Stat}; @@ -101,8 +115,15 @@ pub(crate) fn merge_etc_into_deployment(root_path: &Path, deploy_dir: &Path) -> /// `src_var` is the running system's `/var` (at `/var`). /// `new_var` is the new deployment's empty `var/` directory. pub(crate) fn preserve_var(src_var: &Path, new_var: &Path, exclusions: &[String]) -> Result<()> { - let src_var = CapStdDir::open_ambient_dir(src_var, ambient_authority()) - .with_context(|| format!("Opening source {}", src_var.display()))?; + // Resolve symlink aliases and path components before comparing against the + // absolute, kernel-resolved mountpoints in mountinfo. + let canonical_src_var = std::fs::canonicalize(src_var) + .with_context(|| format!("Canonicalizing source /var {}", src_var.display()))?; + let mountinfo_snapshot = read_mountinfo_snapshot()?; + let nested_mounts = + nested_mountpoints(&canonical_src_var, parse_mountinfo(&mountinfo_snapshot)?)?; + let src_var = CapStdDir::open_ambient_dir(&canonical_src_var, ambient_authority()) + .with_context(|| format!("Opening source {}", canonical_src_var.display()))?; let new_var = open_or_create_var_dir(new_var)?; let mut effective_exclusions = ["tmp", "cache", "log/journal", "lib/containers"] @@ -111,13 +132,189 @@ pub(crate) fn preserve_var(src_var: &Path, new_var: &Path, exclusions: &[String] .collect::>(); effective_exclusions.extend(exclusions.iter().cloned()); - if supports_reflink(&src_var, &new_var)? { + let copy_result = if supports_reflink(&src_var, &new_var)? { println!(" Filesystem supports reflinks — using copy-on-write clone (Strategy C)"); - preserve_var_reflink(&src_var, &new_var, &effective_exclusions) + preserve_var_reflink_mounts(&src_var, &new_var, &effective_exclusions, &nested_mounts) } else { println!(" Filesystem does not support reflinks — falling back to full copy (Strategy D)"); - preserve_var_copy(&src_var, &new_var, &effective_exclusions) + preserve_var_copy_mounts(&src_var, &new_var, &effective_exclusions, &nested_mounts) + }; + copy_result?; + + Ok(()) +} + +/// Read one complete installer mount namespace snapshot. The initial snapshot +/// is used to select nested mounts for this copy; changes during copying can +/// race with it, just like concurrent changes to files beneath `/var`. +fn read_mountinfo_snapshot() -> Result> { + std::fs::read("/proc/self/mountinfo") + .context("Reading installer /proc/self/mountinfo for /var migration") +} + +fn nested_mountpoints( + src_var: &Path, + mountpoints: impl IntoIterator, +) -> Result> { + anyhow::ensure!(src_var.is_absolute(), "Source /var path must be absolute"); + let canonical_src_var = std::fs::canonicalize(src_var) + .with_context(|| format!("Canonicalizing source /var {}", src_var.display()))?; + let mut nested = BTreeSet::new(); + for mountpoint in mountpoints { + let Ok(relative) = mountpoint.strip_prefix(&canonical_src_var) else { + continue; + }; + // The mount on /var itself is not a nested mount. Only child paths are + // filtered; Path::strip_prefix provides component-safe containment. + if relative.as_os_str().is_empty() { + continue; + } + anyhow::ensure!( + relative + .components() + .all(|component| matches!(component, Component::Normal(_))), + "Invalid nested mountpoint below {}: {}", + canonical_src_var.display(), + mountpoint.display() + ); + nested.insert(relative.to_path_buf()); + } + Ok(nested) +} + +/// Parse the byte-oriented Linux mountinfo format without requiring UTF-8. +/// Kernel pathname escapes are decoded before paths are constructed. +fn parse_mountinfo(data: &[u8]) -> Result> { + let mut mountpoints = Vec::new(); + let lines = data.split(|byte| *byte == b'\n'); + let line_count = lines.clone().count(); + for (index, line) in lines.enumerate() { + if line.is_empty() && index + 1 == line_count { + continue; // A final newline is conventional. + } + anyhow::ensure!(!line.is_empty(), "Empty mountinfo line {}", index + 1); + anyhow::ensure!( + !line.contains(&0), + "NUL byte in mountinfo line {}", + index + 1 + ); + let fields = line + .split(|byte| byte.is_ascii_whitespace()) + .filter(|field| !field.is_empty()) + .collect::>(); + let separator = fields + .iter() + .enumerate() + .skip(6) + .find_map(|(index, field)| (*field == b"-").then_some(index)) + .with_context(|| { + format!("Malformed mountinfo line {}: missing separator", index + 1) + })?; + anyhow::ensure!( + separator >= 6 && fields.len() == separator + 4, + "Malformed mountinfo line {}: invalid field count", + index + 1 + ); + anyhow::ensure!( + is_positive_decimal(fields[0]) + && is_decimal(fields[1]) + && is_device_number(fields[2]) + && !fields[5].is_empty() + && !fields[separator + 1].is_empty() + && !fields[separator + 2].is_empty() + && !fields[separator + 3].is_empty(), + "Malformed mountinfo line {}: invalid required field", + index + 1 + ); + // The root field is not used for mount selection. Decode it to ensure + // its escape syntax is valid, but don't impose pathname constraints: + // unlike the mountpoint, it may be relative or noncanonical. + decode_mountinfo_escapes(fields[3]) + .with_context(|| format!("Malformed root path on mountinfo line {}", index + 1))?; + let mountpoint = decode_mountinfo_path(fields[4]) + .with_context(|| format!("Malformed mountpoint on mountinfo line {}", index + 1))?; + decode_mountinfo_escapes(fields[separator + 2]) + .with_context(|| format!("Malformed source path on mountinfo line {}", index + 1))?; + mountpoints.push(mountpoint); + } + anyhow::ensure!(!mountpoints.is_empty(), "Mountinfo contained no records"); + Ok(mountpoints) +} + +fn is_decimal(value: &[u8]) -> bool { + !value.is_empty() + && value.iter().all(u8::is_ascii_digit) + && std::str::from_utf8(value) + .ok() + .and_then(|value| value.parse::().ok()) + .is_some() +} + +fn is_positive_decimal(value: &[u8]) -> bool { + is_decimal(value) + && std::str::from_utf8(value) + .ok() + .and_then(|value| value.parse::().ok()) + .is_some_and(|value| value > 0) +} + +fn is_device_number(value: &[u8]) -> bool { + let Some(separator) = value.iter().position(|byte| *byte == b':') else { + return false; + }; + is_decimal(&value[..separator]) && is_decimal(&value[separator + 1..]) +} + +fn decode_mountinfo_path(value: &[u8]) -> Result { + let decoded = decode_mountinfo_escapes(value)?; + anyhow::ensure!( + decoded.first() == Some(&b'/'), + "Mountinfo path is not absolute" + ); + let mut components = decoded[1..].split(|byte| *byte == b'/'); + anyhow::ensure!( + decoded.len() == 1 + || components.all(|component| { + !component.is_empty() && component != b"." && component != b".." + }), + "Mountinfo path is not canonical" + ); + Ok(PathBuf::from(std::ffi::OsString::from_vec(decoded))) +} + +fn decode_mountinfo_escapes(value: &[u8]) -> Result> { + let mut decoded = Vec::with_capacity(value.len()); + let mut index = 0; + while index < value.len() { + if value[index] != b'\\' { + decoded.push(value[index]); + index += 1; + continue; + } + anyhow::ensure!( + index + 3 < value.len() + && value[index + 1..index + 4] + .iter() + .all(|byte| (b'0'..=b'7').contains(byte)), + "Invalid octal escape in mountinfo path" + ); + // Mountinfo escapes are three octal digits. Decode wider than a byte + // so malformed values such as `\\777` cannot overflow or wrap. + let escaped = u16::from(value[index + 1] - b'0') * 64 + + u16::from(value[index + 2] - b'0') * 8 + + u16::from(value[index + 3] - b'0'); + anyhow::ensure!( + escaped <= u16::from(u8::MAX), + "Octal escape exceeds one byte" + ); + anyhow::ensure!( + matches!(escaped, 0o11 | 0o12 | 0o40 | 0o134), + "Unsupported octal escape in mountinfo path" + ); + decoded.push(escaped as u8); + index += 4; } + Ok(decoded) } /// Open the destination through its parent capability, creating just the @@ -141,8 +338,7 @@ fn open_or_create_var_dir(path: &Path) -> Result { } Err(e) => return Err(e).with_context(|| format!("Creating {}", path.display())), } - parent - .open_dir(name) + open_child_dir_nofollow(&parent, name) .with_context(|| format!("Opening destination {}", path.display())) } @@ -163,6 +359,7 @@ pub(crate) fn deployment_var_path(deploy_dir: &Path) -> Result Result<()> { - let entries = src_var.read_dir(".").context("Reading source /var")?; - - for entry in entries { - let entry = entry.context("Reading entry in source /var")?; - let name = entry.file_name(); - let name_str = name.to_string_lossy(); - - if exclusions.iter().any(|s| name.to_str() == Some(s.as_str())) { - println!(" Skipping {} (ephemeral)", name_str); - continue; - } + preserve_var_reflink_mounts(src_var, new_var, exclusions, &BTreeSet::new()) +} - let skips = nested_exclusions(&name, exclusions); - if !skips.is_empty() && src_var.symlink_metadata(&name)?.is_dir() { - copy_dir_skip_subdir(src_var, new_var, &name, &skips, true)?; - continue; - } +fn preserve_var_reflink_mounts( + src_var: &CapStdDir, + new_var: &CapStdDir, + exclusions: &[String], + mountpoints: &BTreeSet, +) -> Result<()> { + copy_var_filtered( + src_var, + new_var, + Path::new(""), + exclusions, + mountpoints, + true, + false, + OsStr::new("cp"), + ) + .context("Reflink-copying /var into new deployment (Strategy C)")?; + println!(" /var reflink copy complete."); + Ok(()) +} - println!(" Reflink-copying {} → {}", name_str, name_str); - let status = run_cp( - src_var, - &[&name], - new_var, - None, - &["--reflink=always", "-a", "--no-clobber"], - Stdio::inherit(), - Stdio::inherit(), - )?; +/// Copy a directory's ordinary children together with cp -a, recursing only +/// into directories whose descendants contain a mountpoint or exclusion. +/// Batching ordinary siblings in one cp invocation retains hardlinks between +/// those entries. Hardlinks crossing a mountpoint or exclusion split, or +/// crossing separate copies while merging into an existing destination +/// directory, may not remain linked. Untouched subtrees remain entirely under +/// cp -a's control. +fn copy_var_filtered( + src_dir: &CapStdDir, + dst_dir: &CapStdDir, + relative_dir: &Path, + exclusions: &[String], + mountpoints: &BTreeSet, + reflink: bool, + destination_created: bool, + cp_program: &OsStr, +) -> Result<()> { + // Existing directories are merged in place, so capture their own metadata + // before reading or copying children can alter their atime/mtime. The same + // wrapper restores source metadata on newly-created destination directories. + let source_metadata = if destination_created { + Some(snapshot_directory_metadata(src_dir).with_context(|| { + format!( + "Snapshotting source metadata for /var/{}", + relative_dir.display() + ) + })?) + } else { + None + }; + let destination_metadata = if destination_created { + None + } else { + Some(dst_dir.metadata(".").with_context(|| { + format!( + "Snapshotting existing destination timestamps for /var/{}", + relative_dir.display() + ) + })?) + }; - anyhow::ensure!( - status.success(), - "cp --reflink=always failed for {}", - name_str - ); + let copy_result = copy_var_filtered_inner( + src_dir, + dst_dir, + relative_dir, + exclusions, + mountpoints, + reflink, + cp_program, + ); + let restore_result = match (source_metadata.as_ref(), destination_metadata.as_ref()) { + (Some(metadata), _) => restore_directory_metadata(dst_dir, metadata), + (None, Some(metadata)) => restore_directory_timestamps(dst_dir, metadata), + (None, None) => Ok(()), } + .with_context(|| { + format!( + "Restoring directory metadata for /var/{}", + relative_dir.display() + ) + }); - println!(" /var reflink copy complete."); - Ok(()) + match (copy_result, restore_result) { + (Ok(()), Ok(())) => Ok(()), + (Ok(()), Err(restore_error)) => Err(restore_error), + (Err(copy_error), Ok(())) => Err(copy_error), + (Err(copy_error), Err(restore_error)) => Err(copy_error.context(format!( + "Additionally failed to restore /var/{} metadata: {restore_error:#}", + relative_dir.display() + ))), + } } -/// Copy a directory recursively, skipping selected child directories. -/// -/// Used by both Strategy C and Strategy D to copy `var/log/` while excluding -/// `var/log/journal/`. `reflink` selects whether `cp --reflink=always` or -/// plain `cp -a` is used. Included sibling entries are passed in one cp process -/// so hardlinks between them remain linked. As in the previous implementation, -/// this filtered path does not copy metadata from the containing directory -/// itself; ordinary (unfiltered) entries are copied wholly by cp -a. -fn copy_dir_skip_subdir( - src_root: &CapStdDir, - dst_root: &CapStdDir, - dir_name: &OsStr, - skip_names: &[&str], +fn copy_var_filtered_inner( + src_dir: &CapStdDir, + dst_dir: &CapStdDir, + relative_dir: &Path, + exclusions: &[String], + mountpoints: &BTreeSet, reflink: bool, + cp_program: &OsStr, ) -> Result<()> { - let src = open_child_dir_nofollow(src_root, dir_name) - .with_context(|| format!("Opening /var/{}", dir_name.to_string_lossy()))?; - let dst = open_or_create_child_dir(dst_root, dir_name)?; - let entries = src + let entries = src_dir .read_dir(".") - .context("Reading excluded-copy source directory")?; + .with_context(|| format!("Reading /var/{}", relative_dir.display()))?; let mut included_entries = Vec::new(); for entry in entries { - let entry = entry.context("Reading entry in source child directory")?; + let entry = entry.context("Reading entry in /var during migration")?; let name = entry.file_name(); - let name_str = name.to_string_lossy(); + let relative = relative_dir.join(&name); - if skip_names.iter().any(|skip| name == OsStr::new(skip)) { + if exclusions + .iter() + .any(|excluded| Path::new(excluded) == relative) + { + println!(" Skipping {} (excluded)", relative.display()); + continue; + } + + if mountpoints.contains(&relative) { println!( - " Skipping {}/{} (ephemeral)", - dir_name.to_string_lossy(), - name_str + " Creating empty mountpoint placeholder for {}", + relative.display() ); + create_staged_placeholder(src_dir, dst_dir, &name)?; + continue; + } + + let has_nested_mount = mountpoints + .iter() + .any(|mountpoint| mountpoint != &relative && mountpoint.starts_with(&relative)); + let has_nested_exclusion = exclusions.iter().any(|excluded| { + let excluded = Path::new(excluded); + excluded != relative && excluded.starts_with(&relative) + }); + let source_metadata = src_dir.symlink_metadata(&name)?; + if source_metadata.is_dir() { + let child_src = open_child_dir_nofollow(src_dir, &name) + .with_context(|| format!("Opening /var/{}", relative.display()))?; + // Use the opened directory as the source of truth for metadata. + // This keeps the directory metadata and xattrs tied to the same + // inode even if its parent entry is replaced after the open. + let existing_dst = open_existing_child_dir(dst_dir, &name)?; + if let Some(child_dst) = existing_dst { + copy_var_filtered( + &child_src, + &child_dst, + &relative, + exclusions, + mountpoints, + reflink, + false, + cp_program, + )?; + } else if destination_entry_exists(dst_dir, &name)? { + // A destination of a different type is an existing entry, not + // permission to replace it with the source directory. + continue; + } else if has_nested_mount || has_nested_exclusion { + copy_filtered_directory_staged( + &child_src, + dst_dir, + &name, + &relative, + exclusions, + mountpoints, + reflink, + cp_program, + )?; + } else { + included_entries.push(name); + } + continue; + } + if destination_entry_exists(dst_dir, &name)? { + // Match cp --no-clobber without ever opening the existing object. continue; } included_entries.push(name); @@ -303,28 +608,478 @@ fn copy_dir_skip_subdir( .iter() .map(|name| name.as_os_str()) .collect::>(); + copy_batch_staged( + cp_program, + src_dir, + &names, + dst_dir, + reflink, + &format!("/var/{}", relative_dir.display()), + )?; + } + + Ok(()) +} + +static NEXT_STAGING_ID: AtomicU64 = AtomicU64::new(0); + +/// Make an empty, uniquely-named staging directory beneath the destination +/// capability. Publishing from the same parent keeps the final rename atomic. +fn create_staging_dir(parent: &CapStdDir) -> Result<(std::ffi::OsString, CapStdDir)> { + use rustix::fs::{Mode, mkdirat}; + + let parent_fd = parent.as_filelike_view::(); + for _ in 0..100 { + let id = NEXT_STAGING_ID.fetch_add(1, Ordering::Relaxed); + let name = + std::ffi::OsString::from(format!(".bootc-preserve-var.{}.{}", std::process::id(), id)); + match mkdirat(parent_fd.as_fd(), &name, Mode::from_bits_truncate(0o700)) { + Ok(()) => match open_child_dir_nofollow(parent, &name) { + Ok(directory) => return Ok((name, directory)), + Err(error) => { + parent.remove_dir_all(&name).with_context(|| { + format!("Cleaning staging directory after open failure: {error}") + })?; + return Err(error).context("Opening /var migration staging directory"); + } + }, + Err(error) if error == rustix::io::Errno::EXIST => continue, + Err(error) => { + return Err(error).context("Creating /var migration staging directory"); + } + } + } + anyhow::bail!("Unable to allocate a unique /var migration staging directory") +} + +/// Ensure temporary staging entries are removed on both success and failure. +/// Any entries already published remain complete; transactions are per entry, +/// not a global rollback of earlier successful publications. +fn with_staging_dir( + parent: &CapStdDir, + operation: impl FnOnce(&OsStr, &CapStdDir) -> Result, +) -> Result { + let (name, directory) = create_staging_dir(parent)?; + let result = operation(&name, &directory); + drop(directory); + let cleanup = parent + .remove_dir_all(&name) + .context("Removing /var migration staging data"); + match (result, cleanup) { + (Ok(value), Ok(())) => Ok(value), + (Err(error), Ok(())) => Err(error), + (Ok(_), Err(error)) => Err(error), + (Err(error), Err(cleanup_error)) => Err(error.context(format!( + "Additionally failed to clean migration staging data: {cleanup_error:#}" + ))), + } +} + +fn destination_entry_exists(parent: &CapStdDir, name: &OsStr) -> Result { + match parent.symlink_metadata(name) { + Ok(_) => Ok(true), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false), + Err(error) => Err(error.into()), + } +} + +fn publish_staged_entry( + staging: &CapStdDir, + destination: &CapStdDir, + name: &OsStr, +) -> Result { + use rustix::fs::{RenameFlags, renameat_with}; + + match renameat_with(staging, name, destination, name, RenameFlags::NOREPLACE) { + Ok(()) => Ok(true), + // A destination can appear after the initial no-clobber check. Merge + // staged directories into a same-type destination without replacing + // existing children; other collisions remain untouched. + Err(error) if error == rustix::io::Errno::EXIST => { + if staging.symlink_metadata(name)?.is_dir() { + if let Some(destination_dir) = open_existing_child_dir(destination, name)? { + let staging_dir = open_child_dir_nofollow(staging, name) + .context("Opening staged directory after publish collision")?; + merge_staged_directory(&staging_dir, &destination_dir)?; + } + } + Ok(false) + } + Err(error) => Err(error.into()), + } +} + +/// Move staged children into an existing destination directory without +/// replacing entries already present there. Keep the destination directory's +/// metadata unchanged; moving children updates its timestamps, so restore its +/// original metadata after the merge. +fn merge_staged_directory(staging: &CapStdDir, destination: &CapStdDir) -> Result<()> { + let metadata = snapshot_directory_metadata(destination)?; + let names = staging + .read_dir(".")? + .map(|entry| entry.map(|entry| entry.file_name())) + .collect::>>()?; + + let merge_result = (|| { + for name in names { + publish_staged_entry(staging, destination, &name)?; + } + Ok(()) + })(); + let restore_result = restore_directory_metadata(destination, &metadata); + + match (merge_result, restore_result) { + (Ok(()), Ok(())) => Ok(()), + (Ok(()), Err(error)) => Err(error), + (Err(error), Ok(())) => Err(error), + (Err(error), Err(restore_error)) => Err(error.context(format!( + "Additionally failed to restore merged destination directory metadata: {restore_error:#}" + ))), + } +} + +fn copy_batch_staged( + cp_program: &OsStr, + source: &CapStdDir, + names: &[&OsStr], + destination: &CapStdDir, + reflink: bool, + description: &str, +) -> Result<()> { + with_staging_dir(destination, |_, staging| { let options = if reflink { - &["--reflink=always", "-a", "--no-clobber"][..] + &["--reflink=always", "-a"][..] } else { - &["-a", "--no-clobber"][..] + &["-a"][..] }; let status = run_cp( - &src, - &names, - &dst, + cp_program, + source, + names, + staging, None, options, Stdio::inherit(), Stdio::inherit(), )?; + anyhow::ensure!(status.success(), "cp failed while copying {description}"); + + for name in names { + publish_staged_entry(staging, destination, name)?; + } + Ok(()) + }) +} + +fn copy_filtered_directory_staged( + source: &CapStdDir, + destination_parent: &CapStdDir, + name: &OsStr, + relative: &Path, + exclusions: &[String], + mountpoints: &BTreeSet, + reflink: bool, + cp_program: &OsStr, +) -> Result<()> { + with_staging_dir(destination_parent, |_, staging| { + staging.create_dir(name)?; + let destination = open_child_dir_nofollow(staging, name)?; + copy_var_filtered( + source, + &destination, + relative, + exclusions, + mountpoints, + reflink, + true, + cp_program, + )?; + publish_staged_entry(staging, destination_parent, name)?; + Ok(()) + }) +} + +fn create_staged_placeholder( + src_parent: &CapStdDir, + dst_parent: &CapStdDir, + name: &OsStr, +) -> Result<()> { + with_staging_dir(dst_parent, |_, staging| { + create_mountpoint_placeholder(src_parent, staging, name)?; + if !publish_staged_entry(staging, dst_parent, name)? { + validate_empty_placeholder(dst_parent, name, src_parent.symlink_metadata(name)?)?; + } + Ok(()) + }) +} +fn validate_empty_placeholder( + destination_parent: &CapStdDir, + name: &OsStr, + source_metadata: Metadata, +) -> Result<()> { + let destination_metadata = destination_parent.symlink_metadata(name)?; + if source_metadata.is_dir() { + anyhow::ensure!( + destination_metadata.is_dir(), + "Mountpoint placeholder {} exists with the wrong type", + name.to_string_lossy() + ); + let destination = open_child_dir_nofollow(destination_parent, name)?; + let mut entries = destination.read_dir(".")?; anyhow::ensure!( - status.success(), - "cp failed while copying {}", - dir_name.to_string_lossy() + entries.next().transpose()?.is_none(), + "Mountpoint placeholder {} is not empty", + name.to_string_lossy() + ); + } else { + anyhow::ensure!( + source_metadata.is_file() && destination_metadata.is_file(), + "Mountpoint placeholder {} exists with the wrong type", + name.to_string_lossy() + ); + anyhow::ensure!( + destination_metadata.len() == 0, + "Mountpoint placeholder {} is not empty", + name.to_string_lossy() ); } + Ok(()) +} +fn create_mountpoint_placeholder( + src_parent: &CapStdDir, + dst_parent: &CapStdDir, + name: &OsStr, +) -> Result<()> { + let metadata = src_parent.symlink_metadata(name)?; + if metadata.is_dir() { + let src_dir = open_child_dir_nofollow(src_parent, name) + .context("Opening mountpoint source directory")?; + let directory_metadata = src_dir + .metadata(".") + .context("Reading mountpoint source directory metadata")?; + dst_parent.create_dir(name).with_context(|| { + format!( + "Creating mountpoint directory placeholder {}", + name.to_string_lossy() + ) + })?; + let dst_dir = open_child_dir_nofollow(dst_parent, name) + .context("Opening mountpoint directory placeholder")?; + preserve_directory_metadata(&src_dir, &dst_dir, &directory_metadata) + .context("Preserving mountpoint directory metadata")?; + return Ok(()); + } + anyhow::ensure!( + metadata.is_file(), + "Cannot create safe placeholder for non-file mountpoint {}", + name.to_string_lossy() + ); + use cap_std_ext::cap_std::fs::OpenOptions; + let dst_file = dst_parent + .open_with(name, OpenOptions::new().write(true).create_new(true)) + .context("Creating empty mountpoint file placeholder")?; + preserve_metadata( + src_parent, + name, + &metadata, + dst_file.as_filelike_view::().as_fd(), + ) + .context("Preserving mountpoint file metadata") +} + +/// Reproduce metadata for a directory which was split into filtered children +/// instead of being copied by `cp -a` as one subtree. +fn preserve_directory_metadata( + src_dir: &CapStdDir, + dst_dir: &CapStdDir, + metadata: &Metadata, +) -> Result<()> { + restore_directory_metadata( + dst_dir, + &snapshot_directory_metadata_from_source(src_dir, metadata)?, + ) +} + +struct DirectoryMetadataSnapshot { + metadata: Metadata, + xattrs: Vec<(std::ffi::OsString, Vec)>, +} + +fn snapshot_directory_metadata(directory: &CapStdDir) -> Result { + let metadata = directory.metadata(".")?; + snapshot_directory_metadata_from_source(directory, &metadata) +} + +fn snapshot_directory_metadata_from_source( + directory: &CapStdDir, + metadata: &Metadata, +) -> Result { + use cap_std_ext::dirext::CapStdExtDirExt; + + let mut xattrs = Vec::new(); + for attribute in directory.listxattrs(".")?.iter() { + if let Some(value) = directory.getxattr(".", attribute)? { + xattrs.push((attribute.to_os_string(), value)); + } + } + Ok(DirectoryMetadataSnapshot { + metadata: metadata.clone(), + xattrs, + }) +} + +fn restore_directory_metadata( + directory: &CapStdDir, + snapshot: &DirectoryMetadataSnapshot, +) -> Result<()> { + use rustix::fs::{Mode, fchmod}; + + let dst_file = directory.as_filelike_view::(); + let dst_fd = dst_file.as_fd(); + let mut errors = Vec::new(); + if let Err(error) = preserve_metadata_owner(&snapshot.metadata, dst_fd) { + errors.push(format!("owner/group: {error:#}")); + } + if let Err(error) = fchmod( + dst_fd, + Mode::from_bits_truncate(snapshot.metadata.mode() & 0o7777), + ) { + errors.push(format!("mode: {error:#}")); + } + if let Err(error) = apply_xattrs_exact(dst_fd, &snapshot.xattrs) { + errors.push(format!("xattrs: {error:#}")); + } + if let Err(error) = preserve_metadata_timestamps(&snapshot.metadata, dst_fd) { + errors.push(format!("timestamps: {error:#}")); + } + if !errors.is_empty() { + anyhow::bail!("restoring directory metadata failed: {}", errors.join("; ")); + } + Ok(()) +} + +fn restore_directory_timestamps(directory: &CapStdDir, metadata: &Metadata) -> Result<()> { + let dst_file = directory.as_filelike_view::(); + preserve_metadata_timestamps(metadata, dst_file.as_fd()) +} + +/// Preserve ownership, mode, timestamps, and available xattrs (including ACL +/// and SELinux labels) on a newly-created destination object. Callers only use +/// this for objects created by this migration, preserving the no-clobber rule. +/// For mountpoint file placeholders, source metadata and xattrs still use the +/// parent/name pair captured before the destination is opened; replacing that +/// source entry concurrently can therefore race those reads. Destination +/// updates remain anchored to the opened destination fd. Split directory +/// metadata and xattrs instead use the opened source directory handle. +fn preserve_metadata( + src_parent: &CapStdDir, + src_name: &OsStr, + metadata: &Metadata, + dst_fd: Fd, +) -> Result<()> { + use cap_std_ext::dirext::CapStdExtDirExt; + + let mut source_xattrs = Vec::new(); + for attribute in src_parent.listxattrs(src_name)?.iter() { + if let Some(value) = src_parent.getxattr(src_name, attribute)? { + source_xattrs.push((attribute.to_os_string(), value)); + } + } + preserve_metadata_owner_and_mode(metadata, &dst_fd)?; + apply_xattrs_exact(&dst_fd, &source_xattrs)?; + preserve_metadata_timestamps(metadata, &dst_fd) +} + +/// Make the destination xattr set exactly match the source snapshot. Removing +/// extras first matters for inherited ACLs and labels as well as user xattrs. +fn apply_xattrs_exact( + destination: Fd, + source_xattrs: &[(std::ffi::OsString, Vec)], +) -> Result<()> { + use rustix::fs::{fremovexattr, fsetxattr}; + use std::collections::BTreeSet; + + let destination_names = list_fd_xattrs(&destination)?; + let source_names = source_xattrs + .iter() + .map(|(name, _)| name.as_os_str()) + .collect::>(); + for name in destination_names { + if !source_names.contains(name.as_os_str()) { + fremovexattr(&destination, &name).with_context(|| { + format!("Removing destination xattr {}", name.to_string_lossy()) + })?; + } + } + for (name, value) in source_xattrs { + fsetxattr(&destination, name, value, rustix::fs::XattrFlags::empty()) + .with_context(|| format!("Setting source xattr {}", name.to_string_lossy()))?; + } + Ok(()) +} + +fn list_fd_xattrs(fd: Fd) -> Result> { + use std::os::unix::ffi::OsStringExt; + + // Linux caps the complete xattr-name list at XATTR_LIST_MAX (64 KiB). + let mut names = vec![0; 64 * 1024]; + let length = rustix::fs::flistxattr(fd, &mut names[..])?; + names.truncate(length); + if names.is_empty() { + return Ok(Vec::new()); + } + anyhow::ensure!(names.last() == Some(&0), "Malformed xattr name list"); + names.pop(); + names + .split(|byte| *byte == 0) + .filter(|name| !name.is_empty()) + .map(|name| Ok(std::ffi::OsString::from_vec(name.to_vec()))) + .collect() +} + +fn preserve_metadata_owner_and_mode(metadata: &Metadata, dst_fd: Fd) -> Result<()> { + use rustix::fs::{Mode, fchmod}; + + preserve_metadata_owner(metadata, &dst_fd)?; + fchmod(&dst_fd, Mode::from_bits_truncate(metadata.mode() & 0o7777))?; + Ok(()) +} + +fn preserve_metadata_owner(metadata: &Metadata, dst_fd: Fd) -> Result<()> { + use rustix::fs::{Gid, Uid, fchown}; + + let destination_metadata = rustix::fs::fstat(&dst_fd)?; + if destination_metadata.st_uid != metadata.uid() + || destination_metadata.st_gid != metadata.gid() + { + fchown( + &dst_fd, + Some(Uid::from_raw(metadata.uid())), + Some(Gid::from_raw(metadata.gid())), + )?; + } + Ok(()) +} + +fn preserve_metadata_timestamps(metadata: &Metadata, dst_fd: Fd) -> Result<()> { + use rustix::fs::{Timestamps, futimens}; + use rustix::time::Timespec; + + futimens( + &dst_fd, + &Timestamps { + last_access: Timespec { + tv_sec: metadata.atime(), + tv_nsec: metadata.atime_nsec() as _, + }, + last_modification: Timespec { + tv_sec: metadata.mtime(), + tv_nsec: metadata.mtime_nsec() as _, + }, + }, + )?; Ok(()) } @@ -356,62 +1111,37 @@ fn copy_dir_skip_subdir( /// a consistent snapshot. /// /// See: +#[cfg(test)] #[context("Copying /var into new deployment (Strategy D)")] fn preserve_var_copy( src_var: &CapStdDir, new_var: &CapStdDir, exclusions: &[String], ) -> Result<()> { - let entries = src_var.read_dir(".").context("Reading source /var")?; - - for entry in entries { - let entry = entry.context("Reading entry in source /var")?; - let name = entry.file_name(); - let name_str = name.to_string_lossy(); - - if exclusions.iter().any(|s| name.to_str() == Some(s.as_str())) { - println!(" Skipping {} (ephemeral)", name_str); - continue; - } - - let skips = nested_exclusions(&name, exclusions); - if !skips.is_empty() && src_var.symlink_metadata(&name)?.is_dir() { - copy_dir_skip_subdir(src_var, new_var, &name, &skips, false)?; - continue; - } - - println!(" Copying {} → {}", name_str, name_str); - let status = run_cp( - src_var, - &[&name], - new_var, - None, - &["-a", "--no-clobber"], - Stdio::inherit(), - Stdio::inherit(), - )?; - - anyhow::ensure!(status.success(), "cp -a failed for {}", name_str); - } + preserve_var_copy_mounts(src_var, new_var, exclusions, &BTreeSet::new()) +} +fn preserve_var_copy_mounts( + src_var: &CapStdDir, + new_var: &CapStdDir, + exclusions: &[String], + mountpoints: &BTreeSet, +) -> Result<()> { + copy_var_filtered( + src_var, + new_var, + Path::new(""), + exclusions, + mountpoints, + false, + false, + OsStr::new("cp"), + ) + .context("Copying /var into new deployment (Strategy D)")?; println!(" /var copy complete."); Ok(()) } -fn nested_exclusions<'a>(name: &OsStr, exclusions: &'a [String]) -> Vec<&'a str> { - let Some(name) = name.to_str() else { - return Vec::new(); - }; - let prefix = format!("{name}/"); - exclusions - .iter() - .filter_map(|path| { - path.strip_prefix(&prefix) - .filter(|rest| !rest.contains('/')) - }) - .collect() -} - /// Open a single source child directory without following a symlink swapped /// into place after the caller's metadata check. The entry name comes directly /// from `read_dir`, so this is one path component beneath the capability root. @@ -427,21 +1157,46 @@ fn open_child_dir_nofollow(parent: &CapStdDir, name: &OsStr) -> std::io::Result< Ok(CapStdDir::from_std_file(std::fs::File::from(fd))) } -fn open_or_create_child_dir(parent: &CapStdDir, name: &OsStr) -> Result { - match parent.create_dir(name) { - Ok(()) => {} +#[cfg(test)] +fn open_or_create_child_dir(parent: &CapStdDir, name: &OsStr) -> Result<(CapStdDir, bool)> { + let created = match parent.create_dir(name) { + Ok(()) => true, Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { anyhow::ensure!( parent.symlink_metadata(name)?.is_dir(), "Destination child {} exists and is not a directory", name.to_string_lossy() ); + false } Err(e) => return Err(e).context("Creating destination child directory"), - } - parent - .open_dir(name) - .context("Opening destination child directory") + }; + let dir = + open_child_dir_nofollow(parent, name).context("Opening destination child directory")?; + Ok((dir, created)) +} + +/// Open an existing destination directory without following symlinks. The +/// opened handle pins the directory used for the recursive merge. +fn open_existing_child_dir(parent: &CapStdDir, name: &OsStr) -> Result> { + let existing: Result> = match open_child_dir_nofollow(parent, name) { + Ok(directory) => Ok(Some(directory)), + Err(error) + if matches!( + error.kind(), + std::io::ErrorKind::NotFound | std::io::ErrorKind::NotADirectory + ) || error.raw_os_error() == Some(libc::ELOOP) => + { + Ok(None) + } + Err(error) => Err(error.into()), + }; + existing.with_context(|| { + format!( + "Opening existing destination directory {}", + name.to_string_lossy() + ) + }) } /// Validate exclusions before touching either tree. Paths are relative to `/var`. @@ -507,7 +1262,10 @@ mod tests { use cap_std_ext::dirext::CapStdExtDirExt; use std::{ fs, - os::unix::fs::{MetadataExt, PermissionsExt, symlink}, + os::unix::{ + ffi::OsStrExt, + fs::{MetadataExt as StdMetadataExt, PermissionsExt, symlink}, + }, time::{Duration, SystemTime}, }; @@ -536,12 +1294,655 @@ mod tests { } #[test] - fn plain_copy_skips_multiple_children_under_parent() -> Result<()> { - let exclusions = ["tmp".into(), "log/journal".into(), "log/private".into()]; + fn mountinfo_decodes_escaped_and_non_utf8_paths() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_var = temp.path().join("var"); + fs::create_dir(&src_var)?; + let source = src_var.as_os_str().as_bytes(); + let mut mountinfo = format!( + "36 0 0:32 / {0} rw,relatime - tmpfs tmpfs rw\n37 25 0:33 / {0}/rpc\\040pipefs rw - nfs nfs rw\n", + src_var.display() + ) + .into_bytes(); + mountinfo.extend_from_slice(b"38 25 0:34 / "); + mountinfo.extend_from_slice(source); + mountinfo.extend_from_slice(b"/\xffstate rw - tmpfs tmpfs rw\n"); + let parsed = parse_mountinfo(&mountinfo)?; + let escaped_name = src_var.join("rpc pipefs"); + assert_eq!(parsed[1], escaped_name); + let mut non_utf8 = source.to_vec(); + non_utf8.extend_from_slice(b"/\xffstate"); + assert_eq!(parsed[2].as_os_str().as_bytes(), non_utf8); + + let nested = nested_mountpoints(&src_var, parsed)?; + assert_eq!( + nested, + BTreeSet::from([ + PathBuf::from("rpc pipefs"), + PathBuf::from(OsStr::from_bytes(b"\xffstate")) + ]) + ); + Ok(()) + } + + #[test] + fn unrelated_mount_records_do_not_change_nested_mount_selection() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_var = temp.path().join("target/var"); + fs::create_dir_all(&src_var)?; + let base = format!( + "36 0 0:32 / {0} rw - tmpfs tmpfs rw\n37 25 0:33 / {0}/lib/data rw - nfs nfs rw\n", + src_var.display() + ); + let with_unrelated = format!( + "21 0 0:21 / / rw - overlay overlay rw\n{base}38 25 0:34 / /unrelated rw - tmpfs tmpfs rw\n" + ); + + let selected = |mountinfo: &str| -> Result> { + nested_mountpoints(&src_var, parse_mountinfo(mountinfo.as_bytes())?) + }; + assert_eq!( + selected(&base)?, + BTreeSet::from([PathBuf::from("lib/data")]) + ); + assert_eq!(selected(&with_unrelated)?, selected(&base)?); + Ok(()) + } + + #[test] + fn mountinfo_allows_relative_and_noncanonical_roots_with_valid_nested_mountpoints() -> Result<()> + { + let temp = tempfile::tempdir()?; + let src_var = temp.path().join("target/var"); + fs::create_dir_all(&src_var)?; + + for root in ["/some/../root", "relative/root"] { + let mountinfo = format!( + "36 0 0:32 / {0} rw - tmpfs tmpfs rw\n37 25 0:33 {1} {0}/lib/data rw - nfs nfs rw\n", + src_var.display(), + root + ); + + assert_eq!( + nested_mountpoints(&src_var, parse_mountinfo(mountinfo.as_bytes())?)?, + BTreeSet::from([PathBuf::from("lib/data")]), + "root field {root:?} should not affect mountpoint selection" + ); + } + Ok(()) + } + + #[test] + fn mountinfo_rejects_malformed_records_and_escapes() { + for mountinfo in [ + b"".as_slice(), + b"not a mountinfo record\n", + b"36 25 0:32 /bad\\04x /target/var rw - tmpfs tmpfs rw\n", + b"36 25 0:32 / /target/var\\04x rw - tmpfs tmpfs rw\n", + b"36 25 0:32 / /target/var\\400 rw - tmpfs tmpfs rw\n", + b"36 25 0:32 / /target/var\\777 rw - tmpfs tmpfs rw\n", + b"36 25 0:32 / /target/var\\041 rw - tmpfs tmpfs rw\n", + b"36 25 0:32 / relative rw - tmpfs tmpfs rw\n", + b"36 25 0:32 / /target/var/../outside rw - tmpfs tmpfs rw\n", + b"36 25 0:32 / /target/var rw - tmpfs tmpfs rw extra\n", + ] { + assert!( + parse_mountinfo(mountinfo).is_err(), + "accepted {mountinfo:?}" + ); + } + } + + #[test] + fn nested_mountpoint_detection_is_component_safe() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_var = temp.path().join("target/var"); + fs::create_dir_all(&src_var)?; + let alias = temp.path().join("var-alias"); + symlink(&src_var, &alias)?; + let paths = [ + src_var.clone(), + src_var.join("lib/data"), + temp.path().join("target/var-old/other"), + PathBuf::from("/elsewhere/var/mount"), + ]; + assert_eq!( + nested_mountpoints(&alias, paths)?, + BTreeSet::from([PathBuf::from("lib/data")]) + ); + assert!(nested_mountpoints(&temp.path().join("missing"), []).is_err()); + Ok(()) + } + + #[test] + fn both_copy_strategies_leave_nested_mountpoints_empty() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir_all(src_path.join("lib/bound"))?; + fs::create_dir_all(src_path.join("lib/sibling"))?; + fs::write(src_path.join("lib/file-bound"), b"do not migrate")?; + fs::write( + src_path.join("lib/bound/mounted-sentinel"), + b"do not migrate", + )?; + fs::write(src_path.join("lib/sibling/sentinel"), b"migrate")?; + fs::create_dir(&dst_path)?; + let mounts = BTreeSet::from([PathBuf::from("lib/bound"), PathBuf::from("lib/file-bound")]); + let src = cap_dir(&src_path); + let dst = cap_dir(&dst_path); + + preserve_var_copy_mounts(&src, &dst, &[], &mounts)?; + assert!(dst_path.join("lib/bound").is_dir()); + assert_eq!(fs::read_dir(dst_path.join("lib/bound"))?.count(), 0); + assert_eq!(fs::read(dst_path.join("lib/file-bound"))?, b""); + assert_eq!(fs::read(dst_path.join("lib/sibling/sentinel"))?, b"migrate"); + + let excluded_dst_path = temp.path().join("excluded-dst"); + fs::create_dir(&excluded_dst_path)?; + preserve_var_copy_mounts( + &src, + &cap_dir(&excluded_dst_path), + &["lib/bound".into()], + &mounts, + )?; + assert!(!excluded_dst_path.join("lib/bound").exists()); + + let reflink_dst = temp.path().join("reflink-dst"); + fs::create_dir(&reflink_dst)?; + let reflink_dst = cap_dir(&reflink_dst); + if supports_reflink(&src, &reflink_dst)? { + preserve_var_reflink_mounts(&src, &reflink_dst, &[], &mounts)?; + assert!(reflink_dst.symlink_metadata("lib/bound")?.is_dir()); + assert_eq!( + fs::read_dir(temp.path().join("reflink-dst/lib/bound"))?.count(), + 0 + ); + assert_eq!( + fs::read(temp.path().join("reflink-dst/lib/file-bound"))?, + b"" + ); + assert_eq!( + fs::read(temp.path().join("reflink-dst/lib/sibling/sentinel"))?, + b"migrate" + ); + } + Ok(()) + } + + #[test] + fn split_directories_and_mountpoint_placeholders_keep_metadata() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir_all(src_path.join("lib/bound"))?; + fs::create_dir(&dst_path)?; + fs::set_permissions(src_path.join("lib"), fs::Permissions::from_mode(0o751))?; + fs::set_permissions( + src_path.join("lib/bound"), + fs::Permissions::from_mode(0o711), + )?; + // Old atimes make traversal update them on relatime filesystems; the + // copy must use the metadata captured before reading child entries. + let expected_atime = SystemTime::UNIX_EPOCH + Duration::from_secs(1_600_000_001); + let expected_mtime = SystemTime::UNIX_EPOCH + Duration::from_secs(1_600_000_002); + for path in [src_path.join("lib"), src_path.join("lib/bound")] { + fs::File::open(path)?.set_times( + fs::FileTimes::new() + .set_accessed(expected_atime) + .set_modified(expected_mtime), + )?; + } + + let src = cap_dir(&src_path); + let dst = cap_dir(&dst_path); + let xattrs_supported = src + .setxattr("lib", "user.bootc_migrate_test", b"parent") + .is_ok() + && src + .setxattr("lib/bound", "user.bootc_migrate_test", b"placeholder") + .is_ok(); + preserve_var_copy_mounts( + &src, + &dst, + &[], + &BTreeSet::from([PathBuf::from("lib/bound")]), + )?; + + for (source, destination, expected_mode, expected_xattr) in [ + ("lib", "lib", 0o751, b"parent".as_slice()), + ("lib/bound", "lib/bound", 0o711, b"placeholder".as_slice()), + ] { + let source_metadata = fs::metadata(src_path.join(source))?; + let destination_metadata = fs::metadata(dst_path.join(destination))?; + assert_eq!(destination_metadata.mode() & 0o7777, expected_mode); + assert_eq!(destination_metadata.uid(), source_metadata.uid()); + assert_eq!(destination_metadata.gid(), source_metadata.gid()); + assert_eq!( + destination_metadata.atime(), + expected_atime + .duration_since(SystemTime::UNIX_EPOCH)? + .as_secs() as i64 + ); + assert_eq!( + destination_metadata.mtime(), + expected_mtime + .duration_since(SystemTime::UNIX_EPOCH)? + .as_secs() as i64 + ); + assert_eq!(destination_metadata.atime_nsec(), 0); + assert_eq!(destination_metadata.mtime_nsec(), 0); + if xattrs_supported { + assert_eq!( + dst.getxattr(destination, "user.bootc_migrate_test")?, + Some(expected_xattr.to_vec()) + ); + } + } + Ok(()) + } + + #[test] + fn split_directory_xattrs_match_source_exactly_when_supported() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir(&src_path)?; + fs::create_dir(&dst_path)?; + let source = cap_dir(&src_path); + let destination = cap_dir(&dst_path); + if source + .setxattr(".", "user.bootc_expected", b"source") + .is_err() + || destination + .setxattr(".", "user.bootc_inherited", b"extra") + .is_err() + { + return Ok(()); + } + + let snapshot = snapshot_directory_metadata(&source)?; + restore_directory_metadata(&destination, &snapshot)?; + + assert_eq!( + destination.getxattr(".", "user.bootc_expected")?, + Some(b"source".to_vec()) + ); + assert_eq!(destination.getxattr(".", "user.bootc_inherited")?, None); + Ok(()) + } + + #[test] + fn filtered_copy_does_not_change_existing_destination_directory() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir_all(src_path.join("tree/excluded"))?; + fs::write(src_path.join("tree/sentinel"), b"source")?; + fs::write(src_path.join("tree/new-child"), b"new child")?; + fs::write(src_path.join("root-new-child"), b"new root child")?; + fs::create_dir_all(dst_path.join("tree"))?; + fs::write(dst_path.join("tree/sentinel"), b"destination")?; + fs::set_permissions(src_path.join("tree"), fs::Permissions::from_mode(0o751))?; + fs::set_permissions(&dst_path, fs::Permissions::from_mode(0o710))?; + fs::set_permissions(dst_path.join("tree"), fs::Permissions::from_mode(0o700))?; + let expected_atime = SystemTime::UNIX_EPOCH + Duration::from_secs(1_600_000_101); + let expected_mtime = SystemTime::UNIX_EPOCH + Duration::from_secs(1_600_000_102); + let expected_root_atime = SystemTime::UNIX_EPOCH + Duration::from_secs(1_600_000_104); + let expected_root_mtime = SystemTime::UNIX_EPOCH + Duration::from_secs(1_600_000_103); + fs::File::open(dst_path.join("tree"))?.set_times( + fs::FileTimes::new() + .set_accessed(expected_atime) + .set_modified(expected_mtime), + )?; + fs::File::open(&dst_path)?.set_times( + fs::FileTimes::new() + .set_accessed(expected_root_atime) + .set_modified(expected_root_mtime), + )?; + let destination = cap_dir(&dst_path); + let xattrs_supported = destination + .setxattr("tree", "user.bootc_migrate_test", b"existing") + .is_ok(); + let expected_owner = fs::metadata(dst_path.join("tree"))?; + + preserve_var_copy_mounts( + &cap_dir(&src_path), + &destination, + &["tree/excluded".into()], + &BTreeSet::new(), + )?; + + let destination_metadata = fs::metadata(dst_path.join("tree"))?; + assert_eq!(destination_metadata.mode() & 0o7777, 0o700); + assert_eq!(destination_metadata.uid(), expected_owner.uid()); + assert_eq!(destination_metadata.gid(), expected_owner.gid()); + assert_eq!( + destination_metadata.atime(), + expected_atime + .duration_since(SystemTime::UNIX_EPOCH)? + .as_secs() as i64 + ); + assert_eq!( + destination_metadata.mtime(), + expected_mtime + .duration_since(SystemTime::UNIX_EPOCH)? + .as_secs() as i64 + ); + assert_eq!(fs::read(dst_path.join("tree/sentinel"))?, b"destination"); + assert_eq!(fs::read(dst_path.join("tree/new-child"))?, b"new child"); + assert_eq!( + fs::read(dst_path.join("root-new-child"))?, + b"new root child" + ); + assert_eq!(fs::read_dir(dst_path.join("tree"))?.count(), 2); + assert_eq!(destination_metadata.modified()?, expected_mtime); + + let root_metadata = fs::metadata(&dst_path)?; + assert_eq!(root_metadata.mode() & 0o7777, 0o710); assert_eq!( - nested_exclusions(OsStr::new("log"), &exclusions), - ["journal", "private"] + root_metadata.atime(), + expected_root_atime + .duration_since(SystemTime::UNIX_EPOCH)? + .as_secs() as i64 ); + assert_eq!(root_metadata.modified()?, expected_root_mtime); + if xattrs_supported { + assert_eq!( + destination.getxattr("tree", "user.bootc_migrate_test")?, + Some(b"existing".to_vec()) + ); + } + Ok(()) + } + + #[test] + fn staged_directory_collision_merges_missing_children_without_clobbering() -> Result<()> { + let temp = tempfile::tempdir()?; + let destination_path = temp.path().join("destination"); + fs::create_dir(&destination_path)?; + let destination = cap_dir(&destination_path); + with_staging_dir(&destination, |_, staging| { + staging.create_dir("tree")?; + staging.write("tree/existing", b"staged collision")?; + staging.write("tree/missing", b"new child")?; + + // Simulate a concurrent creator winning after staging completed. + fs::create_dir(destination_path.join("tree"))?; + fs::write(destination_path.join("tree/existing"), b"destination")?; + fs::set_permissions( + destination_path.join("tree"), + fs::Permissions::from_mode(0o700), + )?; + let destination_mtime = SystemTime::UNIX_EPOCH + Duration::from_secs(1_600_000_301); + fs::File::open(destination_path.join("tree"))? + .set_times(fs::FileTimes::new().set_modified(destination_mtime))?; + let expected_owner = fs::metadata(destination_path.join("tree"))?; + + assert!(!publish_staged_entry( + staging, + &destination, + OsStr::new("tree") + )?); + assert_eq!( + fs::read(destination_path.join("tree/existing"))?, + b"destination" + ); + assert_eq!( + fs::read(destination_path.join("tree/missing"))?, + b"new child" + ); + + let metadata = fs::metadata(destination_path.join("tree"))?; + assert_eq!(metadata.mode() & 0o7777, 0o700); + assert_eq!(metadata.uid(), expected_owner.uid()); + assert_eq!(metadata.gid(), expected_owner.gid()); + assert_eq!(metadata.modified()?, destination_mtime); + Ok(()) + })?; + Ok(()) + } + + #[test] + fn unfiltered_directory_collisions_preserve_destination_metadata() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + fs::create_dir_all(src_path.join("state"))?; + fs::write(src_path.join("state/existing"), b"source")?; + fs::write(src_path.join("state/new"), b"new")?; + fs::write(src_path.join("collision-file"), b"source collision")?; + fs::create_dir(src_path.join("directory-vs-file"))?; + fs::write(src_path.join("directory-vs-file/new"), b"must not replace")?; + fs::set_permissions(src_path.join("state"), fs::Permissions::from_mode(0o751))?; + let source_mtime = SystemTime::UNIX_EPOCH + Duration::from_secs(1_600_000_201); + fs::File::open(src_path.join("state"))? + .set_times(fs::FileTimes::new().set_modified(source_mtime))?; + + let src = cap_dir(&src_path); + for (strategy, reflink) in [("copy", false), ("reflink", true)] { + let dst_path = temp.path().join(format!("dst-{strategy}")); + fs::create_dir_all(dst_path.join("state"))?; + fs::write(dst_path.join("state/existing"), b"destination")?; + fs::write(dst_path.join("collision-file"), b"keep collision")?; + fs::write(dst_path.join("directory-vs-file"), b"keep file")?; + fs::set_permissions(dst_path.join("state"), fs::Permissions::from_mode(0o700))?; + let destination_mtime = SystemTime::UNIX_EPOCH + Duration::from_secs(1_600_000_202); + fs::File::open(dst_path.join("state"))? + .set_times(fs::FileTimes::new().set_modified(destination_mtime))?; + let dst = cap_dir(&dst_path); + if reflink && !supports_reflink(&src, &dst)? { + continue; + } + + if reflink { + preserve_var_reflink(&src, &dst, &[])?; + } else { + preserve_var_copy(&src, &dst, &[])?; + } + + let destination_metadata = fs::metadata(dst_path.join("state"))?; + assert_eq!(destination_metadata.mode() & 0o7777, 0o700); + assert_eq!(destination_metadata.modified()?, destination_mtime); + assert_eq!(fs::read(dst_path.join("state/existing"))?, b"destination"); + assert_eq!(fs::read(dst_path.join("state/new"))?, b"new"); + assert_eq!( + fs::read(dst_path.join("collision-file"))?, + b"keep collision" + ); + assert_eq!(fs::read(dst_path.join("directory-vs-file"))?, b"keep file"); + assert!(!dst_path.join("directory-vs-file/new").exists()); + } + Ok(()) + } + + #[test] + fn failed_copy_batch_is_cleaned_and_can_be_retried() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + fs::create_dir(&src_path)?; + fs::write(src_path.join("first"), b"linked data")?; + fs::hard_link(src_path.join("first"), src_path.join("second"))?; + let src = cap_dir(&src_path); + + let failing_cp = temp.path().join("cp-fails-after-copy"); + fs::write( + &failing_cp, + b"#!/bin/sh\nreflink=no\nwhile [ \"$1\" != -- ]; do\n [ \"$1\" = --reflink=always ] && reflink=yes\n shift\ndone\nshift\nsource=$1\nshift\nwhile [ \"$#\" -gt 1 ]; do shift; done\ndestination=$1\nif [ \"$reflink\" = yes ]; then\n /bin/cp --reflink=always -a -- \"$source\" \"$destination\"\nelse\n /bin/cp -a -- \"$source\" \"$destination\"\nfi\nexit 1\n", + )?; + fs::set_permissions(&failing_cp, fs::Permissions::from_mode(0o755))?; + + for (strategy, reflink) in [("copy", false), ("reflink", true)] { + let dst_path = temp.path().join(format!("dst-{strategy}")); + fs::create_dir(&dst_path)?; + let dst = cap_dir(&dst_path); + if reflink && !supports_reflink(&src, &dst)? { + continue; + } + + let error = copy_var_filtered( + &src, + &dst, + Path::new(""), + &[], + &BTreeSet::new(), + reflink, + false, + failing_cp.as_os_str(), + ); + assert!(error.is_err()); + assert_eq!(fs::read_dir(&dst_path)?.count(), 0); + + if reflink { + preserve_var_reflink(&src, &dst, &[])?; + } else { + preserve_var_copy(&src, &dst, &[])?; + } + let first = fs::metadata(dst_path.join("first"))?; + let second = fs::metadata(dst_path.join("second"))?; + assert_eq!(first.ino(), second.ino()); + assert_eq!(fs::read(dst_path.join("first"))?, b"linked data"); + } + Ok(()) + } + + #[test] + fn empty_mountpoint_placeholders_are_retry_safe_and_type_checked() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + fs::create_dir_all(src_path.join("directory-mount"))?; + fs::write(src_path.join("file-mount"), b"mounted data")?; + let src = cap_dir(&src_path); + + for (strategy, reflink) in [("copy", false), ("reflink", true)] { + let dst_path = temp.path().join(format!("dst-{strategy}")); + fs::create_dir(&dst_path)?; + let dst = cap_dir(&dst_path); + if reflink && !supports_reflink(&src, &dst)? { + continue; + } + + fs::create_dir(dst_path.join("directory-mount"))?; + fs::write(dst_path.join("file-mount"), b"")?; + let mounts = BTreeSet::from([ + PathBuf::from("directory-mount"), + PathBuf::from("file-mount"), + ]); + if reflink { + preserve_var_reflink_mounts(&src, &dst, &[], &mounts)?; + preserve_var_reflink_mounts(&src, &dst, &[], &mounts)?; + } else { + preserve_var_copy_mounts(&src, &dst, &[], &mounts)?; + preserve_var_copy_mounts(&src, &dst, &[], &mounts)?; + } + assert_eq!(fs::read_dir(dst_path.join("directory-mount"))?.count(), 0); + assert_eq!(fs::metadata(dst_path.join("file-mount"))?.len(), 0); + + fs::write(dst_path.join("directory-mount/sentinel"), b"keep")?; + let error = if reflink { + preserve_var_reflink_mounts(&src, &dst, &[], &mounts) + } else { + preserve_var_copy_mounts(&src, &dst, &[], &mounts) + }; + assert!(error.is_err()); + assert_eq!( + fs::read(dst_path.join("directory-mount/sentinel"))?, + b"keep" + ); + fs::remove_file(dst_path.join("directory-mount/sentinel"))?; + fs::write(dst_path.join("file-mount"), b"keep file")?; + let error = if reflink { + preserve_var_reflink_mounts(&src, &dst, &[], &mounts) + } else { + preserve_var_copy_mounts(&src, &dst, &[], &mounts) + }; + assert!(error.is_err()); + assert_eq!(fs::read(dst_path.join("file-mount"))?, b"keep file"); + + let wrong_type = temp.path().join(format!("wrong-type-{strategy}")); + fs::create_dir(&wrong_type)?; + fs::write(wrong_type.join("directory-mount"), b"keep wrong type")?; + fs::create_dir(wrong_type.join("file-mount"))?; + fs::write(wrong_type.join("file-mount/sentinel"), b"keep wrong type")?; + let wrong_dst = cap_dir(&wrong_type); + let error = if reflink { + preserve_var_reflink_mounts(&src, &wrong_dst, &[], &mounts) + } else { + preserve_var_copy_mounts(&src, &wrong_dst, &[], &mounts) + }; + assert!(error.is_err()); + assert_eq!( + fs::read(wrong_type.join("directory-mount"))?, + b"keep wrong type" + ); + assert_eq!( + fs::read(wrong_type.join("file-mount/sentinel"))?, + b"keep wrong type" + ); + } + Ok(()) + } + + #[test] + fn mountpoint_placeholders_fail_without_reusing_existing_destinations() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir_all(src_path.join("directory-mount"))?; + fs::write(src_path.join("file-mount"), b"source")?; + fs::create_dir_all(dst_path.join("directory-mount"))?; + fs::write(dst_path.join("directory-mount/sentinel"), b"keep")?; + fs::create_dir(dst_path.join("file-mount"))?; + fs::write(dst_path.join("file-mount/sentinel"), b"also keep")?; + + let src = cap_dir(&src_path); + let dst = cap_dir(&dst_path); + assert!(create_mountpoint_placeholder(&src, &dst, OsStr::new("directory-mount")).is_err()); + assert!(create_mountpoint_placeholder(&src, &dst, OsStr::new("file-mount")).is_err()); + assert_eq!( + fs::read(dst_path.join("directory-mount/sentinel"))?, + b"keep" + ); + assert_eq!( + fs::read(dst_path.join("file-mount/sentinel"))?, + b"also keep" + ); + Ok(()) + } + + #[test] + fn destination_directory_symlinks_are_rejected_without_following() -> Result<()> { + let temp = tempfile::tempdir()?; + let target = temp.path().join("target"); + fs::create_dir(&target)?; + fs::write(target.join("sentinel"), b"unchanged")?; + + let var_link = temp.path().join("var-link"); + symlink(&target, &var_link)?; + assert!(open_or_create_var_dir(&var_link).is_err()); + + let parent_path = temp.path().join("parent"); + fs::create_dir(&parent_path)?; + symlink(&target, parent_path.join("child-link"))?; + let parent = cap_dir(&parent_path); + assert!(open_or_create_child_dir(&parent, OsStr::new("child-link")).is_err()); + + let src_path = temp.path().join("src"); + fs::create_dir(&src_path)?; + fs::create_dir(src_path.join("mount"))?; + symlink(&target, parent_path.join("mount"))?; + assert!( + create_mountpoint_placeholder(&cap_dir(&src_path), &parent, OsStr::new("mount")) + .is_err() + ); + + assert_eq!(fs::read(target.join("sentinel"))?, b"unchanged"); + assert_eq!(fs::read_dir(target)?.count(), 1); + Ok(()) + } + + #[test] + fn plain_copy_skips_multiple_children_under_parent() -> Result<()> { + let exclusions = ["tmp".into(), "log/journal".into(), "log/private".into()]; let temp = tempfile::tempdir()?; let src_path = temp.path().join("src"); diff --git a/docs/src/bootc-installation.7.md b/docs/src/bootc-installation.7.md index 6a2cad0502..4f8e063474 100644 --- a/docs/src/bootc-installation.7.md +++ b/docs/src/bootc-installation.7.md @@ -305,6 +305,22 @@ excluded. Repeat `--preserve-var-skip PATH` to add further exclusions using path relative to `/var`: each exclusion must be either one component (such as `lib`) or exactly one child (such as `lib/pgsql`). `/var` itself and deeper paths are not accepted. Excluded data is not migrated and must be handled separately. The +contents of nested mounts under `/var` are not migrated. Each non-excluded +nested mountpoint is represented by an empty placeholder in the new stateroot; +an explicit exclusion takes precedence and omits that path entirely. Mount +definitions merged into the new `/etc` (for example, `/etc/fstab`) can +re-establish configured mounts at boot. Runtime-only mounts need separate +configuration. Hardlinks wholly within one copied batch are preserved, but +hardlinks crossing a mountpoint or exclusion split, or separate copies while +merging into an already-existing destination directory, may not remain linked. +Nested mountpoints are selected from a point-in-time, best-effort mountinfo +snapshot; mounts added or removed beneath `/var` during copying can race with +the copy, just like concurrent file changes. +Reflinks avoid immediate data duplication but are created per file and are not +a snapshot. Each missing destination entry is staged and published atomically +without replacing existing data. A failed entry leaves no partial final entry, +but the migration is not a global transaction: earlier completed entries can +remain if a later entry fails. The ostree-only `--merge-etc` option instead performs a three-way merge of the host's `/etc` customizations into the new deployment before the first reboot. See the [to-existing-root man page](man/bootc-install-to-existing-root.8.md) for details. diff --git a/docs/src/man/bootc-install-to-existing-root.8.md b/docs/src/man/bootc-install-to-existing-root.8.md index 5290bd6a9e..23a5bb4771 100644 --- a/docs/src/man/bootc-install-to-existing-root.8.md +++ b/docs/src/man/bootc-install-to-existing-root.8.md @@ -30,17 +30,31 @@ provide these migrations on the **ostree backend only**. They run after the installation completes and before the first reboot. `--preserve-var` copies the old `/var` into the new stateroot. Where supported, -the copy uses filesystem reflinks; otherwise it performs a regular copy of the -non-excluded contents. A regular copy -can be unsafe while databases or other stateful services are running. Reflinks -clone individual files, but the directory-tree copy is not an atomic database -snapshot; concurrent changes can still result in inconsistent data. Stop those -services before copying. The paths +the copy uses filesystem reflinks, which avoid immediate data duplication but +are created per file rather than as a snapshot; otherwise it performs a regular +copy of the non-excluded contents. A regular copy can be unsafe while databases +or other stateful services are running. Concurrent changes can still result in +inconsistent data, even with reflinks. Stop those services before copying. +Hardlinks wholly within one copied batch are preserved, but hardlinks crossing +a mountpoint or exclusion split, or separate copies while merging into an +already-existing destination directory, may not remain linked. The paths `tmp`, `cache`, `log/journal`, and `lib/containers` are always excluded. Use the repeatable `--preserve-var-skip` option to add exclusions relative to `/var`; each path must be either one component (for example, `lib`) or exactly one child (for example, `lib/pgsql`). `/var` itself and deeper paths such as `lib/pgsql/data` are not accepted. Excluded data is not migrated and must be handled separately. +The contents of nested mounts under `/var` are not migrated. Each non-excluded +nested mountpoint is represented by an empty placeholder; an explicit exclusion +takes precedence and omits the path entirely. Mount configuration merged into the +new `/etc`, such as `/etc/fstab`, can re-establish configured mounts at boot. +Runtime-only mounts need separate configuration. +Nested mountpoints are selected from a point-in-time, best-effort mountinfo +snapshot; mounts added or removed beneath `/var` during copying can race with +the copy, just like concurrent file changes. +Each missing destination entry is staged and published atomically without +replacing existing data. A failed entry leaves no partial final entry, but the +migration is not a global transaction: entries completed before a later failure +can remain and a retry will preserve them. `--merge-etc` performs a three-way merge that applies the running host's `/etc` customizations to the new deployment before its first reboot. Without this @@ -258,7 +272,14 @@ of migrating the fstab entries. See the "Injecting kernel arguments" section abo **--preserve-var** - Preserve the running system's `/var` data into the new bootc deployment + Preserve non-excluded `/var` data in the new deployment. Reflinks, when + supported, are per-file and do not provide a snapshot. Nested mount contents + are not copied; non-excluded mountpoints get empty placeholders, while an + explicit exclusion omits that path. Mount configuration merged into `/etc` + can restore configured mounts at boot; runtime-only mounts need separate + configuration. Each missing entry is staged and published atomically + without replacement; earlier complete entries can remain if a later entry + fails. **--preserve-var-skip**=*PATH* diff --git a/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh b/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh index 1587b91cb3..f3dce01fb1 100644 --- a/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh +++ b/tmt/tests/booted/package-mode/test-to-existing-root-preserve-var-merge-etc.sh @@ -5,7 +5,69 @@ set -xeuo pipefail target_image=localhost/bootc-to-existing-root-preserve-var-merge-etc:latest var_marker=/var/lib/bootc-tmt-preserve-var/sentinel +bind_dir=/var/lib/bootc-tmt-preserve-var-bind +bind_source=$bind_dir/source +bind_target=$bind_dir/target etc_dir=/etc/bootc-tmt-merge +tmt_plan_data_suffix=/tmt/plans/package-mode/data +tmt_workdir_parent=/var/tmp/tmt +tmt_workdir_root=/var/lib/bootc-tmt-workdirs +tmt_test_data_sentinel_name=workdir-persistence-sentinel +tmt_test_data_post_reboot_marker_name=post-reboot-marker + +get_tmt_run_dir() { + local plan_data=${TMT_PLAN_DATA:-} + local run_dir + local resolved_plan_data + local persistent_run_dir + local link_target + + if [[ "$plan_data" != *"$tmt_plan_data_suffix" ]]; then + printf 'TMT_PLAN_DATA must end in %s (got %q)\n' "$tmt_plan_data_suffix" "$plan_data" >&2 + return 1 + fi + run_dir=${plan_data%"$tmt_plan_data_suffix"} + if [[ ! $run_dir =~ ^/var/tmp/tmt/run-[[:alnum:]_.-]+$ || $run_dir == *..* ]]; then + printf 'Expected a safe TMT run directory under %s (got %q)\n' \ + "$tmt_workdir_parent" "$run_dir" >&2 + return 1 + fi + case ${TMT_REBOOT_COUNT:-0} in + 0) + if ! resolved_plan_data=$(realpath -e -- "$plan_data"); then + printf 'TMT_PLAN_DATA must be an existing path without symlinked components (got %q)\n' \ + "$plan_data" >&2 + return 1 + fi + if [[ $resolved_plan_data != "$plan_data" ]]; then + printf 'TMT_PLAN_DATA must be a canonical path without symlinked components (got %q, resolves to %q)\n' \ + "$plan_data" "$resolved_plan_data" >&2 + return 1 + fi + ;; + 1) + persistent_run_dir=$tmt_workdir_root/${run_dir##*/} + if [[ ! -L $run_dir ]] || ! link_target=$(readlink -- "$run_dir") || + [[ $link_target != "$persistent_run_dir" ]]; then + printf 'TMT run directory must link directly to %s (got %q)\n' \ + "$persistent_run_dir" "$(readlink -- "$run_dir" 2>/dev/null || true)" >&2 + return 1 + fi + if ! resolved_plan_data=$(realpath -e -- "$plan_data") || + [[ $resolved_plan_data != "$persistent_run_dir$tmt_plan_data_suffix" ]]; then + printf 'TMT_PLAN_DATA must resolve to %s (got %q, resolves to %q)\n' \ + "$persistent_run_dir$tmt_plan_data_suffix" "$plan_data" "${resolved_plan_data:-}" >&2 + return 1 + fi + ;; + *) + printf 'Unexpected TMT_REBOOT_COUNT=%s\n' "${TMT_REBOOT_COUNT}" >&2 + return 1 + ;; + esac + + printf '%s\n' "$run_dir" +} case ${TMT_REBOOT_COUNT:-0} in 0) @@ -16,7 +78,7 @@ case ${TMT_REBOOT_COUNT:-0} in # Exercise both an admin-created file and an admin modification of an # image default. The derived image below supplies the pristine values. - mkdir -p "$etc_dir" "${var_marker%/*}" + mkdir -p "$etc_dir" "${var_marker%/*}" "$bind_source" "$bind_target" # TMT connects as root. Package-mode hosts keep root's home at /root, # while bootc images link /root to /var/roothome, so preserve its key @@ -28,6 +90,123 @@ case ${TMT_REBOOT_COUNT:-0} in printf '%s\n' admin-modified | tee "$etc_dir/default.conf" printf '%s\n' admin-only | tee "$etc_dir/admin-only.conf" printf '%s\n' preserved-var | tee "$var_marker" + printf '%s\n' bind-source-data | tee "$bind_source/sentinel" + printf '%s\n' mounted-only-data | tee "$bind_source/mounted-sentinel" + printf '%s\n' underlying-target-data | tee "$bind_target/underlying-sentinel" + mount --bind "$bind_source" "$bind_target" + printf '%s\n' "$bind_source $bind_target none bind 0 0" >> /etc/fstab + + tmt_run_dir=$(get_tmt_run_dir) + tmt_run_id=${tmt_run_dir##*/} + if [[ ! -d "$tmt_run_dir" || -L "$tmt_run_dir" ]]; then + printf 'TMT run directory is missing or not a directory: %s\n' "$tmt_run_dir" >&2 + exit 1 + fi + tmt_run_dir_identity=$(stat -c '%d:%i' -- "$tmt_run_dir") + persistent_tmt_run_dir=$tmt_workdir_root/$tmt_run_id + if [[ -e "$persistent_tmt_run_dir" || -L "$persistent_tmt_run_dir" ]]; then + printf 'Persistent TMT workdir already exists: %s\n' "$persistent_tmt_run_dir" >&2 + exit 1 + fi + test -d "$TMT_TEST_DATA" + tmt_test_data_sentinel=$TMT_TEST_DATA/$tmt_test_data_sentinel_name + test ! -e "$tmt_test_data_sentinel" + printf '%s\n' workdir-persistence-sentinel > "$tmt_test_data_sentinel" + # --preserve-var excludes /var/tmp. Stage a full copy on /var, then + # redirect the original path so TMT's post-reboot writes survive. + install -d -m 0755 "$tmt_workdir_root" + staging_parent=$(mktemp -d "$tmt_workdir_root/.${tmt_run_id}.XXXXXX") + trap 'if [[ -n ${staging_parent:-} ]]; then rm -rf -- "$staging_parent"; fi' EXIT + cp -a -- "$tmt_run_dir" "$staging_parent/" + staged_run_dir=$staging_parent/$tmt_run_id + mv -T --no-clobber -- "$staged_run_dir" "$persistent_tmt_run_dir" + if [[ -e "$staged_run_dir" || -L "$staged_run_dir" ]]; then + printf 'Persistent TMT workdir appeared during staging: %s\n' \ + "$persistent_tmt_run_dir" >&2 + exit 1 + fi + rmdir -- "$staging_parent" + staging_parent= + trap - EXIT + + install -d -m 0755 /etc/tmpfiles.d + printf 'd %s 0755 root root -\nL+ %s - - - - %s\n' \ + "$tmt_workdir_parent" "$tmt_run_dir" "$persistent_tmt_run_dir" \ + > /etc/tmpfiles.d/bootc-tmt-workdir.conf + + disposable_run_dir=$tmt_workdir_parent/.${tmt_run_id}.original.$$ + if [[ -e "$disposable_run_dir" || -L "$disposable_run_dir" ]]; then + printf 'Disposable TMT workdir already exists: %s\n' "$disposable_run_dir" >&2 + exit 1 + fi + + tmt_run_dir_moved=0 + tmt_run_dir_swap_complete=0 + cleanup_tmt_run_dir_swap() { + local status=$? + local disposable_identity + local original_identity + + trap - EXIT HUP INT TERM + if (( tmt_run_dir_moved )); then + # A signal can arrive after ln succeeds but before the flag is + # updated, so recognize the completed swap from the link too. + if [[ -L $tmt_run_dir ]] && + [[ $(readlink -- "$tmt_run_dir") == "$persistent_tmt_run_dir" ]]; then + tmt_run_dir_swap_complete=1 + fi + + if (( ! tmt_run_dir_swap_complete )); then + disposable_identity=$(stat -c '%d:%i' -- "$disposable_run_dir" 2>/dev/null || true) + if [[ $disposable_identity == "$tmt_run_dir_identity" ]]; then + if [[ ! -e $tmt_run_dir && ! -L $tmt_run_dir ]]; then + # --no-clobber ensures a replacement path created after + # the existence check is never overwritten. + mv -T --no-clobber -- "$disposable_run_dir" "$tmt_run_dir" || true + original_identity=$(stat -c '%d:%i' -- "$tmt_run_dir" 2>/dev/null || true) + if [[ $original_identity != "$tmt_run_dir_identity" ]]; then + disposable_identity=$(stat -c '%d:%i' -- "$disposable_run_dir" 2>/dev/null || true) + if [[ $disposable_identity == "$tmt_run_dir_identity" ]]; then + printf 'Could not restore TMT workdir because no-replace skipped the move; the original remains at %s\n' \ + "$disposable_run_dir" >&2 + else + printf 'Failed to restore TMT workdir from %s to %s\n' \ + "$disposable_run_dir" "$tmt_run_dir" >&2 + fi + fi + else + printf 'Cannot restore TMT workdir because the original path now exists; the original remains at %s\n' \ + "$disposable_run_dir" >&2 + fi + elif [[ -e $disposable_run_dir || -L $disposable_run_dir ]]; then + printf 'Leaving replacement at disposable TMT workdir path untouched: %s\n' \ + "$disposable_run_dir" >&2 + fi + fi + fi + exit "$status" + } + trap cleanup_tmt_run_dir_swap EXIT + trap 'exit 1' HUP INT TERM + tmt_run_dir_moved=1 + mv -T --no-clobber -- "$tmt_run_dir" "$disposable_run_dir" + disposable_run_dir_identity=$(stat -c '%d:%i' -- "$disposable_run_dir" 2>/dev/null || true) + if [[ $disposable_run_dir_identity != "$tmt_run_dir_identity" ]]; then + tmt_run_dir_moved=0 + printf 'Failed to move original TMT workdir to %s; the destination may have appeared concurrently\n' \ + "$disposable_run_dir" >&2 + exit 1 + fi + ln -s -- "$persistent_tmt_run_dir" "$tmt_run_dir" + tmt_run_dir_swap_complete=1 + # Keep the old tree: it is disposable under /var/tmp and vanishes on + # reboot, while retaining it keeps this shell's cwd valid until we leave. + tmt_run_dir_moved=0 + trap - EXIT HUP INT TERM + + # The shell may still be inside the renamed workdir; leave it before + # installing or rebooting so the old cwd is no longer in use. + cd / # The flags require the OSTree backend. Run the current build-under-test # as a privileged installer, with the package-mode root mounted at /target. @@ -38,10 +217,32 @@ case ${TMT_REBOOT_COUNT:-0} in -v /var/lib/containers:/var/lib/containers \ --security-opt label=type:unconfined_t \ "$target_image" \ - bootc install to-existing-root \ - --preserve-var \ - --merge-etc \ - --acknowledge-destructive + sh -ec ' + test "$(cat /target/var/lib/bootc-tmt-preserve-var-bind/target/mounted-sentinel)" = mounted-only-data + test ! -e /target/var/lib/bootc-tmt-preserve-var-bind/target/underlying-sentinel + exec bootc install to-existing-root \ + --preserve-var \ + --merge-etc \ + --acknowledge-destructive + ' + + # Find the newly created stateroot by its copied bind-source sentinel. + # Auto-generated stateroot names are intentionally not assumed here. + new_var= + for candidate in /ostree/deploy/*/var; do + if [[ -f "$candidate${bind_source#/var}/sentinel" ]]; then + new_var=$candidate + break + fi + done + test -n "$new_var" + # Its /var has the ordinary sibling but only an empty directory for the + # bind mount itself. + test "$(<"$new_var/lib/bootc-tmt-preserve-var-bind/source/sentinel")" = bind-source-data + test -d "$new_var/lib/bootc-tmt-preserve-var-bind/target" + test -z "$(find "$new_var/lib/bootc-tmt-preserve-var-bind/target" -mindepth 1 -print -quit)" + test ! -e "$new_var/lib/bootc-tmt-preserve-var-bind/target/mounted-sentinel" + test ! -e "$new_var/lib/bootc-tmt-preserve-var-bind/target/underlying-sentinel" bootc status # Ask the guest to reboot itself; this is more reliable than @@ -51,6 +252,21 @@ case ${TMT_REBOOT_COUNT:-0} in 1) test -e /run/ostree-booted + tmt_run_dir=$(get_tmt_run_dir) + tmt_run_id=${tmt_run_dir##*/} + persistent_tmt_run_dir=$tmt_workdir_root/$tmt_run_id + test -L "$tmt_run_dir" + test "$(readlink -f -- "$tmt_run_dir")" = "$persistent_tmt_run_dir" + test -d "$TMT_PLAN_DATA" + test -d "$TMT_TEST_DATA" + test -f "$TMT_TEST_METADATA" + tmt_test_data_sentinel=$TMT_TEST_DATA/$tmt_test_data_sentinel_name + tmt_test_data_post_reboot_marker=$TMT_TEST_DATA/$tmt_test_data_post_reboot_marker_name + test -f "$tmt_test_data_sentinel" + test "$(<"$tmt_test_data_sentinel")" = workdir-persistence-sentinel + printf '%s\n' post-reboot-workdir-writable > "$tmt_test_data_post_reboot_marker" + test "$(<"$tmt_test_data_post_reboot_marker")" = post-reboot-workdir-writable + booted_image=$(bootc status --json | jq -r '.status.booted.image.image.image') test "$booted_image" = "$target_image" @@ -58,6 +274,8 @@ case ${TMT_REBOOT_COUNT:-0} in test "$(<"$etc_dir/default.conf")" = admin-modified test "$(<"$etc_dir/admin-only.conf")" = admin-only test "$(<"$etc_dir/image-only.conf")" = image-only + mountpoint -q "$bind_target" + test "$(<"$bind_target/mounted-sentinel")" = mounted-only-data ;; *) printf 'Unexpected TMT_REBOOT_COUNT=%s\n' "$TMT_REBOOT_COUNT" >&2 From e14890dbdc8b2b3625c0b43b3ad78d71908b1abf Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Thu, 15 Oct 2026 09:58:11 -0500 Subject: [PATCH 5/5] install: Reopen O_PATH dirs for var metadata The mount-aware preserve-var tests use O_PATH-backed directory capabilities. Reopen the pinned directory before fd-based metadata operations so these calls work without following mutable pathnames. Assisted-by: AI Signed-off-by: ckyrouac --- crates/lib/src/install/migrate.rs | 80 +++++++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 8 deletions(-) diff --git a/crates/lib/src/install/migrate.rs b/crates/lib/src/install/migrate.rs index 1a13192769..d13a30a42b 100644 --- a/crates/lib/src/install/migrate.rs +++ b/crates/lib/src/install/migrate.rs @@ -936,22 +936,21 @@ fn restore_directory_metadata( ) -> Result<()> { use rustix::fs::{Mode, fchmod}; - let dst_file = directory.as_filelike_view::(); - let dst_fd = dst_file.as_fd(); + let dst_fd = open_directory_for_metadata(directory)?; let mut errors = Vec::new(); - if let Err(error) = preserve_metadata_owner(&snapshot.metadata, dst_fd) { + if let Err(error) = preserve_metadata_owner(&snapshot.metadata, &dst_fd) { errors.push(format!("owner/group: {error:#}")); } if let Err(error) = fchmod( - dst_fd, + &dst_fd, Mode::from_bits_truncate(snapshot.metadata.mode() & 0o7777), ) { errors.push(format!("mode: {error:#}")); } - if let Err(error) = apply_xattrs_exact(dst_fd, &snapshot.xattrs) { + if let Err(error) = apply_xattrs_exact(&dst_fd, &snapshot.xattrs) { errors.push(format!("xattrs: {error:#}")); } - if let Err(error) = preserve_metadata_timestamps(&snapshot.metadata, dst_fd) { + if let Err(error) = preserve_metadata_timestamps(&snapshot.metadata, &dst_fd) { errors.push(format!("timestamps: {error:#}")); } if !errors.is_empty() { @@ -961,8 +960,23 @@ fn restore_directory_metadata( } fn restore_directory_timestamps(directory: &CapStdDir, metadata: &Metadata) -> Result<()> { - let dst_file = directory.as_filelike_view::(); - preserve_metadata_timestamps(metadata, dst_file.as_fd()) + let dst_fd = open_directory_for_metadata(directory)?; + preserve_metadata_timestamps(metadata, &dst_fd) +} + +/// Open a usable fd for fd-based metadata operations while remaining anchored +/// to the pinned directory capability. CapStdDir may hold an O_PATH fd, which +/// cannot be used with fchmod, xattr, or timestamp operations. +fn open_directory_for_metadata(directory: &CapStdDir) -> Result { + use rustix::fs::{Mode, OFlags, openat}; + + let directory_file = directory.as_filelike_view::(); + Ok(openat( + directory_file.as_fd(), + ".", + OFlags::CLOEXEC | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::RDONLY, + Mode::empty(), + )?) } /// Preserve ownership, mode, timestamps, and available xattrs (including ACL @@ -1273,6 +1287,20 @@ mod tests { CapStdDir::open_ambient_dir(path, ambient_authority()).unwrap() } + fn cap_dir_path(path: &Path) -> CapStdDir { + use rustix::fs::{Mode, OFlags, openat}; + + let directory = cap_dir(path); + let fd = openat( + directory.as_filelike_view::().as_fd(), + ".", + OFlags::CLOEXEC | OFlags::DIRECTORY | OFlags::PATH, + Mode::empty(), + ) + .unwrap(); + CapStdDir::from_std_file(std::fs::File::from(fd)) + } + #[test] fn exclusions_are_relative_and_normalized() { assert_eq!( @@ -1572,6 +1600,42 @@ mod tests { Ok(()) } + #[test] + fn directory_metadata_restoration_accepts_path_capability() -> Result<()> { + let temp = tempfile::tempdir()?; + let src_path = temp.path().join("src"); + let dst_path = temp.path().join("dst"); + fs::create_dir(&src_path)?; + fs::create_dir(&dst_path)?; + fs::set_permissions(&src_path, fs::Permissions::from_mode(0o751))?; + + let source = cap_dir(&src_path); + let has_user_xattr = source + .setxattr(".", "user.bootc_path_fd", b"metadata") + .is_ok(); + let destination = cap_dir_path(&dst_path); + let snapshot = snapshot_directory_metadata(&source)?; + restore_directory_metadata(&destination, &snapshot)?; + assert_eq!( + fs::metadata(&dst_path)?.permissions().mode() & 0o7777, + snapshot.metadata.mode() & 0o7777 + ); + if has_user_xattr { + assert_eq!( + destination.getxattr(".", "user.bootc_path_fd")?, + Some(b"metadata".to_vec()) + ); + } + + restore_directory_timestamps(&destination, &snapshot.metadata)?; + let restored = fs::metadata(&dst_path)?; + assert_eq!(restored.atime(), snapshot.metadata.atime()); + assert_eq!(restored.atime_nsec(), snapshot.metadata.atime_nsec()); + assert_eq!(restored.mtime(), snapshot.metadata.mtime()); + assert_eq!(restored.mtime_nsec(), snapshot.metadata.mtime_nsec()); + Ok(()) + } + #[test] fn filtered_copy_does_not_change_existing_destination_directory() -> Result<()> { let temp = tempfile::tempdir()?;