diff --git a/Makefile b/Makefile index b72ab203..1bd2234c 100644 --- a/Makefile +++ b/Makefile @@ -245,7 +245,7 @@ CONTAINER_RUN := $(CONTAINER_RUN_BASE) --ulimit nproc=65536:65536 --pids-limit=6 _test-unit _test-agent-unit _test-fast _test-all _test-root _setup-fcvm _bench \ container-build container-test container-test-unit container-test-fast container-test-all container-test-fc-mock \ container-setup-fcvm container-shell container-clean container-bench \ - cargo-target-link build-host-tools setup-btrfs setup-default release-default-kernel setup-fcvm setup-passt setup-pjdfstest setup-hugepages bench bench-vm bench-hugepages bench-hugepages-test \ + cargo-target-link build-host-tools setup-btrfs setup-default release-default-kernel setup-fcvm setup-cloud-hypervisor setup-passt setup-pjdfstest setup-hugepages bench bench-vm bench-hugepages bench-hugepages-test \ bench-container-import bench-chromium analyze-chromium-request analyze-chromium-campaign bench-clone-latency test-chromium-request \ bench-chromium-request-build bench-webkit-request-build bench-webkit-request-golden bench-webkit-request-verify bench-webkit-request-run test-chromium bench-chromium-request-golden bench-chromium-request-verify \ bench-chromium-corpus bench-chromium-corpus-extra bench-stop \ @@ -594,7 +594,7 @@ test-unit: show-notes check-disk build _test-unit test-agent-unit: show-notes check-disk cargo-target-link _test-agent-unit test-fast: show-notes check-disk setup-fcvm _test-fast test-all: show-notes check-disk setup-fcvm _test-all -test-root: show-notes check-disk setup-fcvm setup-pjdfstest setup-hugepages _test-root +test-root: show-notes check-disk setup-fcvm setup-pjdfstest setup-hugepages setup-cloud-hypervisor _test-root test: test-root # Seeded lifecycle chaos fuzz (tests/test_fuzz_chaos.rs): one rootless VM per @@ -679,6 +679,10 @@ container-clean: podman rmi $(CONTAINER_TAG) 2>/dev/null || true # Setup targets +setup-cloud-hypervisor: private SHELL := $(TARGET_LEASE_SHELL) +setup-cloud-hypervisor: build setup-btrfs + ./target/release/fcvm setup --cloud-hypervisor + setup-passt: ./scripts/build-passt.sh diff --git a/src/hypervisor/cloud_hypervisor/api.rs b/src/hypervisor/cloud_hypervisor/api.rs index 68587c3c..4c8eef01 100644 --- a/src/hypervisor/cloud_hypervisor/api.rs +++ b/src/hypervisor/cloud_hypervisor/api.rs @@ -176,6 +176,12 @@ pub struct MemoryConfig { pub size: u64, /// Back guest RAM with a shared mmap (required for vhost-user / some restore modes). pub shared: bool, + /// When true (CH's default) CH madvises guest RAM MADV_HUGEPAGE. Under the host's + /// `defrag=madvise` each guest fault on that memory can then compact synchronously. + /// On a fragmented CI host that kept kcompactd near 100%, put every CH VMM at + /// 100-180% of a core in the kernel, and stretched a file restore to 183 s from + /// 11 s. Firecracker guest RAM is not madvised either. + pub thp: bool, } #[derive(Debug, Serialize)] diff --git a/src/hypervisor/cloud_hypervisor/mod.rs b/src/hypervisor/cloud_hypervisor/mod.rs index be10bba2..76e7668b 100644 --- a/src/hypervisor/cloud_hypervisor/mod.rs +++ b/src/hypervisor/cloud_hypervisor/mod.rs @@ -17,10 +17,13 @@ pub mod api; use anyhow::{anyhow, bail, Context, Result}; use std::collections::VecDeque; +use std::os::fd::AsRawFd; +use std::os::unix::net::UnixStream; use std::path::{Path, PathBuf}; use std::process::ExitStatus; use std::sync::{Arc, Mutex}; use std::time::Duration; +use tokio::io::{AsyncBufRead, AsyncBufReadExt, BufReader}; use tokio::process::{Child, Command}; use tokio::sync::mpsc; use tokio::task::JoinHandle; @@ -87,6 +90,8 @@ pub struct CloudHypervisorBackend { /// Guest console (hvc0) lines observed by the console tail since spawn /// (see [`Hypervisor::console_line_counter`]). console_lines: Arc, + /// One event reader per VMM child. It must finish before the API path is reused. + reboot_monitor: Option>>, } impl CloudHypervisorBackend { @@ -110,6 +115,7 @@ impl CloudHypervisorBackend { vsock_path: None, console_tail: None, console_lines: Arc::new(std::sync::atomic::AtomicU64::new(0)), + reboot_monitor: None, } } @@ -117,6 +123,15 @@ impl CloudHypervisorBackend { self.client.as_ref().context("Cloud Hypervisor not started") } + async fn stop_reboot_monitor(&mut self) { + // Keep ownership across await: wait() can be cancelled by the VM loop. + if let Some(monitor) = self.reboot_monitor.as_mut() { + monitor.abort(); + let _ = monitor.await; + } + self.reboot_monitor = None; + } + /// Merge a spawn spec into the retained namespace isolation: only fields the spec /// actually provides overwrite the retained values. A guest reboot relaunches via /// [`Hypervisor::spawn`] with a minimal spec (binary + args only), so this preserves @@ -235,6 +250,7 @@ impl CloudHypervisorBackend { memory: MemoryConfig { size: self.pending.mem_mib as u64 * 1024 * 1024, shared: false, + thp: false, }, payload: PayloadConfig { kernel: kernel.display().to_string(), @@ -299,6 +315,7 @@ impl Hypervisor for CloudHypervisorBackend { } async fn spawn(&mut self, spec: &ProcessSpec) -> Result<()> { + self.stop_reboot_monitor().await; // A reboot relaunch re-enters spawn() with a minimal spec (binary + args). Update // retained state only when the spec provides a value, so the namespace isolation // and name captured on the first spawn persist across reboots (mirrors the @@ -320,6 +337,11 @@ impl Hypervisor for CloudHypervisorBackend { let mut cmd = Command::new(&spec.binary); cmd.arg("--api-socket").arg(&self.api_socket); + let (events, child_events) = UnixStream::pair().context("creating CH event socketpair")?; + events.set_nonblocking(true)?; + let events = tokio::net::UnixStream::from_std(events)?; + let event_fd = child_events.as_raw_fd(); + cmd.arg("--event-monitor").arg(format!("fd={event_fd}")); if let Some(log_path) = &self.log_path { cmd.arg("--log-file").arg(log_path); cmd.arg("-v"); @@ -330,6 +352,18 @@ impl Hypervisor for CloudHypervisorBackend { } } + // Clear CLOEXEC only in this child. Changing it in the parent would let + // unrelated concurrent spawns inherit the event socket. Namespace setup + // remains the last pre_exec so its post-setns PDEATHSIG is preserved. + // SAFETY: fcntl is async-signal-safe, and child_events owns the fd until spawn returns. + unsafe { + cmd.pre_exec(move || { + if libc::fcntl(event_fd, libc::F_SETFD, 0) == -1 { + return Err(std::io::Error::last_os_error()); + } + Ok(()) + }); + } install_namespace_pre_exec(&mut cmd, &self.namespace)?; let stderr_tail = Arc::clone(&self.stderr_tail); @@ -353,11 +387,23 @@ impl Hypervisor for CloudHypervisorBackend { } }) .context("spawning Cloud Hypervisor process")?; + drop(child_events); self.process = Some(spawned.child); self.stderr_reader = Some(spawned.stderr_reader); self.wait_for_api().await?; self.client = Some(ChClient::new(self.api_socket.clone())); + let client = self.client()?.clone(); + self.reboot_monitor = Some(tokio::spawn(async move { + if read_reboot_event(BufReader::new(events)).await? { + // The guest's vsock notification is early intent, before shutdown + // writeback. CH emits this event only when it handles the actual + // reset. Exit the VMM here so fcvm consumes intent and cold-relaunches. + info!("Cloud Hypervisor guest reset, shutting down VMM for host relaunch"); + client.shutdown_vmm().await?; + } + Ok(()) + })); // Tail the guest console (hvc0 → file) into fcvm's tracing logs, mirroring the // Firecracker serial-to-stdout capture. Abort any prior tail first: a reboot @@ -387,6 +433,9 @@ impl Hypervisor for CloudHypervisorBackend { match self.process.as_mut() { Some(p) => match p.try_wait().context("checking Cloud Hypervisor status")? { Some(status) => { + if let Some(monitor) = &self.reboot_monitor { + monitor.abort(); + } self.process = None; Ok(Some(status)) } @@ -397,17 +446,43 @@ impl Hypervisor for CloudHypervisorBackend { } async fn wait(&mut self) -> Result { - match self.process.as_mut() { - Some(p) => { - let status = p.wait().await.context("waiting for Cloud Hypervisor")?; - self.process = None; - Ok(status) + let process = self + .process + .as_mut() + .context("Cloud Hypervisor process not running")?; + let mut monitor_failure = None; + let status = if let Some(monitor) = self.reboot_monitor.as_mut() { + tokio::select! { + biased; + status = process.wait() => status, + result = monitor => { + self.reboot_monitor = None; + if let Err(error) = result.context("CH reboot monitor task failed").and_then(|r| r) { + warn!(%error, "CH reboot monitor failed, terminating VMM"); + // The caller treats any wait() return as child termination. + // Never report this failure while the VMM can still be alive. + process.start_kill().context("terminating CH after event monitor failure")?; + monitor_failure = Some(error); + } + process.wait().await + } } - None => bail!("Cloud Hypervisor process not running"), + } else { + process.wait().await + } + .context("waiting for Cloud Hypervisor")?; + self.stop_reboot_monitor().await; + self.process = None; + if let Some(error) = monitor_failure { + return Err(error); } + Ok(status) } fn start_kill(&mut self) -> Result<()> { + if let Some(monitor) = &self.reboot_monitor { + monitor.abort(); + } if let Some(tail) = self.console_tail.take() { tail.abort(); } @@ -420,6 +495,7 @@ impl Hypervisor for CloudHypervisorBackend { } async fn reap(&mut self) { + self.stop_reboot_monitor().await; if let Some(mut p) = self.process.take() { let _ = p.wait().await; } @@ -536,6 +612,43 @@ impl Hypervisor for CloudHypervisorBackend { } } +impl Drop for CloudHypervisorBackend { + fn drop(&mut self) { + if let Some(monitor) = &self.reboot_monitor { + monitor.abort(); + } + } +} + +/// CH writes pretty-printed JSON objects separated by a blank line, not JSONL. +async fn read_reboot_event(reader: impl AsyncBufRead + Unpin) -> Result { + #[derive(serde::Deserialize)] + struct Event { + source: String, + event: String, + } + + let mut lines = reader.lines(); + let mut frame = String::new(); + while let Some(line) = lines.next_line().await.context("reading CH events")? { + if line.is_empty() { + if frame.is_empty() { + continue; + } + let event: Event = serde_json::from_str(&frame).context("decoding CH event")?; + if event.source == "vm" && event.event == "rebooting" { + return Ok(true); + } + frame.clear(); + } else { + frame.push_str(&line); + frame.push('\n'); + } + } + anyhow::ensure!(frame.is_empty(), "CH event stream ended within an event"); + Ok(false) +} + /// The default guest CID Cloud Hypervisor uses for the host↔guest vsock device. pub const fn default_guest_cid() -> u32 { GUEST_CID @@ -602,6 +715,86 @@ async fn tail_console_to_tracing(path: PathBuf, console_lines: Arc(); + // A started blocking task models a reader still in its current poll: + // abort requests cancellation but cannot finish the join until it yields. + be.reboot_monitor = Some(tokio::task::spawn_blocking(move || { + let _ = started_tx.send(()); + let _ = release_rx.recv(); + Ok(()) + })); + started_rx.await.unwrap(); + let pending = { + let mut cleanup = std::pin::pin!(be.stop_reboot_monitor()); + let mut context = std::task::Context::from_waker(std::task::Waker::noop()); + cleanup.as_mut().poll(&mut context).is_pending() + }; + let retained = be.reboot_monitor.is_some(); + // Release even on a failed assertion, so the fixture cannot strand a worker. + drop(release_tx); + be.stop_reboot_monitor().await; + assert!(pending, "cleanup must wait for the reader to finish"); + assert!( + retained, + "cancelled cleanup must retain the reader's join handle" + ); + assert!(be.reboot_monitor.is_none()); + } + + #[tokio::test] + async fn reboot_monitor_failure_reaps_child_before_returning() { + let mut be = backend(); + be.process = Some( + Command::new("sleep") + .arg("60") + .kill_on_drop(true) + .spawn() + .unwrap(), + ); + be.reboot_monitor = Some(tokio::spawn(async { bail!("injected CH event failure") })); + let result = tokio::time::timeout(Duration::from_secs(2), be.wait()).await; + let reaped = be.process.is_none(); + be.start_kill().unwrap(); + be.reap().await; + assert!(result + .unwrap() + .unwrap_err() + .to_string() + .contains("injected CH event failure")); + assert!( + reaped, + "wait must not report a monitor failure with a live VMM child" + ); + } + + #[tokio::test] + async fn reboot_monitor_requires_complete_vm_reset_event() { + let other_events = concat!( + "{\n \"source\": \"vmm\",\n \"event\": \"rebooting\"\n}\n\n", + "{\n \"source\": \"vm\",\n \"event\": \"rebooted\"\n}\n\n", + ); + assert!(!read_reboot_event(other_events.as_bytes()).await.unwrap()); + let events = format!( + "{other_events}{{\n \"source\": \"vm\",\n \"event\": \"rebooting\",\n \"properties\": null\n}}\n\n" + ); + // One-byte reads split both the JSON and its delimiter across reads. + assert!( + read_reboot_event(BufReader::with_capacity(1, events.as_bytes())) + .await + .unwrap() + ); + assert!(read_reboot_event(b"{\n \"source\": \"vm\"".as_slice()) + .await + .is_err()); + assert!(read_reboot_event(b"not-json\n\n".as_slice()).await.is_err()); + } + fn backend() -> CloudHypervisorBackend { CloudHypervisorBackend::new( "vm-test".to_string(), @@ -610,6 +803,21 @@ mod tests { ) } + /// CH madvises guest RAM MADV_HUGEPAGE unless the request says `thp: false`, so an + /// absent field opts in. See `MemoryConfig::thp` for what that cost on CI. + #[test] + fn vm_config_opts_guest_memory_out_of_transparent_hugepages() { + let mut be = backend(); + be.pending.kernel = Some(PathBuf::from("/boot/Image")); + let config = serde_json::to_value(be.build_vm_config().unwrap()).unwrap(); + assert_eq!( + config["memory"]["thp"], + serde_json::Value::Bool(false), + "vm.create memory config: {}", + config["memory"] + ); + } + /// Codex #632 P1 #1: a reboot relaunches with a minimal spec (binary + args only). /// The namespace isolation captured on the FIRST spawn must persist, or the /// relaunched VMM would run outside its namespaces. Before the fix, `spawn` read the diff --git a/src/setup/kernel.rs b/src/setup/kernel.rs index 4a84ab92..ea6abf47 100644 --- a/src/setup/kernel.rs +++ b/src/setup/kernel.rs @@ -2642,13 +2642,11 @@ pub async fn ensure_cloud_hypervisor(repo: &str, branch: &str) -> Result = makefile + .lines() + .find_map(|line| line.strip_prefix("test-root:")) + .expect( + "the Makefile defines no `test-root:` rule, so this cannot fail for the right reason", + ) + .split_whitespace() + .collect(); + assert!( + prerequisites.contains(&"setup-fcvm"), + "`test-root:` names no `setup-fcvm` prerequisite ({prerequisites:?}), so the parse is not \ + reading the rule it thinks it is" + ); + + let reboot = repo_file("tests/test_reboot.rs"); + assert!( + reboot.contains("fn test_cloud_hypervisor_reboot_recovers_and_then_exits") + && reboot.contains("find_cloud_hypervisor()"), + "the Cloud Hypervisor reboot test is gone or no longer requires the backend; remove this \ + pin with it" + ); + + assert!( + prerequisites.contains(&"setup-cloud-hypervisor"), + "`make test-root` runs test_cloud_hypervisor_reboot_recovers_and_then_exits, which needs \ + the Cloud Hypervisor binary, but does not depend on `setup-cloud-hypervisor`: \ + {prerequisites:?}" + ); +} + +/// A target that runs `fcvm setup` must mount the assets store before its recipe runs. +/// +/// `fcvm setup` writes under /mnt/fcvm-btrfs, which `setup-btrfs` mounts as a loopback on a host +/// that is not btrfs. Make runs a recipe after that target's own prerequisites only, and sibling +/// prerequisites run in any order under `make -j`, so a target that reaches `setup-btrfs` only +/// through a sibling can write into the bare directory and have the mount hide what it wrote. +/// `setup-cloud-hypervisor` did that under `test-root` (CodeRabbit on #921). Excluded: +/// `--generate-config`, which writes the user's config and not the store, and `_`-prefixed +/// targets, which run inside the container their host-side wrapper starts after that wrapper's +/// own prerequisites (`container-setup-fcvm` runs `_setup-fcvm`). +#[test] +fn targets_that_run_fcvm_setup_mount_the_assets_store_first() { + let makefile = repo_file("Makefile"); + let mut prerequisites: std::collections::HashMap<&str, Vec<&str>> = Default::default(); + let mut runs_setup: Vec<&str> = Vec::new(); + let mut current: Option<&str> = None; + for line in makefile.lines() { + if let Some(command) = line.strip_prefix('\t') { + if let Some(target) = current { + if command.contains("target/release/fcvm setup") + && !command.contains("--generate-config") + && !runs_setup.contains(&target) + { + runs_setup.push(target); + } + } + continue; + } + let rule = line.split_once(':').filter(|(name, _)| { + !name.is_empty() + && name + .chars() + .all(|c| c.is_ascii_alphanumeric() || "_.-".contains(c)) + }); + match rule { + Some((name, rest)) => { + current = Some(name); + // `target: private SHELL := ...` sets a variable; it names no prerequisites. + if !rest.contains('=') { + prerequisites + .entry(name) + .or_default() + .extend(rest.split_whitespace()); + } + } + None if !line.trim().is_empty() => current = None, + None => {} + } + } + + for expected in ["setup-default", "setup-fcvm", "setup-cloud-hypervisor"] { + assert!( + runs_setup.contains(&expected), + "`{expected}` no longer runs `fcvm setup` as far as this parse can tell \ + ({runs_setup:?}), so it is not reading the recipes it thinks it is" + ); + } + + for target in runs_setup.iter().filter(|t| !t.starts_with('_')) { + let mut seen: Vec<&str> = Vec::new(); + let mut stack = vec![*target]; + while let Some(t) = stack.pop() { + if seen.contains(&t) { + continue; + } + seen.push(t); + stack.extend(prerequisites.get(t).into_iter().flatten().copied()); + } + assert!( + seen.contains(&"setup-btrfs"), + "`{target}` runs `fcvm setup`, which writes under /mnt/fcvm-btrfs, but `setup-btrfs` \ + is not among its prerequisites. Under `make -j` it can run before the mount and \ + write into a directory the mount then hides. Prerequisites reached: {seen:?}" + ); + } +} diff --git a/tests/test_reboot.rs b/tests/test_reboot.rs index 3f3f5e10..2dde176d 100644 --- a/tests/test_reboot.rs +++ b/tests/test_reboot.rs @@ -4,7 +4,7 @@ //! relaunches in place from the same provisioned disk and comes back healthy, with //! the container's writable layer ("the work") preserved and its identity //! regenerated. The fcvm process (and therefore its PID) stays stable across the -//! reboot — only the Firecracker child restarts. +//! reboot, only the VMM child restarts. //! //! Both VM lifecycle paths are covered: //! * fresh `podman run` boot (`--no-snapshot` pins the run_vm_loop path) @@ -24,17 +24,47 @@ fn process_alive(pid: u32) -> bool { .unwrap_or(false) } +/// True when `after` is a machine-id that differs from the one read before the reboot. +fn machine_id_regenerated(before: &str, after: &str) -> bool { + let (before, after) = (before.trim(), after.trim()); + !before.is_empty() && !after.is_empty() && after != before +} + +/// The regeneration witness must compare against a real pre-reboot machine-id. An empty baseline, +/// which a failed read produced through `unwrap_or_default()`, made any machine-id read after the +/// reboot count as regenerated, including one from a VM that never rebooted (CodeRabbit on #921). +#[test] +fn a_regenerated_machine_id_needs_a_real_baseline() { + let before = "0123456789abcdef0123456789abcdef"; + let after = "fedcba9876543210fedcba9876543210\n"; + assert!(machine_id_regenerated(before, after)); + assert!(!machine_id_regenerated(before, before)); + assert!(!machine_id_regenerated(before, "")); + assert!( + !machine_id_regenerated("", after), + "an empty pre-reboot machine-id witnesses nothing" + ); + assert!( + !machine_id_regenerated("\n", after), + "a blank pre-reboot machine-id witnesses nothing" + ); +} + /// Reboot the guest and assert the SAME fcvm process relaunches it in place: /// machine-id regenerates (positive witness of the re-boot), health recovers, /// and the container's writable layer survives. async fn reboot_and_assert_relaunch(pid: u32, token: &str) -> Result<()> { let mid_before = common::exec_in_vm(pid, &["cat", "/etc/machine-id"]) .await - .unwrap_or_default(); + .context("reading the machine-id before the reboot")?; + anyhow::ensure!( + !mid_before.trim().is_empty(), + "the machine-id read before the reboot is empty, so a regenerated one cannot be told apart" + ); // `reboot` goes through systemd, which starts fcvm-reboot-notify.service // (WantedBy=reboot.target) -> fc-agent --notify-reboot -> host relaunches - // Firecracker in place. The exec dies mid-command as the VM resets. + // the VMM in place. The exec dies mid-command as the VM resets. let _ = common::exec_in_vm(pid, &["reboot"]).await; let deadline = Instant::now() + Duration::from_secs(150); @@ -45,8 +75,7 @@ async fn reboot_and_assert_relaunch(pid: u32, token: &str) -> Result<()> { "fcvm process (pid {pid}) must stay alive across an in-place reboot" ); if let Ok(mid) = common::exec_in_vm(pid, &["cat", "/etc/machine-id"]).await { - let mid = mid.trim().to_string(); - if !mid.is_empty() && mid != mid_before.trim() { + if machine_id_regenerated(&mid_before, &mid) { recovered = true; break; } @@ -124,6 +153,54 @@ async fn test_vm_reboot_comes_back_healthy_and_preserves_work() -> Result<()> { Ok(()) } +/// A normal Cloud Hypervisor VM must consume reboot intent before its final exit. +// Host-Root CI provisions Cloud Hypervisor; container-test-all does not. +#[cfg(feature = "privileged-tests")] +#[tokio::test] +async fn test_cloud_hypervisor_reboot_recovers_and_then_exits() -> Result<()> { + fcvm::commands::common::find_cloud_hypervisor() + .context("CH reboot test requires the backend")?; + let (name, _, _, _) = common::unique_names("ch-reboot"); + let (mut child, pid) = common::spawn_fcvm_with_logs( + &[ + "podman", + "run", + "--name", + &name, + "--hypervisor", + "cloud-hypervisor", + "--no-snapshot", + "nginx:alpine", + "sh", + "-c", + "while [ ! -e /stop ]; do sleep 1; done", + ], + "ch-reboot-base", + ) + .await?; + let result = async { + common::poll_health_by_pid(pid, 120).await?; + let token = format!("ch-reboot-token-{pid}"); + write_work_marker(pid, &token).await?; + reboot_and_assert_relaunch(pid, &token).await?; + + // The exec response may disappear when its container exits; the process + // exit below proves the stop marker was consumed and the final boot ended. + let _ = common::exec_in_container(pid, &["touch", "/stop"]).await; + let status = tokio::time::timeout(Duration::from_secs(60), child.wait()) + .await + .context("CH VM did not terminate after its container exited")??; + anyhow::ensure!(status.success(), "CH VM final exit must be zero: {status}"); + Ok::<_, anyhow::Error>(()) + } + .await; + if child.try_wait()?.is_none() { + common::kill_process(pid).await; + let _ = child.kill().await; + } + result +} + /// Data-disk preservation: an in-place reboot must NOT rebuild --disk-dir /// images from the host directory — guest writes to the data disk live only in /// the per-VM image and would be silently destroyed (confirmed review finding). @@ -168,7 +245,11 @@ async fn test_vm_reboot_preserves_disk_dir_writes() -> Result<()> { // Reboot; wait for the relaunch (machine-id change is the positive witness). let mid_before = common::exec_in_vm(pid, &["cat", "/etc/machine-id"]) .await - .unwrap_or_default(); + .context("reading the machine-id before the reboot")?; + anyhow::ensure!( + !mid_before.trim().is_empty(), + "the machine-id read before the reboot is empty, so a regenerated one cannot be told apart" + ); let _ = common::exec_in_vm(pid, &["reboot"]).await; let deadline = Instant::now() + Duration::from_secs(150); loop { @@ -177,7 +258,7 @@ async fn test_vm_reboot_preserves_disk_dir_writes() -> Result<()> { "fcvm process must stay alive across the reboot" ); if let Ok(mid) = common::exec_in_vm(pid, &["cat", "/etc/machine-id"]).await { - if !mid.trim().is_empty() && mid.trim() != mid_before.trim() { + if machine_id_regenerated(&mid_before, &mid) { break; } }