From fe364208ec773a3f991387aeae05532e41b3d185 Mon Sep 17 00:00:00 2001 From: Joseph Marrero Corchado Date: Wed, 23 Jul 2025 09:40:34 -0400 Subject: [PATCH] Revert "blockdev: implement signal-safe loopback device cleanup helper" This reverts commit c2c918cc805fe5cfcc9434e4da419b98460abb60. As it makes install to-disk fail: https://github.com/bootc-dev/bootc/issues/1439 --- Cargo.lock | 4 -- crates/blockdev/Cargo.toml | 4 -- crates/blockdev/src/blockdev.rs | 94 +-------------------------------- crates/lib/src/cli.rs | 9 ---- crates/lib/src/lib.rs | 3 -- 5 files changed, 1 insertion(+), 113 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 822826cc..f1aba1ba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -179,13 +179,9 @@ dependencies = [ "camino", "fn-error-context", "indoc", - "libc", "regex", - "rustix 1.0.3", "serde", "serde_json", - "tempfile", - "tokio", "tracing", ] diff --git a/crates/blockdev/Cargo.toml b/crates/blockdev/Cargo.toml index eee8dd70..bab2fe37 100644 --- a/crates/blockdev/Cargo.toml +++ b/crates/blockdev/Cargo.toml @@ -11,13 +11,9 @@ anyhow = { workspace = true } bootc-utils = { package = "bootc-internal-utils", path = "../utils", version = "0.0.0" } camino = { workspace = true, features = ["serde1"] } fn-error-context = { workspace = true } -libc = { workspace = true } regex = "1.10.4" -rustix = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } -tempfile = { workspace = true } -tokio = { workspace = true, features = ["signal"] } tracing = { workspace = true } [dev-dependencies] diff --git a/crates/blockdev/src/blockdev.rs b/crates/blockdev/src/blockdev.rs index 54198799..667defe0 100644 --- a/crates/blockdev/src/blockdev.rs +++ b/crates/blockdev/src/blockdev.rs @@ -181,14 +181,6 @@ pub fn partitions_of(dev: &Utf8Path) -> Result { pub struct LoopbackDevice { pub dev: Option, - // Handle to the cleanup helper process - cleanup_handle: Option, -} - -/// Handle to manage the cleanup helper process for loopback devices -struct LoopbackCleanupHandle { - /// Child process handle - child: std::process::Child, } impl LoopbackDevice { @@ -216,15 +208,7 @@ impl LoopbackDevice { .run_get_string()?; let dev = Utf8PathBuf::from(dev.trim()); tracing::debug!("Allocated loopback {dev}"); - - // Try to spawn cleanup helper process - if it fails, make it fatal - let cleanup_handle = Self::spawn_cleanup_helper(dev.as_str()) - .context("Failed to spawn loopback cleanup helper")?; - - Ok(Self { - dev: Some(dev), - cleanup_handle: Some(cleanup_handle), - }) + Ok(Self { dev: Some(dev) }) } // Access the path to the loopback block device. @@ -233,35 +217,6 @@ impl LoopbackDevice { self.dev.as_deref().unwrap() } - /// Spawn a cleanup helper process that will clean up the loopback device - /// if the parent process dies unexpectedly - fn spawn_cleanup_helper(device_path: &str) -> Result { - use std::process::{Command, Stdio}; - - // Get the path to our own executable - let self_exe = - std::fs::read_link("/proc/self/exe").context("Failed to read /proc/self/exe")?; - - // Create the helper process - let mut cmd = Command::new(self_exe); - cmd.args(["loopback-cleanup-helper", "--device", device_path]); - - // Set environment variable to indicate this is a cleanup helper - cmd.env("BOOTC_LOOPBACK_CLEANUP_HELPER", "1"); - - // Set up stdio to redirect to /dev/null - cmd.stdin(Stdio::null()); - cmd.stdout(Stdio::null()); - cmd.stderr(Stdio::null()); - - // Spawn the process - let child = cmd - .spawn() - .context("Failed to spawn loopback cleanup helper")?; - - Ok(LoopbackCleanupHandle { child }) - } - // Shared backend for our `close` and `drop` implementations. fn impl_close(&mut self) -> Result<()> { // SAFETY: This is the only place we take the option @@ -269,13 +224,6 @@ impl LoopbackDevice { tracing::trace!("loopback device already deallocated"); return Ok(()); }; - - // Kill the cleanup helper since we're cleaning up normally - if let Some(mut cleanup_handle) = self.cleanup_handle.take() { - // Send SIGTERM to the child process - let _ = cleanup_handle.child.kill(); - } - Command::new("losetup").args(["-d", dev.as_str()]).run() } @@ -292,46 +240,6 @@ impl Drop for LoopbackDevice { } } -/// Main function for the loopback cleanup helper process -/// This function does not return - it either exits normally or via signal -pub async fn run_loopback_cleanup_helper(device_path: &str) -> Result<()> { - // Check if we're running as a cleanup helper - if std::env::var("BOOTC_LOOPBACK_CLEANUP_HELPER").is_err() { - anyhow::bail!("This function should only be called as a cleanup helper"); - } - - // Set up death signal notification - we want to be notified when parent dies - rustix::process::set_parent_process_death_signal(Some(rustix::process::Signal::TERM)) - .context("Failed to set parent death signal")?; - - // Wait for SIGTERM (either from parent death or normal cleanup) - tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate()) - .expect("Failed to create signal stream") - .recv() - .await; - - // Clean up the loopback device - let status = std::process::Command::new("losetup") - .args(["-d", device_path]) - .status(); - - match status { - Ok(exit_status) if exit_status.success() => { - // Log to systemd journal instead of stderr - tracing::info!("Cleaned up leaked loopback device {}", device_path); - std::process::exit(0); - } - Ok(_) => { - tracing::error!("Failed to clean up loopback device {}", device_path); - std::process::exit(1); - } - Err(e) => { - tracing::error!("Error cleaning up loopback device {}: {}", device_path, e); - std::process::exit(1); - } - } -} - /// Parse key-value pairs from lsblk --pairs. /// Newer versions of lsblk support JSON but the one in CentOS 7 doesn't. fn split_lsblk_line(line: &str) -> HashMap { diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index df5d4171..d6c67644 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -462,12 +462,6 @@ pub(crate) enum InternalsOpts { #[clap(allow_hyphen_values = true)] args: Vec, }, - /// Loopback device cleanup helper (internal use only) - LoopbackCleanupHelper { - /// Device path to clean up - #[clap(long)] - device: String, - }, /// Invoked from ostree-ext to complete an installation. BootcInstallCompletion { /// Path to the sysroot @@ -1269,9 +1263,6 @@ async fn run_from_opt(opt: Opt) -> Result<()> { let rootfs = &Dir::open_ambient_dir("/", cap_std::ambient_authority())?; crate::install::completion::run_from_ostree(rootfs, &sysroot, &stateroot).await } - InternalsOpts::LoopbackCleanupHelper { device } => { - crate::blockdev::run_loopback_cleanup_helper(&device).await - } #[cfg(feature = "rhsm")] InternalsOpts::PublishRhsmFacts => crate::rhsm::publish_facts(&root).await, }, diff --git a/crates/lib/src/lib.rs b/crates/lib/src/lib.rs index b5343485..37bce148 100644 --- a/crates/lib/src/lib.rs +++ b/crates/lib/src/lib.rs @@ -38,6 +38,3 @@ mod kernel; #[cfg(feature = "rhsm")] mod rhsm; - -// Re-export blockdev crate for internal use -pub(crate) use bootc_blockdev as blockdev;