From d40301b9f0ae69b6c0a2b5cf97a6c67095efd982 Mon Sep 17 00:00:00 2001 From: Vineeth Sai Narajala Date: Mon, 2 Nov 2026 20:25:50 +0000 Subject: [PATCH 1/5] fix(vm): recover full overlay disks before boot Signed-off-by: Vineeth Sai Narajala --- crates/openshell-driver-vm/README.md | 15 +- crates/openshell-driver-vm/src/driver.rs | 58 +++- crates/openshell-driver-vm/src/rootfs.rs | 295 ++++++++++++++++++- docs/how-it-works/gateways/configuration.mdx | 3 + docs/how-it-works/sandboxes/runtimes.mdx | 11 + skills/debug-openshell-cluster/SKILL.md | 3 +- 6 files changed, 372 insertions(+), 13 deletions(-) diff --git a/crates/openshell-driver-vm/README.md b/crates/openshell-driver-vm/README.md index a0371bacfb..6464438a7d 100644 --- a/crates/openshell-driver-vm/README.md +++ b/crates/openshell-driver-vm/README.md @@ -247,6 +247,19 @@ the overlay while cached image disks remain unchanged. The overlay disk must be large enough to hold the compressed payload, unpacked rootfs, and sandbox writes during the first prepare. +On restart, the driver checks a preserved overlay with `e2fsck` before writing +new launch files. If fewer than 32 MiB are available to the sandbox after the +check, it grows the sparse disk and ext4 filesystem by 64 MiB before booting. +If preen stops on an unattached inode in a nearly full overlay, the driver runs +noninteractive repair. Failures without that signal still stop startup. +The driver retains the overlay image and publishes an `OverlayDiskFull` +warning when it grows the disk. Repeated full-disk restarts can add another +64 MiB each time. `overlay_disk_mib` sets the initial size of new overlays; it +is not a hard cap on recovery growth, and changing it does not resize existing +overlays. Recovery requires `resize2fs` on the gateway host. If repair or +growth fails, the start error names `OverlayDiskFull` and the config key, and +the existing overlay is retained for another attempt. + The driver also writes the accepted `DriverSandbox` launch request to `/sandboxes//sandbox.pb`. If the gateway restarts, it starts a new VM driver process. During graceful shutdown, the gateway first sends the @@ -297,7 +310,7 @@ create TAP devices or install nftables/iptables rules. - macOS on Apple Silicon, or Linux on aarch64/x86_64 with KVM - Rust toolchain -- e2fsprogs (`mke2fs` or `mkfs.ext4`, plus `debugfs`) for root and overlay disk image creation and QEMU environment injection +- e2fsprogs (`mke2fs` or `mkfs.ext4`, `debugfs`, `e2fsck`, and `resize2fs`) for root and overlay disk image creation and recovery - Guest-sandbox cross-compile toolchain (needed on macOS, and on Linux when host arch differs from the guest): - Matching static guest target: `rustup target add aarch64-unknown-linux-musl` (or `x86_64-unknown-linux-musl` for an amd64 guest) - `cargo install --locked cargo-zigbuild` and `brew install zig` (or distro equivalent). `vm:supervisor` cross-compiles the Linux guest `openshell-sandbox` and its matching `openshell-supervisor`. diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 6260a52842..4fcb458e4a 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -1815,7 +1815,11 @@ impl VmDriver { self.fail_provisioning( &sandbox_id, &state_dir, - "ProvisioningFailed", + if err.message().contains("OverlayDiskFull:") { + "OverlayDiskFull" + } else { + "ProvisioningFailed" + }, err.message(), overlay_preparation == OverlayPreparation::Fresh, ) @@ -1932,6 +1936,10 @@ impl VmDriver { "Preparing writable VM overlay disk".to_string(), ), ); + let previous_overlay_size = tokio::fs::metadata(&overlay_disk) + .await + .ok() + .map(|m| m.len()); let sandbox_owner_state = self .prepare_overlay_in_worker( &sandbox.id, @@ -1944,6 +1952,26 @@ impl VmDriver { ) .await .map_err(|err| Status::internal(format!("prepare guest overlay disk failed: {err}")))?; + if let Some(previous_size) = previous_overlay_size { + let recovered_size = tokio::fs::metadata(&overlay_disk) + .await + .map_err(|error| Status::internal(format!("stat recovered overlay: {error}")))? + .len(); + if recovered_size > previous_size { + self.publish_platform_event( + sandbox.id.clone(), + platform_event( + "vm", + "Warning", + "OverlayDiskFull", + format!( + "VM overlay was nearly full and grew by {} MiB for recovery; remove files or increase [openshell.drivers.vm] overlay_disk_mib for new sandboxes", + (recovered_size - previous_size) / (1024 * 1024) + ), + ), + ); + } + } self.ensure_provisioning_active(&sandbox.id).await?; if let Err(err) = @@ -7598,12 +7626,12 @@ fn prepare_sandbox_overlay_image( ) -> Result<(), String> { if preparation == OverlayPreparation::PreserveExisting { match fs::metadata(overlay_disk) { - Ok(metadata) if metadata.is_file() && metadata.len() == expected_size_bytes => { + Ok(metadata) if metadata.is_file() && metadata.len() >= expected_size_bytes => { return Ok(()); } Ok(metadata) if metadata.is_file() => { return Err(format!( - "existing overlay disk '{}' has size {}, expected {}", + "existing overlay disk '{}' has size {}, expected at least {}", overlay_disk.display(), metadata.len(), expected_size_bytes @@ -11318,6 +11346,30 @@ mod tests { let _ = std::fs::remove_dir_all(base); } + #[test] + fn prepare_sandbox_overlay_preserves_recovery_growth_on_later_starts() { + let base = unique_temp_dir(); + std::fs::create_dir_all(&base).unwrap(); + let template = base.join("template.ext4"); + let overlay = base.join("overlay.ext4"); + std::fs::write(&template, b"fresh-overlay").unwrap(); + std::fs::write(&overlay, b"saved-overlay-with-recovery-space").unwrap(); + + prepare_sandbox_overlay_image( + &template, + &overlay, + OverlayPreparation::PreserveExisting, + "saved-overlay".len() as u64, + ) + .expect("preserve grown overlay"); + + assert_eq!( + std::fs::read(&overlay).unwrap(), + b"saved-overlay-with-recovery-space" + ); + let _ = std::fs::remove_dir_all(base); + } + #[test] fn prepare_sandbox_overlay_creates_missing_overlay_on_start() { let base = unique_temp_dir(); diff --git a/crates/openshell-driver-vm/src/rootfs.rs b/crates/openshell-driver-vm/src/rootfs.rs index 138c7e1a90..cee3441b2e 100644 --- a/crates/openshell-driver-vm/src/rootfs.rs +++ b/crates/openshell-driver-vm/src/rootfs.rs @@ -30,6 +30,8 @@ const SANDBOX_VM_INIT_PATH: &str = "/opt/openshell/bin/openshell-vm-init"; const ROOTFS_IMAGE_MIN_SIZE_BYTES: u64 = 512 * 1024 * 1024; const ROOTFS_IMAGE_MIN_HEADROOM_BYTES: u64 = 256 * 1024 * 1024; const EXT4_IMAGE_MIN_HEADROOM_BYTES: u64 = 16 * 1024 * 1024; +const OVERLAY_RECOVERY_MIN_AVAILABLE_BYTES: u64 = 32 * 1024 * 1024; +const OVERLAY_RECOVERY_GROWTH_BYTES: u64 = 64 * 1024 * 1024; static INJECTION_COUNTER: AtomicU64 = AtomicU64::new(0); pub const fn sandbox_guest_init_path() -> &'static str { @@ -367,23 +369,202 @@ pub fn set_rootfs_image_file_mode( ) } -/// Replay the ext4 journal and repair automatically correctable filesystem -/// state before the driver mutates a preserved guest disk offline. +/// Repair a preserved overlay before the driver mutates it offline. A full +/// overlay needs space for guest init even when the workload has stopped. pub fn recover_rootfs_image(image_path: &Path) -> Result<(), String> { + let initial_space = ext4_image_space(image_path).ok(); + let full_overlay = initial_space + .is_some_and(|space| space.available_bytes < OVERLAY_RECOVERY_MIN_AVAILABLE_BYTES); let mut command = e2fs_command("e2fsck")?; let path = PathBuf::from(command.get_program()); - let output = command.arg("-p").arg("-f").arg(image_path).output(); - match output { - Ok(output) if matches!(output.status.code(), Some(0..=2)) => Ok(()), - Ok(output) => Err(format!( + let output = command + .env("LC_ALL", "C") + .arg("-p") + .arg("-f") + .arg(image_path) + .output() + .map_err(|error| { + if full_overlay { + format!( + "OverlayDiskFull: [openshell.drivers.vm] overlay_disk_mib sizes new overlays; run {}: {error}", + path.display() + ) + } else { + format!("run {}: {error}", path.display()) + } + })?; + if !matches!(output.status.code(), Some(0..=3)) { + let error = format!( "{} failed with status {}\nstdout: {}\nstderr: {}", path.display(), output.status, String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr) - )), - Err(error) => Err(format!("run {}: {error}", path.display())), + ); + // Preen refuses some orphaned inodes left by a failed full-disk boot. + // Restrict noninteractive repair to overlays that were already short + // of usable space before fsck; other corruption still requires review. + if !full_overlay + || !matches!(output.status.code(), Some(4..=7)) + || !String::from_utf8_lossy(&output.stdout).contains("Unattached inode") + { + return Err(if full_overlay { + format!( + "OverlayDiskFull: filesystem repair needs review; [openshell.drivers.vm] overlay_disk_mib sizes new overlays: {error}" + ) + } else { + error + }); + } + let repair = Command::new(&path) + .env("LC_ALL", "C") + .arg("-y") + .arg("-f") + .arg(image_path) + .output() + .map_err(|error| { + format!( + "OverlayDiskFull: [openshell.drivers.vm] overlay_disk_mib sizes new overlays; run {}: {error}", + path.display() + ) + })?; + if !matches!(repair.status.code(), Some(0..=3)) { + return Err(format!( + "OverlayDiskFull: filesystem repair failed; [openshell.drivers.vm] overlay_disk_mib sizes new overlays. {error}\nrepair stdout: {}\nrepair stderr: {}", + String::from_utf8_lossy(&repair.stdout), + String::from_utf8_lossy(&repair.stderr) + )); + } + } + + let space = ext4_image_space(image_path)?; + if space.available_bytes >= OVERLAY_RECOVERY_MIN_AVAILABLE_BYTES { + return Ok(()); + } + grow_full_overlay(image_path, space).map_err(|error| { + format!( + "OverlayDiskFull: cannot provide space for guest init; [openshell.drivers.vm] overlay_disk_mib sizes new overlays: {error}" + ) + }) +} + +#[derive(Clone, Copy)] +struct Ext4ImageSpace { + available_bytes: u64, + filesystem_bytes: u64, +} + +fn ext4_image_space(image_path: &Path) -> Result { + // ext4's primary superblock starts 1024 bytes into the image. Read only + // its fixed-width counters; e2fsck validates the filesystem itself. + let mut image = File::open(image_path) + .map_err(|error| format!("open overlay {}: {error}", image_path.display()))?; + image + .seek(SeekFrom::Start(1024)) + .map_err(|error| format!("seek overlay superblock {}: {error}", image_path.display()))?; + let mut superblock = [0_u8; 0x15c]; + image + .read_exact(&mut superblock) + .map_err(|error| format!("read overlay superblock {}: {error}", image_path.display()))?; + let u32_at = |offset: usize| { + u32::from_le_bytes([ + superblock[offset], + superblock[offset + 1], + superblock[offset + 2], + superblock[offset + 3], + ]) + }; + if u16::from_le_bytes([superblock[0x38], superblock[0x39]]) != 0xef53 { + return Err(format!("overlay {} is not ext4", image_path.display())); + } + let block_size = 1024_u64 + .checked_shl(u32_at(0x18)) + .filter(|size| *size <= 65_536) + .ok_or_else(|| format!("invalid ext4 block size in {}", image_path.display()))?; + let has_64bit_counts = u32_at(0x60) & 0x80 != 0; + let count = |low: usize, high: usize| { + u64::from(u32_at(low)) + | if has_64bit_counts { + u64::from(u32_at(high)) << 32 + } else { + 0 + } + }; + let blocks = count(0x04, 0x150); + let reserved = count(0x08, 0x154); + let free = count(0x0c, 0x158); + if blocks == 0 || reserved > blocks || free > blocks { + return Err(format!( + "invalid ext4 block counts in {}", + image_path.display() + )); + } + let filesystem_bytes = blocks + .checked_mul(block_size) + .ok_or_else(|| format!("ext4 size overflows in {}", image_path.display()))?; + let image_bytes = image + .metadata() + .map_err(|error| format!("stat overlay {}: {error}", image_path.display()))? + .len(); + if filesystem_bytes > image_bytes { + return Err(format!( + "ext4 exceeds overlay image {}", + image_path.display() + )); } + let available_bytes = free + .saturating_sub(reserved) + .checked_mul(block_size) + .ok_or_else(|| format!("ext4 free space overflows in {}", image_path.display()))?; + Ok(Ext4ImageSpace { + available_bytes, + filesystem_bytes, + }) +} + +fn grow_full_overlay(image_path: &Path, space: Ext4ImageSpace) -> Result<(), String> { + let image = fs::OpenOptions::new() + .write(true) + .open(image_path) + .map_err(|error| format!("open overlay {} for growth: {error}", image_path.display()))?; + let current_size = image + .metadata() + .map_err(|error| format!("stat overlay {}: {error}", image_path.display()))? + .len(); + let target_size = space + .filesystem_bytes + .checked_add(OVERLAY_RECOVERY_GROWTH_BYTES) + .ok_or_else(|| "overlay recovery size overflows".to_string())?; + if current_size < target_size { + image + .set_len(target_size) + .and_then(|()| image.sync_all()) + .map_err(|error| format!("extend overlay {}: {error}", image_path.display()))?; + } + drop(image); + + let mut command = e2fs_command("resize2fs")?; + let path = PathBuf::from(command.get_program()); + let output = command + .arg(image_path) + .output() + .map_err(|error| format!("run {}: {error}", path.display()))?; + if !output.status.success() { + return Err(format!( + "{} failed with status {}\nstdout: {}\nstderr: {}", + path.display(), + output.status, + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + )); + } + if ext4_image_space(image_path)?.available_bytes < OVERLAY_RECOVERY_MIN_AVAILABLE_BYTES { + return Err(format!( + "overlay {} remains short of free space after resize", + image_path.display() + )); + } + Ok(()) } #[cfg(target_os = "macos")] @@ -1570,6 +1751,104 @@ mod tests { let _ = fs::remove_dir_all(&dir); } + #[test] + fn recover_rootfs_image_grows_nearly_full_overlay_and_keeps_files() { + if ["e2fsck", "debugfs", "resize2fs"] + .iter() + .any(|tool| e2fs_command(tool).is_err()) + { + return; + } + + let dir = unique_temp_dir(); + let source = dir.join("source"); + let image = dir.join("overlay.ext4"); + fs::create_dir_all(source.join("upper")).expect("create source upperdir"); + fs::create_dir_all(source.join("work")).expect("create source workdir"); + fs::write(source.join("upper/work.txt"), b"important work\n").expect("write work"); + fs::write(source.join("upper/fill"), vec![b'X'; 44 * 1024 * 1024]).expect("write filler"); + create_ext4_image_from_dir_with_size(&source, &image, 64 * 1024 * 1024) + .expect("create nearly full ext4 image"); + + recover_rootfs_image(&image).expect("recover nearly full overlay"); + recover_rootfs_image(&image).expect("restart recovered overlay"); + + assert_eq!( + fs::metadata(&image).expect("stat recovered image").len(), + 128 * 1024 * 1024 + ); + let output = e2fs_command("debugfs") + .expect("debugfs") + .arg("-R") + .arg("cat /upper/work.txt") + .arg(&image) + .output() + .expect("read work from image"); + assert!(output.status.success(), "{output:?}"); + assert_eq!(output.stdout, b"important work\n"); + + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn recover_rootfs_image_repairs_unattached_inode_on_full_overlay() { + if ["e2fsck", "debugfs", "resize2fs"] + .iter() + .any(|tool| e2fs_command(tool).is_err()) + { + return; + } + + let dir = unique_temp_dir(); + let source = dir.join("source"); + let image = dir.join("overlay.ext4"); + fs::create_dir_all(source.join("upper")).expect("create source upperdir"); + fs::create_dir_all(source.join("work")).expect("create source workdir"); + fs::write(source.join("upper/work.txt"), b"important work\n").expect("write work"); + fs::write(source.join("upper/orphan.txt"), b"orphaned work\n").expect("write orphan"); + fs::write(source.join("upper/fill"), vec![b'X'; 44 * 1024 * 1024]).expect("write filler"); + create_ext4_image_from_dir_with_size(&source, &image, 64 * 1024 * 1024) + .expect("create nearly full ext4 image"); + let unlink = e2fs_command("debugfs") + .expect("debugfs") + .arg("-w") + .arg("-R") + .arg("unlink /upper/orphan.txt") + .arg(&image) + .output() + .expect("unlink image file"); + assert!(unlink.status.success(), "{unlink:?}"); + let probe_image = dir.join("probe.ext4"); + clone_or_copy_sparse_file(&image, &probe_image).expect("copy image for preen probe"); + let preen = e2fs_command("e2fsck") + .expect("e2fsck") + .env("LC_ALL", "C") + .arg("-p") + .arg("-f") + .arg(&probe_image) + .output() + .expect("check orphaned image"); + assert_eq!(preen.status.code(), Some(4), "{preen:?}"); + assert!( + String::from_utf8_lossy(&preen.stdout).contains("Unattached inode"), + "{preen:?}" + ); + + recover_rootfs_image(&image).expect("repair full overlay with orphaned inode"); + + let work = e2fs_command("debugfs") + .expect("debugfs") + .arg("-R") + .arg("cat /upper/work.txt") + .arg(&image) + .output() + .expect("read saved work"); + assert!(work.status.success(), "{work:?}"); + assert_eq!(work.stdout, b"important work\n"); + + let _ = fs::remove_dir_all(&dir); + } + #[test] fn ext4_image_has_directory_distinguishes_directories_files_and_missing_paths() { if e2fs_command("debugfs").is_err() { diff --git a/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index 3d5680301d..5631ffbf5a 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -1597,6 +1597,7 @@ bootstrap_image = "nvcr.io/nvidia/base/ubuntu:24.04" krun_log_level = 1 vcpus = 2 mem_mib = 2048 +# Initial size for new overlays; full-disk recovery can grow an existing overlay. overlay_disk_mib = 4096 # Resolved sandbox UID/GID for new rootfs /etc/passwd entries. # Defaults to the image's sandbox account, or 1000 when the account is absent; @@ -1736,6 +1737,8 @@ systemctl --user restart openshell-gateway For an explicitly selected local `vm` driver, preflight also checks a formatter (`mke2fs` or `mkfs.ext4`), `debugfs`, and `e2fsck`. Install e2fsprogs 1.43 or newer with your operating system's package manager. OpenShell does not install or manage these host dependencies. +Full-overlay recovery also uses `resize2fs` from e2fsprogs. The current preflight does not check this tool; ensure it is available to the gateway service before relying on recovery. + For example, on macOS: ```shell diff --git a/docs/how-it-works/sandboxes/runtimes.mdx b/docs/how-it-works/sandboxes/runtimes.mdx index 70f6891ce1..4b48fa0b9f 100644 --- a/docs/how-it-works/sandboxes/runtimes.mdx +++ b/docs/how-it-works/sandboxes/runtimes.mdx @@ -244,6 +244,17 @@ When you run `openshell sandbox stop` while the main process is running, OpenShe Flush errors and timeouts produce warnings while shutdown continues. If a signal kills the supervisor, the driver warns that the guest flush was not confirmed and recent writes may be lost. A completed stop does not guarantee persistence after a failed or timed-out flush. This flush does not cover stops after the main process has already exited. +When you start a stopped MicroVM sandbox, the driver checks its overlay filesystem. If fewer than 32 MiB are usable after repair, it grows the sparse disk by 64 MiB before booting. The sandbox watch stream reports an `OverlayDiskFull` warning when the disk grows. If a previous start left the sandbox in Error, stop it before retrying. After it reaches Ready, download its work and remove the files that filled the disk: + +```shell +openshell sandbox stop full +openshell sandbox start full +openshell sandbox download full /sandbox/work ./work +openshell sandbox exec full -- rm -f /sandbox/fill +``` + +If repair or growth fails, start reports `OverlayDiskFull` and retains the overlay for another attempt. Recovery needs `resize2fs` and free space on the gateway host. Each later full-disk restart can grow the disk again. `overlay_disk_mib` sets the initial size of new sandboxes, not a limit on recovery growth; changing it does not resize an existing overlay. + ### MicroVM Mounts The VM driver shares gateway-host directories into the guest with virtiofs. Each entry in `mounts` takes `source` (absolute host directory), `target`, optional `read_only` (default `true`), and optional `type`, which must be `bind` so Docker and Podman bind entries work unchanged: diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index 9e56f7117c..72cf910ae0 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -1487,7 +1487,8 @@ Use the VM driver logs and host diagnostics available in the user's environment. - The VM driver process is running and reachable by the gateway. - The runtime rootfs exists and matches the expected architecture. -- `mke2fs` or `mkfs.ext4`, `debugfs`, and `e2fsck` from e2fsprogs are installed. Run `openshell-gateway config preflight` with the intended local VM configuration, service account, and environment to check the selected paths and versions before startup. A remote endpoint reports that host checks were not performed. Explicit `sandbox_uid`/`sandbox_gid` does not remove this prerequisite. +- `mke2fs` or `mkfs.ext4`, `debugfs`, and `e2fsck` from e2fsprogs are installed. Run `openshell-gateway config preflight` with the intended local VM configuration, service account, and environment to check the selected paths and versions before startup. Full-overlay recovery also needs `resize2fs`, which preflight does not check. A remote endpoint reports that host checks were not performed. Explicit `sandbox_uid`/`sandbox_gid` does not remove this prerequisite. +- If a stopped VM sandbox reports `OverlayDiskFull`, check host free space and `resize2fs`, then retry start. Once it reaches Ready, download its work and remove the files that filled the overlay. The driver retains the overlay if repair or growth fails. - A persisted overlay identity error is resolved from its owner marker, overlay upper layer, prepared rootfs, explicit config, or current image. Do not assign `10001:10001` unless the persisted state reports that legacy identity. From 9618d3f4677190ba59b1f1bb13179effd667517f Mon Sep 17 00:00:00 2001 From: Vineeth Sai Narajala Date: Mon, 2 Nov 2026 21:45:42 +0000 Subject: [PATCH 2/5] fix(vm): preserve overlays and narrow orphan repair Signed-off-by: Vineeth Sai Narajala --- crates/openshell-driver-vm/Cargo.toml | 2 +- crates/openshell-driver-vm/README.md | 5 +- crates/openshell-driver-vm/src/driver.rs | 79 +++++++++++++++++++++--- crates/openshell-driver-vm/src/rootfs.rs | 57 +++++++++++++++-- 4 files changed, 126 insertions(+), 17 deletions(-) diff --git a/crates/openshell-driver-vm/Cargo.toml b/crates/openshell-driver-vm/Cargo.toml index 54a24e416a..9c320aedd0 100644 --- a/crates/openshell-driver-vm/Cargo.toml +++ b/crates/openshell-driver-vm/Cargo.toml @@ -62,6 +62,7 @@ tar = { version = "0.4", optional = true } flate2 = { version = "1", optional = true } sha2 = { version = "0.10", optional = true } zstd = { version = "0.14", optional = true } +tempfile = "3" [features] # Portable test instrumentation, independent of the compute runtime. @@ -117,7 +118,6 @@ defaults-without-telemetry = ["compute-driver"] [dev-dependencies] openshell-otel-test-support = { path = "../openshell-otel-test-support" } temp-env = "0.3" -tempfile = "3" rcgen = { workspace = true } opentelemetry_sdk = { workspace = true, features = ["testing"] } diff --git a/crates/openshell-driver-vm/README.md b/crates/openshell-driver-vm/README.md index 6464438a7d..292c59d8c8 100644 --- a/crates/openshell-driver-vm/README.md +++ b/crates/openshell-driver-vm/README.md @@ -250,8 +250,9 @@ during the first prepare. On restart, the driver checks a preserved overlay with `e2fsck` before writing new launch files. If fewer than 32 MiB are available to the sandbox after the check, it grows the sparse disk and ext4 filesystem by 64 MiB before booting. -If preen stops on an unattached inode in a nearly full overlay, the driver runs -noninteractive repair. Failures without that signal still stop startup. +If preen stops on an unattached inode in a nearly full overlay, the driver +retries preen with orphan reconnection enabled. Other repairs that require +interaction still stop startup. The driver retains the overlay image and publishes an `OverlayDiskFull` warning when it grows the disk. Repeated full-disk restarts can add another 64 MiB each time. `overlay_disk_mib` sets the initial size of new overlays; it diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 4fcb458e4a..c5519b16e6 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -3690,7 +3690,9 @@ impl VmDriver { && overlay_metadata.as_ref().is_ok_and(fs::Metadata::is_file); let publish_new_overlay = preparation == OverlayPreparation::Fresh || matches!(overlay_metadata, Err(error) if error.kind() == std::io::ErrorKind::NotFound); - if !overlay_template_image_ready(&template_path, overlay_size_bytes).await? { + if publish_new_overlay + && !overlay_template_image_ready(&template_path, overlay_size_bytes).await? + { let _cache_guard = self.image_cache_lock.lock().await; let template_path = template_path.clone(); let staging_dir = self.image_staging_dir("overlay-template"); @@ -7626,16 +7628,8 @@ fn prepare_sandbox_overlay_image( ) -> Result<(), String> { if preparation == OverlayPreparation::PreserveExisting { match fs::metadata(overlay_disk) { - Ok(metadata) if metadata.is_file() && metadata.len() >= expected_size_bytes => { - return Ok(()); - } Ok(metadata) if metadata.is_file() => { - return Err(format!( - "existing overlay disk '{}' has size {}, expected at least {}", - overlay_disk.display(), - metadata.len(), - expected_size_bytes - )); + return Ok(()); } Ok(_) => { return Err(format!( @@ -7653,6 +7647,15 @@ fn prepare_sandbox_overlay_image( } } + let template_metadata = fs::metadata(template_path) + .map_err(|error| format!("stat overlay template {}: {error}", template_path.display()))?; + if !template_metadata.is_file() || template_metadata.len() != expected_size_bytes { + return Err(format!( + "overlay template '{}' must be a {} byte file", + template_path.display(), + expected_size_bytes + )); + } create_sandbox_overlay_image_from_template(template_path, overlay_disk) } @@ -11370,6 +11373,62 @@ mod tests { let _ = std::fs::remove_dir_all(base); } + #[tokio::test] + async fn overlay_recovery_preserves_smaller_disk_after_default_increase() { + if ["mke2fs", "e2fsck", "debugfs"] + .iter() + .any(|tool| openshell_core::e2fsprogs::resolve(&[tool]).is_err()) + { + return; + } + + let base = unique_temp_dir(); + let source = base.join("source"); + let overlay = base.join("overlay.ext4"); + std::fs::create_dir_all(source.join("upper")).unwrap(); + std::fs::create_dir_all(source.join("work")).unwrap(); + std::fs::write(source.join("upper/work.txt"), b"saved work\n").unwrap(); + create_ext4_image_from_dir_with_size(&source, &overlay, 64 * 1024 * 1024).unwrap(); + write_sandbox_owner_state( + &base, + SandboxOwnerIdentity { + uid: 1000, + gid: 1001, + }, + ) + .await + .unwrap(); + + let mut driver = test_driver_with_extensions(LifecycleExtensionRegistry::new()); + driver.config.state_dir = base.clone(); + driver.config.overlay_disk_mib = 128; + let template = overlay_template_image(&base, 128 * 1024 * 1024); + driver + .prepare_runtime_overlay( + &base, + &overlay, + Path::new("/must-not-read-image"), + OverlayPreparation::PreserveExisting, + None, + ) + .await + .expect("start disk created under smaller default"); + + assert_eq!(std::fs::metadata(&overlay).unwrap().len(), 64 * 1024 * 1024); + assert!(!template.exists()); + let output = + std::process::Command::new(openshell_core::e2fsprogs::resolve(&["debugfs"]).unwrap()) + .arg("-R") + .arg("cat /upper/work.txt") + .arg(&overlay) + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); + assert_eq!(output.stdout, b"saved work\n"); + + let _ = std::fs::remove_dir_all(base); + } + #[test] fn prepare_sandbox_overlay_creates_missing_overlay_on_start() { let base = unique_temp_dir(); diff --git a/crates/openshell-driver-vm/src/rootfs.rs b/crates/openshell-driver-vm/src/rootfs.rs index cee3441b2e..b61a8193a8 100644 --- a/crates/openshell-driver-vm/src/rootfs.rs +++ b/crates/openshell-driver-vm/src/rootfs.rs @@ -402,8 +402,8 @@ pub fn recover_rootfs_image(image_path: &Path) -> Result<(), String> { String::from_utf8_lossy(&output.stderr) ); // Preen refuses some orphaned inodes left by a failed full-disk boot. - // Restrict noninteractive repair to overlays that were already short - // of usable space before fsck; other corruption still requires review. + // Permit only orphan reconnection on a full overlay; any other repair + // that preen cannot do safely still requires review. if !full_overlay || !matches!(output.status.code(), Some(4..=7)) || !String::from_utf8_lossy(&output.stdout).contains("Unattached inode") @@ -416,9 +416,16 @@ pub fn recover_rootfs_image(image_path: &Path) -> Result<(), String> { error }); } + let mut config = tempfile::NamedTempFile::new() + .map_err(|error| format!("create e2fsck recovery config: {error}"))?; + config + .write_all(b"[problems]\n0x040002 = {\n preen_ok = true\n}\n") + .and_then(|()| config.flush()) + .map_err(|error| format!("write e2fsck recovery config: {error}"))?; let repair = Command::new(&path) .env("LC_ALL", "C") - .arg("-y") + .env("E2FSCK_CONFIG", config.path()) + .arg("-p") .arg("-f") .arg(image_path) .output() @@ -430,7 +437,7 @@ pub fn recover_rootfs_image(image_path: &Path) -> Result<(), String> { })?; if !matches!(repair.status.code(), Some(0..=3)) { return Err(format!( - "OverlayDiskFull: filesystem repair failed; [openshell.drivers.vm] overlay_disk_mib sizes new overlays. {error}\nrepair stdout: {}\nrepair stderr: {}", + "OverlayDiskFull: filesystem repair needs review; [openshell.drivers.vm] overlay_disk_mib sizes new overlays. {error}\nretry stdout: {}\nretry stderr: {}", String::from_utf8_lossy(&repair.stdout), String::from_utf8_lossy(&repair.stderr) )); @@ -1849,6 +1856,48 @@ mod tests { let _ = fs::remove_dir_all(&dir); } + #[test] + fn overlay_recovery_refuses_unrelated_repairs_after_unattached_inode() { + use std::os::unix::fs::PermissionsExt as _; + + if let Some(directory) = std::env::var_os("OPENSHELL_ROOTFS_TOOL_TEST_ROOT") { + let directory = PathBuf::from(directory); + let error = recover_rootfs_image(&directory.join("overlay.ext4")) + .expect_err("unrelated corruption needs review"); + assert!(error.contains("filesystem repair needs review"), "{error}"); + assert!(!directory.join("broad-repair").exists()); + return; + } + if e2fs_command("mke2fs").is_err() { + return; + } + + let directory = tempfile::tempdir().expect("tool fixture"); + let source = directory.path().join("source"); + fs::create_dir_all(source.join("upper")).expect("create upperdir"); + fs::create_dir_all(source.join("work")).expect("create workdir"); + fs::write(source.join("upper/fill"), vec![b'X'; 44 * 1024 * 1024]).expect("write filler"); + create_ext4_image_from_dir_with_size( + &source, + &directory.path().join("overlay.ext4"), + 64 * 1024 * 1024, + ) + .expect("create nearly full ext4 image"); + + let tool = directory.path().join("e2fsck"); + fs::write( + &tool, + "#!/bin/sh\nif [ \"$1\" = \"-y\" ]; then\n printf repaired > \"$OPENSHELL_ROOTFS_TOOL_TEST_ROOT/broad-repair\"\n exit 0\nfi\nif [ -n \"$E2FSCK_CONFIG\" ]; then\n echo 'Unrelated corruption'\n exit 4\nfi\necho 'Unattached inode 15'\nexit 4\n", + ) + .expect("write repair fixture"); + fs::set_permissions(&tool, fs::Permissions::from_mode(0o755)) + .expect("make repair fixture executable"); + run_tool_fixture_test( + "rootfs::tests::overlay_recovery_refuses_unrelated_repairs_after_unattached_inode", + directory.path(), + ); + } + #[test] fn ext4_image_has_directory_distinguishes_directories_files_and_missing_paths() { if e2fs_command("debugfs").is_err() { From 3eeb96b0ceaa2bf5c6e4d02382ef9b8d65d8b0ee Mon Sep 17 00:00:00 2001 From: Vineeth Sai Narajala Date: Mon, 2 Nov 2026 23:05:14 +0000 Subject: [PATCH 3/5] fix(vm): recover full overlay on first restart Signed-off-by: Vineeth Sai Narajala --- crates/openshell-driver-vm/README.md | 11 ++- crates/openshell-driver-vm/src/rootfs.rs | 114 ++++++++++++++++++++--- docs/how-it-works/sandboxes/runtimes.mdx | 4 +- 3 files changed, 109 insertions(+), 20 deletions(-) diff --git a/crates/openshell-driver-vm/README.md b/crates/openshell-driver-vm/README.md index 292c59d8c8..86ed9b8144 100644 --- a/crates/openshell-driver-vm/README.md +++ b/crates/openshell-driver-vm/README.md @@ -249,15 +249,16 @@ during the first prepare. On restart, the driver checks a preserved overlay with `e2fsck` before writing new launch files. If fewer than 32 MiB are available to the sandbox after the -check, it grows the sparse disk and ext4 filesystem by 64 MiB before booting. +check, it grows the sparse disk and ext4 filesystem by at least 64 MiB, +then checks the usable space again before booting. Recovery can grow further in +the same start if ext4 reserved blocks or metadata still leave too little room. If preen stops on an unattached inode in a nearly full overlay, the driver retries preen with orphan reconnection enabled. Other repairs that require interaction still stop startup. The driver retains the overlay image and publishes an `OverlayDiskFull` -warning when it grows the disk. Repeated full-disk restarts can add another -64 MiB each time. `overlay_disk_mib` sets the initial size of new overlays; it -is not a hard cap on recovery growth, and changing it does not resize existing -overlays. Recovery requires `resize2fs` on the gateway host. If repair or +warning when it grows the disk. Later full-disk restarts can grow it again. +`overlay_disk_mib` sets the initial size of new overlays; it is not a hard +cap on recovery growth, and changing it does not resize existing overlays. Recovery requires `resize2fs` on the gateway host. If repair or growth fails, the start error names `OverlayDiskFull` and the config key, and the existing overlay is retained for another attempt. diff --git a/crates/openshell-driver-vm/src/rootfs.rs b/crates/openshell-driver-vm/src/rootfs.rs index b61a8193a8..7114eb96ea 100644 --- a/crates/openshell-driver-vm/src/rootfs.rs +++ b/crates/openshell-driver-vm/src/rootfs.rs @@ -32,6 +32,7 @@ const ROOTFS_IMAGE_MIN_HEADROOM_BYTES: u64 = 256 * 1024 * 1024; const EXT4_IMAGE_MIN_HEADROOM_BYTES: u64 = 16 * 1024 * 1024; const OVERLAY_RECOVERY_MIN_AVAILABLE_BYTES: u64 = 32 * 1024 * 1024; const OVERLAY_RECOVERY_GROWTH_BYTES: u64 = 64 * 1024 * 1024; +const OVERLAY_RECOVERY_RESIZE_CUSHION_BYTES: u64 = 16 * 1024 * 1024; static INJECTION_COUNTER: AtomicU64 = AtomicU64::new(0); pub const fn sandbox_guest_init_path() -> &'static str { @@ -444,21 +445,22 @@ pub fn recover_rootfs_image(image_path: &Path) -> Result<(), String> { } } - let space = ext4_image_space(image_path)?; - if space.available_bytes >= OVERLAY_RECOVERY_MIN_AVAILABLE_BYTES { - return Ok(()); + let mut space = ext4_image_space(image_path)?; + while space.available_bytes < OVERLAY_RECOVERY_MIN_AVAILABLE_BYTES { + space = grow_full_overlay(image_path, space).map_err(|error| { + format!( + "OverlayDiskFull: cannot provide space for guest init; [openshell.drivers.vm] overlay_disk_mib sizes new overlays: {error}" + ) + })?; } - grow_full_overlay(image_path, space).map_err(|error| { - format!( - "OverlayDiskFull: cannot provide space for guest init; [openshell.drivers.vm] overlay_disk_mib sizes new overlays: {error}" - ) - }) + Ok(()) } #[derive(Clone, Copy)] struct Ext4ImageSpace { available_bytes: u64, filesystem_bytes: u64, + recovery_shortfall: u64, } fn ext4_image_space(image_path: &Path) -> Result { @@ -523,13 +525,24 @@ fn ext4_image_space(image_path: &Path) -> Result { .saturating_sub(reserved) .checked_mul(block_size) .ok_or_else(|| format!("ext4 free space overflows in {}", image_path.display()))?; + let recovery_shortfall = (u128::from(reserved) * u128::from(block_size) + + u128::from(OVERLAY_RECOVERY_MIN_AVAILABLE_BYTES)) + .saturating_sub(u128::from(free) * u128::from(block_size)) + .try_into() + .map_err(|error| { + format!( + "ext4 recovery size overflows in {}: {error}", + image_path.display() + ) + })?; Ok(Ext4ImageSpace { available_bytes, filesystem_bytes, + recovery_shortfall, }) } -fn grow_full_overlay(image_path: &Path, space: Ext4ImageSpace) -> Result<(), String> { +fn grow_full_overlay(image_path: &Path, space: Ext4ImageSpace) -> Result { let image = fs::OpenOptions::new() .write(true) .open(image_path) @@ -538,9 +551,20 @@ fn grow_full_overlay(image_path: &Path, space: Ext4ImageSpace) -> Result<(), Str .metadata() .map_err(|error| format!("stat overlay {}: {error}", image_path.display()))? .len(); + // Include the blocks reserved for root, then leave room for ext4 metadata + // and reserved blocks added by resize2fs. A large overlay may need more + // than one 64 MiB increment before guest init can write to it. + let growth_bytes = space + .recovery_shortfall + .checked_add(OVERLAY_RECOVERY_RESIZE_CUSHION_BYTES) + .ok_or_else(|| "overlay recovery size overflows".to_string())? + .max(OVERLAY_RECOVERY_GROWTH_BYTES) + .div_ceil(1024 * 1024) + .checked_mul(1024 * 1024) + .ok_or_else(|| "overlay recovery size overflows".to_string())?; let target_size = space .filesystem_bytes - .checked_add(OVERLAY_RECOVERY_GROWTH_BYTES) + .checked_add(growth_bytes) .ok_or_else(|| "overlay recovery size overflows".to_string())?; if current_size < target_size { image @@ -565,13 +589,16 @@ fn grow_full_overlay(image_path: &Path, space: Ext4ImageSpace) -> Result<(), Str String::from_utf8_lossy(&output.stderr) )); } - if ext4_image_space(image_path)?.available_bytes < OVERLAY_RECOVERY_MIN_AVAILABLE_BYTES { + let grown = ext4_image_space(image_path)?; + if grown.filesystem_bytes <= space.filesystem_bytes + || grown.recovery_shortfall >= space.recovery_shortfall + { return Err(format!( - "overlay {} remains short of free space after resize", + "overlay {} did not gain usable space after resize", image_path.display() )); } - Ok(()) + Ok(grown) } #[cfg(target_os = "macos")] @@ -1797,6 +1824,67 @@ mod tests { let _ = fs::remove_dir_all(&dir); } + #[test] + fn recover_rootfs_image_reaches_headroom_on_first_restart() { + if ["e2fsck", "debugfs", "resize2fs", "tune2fs"] + .iter() + .any(|tool| e2fs_command(tool).is_err()) + { + return; + } + + let dir = unique_temp_dir(); + let source = dir.join("source"); + let image = dir.join("overlay.ext4"); + fs::create_dir_all(source.join("upper")).expect("create source upperdir"); + fs::create_dir_all(source.join("work")).expect("create source workdir"); + fs::write(source.join("upper/work.txt"), b"important work\n").expect("write work"); + fs::write(source.join("upper/fill"), vec![b'X'; 44 * 1024 * 1024]).expect("write filler"); + create_ext4_image_from_dir_with_size(&source, &image, 64 * 1024 * 1024) + .expect("create nearly full ext4 image"); + + // Model a full overlay whose reserved blocks consume most of one + // 64 MiB increase. One growth alone still leaves less than 32 MiB. + let reserve = e2fs_command("tune2fs") + .expect("tune2fs") + .arg("-r") + .arg("6500") + .arg(&image) + .output() + .expect("set reserved blocks"); + assert!(reserve.status.success(), "{reserve:?}"); + + recover_rootfs_image(&image).expect("recover on first restart"); + assert!( + ext4_image_space(&image) + .expect("inspect recovered space") + .available_bytes + >= OVERLAY_RECOVERY_MIN_AVAILABLE_BYTES + ); + assert!( + fs::metadata(&image).expect("stat recovered image").len() > 128 * 1024 * 1024, + "recovery needs more than one 64 MiB increment" + ); + let output = e2fs_command("debugfs") + .expect("debugfs") + .arg("-R") + .arg("cat /upper/work.txt") + .arg(&image) + .output() + .expect("read saved work"); + assert!(output.status.success(), "{output:?}"); + assert_eq!(output.stdout, b"important work\n"); + + let recovered_size = fs::metadata(&image).expect("stat recovered image").len(); + recover_rootfs_image(&image).expect("restart recovered overlay"); + assert_eq!( + fs::metadata(&image).expect("stat reused image").len(), + recovered_size + ); + + let _ = fs::remove_dir_all(&dir); + } + #[test] fn recover_rootfs_image_repairs_unattached_inode_on_full_overlay() { if ["e2fsck", "debugfs", "resize2fs"] diff --git a/docs/how-it-works/sandboxes/runtimes.mdx b/docs/how-it-works/sandboxes/runtimes.mdx index 4b48fa0b9f..eb2e821977 100644 --- a/docs/how-it-works/sandboxes/runtimes.mdx +++ b/docs/how-it-works/sandboxes/runtimes.mdx @@ -244,7 +244,7 @@ When you run `openshell sandbox stop` while the main process is running, OpenShe Flush errors and timeouts produce warnings while shutdown continues. If a signal kills the supervisor, the driver warns that the guest flush was not confirmed and recent writes may be lost. A completed stop does not guarantee persistence after a failed or timed-out flush. This flush does not cover stops after the main process has already exited. -When you start a stopped MicroVM sandbox, the driver checks its overlay filesystem. If fewer than 32 MiB are usable after repair, it grows the sparse disk by 64 MiB before booting. The sandbox watch stream reports an `OverlayDiskFull` warning when the disk grows. If a previous start left the sandbox in Error, stop it before retrying. After it reaches Ready, download its work and remove the files that filled the disk: +When you start a stopped MicroVM sandbox, the driver checks its overlay filesystem. If fewer than 32 MiB are usable after repair, it grows the sparse disk by at least 64 MiB and rechecks free space before booting. It can grow further in the same start if needed. The sandbox watch stream reports an `OverlayDiskFull` warning when the disk grows. If a previous start left the sandbox in Error, stop it before retrying. After it reaches Ready, download its work and remove the files that filled the disk: ```shell openshell sandbox stop full @@ -253,7 +253,7 @@ openshell sandbox download full /sandbox/work ./work openshell sandbox exec full -- rm -f /sandbox/fill ``` -If repair or growth fails, start reports `OverlayDiskFull` and retains the overlay for another attempt. Recovery needs `resize2fs` and free space on the gateway host. Each later full-disk restart can grow the disk again. `overlay_disk_mib` sets the initial size of new sandboxes, not a limit on recovery growth; changing it does not resize an existing overlay. +If repair or growth fails, start reports `OverlayDiskFull` and retains the overlay for another attempt. Recovery needs `resize2fs` and free space on the gateway host. A later full-disk restart can grow the disk again. `overlay_disk_mib` sets the initial size of new sandboxes, not a limit on recovery growth; changing it does not resize an existing overlay. ### MicroVM Mounts From bc221364f008cb9e410f86fbe06d376c0ec4e082 Mon Sep 17 00:00:00 2001 From: Vineeth Sai Narajala Date: Tue, 3 Nov 2026 01:32:30 +0000 Subject: [PATCH 4/5] fix(vm): preserve overlay data during full-disk recovery Signed-off-by: Vineeth Sai Narajala --- crates/openshell-driver-vm/README.md | 33 +- crates/openshell-driver-vm/src/driver.rs | 491 ++++++++++++++++-- crates/openshell-driver-vm/src/rootfs.rs | 265 ++++++++-- .../tests/image_preparation.rs | 3 + .../openshell-sandbox/src/boundary_server.rs | 85 +-- .../src/backend_setup/tests.rs | 2 + .../src/legacy_startup_regression_tests.rs | 1 + crates/openshell-supervisor/src/lib.rs | 24 +- crates/openshell-supervisor/src/main.rs | 20 + .../openshell-supervisor/src/shared_host.rs | 1 + docs/how-it-works/sandboxes/runtimes.mdx | 2 +- 11 files changed, 777 insertions(+), 150 deletions(-) diff --git a/crates/openshell-driver-vm/README.md b/crates/openshell-driver-vm/README.md index 86ed9b8144..192b3d3637 100644 --- a/crates/openshell-driver-vm/README.md +++ b/crates/openshell-driver-vm/README.md @@ -247,20 +247,25 @@ the overlay while cached image disks remain unchanged. The overlay disk must be large enough to hold the compressed payload, unpacked rootfs, and sandbox writes during the first prepare. -On restart, the driver checks a preserved overlay with `e2fsck` before writing -new launch files. If fewer than 32 MiB are available to the sandbox after the -check, it grows the sparse disk and ext4 filesystem by at least 64 MiB, -then checks the usable space again before booting. Recovery can grow further in -the same start if ext4 reserved blocks or metadata still leave too little room. -If preen stops on an unattached inode in a nearly full overlay, the driver -retries preen with orphan reconnection enabled. Other repairs that require -interaction still stop startup. -The driver retains the overlay image and publishes an `OverlayDiskFull` -warning when it grows the disk. Later full-disk restarts can grow it again. -`overlay_disk_mib` sets the initial size of new overlays; it is not a hard -cap on recovery growth, and changing it does not resize existing overlays. Recovery requires `resize2fs` on the gateway host. If repair or -growth fails, the start error names `OverlayDiskFull` and the config key, and -the existing overlay is retained for another attempt. +On restart, the driver checks a preserved overlay with read-only `e2fsck`, +then replays the journal and checks the filesystem on a private copy before +writing new launch files. If fewer than 32 MiB are available after the check, +it grows the sparse disk and ext4 filesystem by at least 64 MiB and rechecks +usable space before booting. Recovery can grow further in the same start. +The driver refuses a full overlay if journal replay or repair would release +saved blocks, or if preen requests orphan reconnection or another interactive +repair. The original image remains untouched in these cases. Guest credential +cleanup runs on the recovered copy after journal replay. VM shutdown flushes +the guest filesystem and records a durable acknowledgement. A restart refuses +an unacknowledged flush, and older overlays with an unreplayed journal are +refused if no acknowledgement exists. +The driver publishes an `OverlayDiskFull` warning when it grows the disk. Later +full-disk restarts can grow it again. `overlay_disk_mib` sets the initial size +of new overlays; it is not a hard cap on recovery growth, and changing it does +not resize existing overlays. Recovery requires `resize2fs` on the gateway +host. A missing previously published overlay is never replaced with an empty +one. A failed recovery reports `OverlayDiskFull` and retains the original +image for inspection. The driver also writes the accepted `DriverSandbox` launch request to `/sandboxes//sandbox.pb`. If the gateway restarts, it starts a diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index c5519b16e6..0142ea9931 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -21,10 +21,10 @@ use crate::lifecycle::{ use crate::rootfs::{ clone_or_copy_sparse_file, create_ext4_image_from_dir_with_size, create_rootfs_image_from_dir, ext4_image_has_directory, extract_host_supervisor, extract_rootfs_archive_to, - prepare_sandbox_rootfs_from_image_root, recover_rootfs_image, remove_rootfs_image_file, - sandbox_guest_init_path, sandbox_guest_runtime_identity, sandbox_guest_user_ids_from_image, - sandbox_guest_user_ids_from_overlay_image, set_rootfs_image_file_mode, - validate_host_supervisor, write_rootfs_image_file, + prepare_sandbox_rootfs_from_image_root, recover_rootfs_image_with_cleanup, + rootfs_image_needs_recovery, sandbox_guest_init_path, sandbox_guest_runtime_identity, + sandbox_guest_user_ids_from_image, sandbox_guest_user_ids_from_overlay_image, + set_rootfs_image_file_mode, validate_host_supervisor, write_rootfs_image_file, }; use crate::runtime::VmBackend; use base64::{Engine as _, engine::general_purpose::STANDARD as BASE64_STANDARD}; @@ -215,6 +215,7 @@ const GUEST_BOUNDARY_CONFIG_ENV: &str = "OPENSHELL_VM_SANDBOX_BOOTSTRAP"; const HOST_AUTH_BUNDLE_FILE: &str = "supervisor-auth.json"; const HOST_RUNTIME_DESCRIPTOR_FILE: &str = "runtime-descriptor.json"; const HOST_BOUNDARY_GENERATION_FILE: &str = "boundary-generation"; +const PENDING_GUEST_GENERATIONS_FILE: &str = "pending-guest-generations"; /// Per-sandbox host-only copy of the gateway-normalized destination trust. /// /// This file is deliberately outside the VM disks. The host supervisor is the @@ -244,6 +245,10 @@ const IMAGE_CACHE_ROOTFS_IMAGE: &str = "rootfs.ext4"; const OVERLAY_TEMPLATE_CACHE_DIR: &str = "overlay-templates"; const OVERLAY_TEMPLATE_CACHE_LAYOUT_VERSION: &str = "sandbox-overlay-ext4-v1"; const SANDBOX_OVERLAY_IMAGE: &str = "overlay.ext4"; +const SANDBOX_OVERLAY_UNPUBLISHED_FILE: &str = "overlay-unpublished"; +const SANDBOX_OVERLAY_PUBLISHED_FILE: &str = "overlay-published"; +const SANDBOX_OVERLAY_RUNNING_FILE: &str = "overlay-running-unconfirmed"; +const SANDBOX_OVERLAY_FLUSH_ACK_FILE: &str = "overlay-flush-ack"; const SANDBOX_OWNER_STATE_FILE: &str = "sandbox-owner-state"; const SANDBOX_OWNER_STATE_V1: &str = "sandbox-owner-v1"; const SANDBOX_OWNER_STATE_V2: &str = "sandbox-owner-v2"; @@ -1516,6 +1521,9 @@ impl VmDriver { ); } configure_main_exit_marker(&mut command, state_dir); + command + .arg("--termination-ack-marker") + .arg(state_dir.join(SANDBOX_OVERLAY_FLUSH_ACK_FILE)); if let Some(tls) = tls_paths { command.env(openshell_core::sandbox_env::TLS_CA, &tls.ca); } @@ -2233,6 +2241,9 @@ impl VmDriver { } self.ensure_provisioning_active(&sandbox.id).await?; + // A restart after an unacknowledged guest flush cannot claim that + // this disk is healthy, even if journal replay happens to be clean. + begin_overlay_run(&state_dir).map_err(Status::internal)?; command .arg("--vm-krun-log-level") @@ -3686,10 +3697,69 @@ impl VmDriver { let template_path = overlay_template_image(&self.config.state_dir, overlay_size_bytes); let overlay_metadata = tokio::fs::metadata(&overlay_disk).await; - let recover_preserved_overlay = preparation == OverlayPreparation::PreserveExisting - && overlay_metadata.as_ref().is_ok_and(fs::Metadata::is_file); - let publish_new_overlay = preparation == OverlayPreparation::Fresh - || matches!(overlay_metadata, Err(error) if error.kind() == std::io::ErrorKind::NotFound); + let overlay_exists = match overlay_metadata { + Ok(metadata) if metadata.is_file() => true, + Ok(_) => { + return Err(format!( + "overlay path {} is not a regular file", + overlay_disk.display() + )); + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => false, + Err(error) => { + return Err(format!("stat overlay {}: {error}", overlay_disk.display())); + } + }; + let guest_flush_acknowledged = verify_overlay_shutdown(state_dir)?; + if preparation == OverlayPreparation::PreserveExisting + && overlay_exists + && !guest_flush_acknowledged + && rootfs_image_needs_recovery(&overlay_disk)? + { + return Err(format!( + "OverlayDiskFull: saved overlay {} has an unconfirmed ext4 journal; refusing to replay it without a guest filesystem flush acknowledgement", + overlay_disk.display() + )); + } + let published_marker = state_dir.join(SANDBOX_OVERLAY_PUBLISHED_FILE); + let unpublished_marker = state_dir.join(SANDBOX_OVERLAY_UNPUBLISHED_FILE); + let published = tokio::fs::try_exists(&published_marker) + .await + .map_err(|error| format!("inspect {}: {error}", published_marker.display()))?; + let unpublished = tokio::fs::try_exists(&unpublished_marker) + .await + .map_err(|error| format!("inspect {}: {error}", unpublished_marker.display()))?; + let publish_new_overlay = match (preparation, overlay_exists) { + (OverlayPreparation::Fresh, false) if !published => { + persist_overlay_marker(state_dir, SANDBOX_OVERLAY_UNPUBLISHED_FILE, true)?; + true + } + (OverlayPreparation::Fresh, _) => { + return Err(format!( + "existing overlay state {} cannot be replaced during fresh preparation", + overlay_disk.display() + )); + } + (OverlayPreparation::PreserveExisting, true) => { + // Legacy overlays acquire the marker before any repair, so a + // later missing image can never be mistaken for a first start. + persist_overlay_marker(state_dir, SANDBOX_OVERLAY_PUBLISHED_FILE, true)?; + false + } + (OverlayPreparation::PreserveExisting, false) if unpublished && !published => true, + (OverlayPreparation::PreserveExisting, false) => { + return Err(format!( + "OverlayDiskFull: saved overlay {} is missing; refusing to create an empty replacement", + overlay_disk.display() + )); + } + }; + let cleanup_paths = if preparation == OverlayPreparation::PreserveExisting && overlay_exists + { + pending_guest_generation_paths(state_dir).await? + } else { + Vec::new() + }; if publish_new_overlay && !overlay_template_image_ready(&template_path, overlay_size_bytes).await? { @@ -3710,6 +3780,7 @@ impl VmDriver { } let overlay_to_recover = overlay_disk.clone(); + let state_dir_for_publish = state_dir.to_path_buf(); let staging_dir = self.image_staging_dir("writable-overlay"); let result = tokio::task::spawn_blocking(move || { if publish_new_overlay { @@ -3722,9 +3793,14 @@ impl VmDriver { prepare_sandbox_overlay_image( &template_path, &staging_overlay, - preparation, + OverlayPreparation::Fresh, overlay_size_bytes, )?; + persist_overlay_marker( + &state_dir_for_publish, + SANDBOX_OVERLAY_PUBLISHED_FILE, + false, + )?; fs::rename(&staging_overlay, &overlay_disk).map_err(|error| error.to_string()) } else { prepare_sandbox_overlay_image( @@ -3738,10 +3814,26 @@ impl VmDriver { .await .map_err(|err| format!("overlay image preparation panicked: {err}"))?; result?; - if recover_preserved_overlay { - tokio::task::spawn_blocking(move || recover_rootfs_image(&overlay_to_recover)) - .await - .map_err(|error| format!("overlay recovery panicked: {error}"))??; + if preparation == OverlayPreparation::PreserveExisting && overlay_exists { + tokio::task::spawn_blocking(move || { + recover_rootfs_image_with_cleanup(&overlay_to_recover, &cleanup_paths) + }) + .await + .map_err(|error| format!("overlay recovery panicked: {error}"))??; + let pending = state_dir.join(PENDING_GUEST_GENERATIONS_FILE); + if let Err(error) = tokio::fs::remove_file(&pending).await + && error.kind() != std::io::ErrorKind::NotFound + { + warn!(path = %pending.display(), %error, "could not clear guest cleanup marker"); + } + } + if publish_new_overlay { + let unpublished = state_dir.join(SANDBOX_OVERLAY_UNPUBLISHED_FILE); + if let Err(error) = tokio::fs::remove_file(&unpublished).await + && error.kind() != std::io::ErrorKind::NotFound + { + warn!(path = %unpublished.display(), %error, "could not clear unpublished overlay marker"); + } } if write_owner_state && !owner_state_written_before_prepare { write_sandbox_owner_state(state_dir, owner_state).await?; @@ -6934,6 +7026,69 @@ fn sandbox_overlay_image(state_dir: &Path) -> PathBuf { state_dir.join(SANDBOX_OVERLAY_IMAGE) } +fn persist_overlay_marker( + state_dir: &Path, + name: &str, + allow_existing: bool, +) -> Result<(), String> { + let path = state_dir.join(name); + let mut file = match fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(&path) + { + Ok(file) => file, + Err(error) if allow_existing && error.kind() == std::io::ErrorKind::AlreadyExists => { + if path.is_file() { + return Ok(()); + } + return Err(format!( + "overlay marker {} is not a regular file", + path.display() + )); + } + Err(error) => return Err(format!("create overlay marker {}: {error}", path.display())), + }; + file.write_all(b"overlay-marker-v1\n") + .and_then(|()| file.sync_all()) + .and_then(|()| fs::File::open(state_dir)?.sync_all()) + .map_err(|error| format!("persist overlay marker {}: {error}", path.display())) +} + +fn remove_overlay_marker(state_dir: &Path, name: &str) -> Result<(), String> { + let path = state_dir.join(name); + match fs::remove_file(&path) { + Ok(()) => fs::File::open(state_dir) + .and_then(|directory| directory.sync_all()) + .map_err(|error| format!("remove overlay marker {}: {error}", path.display())), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(format!("remove overlay marker {}: {error}", path.display())), + } +} + +fn verify_overlay_shutdown(state_dir: &Path) -> Result { + let running = state_dir.join(SANDBOX_OVERLAY_RUNNING_FILE); + if !running.try_exists().map_err(|error| error.to_string())? { + return Ok(false); + } + let ack = state_dir.join(SANDBOX_OVERLAY_FLUSH_ACK_FILE); + if !ack.try_exists().map_err(|error| error.to_string())? { + return Err(format!( + "OverlayDiskFull: guest filesystem flush was not acknowledged for {}; refusing to recover the saved overlay", + state_dir.display() + )); + } + Ok(true) +} + +fn begin_overlay_run(state_dir: &Path) -> Result<(), String> { + // An old acknowledgement must be durably gone before the new run marker + // can be interpreted as belonging to the current VM generation. + remove_overlay_marker(state_dir, SANDBOX_OVERLAY_FLUSH_ACK_FILE)?; + persist_overlay_marker(state_dir, SANDBOX_OVERLAY_RUNNING_FILE, true) +} + fn overlay_template_image(root: &Path, size_bytes: u64) -> PathBuf { image_cache_root_dir(root) .join(OVERLAY_TEMPLATE_CACHE_DIR) @@ -7458,24 +7613,25 @@ async fn remove_runtime_generation_material(state_dir: &Path) -> Result<(), Stri Err(error) => return Err(format!("read boundary generation marker: {error}")), }; if !generation.is_empty() { - let overlay = sandbox_runtime_disk_paths(state_dir).overlay_disk; - let generation_for_cleanup = generation.clone(); - tokio::task::spawn_blocking(move || { - let tls = guest_boundary_tls_paths(&generation_for_cleanup); - for guest_path in [ - PathBuf::from(guest_boundary_config_path(&generation_for_cleanup)), - tls.certificate_chain_path, - tls.private_key_path, - ] { - remove_rootfs_image_file( - &overlay, - &overlay_upper_path(&guest_path.to_string_lossy()), - )?; - } - Ok::<(), String>(()) - }) - .await - .map_err(|error| format!("guest authentication cleanup task failed: {error}"))??; + // The guest may have left ext4 metadata in its journal. Writing with + // debugfs before e2fsck replays that journal can corrupt saved files. + // Queue guest credential cleanup until recovery has completed. + let pending = state_dir.join(PENDING_GUEST_GENERATIONS_FILE); + let mut file = tokio::fs::OpenOptions::new() + .create(true) + .append(true) + .open(&pending) + .await + .map_err(|error| format!("open {}: {error}", pending.display()))?; + file.write_all(format!("{generation}\n").as_bytes()) + .await + .map_err(|error| format!("persist {}: {error}", pending.display()))?; + file.sync_all() + .await + .map_err(|error| format!("sync {}: {error}", pending.display()))?; + restrict_owner_read_write(&pending) + .await + .map_err(|error| format!("protect {}: {error}", pending.display()))?; } for path in [ state_dir.join(HOST_AUTH_BUNDLE_FILE), @@ -7491,6 +7647,35 @@ async fn remove_runtime_generation_material(state_dir: &Path) -> Result<(), Stri Ok(()) } +async fn pending_guest_generation_paths(state_dir: &Path) -> Result, String> { + let pending = state_dir.join(PENDING_GUEST_GENERATIONS_FILE); + let contents = match tokio::fs::read_to_string(&pending).await { + Ok(contents) => contents, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(error) => return Err(format!("read {}: {error}", pending.display())), + }; + let mut paths = Vec::new(); + for generation in contents.lines() { + if generation.is_empty() + || generation.len() > 128 + || !generation + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || byte == b'-') + { + return Err(format!("invalid guest generation in {}", pending.display())); + } + let tls = guest_boundary_tls_paths(generation); + for guest_path in [ + PathBuf::from(guest_boundary_config_path(generation)), + tls.certificate_chain_path, + tls.private_key_path, + ] { + paths.push(overlay_upper_path(&guest_path.to_string_lossy())); + } + } + Ok(paths) +} + async fn clear_explicit_start_markers(state_dir: &Path) -> Result<(), std::io::Error> { // Remove the terminal marker first. If clearing the durable stop marker // fails, the sandbox remains stopped and a later start can safely retry. @@ -7627,24 +7812,21 @@ fn prepare_sandbox_overlay_image( expected_size_bytes: u64, ) -> Result<(), String> { if preparation == OverlayPreparation::PreserveExisting { - match fs::metadata(overlay_disk) { - Ok(metadata) if metadata.is_file() => { - return Ok(()); - } - Ok(_) => { - return Err(format!( - "existing overlay path '{}' is not a file", - overlay_disk.display() - )); - } - Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} - Err(err) => { - return Err(format!( - "stat overlay disk {}: {err}", - overlay_disk.display() - )); - } - } + return match fs::metadata(overlay_disk) { + Ok(metadata) if metadata.is_file() => Ok(()), + Ok(_) => Err(format!( + "existing overlay path '{}' is not a file", + overlay_disk.display() + )), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => Err(format!( + "OverlayDiskFull: saved overlay {} is missing; refusing to create an empty replacement", + overlay_disk.display() + )), + Err(err) => Err(format!( + "stat overlay disk {}: {err}", + overlay_disk.display() + )), + }; } let template_metadata = fs::metadata(template_path) @@ -11373,6 +11555,208 @@ mod tests { let _ = std::fs::remove_dir_all(base); } + #[tokio::test] + async fn missing_published_overlay_refuses_empty_replacement() { + let base = tempfile::tempdir().unwrap(); + let state_dir = base.path(); + let overlay = state_dir.join(SANDBOX_OVERLAY_IMAGE); + write_sandbox_owner_state( + state_dir, + SandboxOwnerIdentity { + uid: 1000, + gid: 1000, + }, + ) + .await + .unwrap(); + persist_overlay_marker(state_dir, SANDBOX_OVERLAY_PUBLISHED_FILE, false).unwrap(); + let mut driver = test_driver_with_extensions(LifecycleExtensionRegistry::new()); + driver.config.state_dir = state_dir.to_path_buf(); + driver.config.overlay_disk_mib = 64; + + let error = driver + .prepare_runtime_overlay( + state_dir, + &overlay, + Path::new("/must-not-read-image"), + OverlayPreparation::PreserveExisting, + None, + ) + .await + .expect_err("previously published overlay must not be replaced"); + assert!(error.contains("OverlayDiskFull"), "{error}"); + assert!(!overlay.exists()); + assert!(!overlay_template_image(state_dir, 64 * 1024 * 1024).exists()); + } + + #[tokio::test] + async fn legacy_dirty_overlay_without_flush_ack_refuses_journal_replay() { + if openshell_core::e2fsprogs::resolve(&["mke2fs"]).is_err() { + return; + } + let base = tempfile::tempdir().unwrap(); + let state_dir = base.path(); + let source = state_dir.join("source"); + let overlay = state_dir.join(SANDBOX_OVERLAY_IMAGE); + std::fs::create_dir_all(source.join("upper")).unwrap(); + std::fs::create_dir_all(source.join("work")).unwrap(); + std::fs::write(source.join("upper/recover.txt"), b"saved marker\n").unwrap(); + create_ext4_image_from_dir_with_size(&source, &overlay, 64 * 1024 * 1024).unwrap(); + let mut image = std::fs::read(&overlay).unwrap(); + image[1024 + 0x60] |= 0x4; // ext4 incompat RECOVER: journal replay pending. + std::fs::write(&overlay, &image).unwrap(); + write_sandbox_owner_state( + state_dir, + SandboxOwnerIdentity { + uid: 1000, + gid: 1000, + }, + ) + .await + .unwrap(); + let mut driver = test_driver_with_extensions(LifecycleExtensionRegistry::new()); + driver.config.state_dir = state_dir.to_path_buf(); + driver.config.overlay_disk_mib = 64; + let error = driver + .prepare_runtime_overlay( + state_dir, + &overlay, + Path::new("/must-not-read-image"), + OverlayPreparation::PreserveExisting, + None, + ) + .await + .expect_err("legacy dirty image cannot be replayed without a flush ack"); + assert!(error.contains("OverlayDiskFull"), "{error}"); + assert!(error.contains("unconfirmed ext4 journal"), "{error}"); + assert_eq!(std::fs::read(&overlay).unwrap(), image); + } + + #[tokio::test] + async fn stopped_full_overlay_keeps_saved_files_or_refuses_start() { + if ["mke2fs", "e2fsck", "debugfs", "resize2fs"] + .iter() + .any(|tool| openshell_core::e2fsprogs::resolve(&[tool]).is_err()) + { + return; + } + + let base = tempfile::tempdir().unwrap(); + let state_dir = base.path(); + let source = state_dir.join("source"); + let overlay = state_dir.join(SANDBOX_OVERLAY_IMAGE); + let generation = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + std::fs::create_dir_all(source.join("upper/.openshell/state")).unwrap(); + std::fs::create_dir_all(source.join("work")).unwrap(); + std::fs::write(source.join("upper/recover.txt"), b"saved marker\n").unwrap(); + std::fs::write(source.join("upper/fill"), vec![b'A'; 55 * 1024 * 1024]).unwrap(); + std::fs::write( + source.join(format!( + "upper/.openshell/state/bootstrap-{generation}.json" + )), + b"old launch configuration", + ) + .unwrap(); + // The production image builder reserves extra initial headroom. Use + // mke2fs directly here to reproduce a guest that later filled it. + std::fs::File::create(&overlay) + .unwrap() + .set_len(64 * 1024 * 1024) + .unwrap(); + let formatter = openshell_core::e2fsprogs::resolve(&["mke2fs", "mkfs.ext4"]).unwrap(); + let mkfs_result = std::process::Command::new(formatter) + .args(["-q", "-F", "-t", "ext4", "-E", "root_owner=0:0", "-d"]) + .arg(&source) + .arg(&overlay) + .output() + .unwrap(); + assert!(mkfs_result.status.success(), "{mkfs_result:?}"); + let image_header = std::fs::read(&overlay).unwrap(); + let superblock = &image_header[1024..]; + let free_blocks = u32::from_le_bytes(superblock[0x0c..0x10].try_into().unwrap()); + let reserved_blocks = u32::from_le_bytes(superblock[0x08..0x0c].try_into().unwrap()); + assert!( + free_blocks <= reserved_blocks, + "the guest must have no usable overlay blocks" + ); + write_sandbox_owner_state( + state_dir, + SandboxOwnerIdentity { + uid: 1000, + gid: 1000, + }, + ) + .await + .unwrap(); + std::fs::write(state_dir.join(HOST_BOUNDARY_GENERATION_FILE), generation).unwrap(); + let saved_image = std::fs::read(&overlay).unwrap(); + + remove_runtime_generation_material(state_dir) + .await + .expect("stop queues guest cleanup"); + assert_eq!( + std::fs::read(&overlay).unwrap(), + saved_image, + "stop must not write to an image with an unreplayed journal" + ); + assert!(state_dir.join(PENDING_GUEST_GENERATIONS_FILE).exists()); + + let mut driver = test_driver_with_extensions(LifecycleExtensionRegistry::new()); + driver.config.state_dir = state_dir.to_path_buf(); + driver.config.overlay_disk_mib = 64; + begin_overlay_run(state_dir).expect("persist unacknowledged VM run"); + let error = driver + .prepare_runtime_overlay( + state_dir, + &overlay, + Path::new("/must-not-read-image"), + OverlayPreparation::PreserveExisting, + None, + ) + .await + .expect_err("unacknowledged guest flush must refuse restart"); + assert!(error.contains("OverlayDiskFull"), "{error}"); + assert_eq!(std::fs::read(&overlay).unwrap(), saved_image); + persist_overlay_marker(state_dir, SANDBOX_OVERLAY_FLUSH_ACK_FILE, false) + .expect("supervisor acknowledged guest flush"); + match driver + .prepare_runtime_overlay( + state_dir, + &overlay, + Path::new("/must-not-read-image"), + OverlayPreparation::PreserveExisting, + None, + ) + .await + { + Ok(_) => { + let debugfs = openshell_core::e2fsprogs::resolve(&["debugfs"]).unwrap(); + let marker = std::process::Command::new(&debugfs) + .args(["-R", "cat /upper/recover.txt"]) + .arg(&overlay) + .output() + .unwrap(); + assert!(marker.status.success(), "{marker:?}"); + assert_eq!(marker.stdout, b"saved marker\n"); + let dump = state_dir.join("saved-fill"); + let output = std::process::Command::new(&debugfs) + .arg("-R") + .arg(format!("dump /upper/fill {}", dump.display())) + .arg(&overlay) + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); + let contents = std::fs::read(&dump).expect("saved filler survived restart"); + assert_eq!(contents, vec![b'A'; 55 * 1024 * 1024]); + assert!(!state_dir.join(PENDING_GUEST_GENERATIONS_FILE).exists()); + } + Err(error) => { + assert!(error.contains("OverlayDiskFull"), "{error}"); + assert_eq!(std::fs::read(&overlay).unwrap(), saved_image); + } + } + } + #[tokio::test] async fn overlay_recovery_preserves_smaller_disk_after_default_increase() { if ["mke2fs", "e2fsck", "debugfs"] @@ -11430,22 +11814,23 @@ mod tests { } #[test] - fn prepare_sandbox_overlay_creates_missing_overlay_on_start() { + fn prepare_sandbox_overlay_refuses_missing_saved_overlay() { let base = unique_temp_dir(); std::fs::create_dir_all(&base).unwrap(); let template = base.join("template.ext4"); let overlay = base.join("overlay.ext4"); std::fs::write(&template, b"fresh-overlay").unwrap(); - prepare_sandbox_overlay_image( + let error = prepare_sandbox_overlay_image( &template, &overlay, OverlayPreparation::PreserveExisting, "fresh-overlay".len() as u64, ) - .expect("create missing overlay"); + .expect_err("a missing saved overlay must not be replaced"); - assert_eq!(std::fs::read(&overlay).unwrap(), b"fresh-overlay"); + assert!(error.contains("OverlayDiskFull"), "{error}"); + assert!(!overlay.exists()); let _ = std::fs::remove_dir_all(base); } diff --git a/crates/openshell-driver-vm/src/rootfs.rs b/crates/openshell-driver-vm/src/rootfs.rs index 7114eb96ea..fc034e045f 100644 --- a/crates/openshell-driver-vm/src/rootfs.rs +++ b/crates/openshell-driver-vm/src/rootfs.rs @@ -372,10 +372,54 @@ pub fn set_rootfs_image_file_mode( /// Repair a preserved overlay before the driver mutates it offline. A full /// overlay needs space for guest init even when the workload has stopped. +#[cfg(test)] pub fn recover_rootfs_image(image_path: &Path) -> Result<(), String> { - let initial_space = ext4_image_space(image_path).ok(); - let full_overlay = initial_space - .is_some_and(|space| space.available_bytes < OVERLAY_RECOVERY_MIN_AVAILABLE_BYTES); + recover_rootfs_image_with_cleanup(image_path, &[]) +} + +/// Recover on a private copy, then atomically replace the saved image only +/// after checking space and removing obsolete guest authentication material. +pub fn recover_rootfs_image_with_cleanup( + image_path: &Path, + guest_paths_to_remove: &[String], +) -> Result<(), String> { + let initial_space = ext4_image_space(image_path)?; + let checked_free_blocks = fsck_free_blocks_without_replay(image_path, &initial_space)?; + let full_overlay = checked_free_blocks + .saturating_sub(initial_space.reserved_blocks) + .saturating_mul(initial_space.block_size) + < OVERLAY_RECOVERY_MIN_AVAILABLE_BYTES; + let parent = image_path + .parent() + .ok_or_else(|| format!("overlay {} has no parent directory", image_path.display()))?; + let staging = tempfile::Builder::new() + .prefix(".overlay-recovery-") + .tempdir_in(parent) + .map_err(|error| format!("stage overlay recovery: {error}"))?; + let staged_image = staging.path().join("overlay.ext4"); + clone_or_copy_sparse_file(image_path, &staged_image).map_err(|error| { + if full_overlay { + format!("OverlayDiskFull: cannot copy saved overlay for recovery: {error}") + } else { + error + } + })?; + recover_rootfs_image_in_place(&staged_image, full_overlay, checked_free_blocks)?; + for guest_path in guest_paths_to_remove { + remove_rootfs_image_file(&staged_image, guest_path)?; + } + File::open(&staged_image) + .and_then(|file| file.sync_all()) + .map_err(|error| format!("sync recovered overlay: {error}"))?; + fs::rename(&staged_image, image_path) + .map_err(|error| format!("publish recovered overlay: {error}")) +} + +fn recover_rootfs_image_in_place( + image_path: &Path, + full_overlay: bool, + checked_free_blocks: u64, +) -> Result<(), String> { let mut command = e2fs_command("e2fsck")?; let path = PathBuf::from(command.get_program()); let output = command @@ -402,50 +446,27 @@ pub fn recover_rootfs_image(image_path: &Path) -> Result<(), String> { String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr) ); - // Preen refuses some orphaned inodes left by a failed full-disk boot. - // Permit only orphan reconnection on a full overlay; any other repair - // that preen cannot do safely still requires review. - if !full_overlay - || !matches!(output.status.code(), Some(4..=7)) - || !String::from_utf8_lossy(&output.stdout).contains("Unattached inode") - { - return Err(if full_overlay { - format!( - "OverlayDiskFull: filesystem repair needs review; [openshell.drivers.vm] overlay_disk_mib sizes new overlays: {error}" - ) - } else { - error - }); - } - let mut config = tempfile::NamedTempFile::new() - .map_err(|error| format!("create e2fsck recovery config: {error}"))?; - config - .write_all(b"[problems]\n0x040002 = {\n preen_ok = true\n}\n") - .and_then(|()| config.flush()) - .map_err(|error| format!("write e2fsck recovery config: {error}"))?; - let repair = Command::new(&path) - .env("LC_ALL", "C") - .env("E2FSCK_CONFIG", config.path()) - .arg("-p") - .arg("-f") - .arg(image_path) - .output() - .map_err(|error| { - format!( - "OverlayDiskFull: [openshell.drivers.vm] overlay_disk_mib sizes new overlays; run {}: {error}", - path.display() - ) - })?; - if !matches!(repair.status.code(), Some(0..=3)) { - return Err(format!( - "OverlayDiskFull: filesystem repair needs review; [openshell.drivers.vm] overlay_disk_mib sizes new overlays. {error}\nretry stdout: {}\nretry stderr: {}", - String::from_utf8_lossy(&repair.stdout), - String::from_utf8_lossy(&repair.stderr) - )); - } + // Reconnecting an orphan into lost+found can make its former guest + // path disappear. Keep the original image for manual recovery. + let now_full = full_overlay + || ext4_image_space(image_path) + .is_ok_and(|space| space.available_bytes < OVERLAY_RECOVERY_MIN_AVAILABLE_BYTES); + return Err(if now_full { + format!( + "OverlayDiskFull: filesystem repair needs review; [openshell.drivers.vm] overlay_disk_mib sizes new overlays: {error}" + ) + } else { + error + }); } let mut space = ext4_image_space(image_path)?; + if space.free_blocks > checked_free_blocks { + return Err(format!( + "OverlayDiskFull: journal replay or filesystem repair would release saved overlay blocks ({} to {} free); refusing to start", + checked_free_blocks, space.free_blocks + )); + } while space.available_bytes < OVERLAY_RECOVERY_MIN_AVAILABLE_BYTES { space = grow_full_overlay(image_path, space).map_err(|error| { format!( @@ -461,6 +482,74 @@ struct Ext4ImageSpace { available_bytes: u64, filesystem_bytes: u64, recovery_shortfall: u64, + block_size: u64, + free_blocks: u64, + reserved_blocks: u64, + needs_recovery: bool, +} + +/// A dirty image without a recorded guest flush cannot be safely replayed. +pub fn rootfs_image_needs_recovery(image_path: &Path) -> Result { + ext4_image_space(image_path).map(|space| space.needs_recovery) +} + +fn fsck_free_blocks_without_replay( + image_path: &Path, + initial_space: &Ext4ImageSpace, +) -> Result { + let mut command = e2fs_command("e2fsck")?; + let path = PathBuf::from(command.get_program()); + let output = command + .env("LC_ALL", "C") + .arg("-n") + .arg("-f") + .arg(image_path) + .output() + .map_err(|error| format!("OverlayDiskFull: run {}: {error}", path.display()))?; + let stdout = String::from_utf8_lossy(&output.stdout); + let (used, total) = stdout + .lines() + .rev() + .find_map(|line| { + let (_, counts) = line.rsplit_once(", ")?; + let counts = counts.strip_suffix(" blocks")?; + let (used, total) = counts.split_once('/')?; + Some((used.parse::().ok()?, total.parse::().ok()?)) + }) + .ok_or_else(|| { + format!( + "OverlayDiskFull: cannot verify saved overlay block usage before journal replay with {} on {}\nstdout: {}\nstderr: {}", + path.display(), + image_path.display(), + stdout, + String::from_utf8_lossy(&output.stderr) + ) + })?; + let counted_free = stdout + .lines() + .filter_map(|line| { + line.split_once("Free blocks count wrong (")? + .1 + .split_once("counted=")? + .1 + .split_once(')')? + .0 + .parse::() + .ok() + }) + .next_back(); + let free = counted_free.unwrap_or_else(|| total.saturating_sub(used)); + if used > total + || free > total + || total.saturating_mul(initial_space.block_size) != initial_space.filesystem_bytes + || output.status.code().is_none_or(|code| code & 8 != 0) + { + return Err(format!( + "OverlayDiskFull: cannot verify saved overlay block usage before journal replay: {}", + image_path.display() + )); + } + Ok(free) } fn ext4_image_space(image_path: &Path) -> Result { @@ -539,6 +628,10 @@ fn ext4_image_space(image_path: &Path) -> Result { available_bytes, filesystem_bytes, recovery_shortfall, + block_size, + free_blocks: free, + reserved_blocks: reserved, + needs_recovery: u32_at(0x60) & 0x4 != 0, }) } @@ -1886,7 +1979,7 @@ mod tests { } #[test] - fn recover_rootfs_image_repairs_unattached_inode_on_full_overlay() { + fn recover_rootfs_image_refuses_unattached_inode_without_changing_original() { if ["e2fsck", "debugfs", "resize2fs"] .iter() .any(|tool| e2fs_command(tool).is_err()) @@ -1929,7 +2022,11 @@ mod tests { "{preen:?}" ); - recover_rootfs_image(&image).expect("repair full overlay with orphaned inode"); + let before = fs::read(&image).expect("read saved overlay"); + let error = + recover_rootfs_image(&image).expect_err("an orphaned path needs manual recovery"); + assert!(error.contains("OverlayDiskFull"), "{error}"); + assert_eq!(fs::read(&image).expect("read refused overlay"), before); let work = e2fs_command("debugfs") .expect("debugfs") @@ -1973,9 +2070,13 @@ mod tests { .expect("create nearly full ext4 image"); let tool = directory.path().join("e2fsck"); + let real_e2fsck = e2fs_command("e2fsck").expect("real e2fsck"); fs::write( &tool, - "#!/bin/sh\nif [ \"$1\" = \"-y\" ]; then\n printf repaired > \"$OPENSHELL_ROOTFS_TOOL_TEST_ROOT/broad-repair\"\n exit 0\nfi\nif [ -n \"$E2FSCK_CONFIG\" ]; then\n echo 'Unrelated corruption'\n exit 4\nfi\necho 'Unattached inode 15'\nexit 4\n", + format!( + "#!/bin/sh\nif [ \"$1\" = \"-n\" ]; then\n exec '{}' \"$@\"\nfi\nif [ \"$1\" = \"-y\" ]; then\n printf repaired > \"$OPENSHELL_ROOTFS_TOOL_TEST_ROOT/broad-repair\"\n exit 0\nfi\necho 'Unattached inode 15'\nexit 4\n", + real_e2fsck.get_program().to_string_lossy() + ), ) .expect("write repair fixture"); fs::set_permissions(&tool, fs::Permissions::from_mode(0o755)) @@ -1986,6 +2087,67 @@ mod tests { ); } + #[test] + fn recovery_refuses_replay_that_discards_full_overlay_files() { + use std::os::unix::fs::PermissionsExt as _; + + if let Some(directory) = std::env::var_os("OPENSHELL_ROOTFS_TOOL_TEST_ROOT") { + let directory = PathBuf::from(directory); + let image = directory.join("overlay.ext4"); + let before = fs::read(&image).expect("read saved overlay"); + let error = recover_rootfs_image(&image) + .expect_err("restart must refuse a replay that loses saved files"); + assert!(error.contains("OverlayDiskFull"), "{error}"); + assert!(error.contains("release saved overlay blocks"), "{error}"); + assert_eq!(fs::read(&image).expect("read refused overlay"), before); + return; + } + if ["mke2fs", "e2fsck", "debugfs"] + .iter() + .any(|tool| e2fs_command(tool).is_err()) + { + return; + } + + let directory = tempfile::tempdir().expect("replay fixture"); + let source = directory.path().join("source"); + let empty = directory.path().join("empty"); + fs::create_dir_all(source.join("upper")).expect("create upperdir"); + fs::create_dir_all(source.join("work")).expect("create workdir"); + fs::create_dir_all(empty.join("upper")).expect("create empty upperdir"); + fs::create_dir_all(empty.join("work")).expect("create empty workdir"); + fs::write(source.join("upper/recover.txt"), b"saved marker\n").expect("write marker"); + fs::write(source.join("upper/fill"), vec![b'A'; 44 * 1024 * 1024]).expect("fill overlay"); + create_ext4_image_from_dir_with_size( + &source, + &directory.path().join("overlay.ext4"), + 64 * 1024 * 1024, + ) + .expect("create full overlay"); + create_ext4_image_from_dir_with_size( + &empty, + &directory.path().join("empty.ext4"), + 64 * 1024 * 1024, + ) + .expect("create empty overlay"); + let real_e2fsck = e2fs_command("e2fsck").expect("real e2fsck"); + let tool = directory.path().join("e2fsck"); + fs::write( + &tool, + format!( + "#!/bin/sh\nif [ \"$1\" = \"-n\" ]; then\n exec '{}' \"$@\"\nfi\n/bin/cp \"$OPENSHELL_ROOTFS_TOOL_TEST_ROOT/empty.ext4\" \"$3\"\n", + real_e2fsck.get_program().to_string_lossy() + ), + ) + .expect("write replay fixture"); + fs::set_permissions(&tool, fs::Permissions::from_mode(0o755)) + .expect("make replay fixture executable"); + run_tool_fixture_test( + "rootfs::tests::recovery_refuses_replay_that_discards_full_overlay_files", + directory.path(), + ); + } + #[test] fn ext4_image_has_directory_distinguishes_directories_files_and_missing_paths() { if e2fs_command("debugfs").is_err() { @@ -2154,6 +2316,15 @@ mod tests { return; } let directory = tempfile::tempdir().expect("tool installation"); + let source = directory.path().join("source"); + fs::create_dir_all(source.join("upper")).expect("create upperdir"); + fs::create_dir_all(source.join("work")).expect("create workdir"); + create_ext4_image_from_dir_with_size( + &source, + &directory.path().join("overlay.ext4"), + 64 * 1024 * 1024, + ) + .expect("create fixture overlay"); run_tool_fixture_test( "rootfs::tests::recovery_preserves_selected_path_and_execution_errors", directory.path(), diff --git a/crates/openshell-driver-vm/tests/image_preparation.rs b/crates/openshell-driver-vm/tests/image_preparation.rs index ffa0992c9e..d78093583b 100644 --- a/crates/openshell-driver-vm/tests/image_preparation.rs +++ b/crates/openshell-driver-vm/tests/image_preparation.rs @@ -56,6 +56,7 @@ fn worker_overlay_case(interrupt_retry: bool) { b"sandbox-owner-v2:1000:1000\n", ) .unwrap(); + fs::write(sandbox.join("overlay-unpublished"), b"unpublished\n").unwrap(); } let source = cache.join("test-image/rootfs.ext4"); fs::create_dir_all(source.parent().unwrap()).unwrap(); @@ -185,6 +186,8 @@ fn worker_overlay_case(interrupt_retry: bool) { serde_json::json!({"Ok": {"Overlay": {"uid": 1000, "gid": 1000}}}) ); assert_eq!(fs::read(sandbox.join("overlay.ext4")).unwrap(), expected); + assert!(sandbox.join("overlay-published").exists()); + assert!(!sandbox.join("overlay-unpublished").exists()); assert_eq!( fs::read(&template).unwrap(), expected, diff --git a/crates/openshell-sandbox/src/boundary_server.rs b/crates/openshell-sandbox/src/boundary_server.rs index 4eb3f1ec02..0871d1146d 100644 --- a/crates/openshell-sandbox/src/boundary_server.rs +++ b/crates/openshell-sandbox/src/boundary_server.rs @@ -2414,14 +2414,14 @@ mod linux { fn for_listener(listener: &BoundaryListenerConfig) -> Option { match listener { BoundaryListenerConfig::Vsock { .. } - | BoundaryListenerConfig::RuntimeInitiatedVsock { .. } => Some(Self { + | BoundaryListenerConfig::RuntimeInitiatedVsock { .. } + | BoundaryListenerConfig::RuntimeInitiatedUnix { .. } => Some(Self { run: Arc::new(flush_root_filesystem), timeout: FILESYSTEM_FLUSH_TIMEOUT, }), BoundaryListenerConfig::Unix { .. } | BoundaryListenerConfig::TlsTcp { .. } - | BoundaryListenerConfig::RuntimeInitiatedTlsTcp { .. } - | BoundaryListenerConfig::RuntimeInitiatedUnix { .. } => None, + | BoundaryListenerConfig::RuntimeInitiatedTlsTcp { .. } => None, } } } @@ -2743,6 +2743,7 @@ mod linux { Frozen { recovery_id: u64 }, Terminating, Terminal, + TerminalFlushFailed, } #[derive(Clone)] @@ -3019,6 +3020,7 @@ mod linux { *connection, SupervisorConnectionState::Terminating | SupervisorConnectionState::Terminal + | SupervisorConnectionState::TerminalFlushFailed ) { self.connections.mark_terminal(); return terminating(); @@ -3036,7 +3038,9 @@ mod linux { let mut connection = lock(&self.supervisor_connection); if matches!( *connection, - SupervisorConnectionState::Terminating | SupervisorConnectionState::Terminal + SupervisorConnectionState::Terminating + | SupervisorConnectionState::Terminal + | SupervisorConnectionState::TerminalFlushFailed ) { self.connections.mark_terminal(); return terminating(); @@ -3254,8 +3258,15 @@ mod linux { async fn terminate_boundary(&self) -> Response { { let mut connection = lock(&self.supervisor_connection); - if *connection == SupervisorConnectionState::Terminal { - return Response::BoundaryTerminated; + match *connection { + SupervisorConnectionState::Terminal => return Response::BoundaryTerminated, + SupervisorConnectionState::TerminalFlushFailed => { + return guest_error( + BoundaryErrorKind::Process, + "sandbox filesystem flush failed before termination", + ); + } + _ => {} } *connection = SupervisorConnectionState::Terminating; } @@ -3277,35 +3288,33 @@ mod linux { }; // The host may end the VM on acknowledgement. Flush dirty writes // first, even if process termination failed. - self.flush_filesystem().await; + let flushed = self.flush_filesystem().await; if let Err(error) = terminated { return guest_error(BoundaryErrorKind::Process, error); } + if let Err(error) = flushed { + *lock(&self.supervisor_connection) = SupervisorConnectionState::TerminalFlushFailed; + return guest_error(BoundaryErrorKind::Process, error); + } *lock(&self.supervisor_connection) = SupervisorConnectionState::Terminal; Response::BoundaryTerminated } - /// Log flush failures and timeouts while allowing termination to finish. - async fn flush_filesystem(&self) { + /// A failed or timed-out flush must prevent a successful shutdown ack. + async fn flush_filesystem(&self) -> Result<(), String> { let Some(filesystem_flush) = &self.filesystem_flush else { - return; + return Ok(()); }; let run = Arc::clone(&filesystem_flush.run); let flush = tokio::task::spawn_blocking(move || run()); match tokio::time::timeout(filesystem_flush.timeout, flush).await { - Ok(Ok(Ok(()))) => {} - Ok(Ok(Err(error))) => { - tracing::warn!(%error, "Sandbox filesystem flush failed before termination"); - } - Ok(Err(error)) => { - tracing::warn!(%error, "Sandbox filesystem flush task failed before termination"); - } - Err(_) => { - tracing::warn!( - timeout = ?filesystem_flush.timeout, - "Sandbox filesystem flush timed out before termination" - ); - } + Ok(Ok(Ok(()))) => Ok(()), + Ok(Ok(Err(error))) => Err(format!("sandbox filesystem flush failed: {error}")), + Ok(Err(error)) => Err(format!("sandbox filesystem flush task failed: {error}")), + Err(_) => Err(format!( + "sandbox filesystem flush timed out after {:?}", + filesystem_flush.timeout + )), } } @@ -6067,6 +6076,14 @@ mod linux { let flush = FilesystemFlush::for_listener(&vsock).expect("the VM backend flushes"); assert_eq!(flush.timeout, FILESYSTEM_FLUSH_TIMEOUT); + let runtime_unix = BoundaryListenerConfig::RuntimeInitiatedUnix { + socket_path: "/run/openshell/vm-control.sock".into(), + tls: tls.clone(), + }; + assert!( + FilesystemFlush::for_listener(&runtime_unix).is_some(), + "the VM's runtime-initiated Unix listener must flush" + ); let unix = BoundaryListenerConfig::Unix { socket_path: "/run/openshell/control.sock".into(), tls: tls.clone(), @@ -6118,24 +6135,28 @@ mod linux { } #[tokio::test(flavor = "multi_thread")] - async fn termination_is_acknowledged_when_the_filesystem_flush_fails() { + async fn termination_refuses_acknowledgement_when_the_filesystem_flush_fails() { let (runtime, _token) = availability_test_runtime_with_flush(FilesystemFlush { run: Arc::new(|| Err(io::Error::other("flush failed"))), timeout: FILESYSTEM_FLUSH_TIMEOUT, }); - assert_eq!( + assert!(matches!( runtime.terminate_boundary().await, - Response::BoundaryTerminated - ); + Response::Error { .. } + )); assert_eq!( *lock(&runtime.supervisor_connection), - SupervisorConnectionState::Terminal + SupervisorConnectionState::TerminalFlushFailed ); + assert!(matches!( + runtime.terminate_boundary().await, + Response::Error { .. } + )); } #[tokio::test(flavor = "multi_thread")] - async fn termination_is_acknowledged_when_the_filesystem_flush_hangs() { + async fn termination_refuses_acknowledgement_when_the_filesystem_flush_hangs() { let (release, blocked) = std::sync::mpsc::channel::<()>(); let blocked = Mutex::new(blocked); let (runtime, _token) = availability_test_runtime_with_flush(FilesystemFlush { @@ -6152,14 +6173,14 @@ mod linux { // Let the blocked flush finish so the runtime can shut down. release.send(()).expect("release the blocked flush"); - assert_eq!(response, Response::BoundaryTerminated); + assert!(matches!(response, Response::Error { .. })); assert!( elapsed < Duration::from_secs(2), - "a hung flush must not hold the acknowledgement: {elapsed:?}" + "a hung flush must be bounded: {elapsed:?}" ); assert_eq!( *lock(&runtime.supervisor_connection), - SupervisorConnectionState::Terminal + SupervisorConnectionState::TerminalFlushFailed ); } diff --git a/crates/openshell-supervisor/src/backend_setup/tests.rs b/crates/openshell-supervisor/src/backend_setup/tests.rs index bdf4035c61..1722fbe579 100644 --- a/crates/openshell-supervisor/src/backend_setup/tests.rs +++ b/crates/openshell-supervisor/src/backend_setup/tests.rs @@ -844,6 +844,7 @@ async fn shared_startup_uses_selected_backend_through_readiness_and_shutdown() { auth_bundle: setup.auth.clone(), admitted_isolation_backend: Some(TEST_BACKEND.into()), main_exit_marker: Some(marker.clone()), + termination_ack_marker: None, trace_context: None, provider_workload_api_socket: None, // The binary loads the proxy trust directory from its environment. @@ -1048,6 +1049,7 @@ async fn suspend_signal_exits_without_terminating_boundary() { auth_bundle: setup.auth.clone(), admitted_isolation_backend: Some(TEST_BACKEND.into()), main_exit_marker: Some(marker.clone()), + termination_ack_marker: None, trace_context: None, provider_workload_api_socket: None, // The binary loads the proxy trust directory from its environment. diff --git a/crates/openshell-supervisor/src/legacy_startup_regression_tests.rs b/crates/openshell-supervisor/src/legacy_startup_regression_tests.rs index 2d3894d563..70882ae6a5 100644 --- a/crates/openshell-supervisor/src/legacy_startup_regression_tests.rs +++ b/crates/openshell-supervisor/src/legacy_startup_regression_tests.rs @@ -301,6 +301,7 @@ async fn config_push_regression_legacy_gateway_never_observes_ready_before_workl auth_bundle, admitted_isolation_backend: Some("test-held".into()), main_exit_marker: None, + termination_ack_marker: None, trace_context: None, provider_workload_api_socket: None, proxy_tls: openshell_supervisor_network::run::ProxyTlsFiles::default(), diff --git a/crates/openshell-supervisor/src/lib.rs b/crates/openshell-supervisor/src/lib.rs index 8b1035beca..501b514cfa 100644 --- a/crates/openshell-supervisor/src/lib.rs +++ b/crates/openshell-supervisor/src/lib.rs @@ -907,6 +907,8 @@ pub struct SandboxInstanceConfig { pub auth_bundle: openshell_core::jwt::SupervisorAuthBundle, pub admitted_isolation_backend: Option, pub main_exit_marker: Option, + /// Written only after the guest acknowledges a successful filesystem flush. + pub termination_ack_marker: Option, /// Parent of the startup span. pub trace_context: Option, /// SPIFFE Workload API socket for provider token grants; absent fails closed. @@ -1070,6 +1072,7 @@ pub async fn run_sandbox_instance( auth_bundle, admitted_isolation_backend, main_exit_marker, + termination_ack_marker, trace_context, provider_workload_api_socket, proxy_tls, @@ -1737,7 +1740,7 @@ pub async fn run_sandbox_instance( .map_err(|error| miette::miette!(error.to_string())) }; tokio::select! { - result = wait => break Some((result?, true)), + result = wait => break Some((result?, true, false)), () = &mut proxy_exited => { let _ = running.terminate().await; return Err(miette::miette!( @@ -1761,7 +1764,7 @@ pub async fn run_sandbox_instance( .map_err(|error| miette::miette!( "sandbox did not acknowledge terminal state: {error}" ))?; - break Some((boundary_exit_code(status), false)); + break Some((boundary_exit_code(status), false, true)); } // The agent wait is cancelled while suspending so a fenced // generation's stream failure cannot race the suspension. @@ -1778,7 +1781,7 @@ pub async fn run_sandbox_instance( } } }; - let Some((exit_code, mut retain_access)) = lifecycle else { + let Some((exit_code, mut retain_access, boundary_terminated)) = lifecycle else { // A suspended boundary is retained by the backend: never send // TerminateBoundary or publish a main-process exit. Dropping the // access plane closes this supervisor's gateway session. @@ -1856,6 +1859,16 @@ pub async fn run_sandbox_instance( info!(backend = %backend_name, "Canonical process exited; retaining control-mode access plane"); retain_remote_access_plane(&mut proxy_exited, &mut shutdown_requested).await?; } + if let Some(marker) = termination_ack_marker.as_deref() { + if !boundary_terminated { + Box::pin(running.terminate()).await.map_err(|error| { + miette::miette!("sandbox did not acknowledge terminal state: {error}") + })?; + } + persist_termination_ack_marker(marker) + .into_diagnostic() + .wrap_err("persist guest filesystem flush acknowledgement")?; + } // Exec sessions served after the main exit may have exported traces; // flush those too before the relay goes away. if let Some(relay) = otlp_relay.take() { @@ -1908,6 +1921,10 @@ fn persist_main_exit_marker(path: &std::path::Path, exit_code: i32) -> std::io:: write_private_file_atomically(path, format!("exit_code={exit_code}\n").as_bytes()) } +fn persist_termination_ack_marker(path: &std::path::Path) -> std::io::Result<()> { + write_private_file_atomically(path, b"guest-filesystem-flushed-v1\n") +} + /// Replace `path` with `contents` via an owner-only (0600) temporary file in /// the same directory, fsync, rename, and directory fsync. fn write_private_file_atomically(path: &std::path::Path, contents: &[u8]) -> std::io::Result<()> { @@ -4642,6 +4659,7 @@ mod tests { auth_bundle, admitted_isolation_backend: None, main_exit_marker: None, + termination_ack_marker: None, trace_context: None, provider_workload_api_socket: None, proxy_tls: openshell_supervisor_network::run::ProxyTlsFiles::default(), diff --git a/crates/openshell-supervisor/src/main.rs b/crates/openshell-supervisor/src/main.rs index 7c35b96826..82704126dc 100644 --- a/crates/openshell-supervisor/src/main.rs +++ b/crates/openshell-supervisor/src/main.rs @@ -213,6 +213,10 @@ struct Args { #[arg(long, hide = true)] main_exit_marker: Option, + /// Driver-owned acknowledgement of a successful guest filesystem flush. + #[arg(long, hide = true)] + termination_ack_marker: Option, + /// Stdin is a driver-owned liveness pipe. EOF means the driver exited. #[arg(long, hide = true)] parent_liveness_stdin: bool, @@ -379,6 +383,7 @@ fn validate_role_arguments(args: &Args) -> Result<()> { || args.health_socket_path.is_some() || args.health_port.is_some() || args.main_exit_marker.is_some() + || args.termination_ack_marker.is_some() || args.parent_liveness_stdin { return Err(miette::miette!( @@ -415,6 +420,17 @@ fn validate_main_exit_marker(marker: Option<&Path>) -> Result<()> { Ok(()) } +fn validate_termination_ack_marker(marker: Option<&Path>) -> Result<()> { + if let Some(marker) = marker + && !marker.is_absolute() + { + return Err(miette::miette!( + "--termination-ack-marker must be an absolute path" + )); + } + Ok(()) +} + fn main() -> Result<()> { let raw_args = std::env::args().collect::>(); if raw_args.get(1).map(String::as_str) == Some(DEBUG_RPC_SUBCOMMAND) { @@ -450,6 +466,7 @@ fn main() -> Result<()> { validate_role_arguments(&args)?; arm_parent_liveness(args.parent_liveness_stdin)?; validate_main_exit_marker(args.main_exit_marker.as_deref())?; + validate_termination_ack_marker(args.termination_ack_marker.as_deref())?; let isolation_inputs = if args.role == SupervisorRole::IsolationBackend { let admitted_backend = std::env::var(openshell_core::sandbox_env::ADMITTED_ISOLATION_BACKEND) @@ -617,6 +634,7 @@ fn main() -> Result<()> { auth_bundle, admitted_isolation_backend: Some(admitted_backend), main_exit_marker: args.main_exit_marker, + termination_ack_marker: args.termination_ack_marker, trace_context: openshell_supervisor::trace_context_from_environment(), provider_workload_api_socket: std::env::var_os( openshell_core::sandbox_env::PROVIDER_SPIFFE_WORKLOAD_API_SOCKET, @@ -1096,5 +1114,7 @@ mod tests { assert!(validate_main_exit_marker(Some(Path::new("relative"))).is_err()); let absolute = std::env::temp_dir().join("openshell-main-exit"); assert!(validate_main_exit_marker(Some(&absolute)).is_ok()); + assert!(validate_termination_ack_marker(Some(Path::new("relative"))).is_err()); + assert!(validate_termination_ack_marker(Some(&absolute)).is_ok()); } } diff --git a/crates/openshell-supervisor/src/shared_host.rs b/crates/openshell-supervisor/src/shared_host.rs index 28f49b426b..07980a4b02 100644 --- a/crates/openshell-supervisor/src/shared_host.rs +++ b/crates/openshell-supervisor/src/shared_host.rs @@ -799,6 +799,7 @@ impl SharedHost { auth_bundle: registration.auth, admitted_isolation_backend: Some(openshell_sandbox_backend::BACKEND_NAME.to_string()), main_exit_marker: None, + termination_ack_marker: None, trace_context: registration.trace_context, // One process cannot hold a per-sandbox workload identity, so // providers that need one fail closed. diff --git a/docs/how-it-works/sandboxes/runtimes.mdx b/docs/how-it-works/sandboxes/runtimes.mdx index eb2e821977..089ed1c1ed 100644 --- a/docs/how-it-works/sandboxes/runtimes.mdx +++ b/docs/how-it-works/sandboxes/runtimes.mdx @@ -253,7 +253,7 @@ openshell sandbox download full /sandbox/work ./work openshell sandbox exec full -- rm -f /sandbox/fill ``` -If repair or growth fails, start reports `OverlayDiskFull` and retains the overlay for another attempt. Recovery needs `resize2fs` and free space on the gateway host. A later full-disk restart can grow the disk again. `overlay_disk_mib` sets the initial size of new sandboxes, not a limit on recovery growth; changing it does not resize an existing overlay. +If recovery would discard saved blocks, needs interactive filesystem repair, or cannot grow the disk, start reports `OverlayDiskFull` and retains the original overlay for inspection. The driver also refuses to replace a missing saved overlay with an empty one, or to replay an unacknowledged dirty journal. Recovery needs `resize2fs` and free space on the gateway host. A later full-disk restart can grow the disk again. `overlay_disk_mib` sets the initial size of new sandboxes, not a limit on recovery growth; changing it does not resize an existing overlay. ### MicroVM Mounts From 2787e6b10b4f87397bad647e4d8f57344730d275 Mon Sep 17 00:00:00 2001 From: Vineeth Sai Narajala Date: Tue, 3 Nov 2026 04:45:25 +0000 Subject: [PATCH 5/5] fix(runtime): preserve sandbox state across failure paths Signed-off-by: Vineeth Sai Narajala --- crates/openshell-driver-vm/src/driver.rs | 172 ++++++++++++++++-- .../openshell-sandbox/src/boundary_server.rs | 54 +++++- crates/openshell-supervisor/src/lib.rs | 106 +++++++++-- 3 files changed, 289 insertions(+), 43 deletions(-) diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 0142ea9931..277c30d2a0 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -2241,9 +2241,6 @@ impl VmDriver { } self.ensure_provisioning_active(&sandbox.id).await?; - // A restart after an unacknowledged guest flush cannot claim that - // this disk is healthy, even if journal replay happens to be clean. - begin_overlay_run(&state_dir).map_err(Status::internal)?; command .arg("--vm-krun-log-level") @@ -2283,7 +2280,7 @@ impl VmDriver { console_output = %console_output.display(), "vm driver: spawning VM launcher" ); - let mut child = match command.spawn() { + let mut child = match spawn_vm_launcher_with_overlay_run(&mut command, &state_dir) { Ok(child) => child, Err(err) => { warn!( @@ -3796,12 +3793,11 @@ impl VmDriver { OverlayPreparation::Fresh, overlay_size_bytes, )?; - persist_overlay_marker( + publish_sandbox_overlay_image( &state_dir_for_publish, - SANDBOX_OVERLAY_PUBLISHED_FILE, - false, - )?; - fs::rename(&staging_overlay, &overlay_disk).map_err(|error| error.to_string()) + &staging_overlay, + &overlay_disk, + ) } else { prepare_sandbox_overlay_image( &template_path, @@ -7056,6 +7052,21 @@ fn persist_overlay_marker( .map_err(|error| format!("persist overlay marker {}: {error}", path.display())) } +fn publish_sandbox_overlay_image( + state_dir: &Path, + staged_overlay: &Path, + overlay_disk: &Path, +) -> Result<(), String> { + fs::rename(staged_overlay, overlay_disk) + .map_err(|error| format!("publish overlay {}: {error}", overlay_disk.display()))?; + // Make the final image name durable before the marker can say it was + // published. A failed marker write leaves an existing image to recover. + fs::File::open(state_dir) + .and_then(|directory| directory.sync_all()) + .map_err(|error| format!("sync overlay directory {}: {error}", state_dir.display()))?; + persist_overlay_marker(state_dir, SANDBOX_OVERLAY_PUBLISHED_FILE, false) +} + fn remove_overlay_marker(state_dir: &Path, name: &str) -> Result<(), String> { let path = state_dir.join(name); match fs::remove_file(&path) { @@ -7082,11 +7093,59 @@ fn verify_overlay_shutdown(state_dir: &Path) -> Result { Ok(true) } -fn begin_overlay_run(state_dir: &Path) -> Result<(), String> { - // An old acknowledgement must be durably gone before the new run marker - // can be interpreted as belonging to the current VM generation. - remove_overlay_marker(state_dir, SANDBOX_OVERLAY_FLUSH_ACK_FILE)?; - persist_overlay_marker(state_dir, SANDBOX_OVERLAY_RUNNING_FILE, true) +#[derive(Debug, Clone, Copy)] +struct PreviousOverlayRunState { + running: bool, + acknowledged: bool, +} + +fn begin_overlay_run(state_dir: &Path) -> Result { + let previous = PreviousOverlayRunState { + running: state_dir + .join(SANDBOX_OVERLAY_RUNNING_FILE) + .try_exists() + .map_err(|error| error.to_string())?, + acknowledged: state_dir + .join(SANDBOX_OVERLAY_FLUSH_ACK_FILE) + .try_exists() + .map_err(|error| error.to_string())?, + }; + // Record the possible VM run before removing the old acknowledgement. + // The launcher cannot start until the acknowledgement is durably gone. + persist_overlay_marker(state_dir, SANDBOX_OVERLAY_RUNNING_FILE, true)?; + if let Err(error) = remove_overlay_marker(state_dir, SANDBOX_OVERLAY_FLUSH_ACK_FILE) { + rollback_overlay_run(state_dir, previous)?; + return Err(error); + } + Ok(previous) +} + +fn rollback_overlay_run(state_dir: &Path, previous: PreviousOverlayRunState) -> Result<(), String> { + if previous.acknowledged { + persist_overlay_marker(state_dir, SANDBOX_OVERLAY_FLUSH_ACK_FILE, true)?; + } + if !previous.running { + remove_overlay_marker(state_dir, SANDBOX_OVERLAY_RUNNING_FILE)?; + } + Ok(()) +} + +fn spawn_vm_launcher_with_overlay_run( + command: &mut Command, + state_dir: &Path, +) -> Result { + // A restart after an unacknowledged guest flush cannot claim that the + // disk is healthy, even if journal replay happens to be clean. + let previous = begin_overlay_run(state_dir)?; + match command.spawn() { + Ok(child) => Ok(child), + Err(error) => { + rollback_overlay_run(state_dir, previous).map_err(|rollback| { + format!("launch VM helper: {error}; restore overlay markers: {rollback}") + })?; + Err(error.to_string()) + } + } } fn overlay_template_image(root: &Path, size_bytes: u64) -> PathBuf { @@ -11555,6 +11614,91 @@ mod tests { let _ = std::fs::remove_dir_all(base); } + #[test] + fn failed_vm_launcher_spawn_restores_previous_overlay_state() { + let directory = tempfile::tempdir().unwrap(); + let state_dir = directory.path(); + let overlay = state_dir.join(SANDBOX_OVERLAY_IMAGE); + std::fs::write(&overlay, b"saved workload data").unwrap(); + persist_overlay_marker(state_dir, SANDBOX_OVERLAY_RUNNING_FILE, false).unwrap(); + persist_overlay_marker(state_dir, SANDBOX_OVERLAY_FLUSH_ACK_FILE, false).unwrap(); + + let mut command = Command::new("/openshell-test/nonexistent-vm-launcher"); + spawn_vm_launcher_with_overlay_run(&mut command, state_dir) + .expect_err("launcher spawn must fail"); + assert!(verify_overlay_shutdown(state_dir).unwrap()); + assert_eq!(std::fs::read(&overlay).unwrap(), b"saved workload data"); + + remove_overlay_marker(state_dir, SANDBOX_OVERLAY_RUNNING_FILE).unwrap(); + remove_overlay_marker(state_dir, SANDBOX_OVERLAY_FLUSH_ACK_FILE).unwrap(); + let mut command = Command::new("/openshell-test/nonexistent-vm-launcher"); + spawn_vm_launcher_with_overlay_run(&mut command, state_dir) + .expect_err("first launcher spawn must fail"); + assert!(!state_dir.join(SANDBOX_OVERLAY_RUNNING_FILE).exists()); + assert!(!state_dir.join(SANDBOX_OVERLAY_FLUSH_ACK_FILE).exists()); + assert_eq!(std::fs::read(&overlay).unwrap(), b"saved workload data"); + } + + #[tokio::test] + async fn failed_publish_marker_preserves_overlay_for_restore() { + if ["mke2fs", "e2fsck", "debugfs"] + .iter() + .any(|tool| openshell_core::e2fsprogs::resolve(&[tool]).is_err()) + { + return; + } + let directory = tempfile::tempdir().unwrap(); + let state_dir = directory.path(); + let source = state_dir.join("source"); + let staging = state_dir.join("staging-overlay.ext4"); + let overlay = state_dir.join(SANDBOX_OVERLAY_IMAGE); + std::fs::create_dir_all(source.join("upper")).unwrap(); + std::fs::create_dir_all(source.join("work")).unwrap(); + std::fs::write(source.join("upper/saved.txt"), b"saved workload data\n").unwrap(); + create_ext4_image_from_dir_with_size(&source, &staging, 64 * 1024 * 1024).unwrap(); + write_sandbox_owner_state( + state_dir, + SandboxOwnerIdentity { + uid: 1000, + gid: 1000, + }, + ) + .await + .unwrap(); + persist_overlay_marker(state_dir, SANDBOX_OVERLAY_UNPUBLISHED_FILE, false).unwrap(); + let marker = state_dir.join(SANDBOX_OVERLAY_PUBLISHED_FILE); + std::fs::create_dir(&marker).unwrap(); + let error = publish_sandbox_overlay_image(state_dir, &staging, &overlay) + .expect_err("marker write must fail after publishing the complete image"); + assert!(error.contains("marker"), "{error}"); + assert!(overlay.exists()); + assert!(!staging.exists()); + std::fs::remove_dir(&marker).unwrap(); + + let mut driver = test_driver_with_extensions(LifecycleExtensionRegistry::new()); + driver.config.state_dir = state_dir.to_path_buf(); + driver.config.overlay_disk_mib = 64; + driver + .prepare_runtime_overlay( + state_dir, + &overlay, + Path::new("/must-not-read-image"), + OverlayPreparation::PreserveExisting, + None, + ) + .await + .expect("restore the already published image after marker failure"); + assert!(marker.is_file()); + let output = + std::process::Command::new(openshell_core::e2fsprogs::resolve(&["debugfs"]).unwrap()) + .args(["-R", "cat /upper/saved.txt"]) + .arg(&overlay) + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); + assert_eq!(output.stdout, b"saved workload data\n"); + } + #[tokio::test] async fn missing_published_overlay_refuses_empty_replacement() { let base = tempfile::tempdir().unwrap(); diff --git a/crates/openshell-sandbox/src/boundary_server.rs b/crates/openshell-sandbox/src/boundary_server.rs index 0871d1146d..c65b28f8da 100644 --- a/crates/openshell-sandbox/src/boundary_server.rs +++ b/crates/openshell-sandbox/src/boundary_server.rs @@ -2414,12 +2414,12 @@ mod linux { fn for_listener(listener: &BoundaryListenerConfig) -> Option { match listener { BoundaryListenerConfig::Vsock { .. } - | BoundaryListenerConfig::RuntimeInitiatedVsock { .. } - | BoundaryListenerConfig::RuntimeInitiatedUnix { .. } => Some(Self { + | BoundaryListenerConfig::RuntimeInitiatedVsock { .. } => Some(Self { run: Arc::new(flush_root_filesystem), timeout: FILESYSTEM_FLUSH_TIMEOUT, }), BoundaryListenerConfig::Unix { .. } + | BoundaryListenerConfig::RuntimeInitiatedUnix { .. } | BoundaryListenerConfig::TlsTcp { .. } | BoundaryListenerConfig::RuntimeInitiatedTlsTcp { .. } => None, } @@ -5788,6 +5788,19 @@ mod linux { /// listener kind selects in production. fn availability_test_runtime_with( flush: Option, + ) -> (Arc, String) { + availability_test_runtime_with_listener( + BoundaryListenerConfig::TlsTcp { + address: "127.0.0.1:5500".parse().unwrap(), + tls: placeholder_server_tls(), + }, + flush, + ) + } + + fn availability_test_runtime_with_listener( + listener: BoundaryListenerConfig, + flush: Option, ) -> (Arc, String) { let (broker, launcher) = test_network_broker(); let (verification_key, token) = test_auth_material("availability"); @@ -5801,10 +5814,7 @@ mod linux { auth_epoch: CredentialEpoch::new(1).expect("auth epoch"), gateway_id: "test-gateway".to_string(), verification_keys: vec![verification_key], - listener: BoundaryListenerConfig::TlsTcp { - address: "127.0.0.1:5500".parse().unwrap(), - tls: placeholder_server_tls(), - }, + listener, resource_claims: std::collections::BTreeMap::new(), resource_claim_files: std::collections::BTreeMap::new(), workload_identity: test_workload_identity(), @@ -6075,14 +6085,22 @@ mod linux { }; let flush = FilesystemFlush::for_listener(&vsock).expect("the VM backend flushes"); assert_eq!(flush.timeout, FILESYSTEM_FLUSH_TIMEOUT); + let runtime_vsock = BoundaryListenerConfig::RuntimeInitiatedVsock { + host_port: 5500, + tls: tls.clone(), + }; + assert!( + FilesystemFlush::for_listener(&runtime_vsock).is_some(), + "the VM's runtime-initiated vsock listener must flush" + ); let runtime_unix = BoundaryListenerConfig::RuntimeInitiatedUnix { - socket_path: "/run/openshell/vm-control.sock".into(), + socket_path: "/run/openshell/container-control.sock".into(), tls: tls.clone(), }; assert!( - FilesystemFlush::for_listener(&runtime_unix).is_some(), - "the VM's runtime-initiated Unix listener must flush" + FilesystemFlush::for_listener(&runtime_unix).is_none(), + "runtime-initiated Unix is used by containers and must not flush" ); let unix = BoundaryListenerConfig::Unix { socket_path: "/run/openshell/control.sock".into(), @@ -6120,6 +6138,24 @@ mod linux { ); } + #[tokio::test(flavor = "multi_thread")] + async fn runtime_initiated_unix_container_termination_does_not_flush() { + let listener = BoundaryListenerConfig::RuntimeInitiatedUnix { + socket_path: "/run/openshell/container-control.sock".into(), + tls: placeholder_server_tls(), + }; + let (runtime, _token) = availability_test_runtime_with_listener(listener, None); + assert!(runtime.filesystem_flush.is_none()); + assert_eq!( + runtime.terminate_boundary().await, + Response::BoundaryTerminated + ); + assert_eq!( + *lock(&runtime.supervisor_connection), + SupervisorConnectionState::Terminal + ); + } + /// Exercise the real syscall and open-error path; persistence requires /// VM stop/start testing. #[test] diff --git a/crates/openshell-supervisor/src/lib.rs b/crates/openshell-supervisor/src/lib.rs index 501b514cfa..c0994d12a5 100644 --- a/crates/openshell-supervisor/src/lib.rs +++ b/crates/openshell-supervisor/src/lib.rs @@ -105,6 +105,41 @@ async fn retain_remote_access_plane( } } +/// A successful terminal response is the guest's flush acknowledgement. Write +/// it before any later lifecycle error can end this supervisor instance. +async fn terminate_with_flush_ack( + termination: F, + marker: Option<&std::path::Path>, +) -> Result<()> +where + F: Future>, + E: std::fmt::Display, +{ + termination + .await + .map_err(|error| miette::miette!("sandbox did not acknowledge terminal state: {error}"))?; + if let Some(marker) = marker { + persist_termination_ack_marker(marker) + .into_diagnostic() + .wrap_err("persist guest filesystem flush acknowledgement")?; + } + Ok(()) +} + +async fn terminate_on_proxy_exit( + termination: F, + marker: Option<&std::path::Path>, +) -> Result +where + F: Future>, + E: std::fmt::Display, +{ + terminate_with_flush_ack(termination, marker).await?; + Err(miette::miette!( + "control-mode proxy accept loop exited unexpectedly" + )) +} + async fn completion_phase_or_shutdown(phase: F, mut shutdown: Pin<&mut S>) -> bool where F: Future, @@ -1742,10 +1777,10 @@ pub async fn run_sandbox_instance( tokio::select! { result = wait => break Some((result?, true, false)), () = &mut proxy_exited => { - let _ = running.terminate().await; - return Err(miette::miette!( - "control-mode proxy accept loop exited unexpectedly" - )); + return Box::pin(terminate_on_proxy_exit( + Box::pin(running.terminate()), + termination_ack_marker.as_deref(), + )).await; } () = &mut shutdown_requested => { let _ = agent @@ -1758,12 +1793,10 @@ pub async fn run_sandbox_instance( agent.wait().await } .map_err(|error| miette::miette!(error.to_string()))?; - running - .terminate() - .await - .map_err(|error| miette::miette!( - "sandbox did not acknowledge terminal state: {error}" - ))?; + Box::pin(terminate_with_flush_ack( + Box::pin(running.terminate()), + termination_ack_marker.as_deref(), + )).await?; break Some((boundary_exit_code(status), false, true)); } // The agent wait is cancelled while suspending so a fenced @@ -1857,17 +1890,27 @@ pub async fn run_sandbox_instance( } if retain_access { info!(backend = %backend_name, "Canonical process exited; retaining control-mode access plane"); - retain_remote_access_plane(&mut proxy_exited, &mut shutdown_requested).await?; - } - if let Some(marker) = termination_ack_marker.as_deref() { - if !boundary_terminated { - Box::pin(running.terminate()).await.map_err(|error| { - miette::miette!("sandbox did not acknowledge terminal state: {error}") - })?; + if let Err(error) = + retain_remote_access_plane(&mut proxy_exited, &mut shutdown_requested).await + { + if let Some(marker) = termination_ack_marker.as_deref() { + return Box::pin(terminate_on_proxy_exit( + Box::pin(running.terminate()), + Some(marker), + )) + .await; + } + return Err(error); } - persist_termination_ack_marker(marker) - .into_diagnostic() - .wrap_err("persist guest filesystem flush acknowledgement")?; + } + if let Some(marker) = termination_ack_marker.as_deref() + && !boundary_terminated + { + Box::pin(terminate_with_flush_ack( + Box::pin(running.terminate()), + Some(marker), + )) + .await?; } // Exec sessions served after the main exit may have exported traces; // flush those too before the relay goes away. @@ -4946,6 +4989,29 @@ mod tests { assert!(write_private_file_atomically(std::path::Path::new("/"), b"x").is_err()); } + #[tokio::test] + async fn successful_termination_persists_flush_ack_before_proxy_error() { + let directory = tempfile::tempdir().unwrap(); + let marker = directory.path().join("overlay-flush-ack"); + let error = terminate_on_proxy_exit(async { Ok::<(), &'static str>(()) }, Some(&marker)) + .await + .expect_err("proxy failure still ends the supervisor instance"); + assert!(error.to_string().contains("proxy accept loop exited")); + assert_eq!( + std::fs::read(&marker).unwrap(), + b"guest-filesystem-flushed-v1\n" + ); + std::fs::remove_file(&marker).unwrap(); + let error = terminate_on_proxy_exit( + async { Err::<(), &'static str>("flush failed") }, + Some(&marker), + ) + .await + .expect_err("failed guest termination cannot acknowledge the flush"); + assert!(error.to_string().contains("flush failed")); + assert!(!marker.exists()); + } + #[tokio::test] async fn remote_access_plane_outlives_main_completion_until_teardown() { let (shutdown_tx, shutdown_rx) = tokio::sync::oneshot::channel();