From f9d3d423300ec531ee13d84f562c244d1e208c13 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Fri, 6 Nov 2026 06:56:08 -0800 Subject: [PATCH 01/15] guest: rehome snapshot device and mount values Keep one canonical mount parser and MMIO-value device identity under guest::snapshot while retaining the legacy activation caller shape. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2610ed93-496e-4bc8-b990-ea9b49ab3462 --- nvx-rs/src/guest/activation.rs | 37 +++++-------- nvx-rs/src/guest/mod.rs | 2 + .../{activation/mounts.rs => snapshot.rs} | 54 ++++++++++++++----- .../{activation/mounts => snapshot}/tests.rs | 0 4 files changed, 55 insertions(+), 38 deletions(-) rename nvx-rs/src/guest/{activation/mounts.rs => snapshot.rs} (80%) rename nvx-rs/src/guest/{activation/mounts => snapshot}/tests.rs (100%) diff --git a/nvx-rs/src/guest/activation.rs b/nvx-rs/src/guest/activation.rs index c88267576..9eecf4e9d 100644 --- a/nvx-rs/src/guest/activation.rs +++ b/nvx-rs/src/guest/activation.rs @@ -273,7 +273,6 @@ //! writes the machine ID's and the resolver's files in the runtime directory, whose binds over //! the root's `/etc/machine-id` and `/etc/resolv.conf` need Linux 6.8 or later. -mod mounts; mod repair; mod state; #[cfg(test)] @@ -285,15 +284,14 @@ use std::io::{self, Read}; use std::mem; use std::path::{Path, PathBuf}; -use self::mounts::DeviceNumber; use super::GenerationId; use super::root::{self, Device, Paths}; +use super::snapshot::{self as mounts, DeviceNumber}; use super::sys; use super::time::{ CaptureOutcome, CaptureRequest, PreCaptureError, Restore, ScratchPolicy, TimeAbi, TimeError, }; -pub use self::mounts::{Mounted, Origin}; pub use self::repair::{ RangeFault, Repair, RepairCause, RepairError, RepairProgress, RepairStage, Repaired, repair, }; @@ -305,10 +303,7 @@ pub use self::state::{ pub use self::state::{ Applied, AppliedTeardownError, ApplyError, NetworkError, RootTemplate, apply, }; - -/// The most bytes of the mount table that a [`CapturePreparation`] reads: 1 MiB, the lines of -/// thousands of mounts. It refuses a longer table, rather than check a part of it. -pub const MOUNT_TABLE_LIMIT: usize = 1 << 20; +pub use super::snapshot::{MOUNT_TABLE_LIMIT, Mounted, Origin}; /// The mount table of the calling process's mount namespace, below procfs. const MOUNT_TABLE: &str = "self/mountinfo"; @@ -421,22 +416,9 @@ impl CapturePreparation { if let Err(error) = barrier.system.pre_capture() { // The barrier restores a suppression that the time ABI holds after a failure, as // after a preparation that succeeded. - barrier.suppressed = error.holds_suppression(); - let error = match error { - PreCaptureError::BootChecks(error) => { - CaptureError::new(CaptureStage::BootChecks, CaptureCause::Time(error)) - } - PreCaptureError::CaptureChecks(error) - | PreCaptureError::PortRelease { error, .. } => { - CaptureError::new(CaptureStage::CaptureChecks, CaptureCause::Time(error)) - } - PreCaptureError::Pending => { - CaptureError::new(CaptureStage::Suppression, CaptureCause::Pending) - } - PreCaptureError::Suppression(error) | PreCaptureError::Unrestored(error) => { - CaptureError::new(CaptureStage::Suppression, CaptureCause::Io(error.into())) - } - }; + let (stage, cause, suppressed) = mounts::preparation_failure(error); + barrier.suppressed = suppressed; + let error = CaptureError::new(stage, cause); return Err(barrier.fail(error)); } barrier.suppressed = true; @@ -463,7 +445,12 @@ fn check_mounts(system: &mut dyn System, devices: &Devices) -> Result<(), Captur format!("the mount table is longer than {MOUNT_TABLE_LIMIT} bytes"), ))); } - match mounts::first_blocking(system, devices, &table).map_err(CaptureCause::Io)? { + let devices = mounts::Devices { + distro: devices.distro.mmio, + scratch: devices.scratch.mmio, + filesystem_tag: devices.filesystem_tag.clone(), + }; + match mounts::first_blocking(system, &devices, &table).map_err(CaptureCause::Io)? { Some(mounted) => Err(CaptureCause::Mounted(mounted)), None => Ok(()), } @@ -779,7 +766,7 @@ impl std::error::Error for CaptureError { } /// What a capture reads of the kernel and asks of the time ABI, which the tests' fake replaces. -trait System: Send { +pub(super) trait System: Send { /// Reads the mount table of the calling process's mount namespace, at most one byte more than /// [`MOUNT_TABLE_LIMIT`] of it. fn mount_table(&mut self) -> io::Result>; diff --git a/nvx-rs/src/guest/mod.rs b/nvx-rs/src/guest/mod.rs index b5833c5e4..8c9085d9f 100644 --- a/nvx-rs/src/guest/mod.rs +++ b/nvx-rs/src/guest/mod.rs @@ -136,6 +136,8 @@ pub mod process; pub mod reseed; #[doc(hidden)] pub mod root; +#[cfg(target_arch = "x86_64")] +pub mod snapshot; pub mod spawn; pub mod terminal; #[cfg(target_arch = "x86_64")] diff --git a/nvx-rs/src/guest/activation/mounts.rs b/nvx-rs/src/guest/snapshot.rs similarity index 80% rename from nvx-rs/src/guest/activation/mounts.rs rename to nvx-rs/src/guest/snapshot.rs index 67c29a883..5d6f4e0b3 100644 --- a/nvx-rs/src/guest/activation/mounts.rs +++ b/nvx-rs/src/guest/snapshot.rs @@ -1,4 +1,4 @@ -//! The kernel's mount table, and the mounts in it that keep a template's guest from capturing. +//! The devices and mount checks of a platform snapshot. //! //! Each line of `/proc/self/mountinfo` describes one mount with fields separated by spaces: its //! ID, its parent's ID, the device number of its file system, `MAJ:MIN`, the root of the mount @@ -13,9 +13,24 @@ use std::io; use std::os::unix::ffi::OsStringExt; use std::path::{Path, PathBuf}; -use super::{Devices, System}; +use super::activation::{CaptureCause, CaptureStage, System}; +use super::time::PreCaptureError; use crate::guest::parse::parse_decimal; +/// The most bytes of the caller's mount table that capture accepts: 1 MiB. +pub const MOUNT_TABLE_LIMIT: usize = 1 << 20; + +/// The devices from which a platform capture must find nothing mounted. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct Devices { + /// The distro device's MMIO base, including all of its partitions. + pub distro: u64, + /// The scratch device's MMIO base. + pub scratch: u64, + /// The host filesystem device's virtio-fs tag. + pub filesystem_tag: String, +} + /// The file system type of an overlay, as the mount table shows it. const OVERLAY: &[u8] = b"overlay"; /// The file system type of a virtio-fs device's mounts, whose source is the device's tag. @@ -194,23 +209,17 @@ impl fmt::Display for Mounted { } } -/// Returns the first mount of the mount table `table` that is from `devices` or an overlay, bind -/// mounts included, or `None` if there is none. +/// Returns the first mount from the named devices or an overlay, bind mounts included. /// -/// A mount is from the distro device or the scratch device when its file system's device, which -/// `system` finds in sysfs by its number, is below the sysfs directory of the device's MMIO -/// window, where the kernel creates the device's block device and partitions. A mount is from the -/// filesystem tag when its file system is virtio-fs and its source is the tag, and from an overlay -/// when its file system is one. A line that is not of the kernel's form, or a file system on a -/// block device that sysfs does not show, fails the check, which never passes a mount that it -/// cannot tell apart. +/// Device numbers are resolved through sysfs; malformed lines and unresolved block devices +/// fail closed rather than passing a mount that cannot be classified. pub(super) fn first_blocking( system: &mut dyn System, devices: &Devices, table: &[u8], ) -> io::Result> { - let distro = system.windows(devices.distro.mmio)?; - let scratch = system.windows(devices.scratch.mmio)?; + let distro = system.windows(devices.distro)?; + let scratch = system.windows(devices.scratch)?; let tag = devices.filesystem_tag.as_bytes(); for (index, text) in table.split(|&byte| byte == b'\n').enumerate() { if text.is_empty() { @@ -250,5 +259,24 @@ pub(super) fn first_blocking( Ok(None) } +/// The existing preparation's stage and cause, and whether it owns suppression to undo. +pub(crate) type PreparationFailure = (CaptureStage, CaptureCause, bool); + +/// Classifies preparation without changing its suppression ownership. +pub(super) fn preparation_failure(error: PreCaptureError) -> PreparationFailure { + let suppressed = error.holds_suppression(); + let (stage, cause) = match error { + PreCaptureError::BootChecks(error) => (CaptureStage::BootChecks, CaptureCause::Time(error)), + PreCaptureError::CaptureChecks(error) | PreCaptureError::PortRelease { error, .. } => { + (CaptureStage::CaptureChecks, CaptureCause::Time(error)) + } + PreCaptureError::Pending => (CaptureStage::Suppression, CaptureCause::Pending), + PreCaptureError::Suppression(error) | PreCaptureError::Unrestored(error) => { + (CaptureStage::Suppression, CaptureCause::Io(error.into())) + } + }; + (stage, cause, suppressed) +} + #[cfg(test)] mod tests; diff --git a/nvx-rs/src/guest/activation/mounts/tests.rs b/nvx-rs/src/guest/snapshot/tests.rs similarity index 100% rename from nvx-rs/src/guest/activation/mounts/tests.rs rename to nvx-rs/src/guest/snapshot/tests.rs From cf93f1cfcfbc7bc7849602cfd205046191d5dc65 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Fri, 6 Nov 2026 07:29:02 -0800 Subject: [PATCH 02/15] guest: add a bounded fresh snapshot capture Compose mount and strict time preparation with one explicit pre-request deadline, including runtime locks and suppression rollback. Clear the deadline before the capture request and report whether the caller may resume or must end the guest. Keep the existing activation capture interface and reject unexpected Fresh restore network settings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2610ed93-496e-4bc8-b990-ea9b49ab3462 --- nvx-rs/src/guest/activation.rs | 58 +++-- nvx-rs/src/guest/activation/tests.rs | 18 +- nvx-rs/src/guest/snapshot.rs | 232 ++++++++++++++++++- nvx-rs/src/guest/snapshot/tests.rs | 313 ++++++++++++++++++++++++++ nvx-rs/src/guest/sys/fd.rs | 44 ++++ nvx-rs/src/guest/sys/fd/tests.rs | 34 +++ nvx-rs/src/guest/time/api.rs | 69 +++++- nvx-rs/src/guest/time/api/tests.rs | 19 ++ nvx-rs/src/guest/time/checks.rs | 31 ++- nvx-rs/src/guest/time/system.rs | 102 ++++++--- nvx-rs/src/guest/time/system/tests.rs | 77 +++++++ 11 files changed, 932 insertions(+), 65 deletions(-) diff --git a/nvx-rs/src/guest/activation.rs b/nvx-rs/src/guest/activation.rs index 9eecf4e9d..2c383cfae 100644 --- a/nvx-rs/src/guest/activation.rs +++ b/nvx-rs/src/guest/activation.rs @@ -283,6 +283,7 @@ use std::fs::{self, File}; use std::io::{self, Read}; use std::mem; use std::path::{Path, PathBuf}; +use std::time::Instant; use super::GenerationId; use super::root::{self, Device, Paths}; @@ -392,11 +393,7 @@ impl CapturePreparation { /// the permission for the time ABI's ports fails with [`TimeError::PortRelease`] instead, /// after which the thread must start neither a thread nor a process, which would inherit it. pub fn prepare(self) -> Result, CaptureError> { - let system = LiveSystem { - paths: Paths::default(), - abi: live_abi(), - }; - self.prepare_with(Box::new(system)) + self.prepare_with(capture_system(None)) } /// Implements [`prepare`](Self::prepare) on `system`. @@ -438,22 +435,12 @@ impl fmt::Debug for CapturePreparation { /// Checks, in the mount table that `system` reads, that nothing is mounted from `devices` or an /// overlay, and fails with the first mount that is, or with why the table could not be checked. fn check_mounts(system: &mut dyn System, devices: &Devices) -> Result<(), CaptureCause> { - let table = system.mount_table().map_err(CaptureCause::Io)?; - if table.len() > MOUNT_TABLE_LIMIT { - return Err(CaptureCause::Io(io::Error::new( - io::ErrorKind::FileTooLarge, - format!("the mount table is longer than {MOUNT_TABLE_LIMIT} bytes"), - ))); - } let devices = mounts::Devices { distro: devices.distro.mmio, scratch: devices.scratch.mmio, filesystem_tag: devices.filesystem_tag.clone(), }; - match mounts::first_blocking(system, &devices, &table).map_err(CaptureCause::Io)? { - Some(mounted) => Err(CaptureCause::Mounted(mounted)), - None => Ok(()), - } + mounts::check_mounts(system, &devices, None) } /// The barrier of a prepared capture, which holds the caller's fence closed and the stall @@ -782,6 +769,12 @@ pub(super) trait System: Send { /// detectors, as [`TimeAbi::pre_capture`] does. fn pre_capture(&mut self) -> Result<(), PreCaptureError>; + /// Prepares strictly by the caller deadline, retaining suppression ownership on failure. + fn pre_capture_until(&mut self, deadline: Instant) -> Result<(), (PreCaptureError, bool)>; + + /// Removes the pre-request deadline before requesting capture. + fn clear_deadline(&mut self); + /// Restores the stall detectors' settings, as [`TimeAbi::cancel_capture`] does. fn cancel_capture(&mut self) -> Result<(), Vec>; @@ -798,6 +791,17 @@ struct LiveSystem { paths: Paths, /// The time ABI of [`live_abi`], whose preparation of a capture enforces the ABI all the same. abi: TimeAbi, + /// The caller's pre-request deadline, cleared before the request. + deadline: Option, +} + +/// Returns the live capture seam with an explicit optional pre-request deadline. +pub(super) fn capture_system(deadline: Option) -> Box { + Box::new(LiveSystem { + paths: Paths::default(), + abi: live_abi(), + deadline, + }) } /// Returns the time ABI of the capture and of the repair: in strict mode, unless the kernel @@ -808,6 +812,7 @@ fn live_abi() -> TimeAbi { impl System for LiveSystem { fn mount_table(&mut self) -> io::Result> { + sys::FileLock::check_deadline(self.deadline)?; let path = self.paths.proc.join(MOUNT_TABLE); let mut table = Vec::new(); File::open(&path) @@ -816,14 +821,17 @@ impl System for LiveSystem { .read_to_end(&mut table) }) .map_err(|error| at(&path, &error))?; + sys::FileLock::check_deadline(self.deadline)?; Ok(table) } fn windows(&mut self, mmio: u64) -> io::Result> { + sys::FileLock::check_deadline(self.deadline)?; let windows = root::window_directories(&self.paths, mmio) .map_err(|error| at(&self.paths.proc.join("iomem"), &error))?; let mut directories = Vec::with_capacity(windows.len()); for window in windows { + sys::FileLock::check_deadline(self.deadline)?; match fs::canonicalize(&window) { Ok(directory) => directories.push(directory), // The kernel lists the window before it adds the window's device, from which @@ -832,24 +840,36 @@ impl System for LiveSystem { Err(error) => return Err(at(&window, &error)), } } + sys::FileLock::check_deadline(self.deadline)?; Ok(directories) } fn block_device(&mut self, device: DeviceNumber) -> io::Result { + sys::FileLock::check_deadline(self.deadline)?; let path = self.paths.sys.join(BLOCK_DEVICES).join(device.to_string()); - fs::canonicalize(&path).map_err(|error| at(&path, &error)) + let directory = fs::canonicalize(&path).map_err(|error| at(&path, &error))?; + sys::FileLock::check_deadline(self.deadline)?; + Ok(directory) } fn pre_capture(&mut self) -> Result<(), PreCaptureError> { self.abi.pre_capture() } + fn pre_capture_until(&mut self, deadline: Instant) -> Result<(), (PreCaptureError, bool)> { + self.abi.pre_capture_until(deadline) + } + + fn clear_deadline(&mut self) { + self.deadline = None; + } + fn cancel_capture(&mut self) -> Result<(), Vec> { - self.abi.cancel_capture() + self.abi.cancel_capture_until(self.deadline) } fn generation_id(&mut self) -> Result { - self.abi.generation_id() + self.abi.generation_id_until(self.deadline) } fn capture(&mut self, request: CaptureRequest) -> Result { diff --git a/nvx-rs/src/guest/activation/tests.rs b/nvx-rs/src/guest/activation/tests.rs index 0b0bb029c..f5b411944 100644 --- a/nvx-rs/src/guest/activation/tests.rs +++ b/nvx-rs/src/guest/activation/tests.rs @@ -287,6 +287,16 @@ impl System for FakeSystem { } } + fn pre_capture_until(&mut self, _deadline: Instant) -> Result<(), (PreCaptureError, bool)> { + let result = self.pre_capture(); + let held = result + .as_ref() + .map_or_else(PreCaptureError::holds_suppression, |()| true); + result.map_err(|error| (error, held)) + } + + fn clear_deadline(&mut self) {} + fn generation_id(&mut self) -> Result { self.calls.push(Call::GenerationId); if self.generation_fails { @@ -997,6 +1007,7 @@ fn the_live_system_finds_windows_and_block_devices_in_procfs_and_sysfs() { let mut system = LiveSystem { paths: paths.clone(), abi: TimeAbi::report_only(), + deadline: None, }; assert_eq!(system.windows(DISTRO).unwrap(), [window]); assert_eq!(system.windows(SCRATCH).unwrap(), Vec::::new()); @@ -1016,7 +1027,12 @@ fn the_live_system_finds_windows_and_block_devices_in_procfs_and_sysfs() { // The check finds a layer in the partition, from the live system's answers. let table = format!("{TEMPLATE_TABLE}30 23 254:2 / /run/nvx/layers/p2 ro - ext4 /dev/vda2 ro\n"); - let mounted = mounts::first_blocking(&mut system, &devices(), table.as_bytes()) + let canonical = mounts::Devices { + distro: DISTRO, + scratch: SCRATCH, + filesystem_tag: TAG.to_owned(), + }; + let mounted = mounts::first_blocking(&mut system, &canonical, table.as_bytes(), None) .unwrap() .unwrap(); assert_eq!(mounted.origin(), Origin::Distro); diff --git a/nvx-rs/src/guest/snapshot.rs b/nvx-rs/src/guest/snapshot.rs index 5d6f4e0b3..859a43362 100644 --- a/nvx-rs/src/guest/snapshot.rs +++ b/nvx-rs/src/guest/snapshot.rs @@ -1,4 +1,16 @@ -//! The devices and mount checks of a platform snapshot. +//! A Fresh platform capture in the calling guest process, without execution control. +//! +//! [`capture`] checks mounts and the time ABI, suppresses the stall detectors, reads the +//! generation ID, and requests capture. All pre-request steps, locks, waits and rollback +//! share one caller deadline. The request has no deadline while the host saves stopped +//! processors. A strict time violation is returned with its status, never powered off here. +//! +//! The caller keeps its executions still. After answering the capture request, its transport +//! sends nothing until this call returns. After [`Captured::Continued`] or a failure with +//! [`Disposition::Resumed`], it reports on the same session. +//! +//! This interface does not repair restored guests, which must end. A Fresh platform restore +//! that carries network settings is explicitly refused rather than silently dropping them. //! //! Each line of `/proc/self/mountinfo` describes one mount with fields separated by spaces: its //! ID, its parent's ID, the device number of its file system, `MAJ:MIN`, the root of the mount @@ -12,10 +24,15 @@ use std::fmt; use std::io; use std::os::unix::ffi::OsStringExt; use std::path::{Path, PathBuf}; +use std::time::Instant; -use super::activation::{CaptureCause, CaptureStage, System}; -use super::time::PreCaptureError; +use super::activation::{self, System}; +use super::sys; +use super::time::{CaptureOutcome, CaptureRequest, PreCaptureError, ScratchPolicy, TimeError}; use crate::guest::parse::parse_decimal; +use crate::protocol::status; + +pub use super::activation::{CaptureCause, CaptureStage}; /// The most bytes of the caller's mount table that capture accepts: 1 MiB. pub const MOUNT_TABLE_LIMIT: usize = 1 << 20; @@ -31,6 +48,209 @@ pub struct Devices { pub filesystem_tag: String, } +/// The disk policy of a capture. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +#[non_exhaustive] +pub enum Disks { + /// Capture before mounting the disks that each clone replaces. + Fresh, +} + +/// How a capture ended in this guest. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[non_exhaustive] +pub enum Captured { + /// No snapshot was taken; the stall detectors were restored and this guest may continue. + Continued, +} + +/// Whether the caller may resume the guest or must end it. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[non_exhaustive] +pub enum Disposition { + /// No detector suppression owned by this call remains. + Resumed, + /// The guest cannot safely continue. + MustEnd { + /// The exit status, preserving a strict time violation's actual status. + status: u8, + }, +} + +/// A capture failure and what the caller must do with the guest. +#[derive(Debug)] +pub struct CaptureFailure { + /// The stage that failed. + pub stage: CaptureStage, + /// The failure, or a failed detector restoration. + pub cause: CaptureCause, + /// Whether the guest may resume or must end. + pub disposition: Disposition, +} + +impl CaptureFailure { + /// Classifies a failure, making request and release failures final. + fn new(stage: CaptureStage, cause: CaptureCause) -> Self { + let must_end = matches!(stage, CaptureStage::Capture | CaptureStage::Release); + let status = match &cause { + CaptureCause::Time(TimeError::Violation(violation)) => Some(violation.status()), + CaptureCause::Time(TimeError::PortRelease(_)) => Some(status::TIME_REPAIR), + _ => must_end.then_some(status::TIME_REPAIR), + }; + Self { + stage, + cause, + disposition: status.map_or(Disposition::Resumed, |status| Disposition::MustEnd { + status, + }), + } + } + + /// Restores only this call's suppression, retaining a port-release cause and fatal status. + fn cleanup(mut self, system: &mut dyn System, suppressed: bool) -> Self { + if suppressed + && let Err(cause) = release(system) + && !matches!(self.cause, CaptureCause::Time(TimeError::PortRelease(_))) + { + let failure = Self::new(CaptureStage::Release, cause); + self.stage = failure.stage; + self.cause = failure.cause; + if self.disposition == Disposition::Resumed { + self.disposition = failure.disposition; + } + } + self + } +} + +impl fmt::Display for CaptureFailure { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(formatter, "failed to {}: {}", self.stage, self.cause) + } +} + +impl std::error::Error for CaptureFailure { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match &self.cause { + CaptureCause::Io(error) => Some(error), + CaptureCause::Time(error) => Some(error), + CaptureCause::Mounted(_) | CaptureCause::Pending => None, + } + } +} + +/// Captures a Fresh template without freezing or inspecting the caller's executions. +/// +/// The deadline bounds pre-request work only. This call starts no helper, drives no transport, +/// repairs no restore, and never powers off the VM; failures return the caller's disposition. +pub fn capture( + devices: &Devices, + disks: Disks, + deadline: Instant, +) -> Result { + let Disks::Fresh = disks; + sys::check_released().map_err(|error| { + CaptureFailure::new( + CaptureStage::CaptureChecks, + CaptureCause::Time(error.into()), + ) + })?; + before(CaptureStage::BootChecks, deadline)?; + capture_with( + activation::capture_system(Some(deadline)).as_mut(), + devices, + deadline, + ) +} + +/// Composes the existing kernel/time operations with one explicit pre-request deadline. +fn capture_with( + system: &mut dyn System, + devices: &Devices, + deadline: Instant, +) -> Result { + let mut suppressed = false; + let prepared: Result<_, CaptureFailure> = (|| { + check_mounts(system, devices, Some(deadline)) + .map_err(|cause| CaptureFailure::new(CaptureStage::Mounts, cause))?; + system + .pre_capture_until(deadline) + .map_err(|(error, held)| { + suppressed = held; + let (stage, cause, _) = preparation_failure(error); + CaptureFailure::new(stage, cause) + })?; + suppressed = true; + before(CaptureStage::GenerationId, deadline)?; + let generation = system.generation_id().map_err(|error| { + CaptureFailure::new(CaptureStage::GenerationId, CaptureCause::Time(error)) + })?; + before(CaptureStage::GenerationId, deadline)?; + Ok(CaptureRequest::in_memory(ScratchPolicy::Fresh, generation)) + })(); + let request = prepared.map_err(|failure| failure.cleanup(system, suppressed))?; + system.clear_deadline(); + let outcome = system.capture(request).map_err(|error| { + CaptureFailure::new(CaptureStage::Capture, CaptureCause::Time(error)) + .cleanup(system, suppressed) + })?; + match outcome { + CaptureOutcome::Continued => { + release(system).map_err(|cause| CaptureFailure::new(CaptureStage::Release, cause))?; + Ok(Captured::Continued) + } + CaptureOutcome::Restored(restore) => { + let detail = if restore.network().is_some() { + "a Fresh platform restore must not carry network settings" + } else { + "this capture interface does not repair restored guests" + }; + Err(CaptureFailure::new( + CaptureStage::Capture, + CaptureCause::Io(io::Error::new(io::ErrorKind::Unsupported, detail)), + ) + .cleanup(system, suppressed)) + } + } +} + +/// Checks the same caller deadline before the next pre-request step. +fn before(stage: CaptureStage, deadline: Instant) -> Result<(), CaptureFailure> { + sys::FileLock::check_deadline(Some(deadline)) + .map_err(|error| CaptureFailure::new(stage, CaptureCause::Io(error.into()))) +} + +/// Restores detectors and prioritizes any port permission that the thread still holds. +fn release(system: &mut dyn System) -> Result<(), CaptureCause> { + let result = system.cancel_capture(); + sys::check_released().map_err(|error| CaptureCause::Time(error.into()))?; + result.map_err(|detail| { + CaptureCause::Io(io::Error::other( + String::from_utf8_lossy(&detail).into_owned(), + )) + }) +} + +/// Checks the one bounded mount table for both capture interfaces. +pub(super) fn check_mounts( + system: &mut dyn System, + devices: &Devices, + deadline: Option, +) -> Result<(), CaptureCause> { + sys::FileLock::check_deadline(deadline).map_err(CaptureCause::Io)?; + let table = system.mount_table().map_err(CaptureCause::Io)?; + if table.len() > MOUNT_TABLE_LIMIT { + return Err(CaptureCause::Io(io::Error::new( + io::ErrorKind::FileTooLarge, + format!("the mount table is longer than {MOUNT_TABLE_LIMIT} bytes"), + ))); + } + match first_blocking(system, devices, &table, deadline).map_err(CaptureCause::Io)? { + Some(mounted) => Err(CaptureCause::Mounted(mounted)), + None => Ok(()), + } +} + /// The file system type of an overlay, as the mount table shows it. const OVERLAY: &[u8] = b"overlay"; /// The file system type of a virtio-fs device's mounts, whose source is the device's tag. @@ -217,11 +437,16 @@ pub(super) fn first_blocking( system: &mut dyn System, devices: &Devices, table: &[u8], + deadline: Option, ) -> io::Result> { + sys::FileLock::check_deadline(deadline)?; let distro = system.windows(devices.distro)?; + sys::FileLock::check_deadline(deadline)?; let scratch = system.windows(devices.scratch)?; + sys::FileLock::check_deadline(deadline)?; let tag = devices.filesystem_tag.as_bytes(); for (index, text) in table.split(|&byte| byte == b'\n').enumerate() { + sys::FileLock::check_deadline(deadline)?; if text.is_empty() { continue; } @@ -239,6 +464,7 @@ pub(super) fn first_blocking( None } else { let device = system.block_device(line.device)?; + sys::FileLock::check_deadline(deadline)?; let below = |windows: &[PathBuf]| windows.iter().any(|window| device.starts_with(window)); if below(&distro) { diff --git a/nvx-rs/src/guest/snapshot/tests.rs b/nvx-rs/src/guest/snapshot/tests.rs index d72361ce0..cc291a688 100644 --- a/nvx-rs/src/guest/snapshot/tests.rs +++ b/nvx-rs/src/guest/snapshot/tests.rs @@ -1,4 +1,317 @@ use super::*; +use crate::guest::GenerationId; +use crate::guest::sys::Errno; +use crate::guest::time::{Restore, Violation}; +use std::thread; +use std::time::Duration; + +/// A capture seam that records ordering and requires each operation to be called once. +struct CaptureSystem { + /// Calls in execution order. + calls: Vec<&'static str>, + /// The caller's deadline until the request context clears it. + deadline: Option, + /// The calling namespace's mount table. + table: Vec, + /// The next preparation's result and suppression ownership. + preparation: Option>, + /// The next generation read. + generation: Option>, + /// The next request's outcome. + outcome: Option>, + /// Whether detector restoration fails. + release_fails: bool, + /// Time spent reading the generation ID. + generation_delay: Duration, + /// Time spent in the request, which must not have a deadline. + request_delay: Duration, +} + +impl CaptureSystem { + /// Returns a successful template capture with the caller's deadline. + fn new(deadline: Instant) -> Self { + Self { + calls: Vec::new(), + deadline: Some(deadline), + table: b"1 1 0:2 / / rw - rootfs rootfs rw\n".to_vec(), + preparation: Some(Ok(())), + generation: Some(Ok(GenerationId::from_bytes([7; GenerationId::LENGTH]))), + outcome: Some(Ok(CaptureOutcome::Continued)), + release_fails: false, + generation_delay: Duration::ZERO, + request_delay: Duration::ZERO, + } + } +} + +impl System for CaptureSystem { + fn mount_table(&mut self) -> io::Result> { + self.calls.push("mounts"); + Ok(self.table.clone()) + } + + fn windows(&mut self, _: u64) -> io::Result> { + self.calls.push("window"); + Ok(Vec::new()) + } + + fn block_device(&mut self, _: DeviceNumber) -> io::Result { + panic!("this fixture has no block-backed mounts"); + } + + fn pre_capture(&mut self) -> Result<(), PreCaptureError> { + panic!("the single-call capture must use the strict bounded preparation"); + } + + fn pre_capture_until(&mut self, deadline: Instant) -> Result<(), (PreCaptureError, bool)> { + assert_eq!(self.deadline, Some(deadline)); + self.calls.push("prepare"); + self.preparation.take().expect("preparation runs once") + } + + fn clear_deadline(&mut self) { + self.calls.push("unbounded"); + self.deadline = None; + } + + fn cancel_capture(&mut self) -> Result<(), Vec> { + self.calls.push("release"); + sys::FileLock::check_deadline(self.deadline) + .map_err(|error| error.desc().as_bytes().to_vec())?; + if self.release_fails { + Err(b"detector restore failed".to_vec()) + } else { + Ok(()) + } + } + + fn generation_id(&mut self) -> Result { + self.calls.push("generation"); + thread::sleep(self.generation_delay); + self.generation.take().expect("generation is read once") + } + + fn capture(&mut self, request: CaptureRequest) -> Result { + assert_eq!(self.deadline, None); + assert_eq!( + request, + CaptureRequest::in_memory( + ScratchPolicy::Fresh, + GenerationId::from_bytes([7; GenerationId::LENGTH]), + ) + ); + self.calls.push("request"); + thread::sleep(self.request_delay); + self.outcome.take().expect("capture is requested once") + } +} + +/// The canonical MMIO identities, independent of the legacy root Device type. +fn capture_devices() -> Devices { + Devices { + distro: 0xd000_3000, + scratch: 0xd000_6000, + filesystem_tag: "microvm".into(), + } +} + +#[test] +fn fresh_capture_orders_the_existing_steps_and_does_not_deadline_the_request() { + let deadline = Instant::now() + Duration::from_millis(50); + let mut system = CaptureSystem::new(deadline); + system.request_delay = Duration::from_millis(100); + assert_eq!( + capture_with(&mut system, &capture_devices(), deadline).unwrap(), + Captured::Continued + ); + assert!(Instant::now() > deadline); + assert_eq!( + system.calls, + [ + "mounts", + "window", + "window", + "prepare", + "generation", + "unbounded", + "request", + "release", + ] + ); +} + +#[test] +fn a_pre_request_failure_resumes_only_after_its_detector_restoration_succeeds() { + for release_fails in [false, true] { + let deadline = Instant::now() + Duration::from_secs(1); + let mut system = CaptureSystem::new(deadline); + system.generation = Some(Err(TimeError::GenerationIdUnavailable)); + system.release_fails = release_fails; + let failure = capture_with(&mut system, &capture_devices(), deadline).unwrap_err(); + if release_fails { + assert_eq!(failure.stage, CaptureStage::Release); + assert_eq!( + failure.disposition, + Disposition::MustEnd { + status: status::TIME_REPAIR + } + ); + } else { + assert_eq!(failure.stage, CaptureStage::GenerationId); + assert_eq!(failure.disposition, Disposition::Resumed); + } + assert_eq!(system.calls.last(), Some(&"release")); + assert!(!system.calls.contains(&"request")); + } +} + +#[test] +fn strict_time_status_is_retained_when_detector_rollback_also_fails() { + for violation in [ + Violation::of_missing_daemon(), + Violation::of_repair("repair failed"), + ] { + let status = violation.status(); + let deadline = Instant::now() + Duration::from_secs(1); + let mut system = CaptureSystem::new(deadline); + system.preparation = Some(Err(( + PreCaptureError::CaptureChecks(TimeError::Violation(violation)), + true, + ))); + system.release_fails = true; + let failure = capture_with(&mut system, &capture_devices(), deadline).unwrap_err(); + assert_eq!(failure.disposition, Disposition::MustEnd { status }); + assert_eq!(failure.stage, CaptureStage::Release); + assert!(!system.calls.contains(&"generation")); + } +} + +#[test] +fn port_release_is_fatal_and_keeps_its_cause_at_each_time_stage_despite_rollback_failure() { + for stage in [ + CaptureStage::CaptureChecks, + CaptureStage::GenerationId, + CaptureStage::Capture, + ] { + let deadline = Instant::now() + Duration::from_secs(1); + let mut system = CaptureSystem::new(deadline); + let port = || TimeError::PortRelease(Errno::ENOMEM.into()); + match stage { + CaptureStage::CaptureChecks => { + system.preparation = Some(Err(( + PreCaptureError::PortRelease { + error: port(), + suppressed: true, + }, + true, + ))); + } + CaptureStage::GenerationId => system.generation = Some(Err(port())), + CaptureStage::Capture => system.outcome = Some(Err(port())), + _ => unreachable!(), + } + system.release_fails = true; + let failure = capture_with(&mut system, &capture_devices(), deadline).unwrap_err(); + assert_eq!(failure.stage, stage); + assert!( + matches!(failure.cause, CaptureCause::Time(TimeError::PortRelease(ref error)) + if error.raw_os_error() == Some(Errno::ENOMEM as i32)) + ); + assert_eq!( + failure.disposition, + Disposition::MustEnd { + status: status::TIME_REPAIR + } + ); + assert_eq!(system.calls.last(), Some(&"release")); + } +} + +#[test] +fn an_expired_pre_request_deadline_prevents_capture_and_does_not_extend_rollback() { + let deadline = Instant::now(); + let mut system = CaptureSystem::new(deadline); + let failure = capture_with(&mut system, &capture_devices(), deadline).unwrap_err(); + assert_eq!(failure.stage, CaptureStage::Mounts); + assert_eq!(failure.disposition, Disposition::Resumed); + assert!(system.calls.is_empty()); + + let deadline = Instant::now() + Duration::from_millis(20); + let mut system = CaptureSystem::new(deadline); + system.generation_delay = Duration::from_millis(40); + let failure = capture_with(&mut system, &capture_devices(), deadline).unwrap_err(); + assert_eq!(failure.stage, CaptureStage::Release); + assert_eq!( + failure.disposition, + Disposition::MustEnd { + status: status::TIME_REPAIR + } + ); + assert!(!system.calls.contains(&"request")); + assert_eq!(system.calls.last(), Some(&"release")); +} + +#[test] +fn a_fresh_restore_refuses_even_an_otherwise_empty_network_policy() { + let deadline = Instant::now() + Duration::from_secs(1); + let mut system = CaptureSystem::new(deadline); + system.outcome = Some(Ok(CaptureOutcome::Restored( + Restore::to(GenerationId::from_bytes([8; GenerationId::LENGTH])).with_network(None, false), + ))); + let failure = capture_with(&mut system, &capture_devices(), deadline).unwrap_err(); + assert_eq!(failure.stage, CaptureStage::Capture); + assert!(matches!(failure.cause, CaptureCause::Io(ref error) + if error.kind() == io::ErrorKind::Unsupported && error.to_string().contains("network"))); + assert_eq!( + failure.disposition, + Disposition::MustEnd { + status: status::TIME_REPAIR + } + ); + assert_eq!(system.calls.last(), Some(&"release")); +} + +#[test] +fn request_and_release_failures_never_report_that_the_guest_resumed() { + for release_fails in [false, true] { + let deadline = Instant::now() + Duration::from_secs(1); + let mut system = CaptureSystem::new(deadline); + system.release_fails = release_fails; + if !release_fails { + system.outcome = Some(Err(TimeError::StateUnavailable)); + } + let failure = capture_with(&mut system, &capture_devices(), deadline).unwrap_err(); + assert_eq!( + failure.stage, + if release_fails { + CaptureStage::Release + } else { + CaptureStage::Capture + } + ); + assert_eq!( + failure.disposition, + Disposition::MustEnd { + status: status::TIME_REPAIR + } + ); + } +} + +#[test] +fn a_mount_refusal_never_prepares_or_requests_capture() { + let deadline = Instant::now() + Duration::from_secs(1); + let mut system = CaptureSystem::new(deadline); + system.table = b"1 1 0:2 / /root rw - overlay overlay rw\n".to_vec(); + let failure = capture_with(&mut system, &capture_devices(), deadline).unwrap_err(); + assert_eq!(failure.stage, CaptureStage::Mounts); + assert!( + matches!(failure.cause, CaptureCause::Mounted(ref mounted) if mounted.origin() == Origin::Overlay) + ); + assert_eq!(failure.disposition, Disposition::Resumed); + assert!(!system.calls.contains(&"prepare")); + assert!(!system.calls.contains(&"release")); +} /// Returns the line that the kernel writes for a mount of `filesystem` from `source` at /// `mount_point`, on the device `device`, with the optional fields `optional`. diff --git a/nvx-rs/src/guest/sys/fd.rs b/nvx-rs/src/guest/sys/fd.rs index 59f633088..c1047d1f9 100644 --- a/nvx-rs/src/guest/sys/fd.rs +++ b/nvx-rs/src/guest/sys/fd.rs @@ -3,6 +3,8 @@ use std::ffi::CStr; use std::io; use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd, RawFd}; +use std::thread; +use std::time::{Duration, Instant}; pub(crate) use nix::fcntl::OFlag as OpenFlags; use nix::fcntl::{FcntlArg, FdFlag, Flock, FlockArg, fcntl}; @@ -292,6 +294,48 @@ pub(crate) struct FileLock { _lock: Flock, } +impl FileLock { + /// Checks an explicit caller deadline; ordinary operations pass `None`. + pub(crate) fn check_deadline(deadline: Option) -> Result<(), Errno> { + match deadline { + Some(deadline) if Instant::now() >= deadline => Err(Errno::ETIMEDOUT), + _ => Ok(()), + } + } + + /// Takes an exclusive file lock, bounding every retry by the same optional deadline. + pub(crate) fn acquire( + path: &CStr, + mode: Mode, + deadline: Option, + ) -> Result { + let Some(deadline) = deadline else { + return lock_file(path, mode); + }; + Self::check_deadline(Some(deadline))?; + let flags = OpenFlags::O_RDWR | OpenFlags::O_CREAT | OpenFlags::O_CLOEXEC; + let mut descriptor = nix::fcntl::open(path, flags, mode)?; + loop { + Self::check_deadline(Some(deadline))?; + match Flock::lock(descriptor, FlockArg::LockExclusiveNonblock) { + Ok(lock) => { + Self::check_deadline(Some(deadline))?; + return Ok(Self { _lock: lock }); + } + Err((returned, Errno::EINTR | Errno::EWOULDBLOCK)) => { + descriptor = returned; + thread::sleep( + deadline + .saturating_duration_since(Instant::now()) + .min(Duration::from_millis(1)), + ); + } + Err((_, error)) => return Err(error), + } + } + } +} + /// Opens `path`, creating it with `mode` if it is missing, and waits for its exclusive lock, /// resuming after interruptions. pub(crate) fn lock_file(path: &CStr, mode: Mode) -> Result { diff --git a/nvx-rs/src/guest/sys/fd/tests.rs b/nvx-rs/src/guest/sys/fd/tests.rs index 2b8fdae5c..5cdd015e4 100644 --- a/nvx-rs/src/guest/sys/fd/tests.rs +++ b/nvx-rs/src/guest/sys/fd/tests.rs @@ -1,6 +1,40 @@ use std::ffi::CString; use super::*; +use std::os::unix::ffi::OsStrExt; + +#[test] +fn a_bounded_file_lock_times_out_without_taking_or_leaking_the_lock() { + let directory = tempfile::tempdir().unwrap(); + let path = CString::new(directory.path().join("lock").as_os_str().as_bytes()).unwrap(); + let held = lock_file(&path, Mode::S_IRUSR | Mode::S_IWUSR).unwrap(); + let deadline = Instant::now() + Duration::from_millis(20); + assert!(matches!( + FileLock::acquire(&path, Mode::S_IRUSR | Mode::S_IWUSR, Some(deadline)), + Err(Errno::ETIMEDOUT) + )); + drop(held); + let next = FileLock::acquire( + &path, + Mode::S_IRUSR | Mode::S_IWUSR, + Some(Instant::now() + Duration::from_secs(1)), + ) + .unwrap(); + drop(next); + lock_file(&path, Mode::S_IRUSR | Mode::S_IWUSR).unwrap(); +} + +#[test] +fn an_expired_lock_deadline_does_not_create_the_lock_file() { + let directory = tempfile::tempdir().unwrap(); + let file = directory.path().join("not-created"); + let path = CString::new(file.as_os_str().as_bytes()).unwrap(); + assert!(matches!( + FileLock::acquire(&path, Mode::S_IRUSR | Mode::S_IWUSR, Some(Instant::now())), + Err(Errno::ETIMEDOUT) + )); + assert!(!file.exists()); +} impl FileDescriptor { /// Formats the descriptor number for inherited-descriptor tests. diff --git a/nvx-rs/src/guest/time/api.rs b/nvx-rs/src/guest/time/api.rs index ced0a0263..5904cb748 100644 --- a/nvx-rs/src/guest/time/api.rs +++ b/nvx-rs/src/guest/time/api.rs @@ -6,6 +6,7 @@ use std::io; use std::net::IpAddr; use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; +use std::time::Instant; use super::checks::{self, Checks}; use super::cpu_mask::CpuMask; @@ -177,6 +178,16 @@ impl TimeAbi { settled(&runtime, read_generation_id(&runtime)) } + /// Reads the generation ID using the same deadline as the capture's preparation. + pub(crate) fn generation_id_until( + &self, + deadline: Option, + ) -> Result { + let mut runtime = self.runtime(); + runtime.deadline = deadline; + settled(&runtime, read_generation_id(&runtime)) + } + /// Requests a capture through the snapshot port, preparation step 5, and, in a VM that a /// snapshot restored, repairs the restore: reads and validates the restore packet, steps and /// slews the wall clock, hands over the packet's fresh entropy, and records the restore, @@ -236,6 +247,26 @@ impl TimeAbi { ) } + /// Prepares strictly by one deadline and retains suppression ownership on every failure. + pub(crate) fn pre_capture_until( + &self, + deadline: Instant, + ) -> Result<(), (PreCaptureError, bool)> { + let mut runtime = self.runtime(); + runtime.deadline = Some(deadline); + let result = pre_capture(&runtime); + let suppressed = result + .as_ref() + .map_or_else(PreCaptureError::holds_suppression, |()| true); + let result = match runtime.take_violation() { + Some(violation) => Err(PreCaptureError::CaptureChecks(TimeError::Violation( + violation, + ))), + None => result, + }; + released_preparation(result, sys::check_released()).map_err(|error| (error, suppressed)) + } + /// Restores the stall detectors' settings that [`pre_capture`](Self::pre_capture) saved, and /// removes their record, as `nvx-time cancel-capture` does, or fails with its diagnostic, /// such as `read /run/nvx/time-suppression: No such file or directory`. It takes no @@ -243,6 +274,11 @@ impl TimeAbi { pub(crate) fn cancel_capture(&self) -> Result<(), Vec> { system::suppression_restore(false) } + + /// Restores detectors with a deadline only before the capture request. + pub(crate) fn cancel_capture_until(&self, deadline: Option) -> Result<(), Vec> { + system::suppression_restore_until(false, deadline) + } } impl Default for TimeAbi { @@ -361,16 +397,19 @@ fn cold_boot(runtime: &Runtime<'_>) -> Result { /// The read of [`TimeAbi::generation_id`]. fn read_generation_id(runtime: &Runtime<'_>) -> Result { + sys::FileLock::check_deadline(runtime.deadline).map_err(|_| TimeError::Deadline)?; let ports = runtime.open_ports()?; if ports.read_u8(Port::PortbStatus) & PORTB_GENERATION_ID_AVAILABLE == 0 { return Err(TimeError::GenerationIdUnavailable); } let mut id = [0_u8; GenerationId::LENGTH]; { - let _lock = system::lock_portb().map_err(|error| TimeError::PortLock(error.into()))?; + let _lock = system::lock_file_until(system::PORTB_LOCK_PATH, runtime.deadline) + .map_err(|error| TimeError::PortLock(error.into()))?; ports.write_u8(Port::PortbStatus, SELECT_GENERATION_ID); portb_read(&ports, Port::PortbData, &mut id); } + sys::FileLock::check_deadline(runtime.deadline).map_err(|_| TimeError::Deadline)?; Ok(GenerationId::from_bytes(id)) } @@ -421,8 +460,13 @@ impl PreCaptureError { /// The preparation of [`TimeAbi::pre_capture`]. fn pre_capture(runtime: &Runtime<'_>) -> Result<(), PreCaptureError> { - let (state, completed) = checks::wait_for_checks(true); + sys::FileLock::check_deadline(runtime.deadline) + .map_err(|_| PreCaptureError::BootChecks(TimeError::Deadline))?; + let (state, completed) = checks::wait_for_checks_until(true, runtime.deadline); if !completed { + if sys::FileLock::check_deadline(runtime.deadline).is_err() { + return Err(PreCaptureError::BootChecks(TimeError::Deadline)); + } return Err(PreCaptureError::BootChecks(check_failed( runtime, Phase::Capture, @@ -430,6 +474,8 @@ fn pre_capture(runtime: &Runtime<'_>) -> Result<(), PreCaptureError> { "the boot checks are still pending", ))); } + sys::FileLock::check_deadline(runtime.deadline) + .map_err(|_| PreCaptureError::BootChecks(TimeError::Deadline))?; let start = clock_ns(sys::Clock::Monotonic); let cpu_start = cpu_time_ns(); runtime.set_generation(state.generation as u32); @@ -441,10 +487,16 @@ fn pre_capture(runtime: &Runtime<'_>) -> Result<(), PreCaptureError> { "cannot read the CPU sets", )) })?; - checks::run_capture_checks(runtime, &mut checks); + checks::run_capture_checks_until(runtime, &mut checks, runtime.deadline) + .map_err(|_| PreCaptureError::CaptureChecks(TimeError::Deadline))?; + if let Some(violation) = runtime.take_violation() { + return Err(PreCaptureError::CaptureChecks(TimeError::Violation( + violation, + ))); + } let elapsed = clock_ns(sys::Clock::Monotonic).wrapping_sub(start); let cpu = cpu_time_ns().wrapping_sub(cpu_start); - match system::suppression_save() { + match system::suppression_save_until(runtime.deadline) { Ok(()) => {} Err(SaveError::Undone(Errno::EEXIST)) => return Err(PreCaptureError::Pending), Err(SaveError::Undone(error)) => return Err(PreCaptureError::Suppression(error)), @@ -464,6 +516,7 @@ fn pre_capture(runtime: &Runtime<'_>) -> Result<(), PreCaptureError> { format!("cannot record the capture check: {}", error.desc()), ); } + sys::FileLock::check_deadline(runtime.deadline).map_err(PreCaptureError::Unrestored)?; Ok(()) } @@ -1298,6 +1351,8 @@ impl Display for Violation { #[derive(Debug)] #[non_exhaustive] pub enum TimeError { + /// The caller's absolute pre-request deadline expired. + Deadline, /// The calling thread could not take the permission for OpenVMM's ports, which needs /// `CAP_SYS_RAWIO`. PortAccess(io::Error), @@ -1340,6 +1395,7 @@ impl From for TimeError { impl Display for TimeError { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { match self { + Self::Deadline => formatter.write_str("the capture preparation deadline expired"), Self::PortAccess(error) => write!(formatter, "ioperm: {}", sys::describe(error)), Self::PortRelease(error) => { write!(formatter, "ioperm revoke: {}", sys::describe(error)) @@ -1369,7 +1425,10 @@ impl std::error::Error for TimeError { | Self::PortLock(error) | Self::Spawn(error) | Self::EntropyFile { source: error, .. } => Some(error), - Self::StateUnavailable | Self::GenerationIdUnavailable | Self::Violation(_) => None, + Self::Deadline + | Self::StateUnavailable + | Self::GenerationIdUnavailable + | Self::Violation(_) => None, } } } diff --git a/nvx-rs/src/guest/time/api/tests.rs b/nvx-rs/src/guest/time/api/tests.rs index e606320aa..2b3e94c45 100644 --- a/nvx-rs/src/guest/time/api/tests.rs +++ b/nvx-rs/src/guest/time/api/tests.rs @@ -33,6 +33,12 @@ impl Restore { self } + /// Returns a restore that carries network settings, including an otherwise empty policy. + pub(crate) fn with_network(mut self, dns: Option, announce: bool) -> Self { + self.network = Some(RestoreNetwork::new(dns, announce)); + self + } + /// Returns the restore with the host's input gated until the guest acknowledges it. pub(crate) fn gated(mut self) -> Self { self.flags |= decoders::PACKET_ACK_REQUIRED; @@ -157,6 +163,19 @@ fn errors_print_the_diagnostics_of_nvx_time() { ); } +#[test] +fn an_expired_preparation_deadline_does_not_wait_for_boot_checks() { + let (error, suppressed) = TimeAbi::report_only() + .pre_capture_until(Instant::now()) + .unwrap_err(); + assert!(matches!( + error, + PreCaptureError::BootChecks(TimeError::Deadline) + )); + assert!(!suppressed); + assert_eq!(sys::check_released(), Ok(())); +} + #[test] fn strict_mode_keeps_report_only_mode_and_spares_the_daemon_and_the_preparation() { let enforcing = TimeAbi { diff --git a/nvx-rs/src/guest/time/checks.rs b/nvx-rs/src/guest/time/checks.rs index 78263aaa4..33ccc9d16 100644 --- a/nvx-rs/src/guest/time/checks.rs +++ b/nvx-rs/src/guest/time/checks.rs @@ -12,7 +12,7 @@ use super::events::{CheckId, DETAIL_MAX, ViolationCode, escape_text}; use super::state::{CheckState, CheckStatus, TimeState}; use super::system::{ DAEMON_NAME, DAEMON_PID_PATH, DEFAULT_STALL_TIMEOUT_S, MsrDescriptor, Runtime, msr_error, - rates_load, state_apply, state_load, + rates_load, state_apply_until, state_load, }; use crate::guest::status::CommandError; use crate::guest::sys::x86::cpuid::{self, Register}; @@ -316,7 +316,7 @@ pub(crate) fn record_check( } else { CheckStatus::Ok }; - state_apply(|state| { + state_apply_until(runtime.deadline, |state| { let generation = if phase == Phase::Boot { 0 } else { @@ -926,14 +926,28 @@ fn compare_cpu_rate(runtime: &Runtime<'_>, checks: &mut Checks) { /// Runs checks that must precede snapshot capture. pub(crate) fn run_capture_checks(runtime: &Runtime<'_>, checks: &mut Checks) { + run_capture_checks_until(runtime, checks, None).expect("unbounded checks cannot time out"); +} + +/// Runs capture checks with one explicit pre-request deadline between their operations. +pub(crate) fn run_capture_checks_until( + runtime: &Runtime<'_>, + checks: &mut Checks, + deadline: Option, +) -> Result<(), Errno> { + sys::FileLock::check_deadline(deadline)?; if let Ok((tsc, lapic)) = rates_load() { checks.tsc_hz = tsc; checks.lapic_hz = lapic; } // Each failure counts a violation in the state before C10 reads the count. + sys::FileLock::check_deadline(deadline)?; checks.record(runtime, check_clocksource()); + sys::FileLock::check_deadline(deadline)?; checks.record(runtime, check_timer_list(checks.online, Duration::ZERO)); + sys::FileLock::check_deadline(deadline)?; checks.record(runtime, check_daemon()); + sys::FileLock::check_deadline(deadline) } /// Runs restore checks, including checks on newly online CPUs. @@ -960,7 +974,16 @@ pub(crate) fn run_restore_checks(runtime: &Runtime<'_>, checks: &mut Checks, new /// Waits for boot-only or all persisted checks and returns completion state. pub(crate) fn wait_for_checks(boot_only: bool) -> (TimeState, bool) { - let deadline = Instant::now() + STATUS_WAIT; + wait_for_checks_until(boot_only, None) +} + +/// Waits no later than the caller deadline or the existing boot-check wait limit. +pub(crate) fn wait_for_checks_until( + boot_only: bool, + deadline: Option, +) -> (TimeState, bool) { + let limit = Instant::now() + STATUS_WAIT; + let deadline = deadline.map_or(limit, |deadline| deadline.min(limit)); loop { let state = state_load().unwrap_or_default(); if !state.pending(boot_only) { @@ -969,7 +992,7 @@ pub(crate) fn wait_for_checks(boot_only: bool) -> (TimeState, bool) { if Instant::now() >= deadline { return (state, false); } - thread::sleep(STATUS_POLL); + thread::sleep(STATUS_POLL.min(deadline.saturating_duration_since(Instant::now()))); } } diff --git a/nvx-rs/src/guest/time/system.rs b/nvx-rs/src/guest/time/system.rs index b2e109462..db00cbd69 100644 --- a/nvx-rs/src/guest/time/system.rs +++ b/nvx-rs/src/guest/time/system.rs @@ -5,6 +5,7 @@ use std::ffi::{CStr, CString}; use std::fmt::{self, Display}; use std::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering}; use std::sync::{Mutex, PoisonError}; +use std::time::Instant; use super::Phase; use super::api::Violation; @@ -264,6 +265,8 @@ pub(crate) struct Runtime<'a> { async_output: AtomicBool, /// Serializes multi-destination violation records. report_lock: Mutex<()>, + /// One absolute pre-request deadline, absent for legacy calls and capture requests. + pub(crate) deadline: Option, } impl<'a> Runtime<'a> { @@ -278,6 +281,7 @@ impl<'a> Runtime<'a> { tsc_hz: AtomicU64::new(0), async_output: AtomicBool::new(false), report_lock: Mutex::new(()), + deadline: None, } } @@ -419,7 +423,7 @@ impl<'a> Runtime<'a> { b"" }; self.ops.write_console(separator, line.as_bytes()); - let _ = count_violation(); + let _ = count_violation(self.deadline); } /// Reports a conformance failure, and powers off unless report-only or strict. @@ -526,8 +530,14 @@ pub(crate) fn replace_file(path: &CStr, temporary: &CStr, text: &[u8]) -> Result /// Opens or creates a runtime lock file and takes its exclusive advisory lock. pub(crate) fn lock_file(path: &CStr) -> Result { + lock_file_until(path, None) +} + +/// Takes a runtime lock using the caller's explicit pre-request deadline, if any. +pub(crate) fn lock_file_until(path: &CStr, deadline: Option) -> Result { + FileLock::check_deadline(deadline)?; make_runtime_directories(); - sys::lock_file(path, PRIVATE_FILE_MODE) + FileLock::acquire(path, PRIVATE_FILE_MODE, deadline) } /// Loads and parses the published time state. @@ -543,18 +553,30 @@ pub(crate) fn state_store(state: &TimeState) -> Result<(), Errno> { /// Applies one read-modify-write update to the time state under its lock. pub(crate) fn state_apply(update: impl FnOnce(&mut TimeState)) -> Result<(), Errno> { - let _lock = lock_file(STATE_LOCK_PATH)?; + state_apply_until(None, update) +} + +/// Updates state under a runtime lock bounded by the same pre-request deadline. +pub(crate) fn state_apply_until( + deadline: Option, + update: impl FnOnce(&mut TimeState), +) -> Result<(), Errno> { + let _lock = lock_file_until(STATE_LOCK_PATH, deadline)?; let mut state = state_load().unwrap_or_default(); + FileLock::check_deadline(deadline)?; update(&mut state); - state_store(&state) + state_store(&state)?; + FileLock::check_deadline(deadline) } /// Increments the persisted violation counter when state exists. -fn count_violation() -> Result<(), Errno> { +fn count_violation(deadline: Option) -> Result<(), Errno> { if !sys::path_exists(STATE_PATH) { return Ok(()); } - state_apply(|state| state.violations = state.violations.wrapping_add(1)) + state_apply_until(deadline, |state| { + state.violations = state.violations.wrapping_add(1) + }) } /// Loads validated TSC and LAPIC rates from state. @@ -567,12 +589,11 @@ pub(crate) fn rates_load() -> Result<(u64, u64), Errno> { } } -/// Saves the stall detectors' settings in the suppression record, and suppresses them for a -/// snapshot capture; see [`Suppression::save`]. A record that exists already, of a pending -/// capture, is `EEXIST`. -pub(crate) fn suppression_save() -> Result<(), SaveError> { +/// Saves suppression with explicitly bounded runtime-lock and detector operations. +pub(crate) fn suppression_save_until(deadline: Option) -> Result<(), SaveError> { + FileLock::check_deadline(deadline).map_err(SaveError::Undone)?; make_runtime_directories(); - SUPPRESSION.save() + SUPPRESSION.save_until(deadline, sys::write_number) } /// Why a save of the stall detectors' settings failed. @@ -593,10 +614,19 @@ fn describe(prefix: &[u8], error: Errno) -> Vec { } /// Restores the stall detectors' settings from the suppression record, and removes the record; -/// see [`Suppression::restore`]. A missing record succeeds when `missing_ok`. +/// see [`Suppression::restore_until`]. A missing record succeeds when `missing_ok`. pub(crate) fn suppression_restore(missing_ok: bool) -> Result<(), Vec> { + suppression_restore_until(missing_ok, None) +} + +/// Restores suppression by an explicit deadline, absent after a capture request. +pub(crate) fn suppression_restore_until( + missing_ok: bool, + deadline: Option, +) -> Result<(), Vec> { + FileLock::check_deadline(deadline).map_err(|error| describe(b"restore detectors: ", error))?; make_runtime_directories(); - SUPPRESSION.restore(missing_ok) + SUPPRESSION.restore_until(missing_ok, deadline) } /// The suppression of the stall detectors for a capture: the detectors, the record of their @@ -619,23 +649,17 @@ const SUPPRESSION: Suppression<'static> = Suppression { }; impl Suppression<'_> { - /// Saves the detectors' settings in the record, and suppresses the detectors, under the - /// lock, so that no restore runs between its reads and its writes. A record that exists - /// already, of a pending capture, is `EEXIST`. A save that fails before it created the - /// record changes nothing; one that fails after undoes its changes and removes the record, - /// or keeps the record if it can't. - fn save(&self) -> Result<(), SaveError> { - self.save_with(sys::write_number) - } - - /// Implements [`save`](Self::save), with `write` writing a detector's setting. - fn save_with( + /// Saves with one deadline shared by locking, reads, writes and any rollback. + fn save_until( &self, + deadline: Option, mut write: impl FnMut(&CStr, i64) -> Result<(), Errno>, ) -> Result<(), SaveError> { - let _lock = sys::lock_file(self.lock, PRIVATE_FILE_MODE).map_err(SaveError::Undone)?; + let _lock = + FileLock::acquire(self.lock, PRIVATE_FILE_MODE, deadline).map_err(SaveError::Undone)?; let mut saved = Vec::new(); for (index, &(path, suppressed)) in self.detectors.iter().enumerate() { + FileLock::check_deadline(deadline).map_err(SaveError::Undone)?; if let Err(error) = sys::check_exists(path) { if index == 0 { return Err(SaveError::Undone(error)); @@ -650,20 +674,29 @@ impl Suppression<'_> { text.extend_from_slice(path.to_bytes()); text.extend_from_slice(format!("={value}\n").as_bytes()); } + FileLock::check_deadline(deadline).map_err(SaveError::Undone)?; let record = CreatedFile::create(self.record, true, PRIVATE_FILE_MODE).map_err(SaveError::Undone)?; record.write_all(&text).map_err(SaveError::Undone)?; // As in the C program, a record that fails to close stays, and the suppression goes on. let _ = record.keep(); for (written, &(path, _, suppressed)) in saved.iter().enumerate() { - if let Err(error) = write(path, suppressed) { + if let Err(error) = + FileLock::check_deadline(deadline).and_then(|()| write(path, suppressed)) + { // The record stays unless every earlier write is undone, so that a restore can // still restore the settings. let mut undone = true; for &(path, value, _) in &saved[..written] { - undone &= write(path, value).is_ok(); + undone &= FileLock::check_deadline(deadline) + .and_then(|()| write(path, value)) + .is_ok(); } - if undone && sys::unlink(self.record).is_ok() { + if undone + && FileLock::check_deadline(deadline) + .and_then(|()| sys::unlink(self.record)) + .is_ok() + { return Err(SaveError::Undone(error)); } return Err(SaveError::Kept(error)); @@ -672,10 +705,10 @@ impl Suppression<'_> { Ok(()) } - /// Restores the detectors' settings from the record, and removes it, under the lock. A - /// missing record succeeds when `missing_ok`. - fn restore(&self, missing_ok: bool) -> Result<(), Vec> { - let _lock = sys::lock_file(self.lock, PRIVATE_FILE_MODE) + /// Restores under the same explicit deadline as the failed preparation. + fn restore_until(&self, missing_ok: bool, deadline: Option) -> Result<(), Vec> { + let expired = |error| describe(b"restore detectors: ", error); + let _lock = FileLock::acquire(self.lock, PRIVATE_FILE_MODE, deadline) .map_err(|error| describe(b"lock /run/nvx/time-suppression.lock: ", error))?; let text = match sys::read_file(self.record) { Ok(text) => text, @@ -686,6 +719,7 @@ impl Suppression<'_> { .split(|byte| *byte == b'\n') .filter(|line| !line.is_empty()) { + FileLock::check_deadline(deadline).map_err(expired)?; let Some(equal) = line.iter().position(|byte| *byte == b'=') else { break; }; @@ -700,8 +734,10 @@ impl Suppression<'_> { )); } } + FileLock::check_deadline(deadline).map_err(expired)?; sys::unlink(self.record) - .map_err(|error| describe(b"remove /run/nvx/time-suppression: ", error)) + .map_err(|error| describe(b"remove /run/nvx/time-suppression: ", error))?; + FileLock::check_deadline(deadline).map_err(expired) } } diff --git a/nvx-rs/src/guest/time/system/tests.rs b/nvx-rs/src/guest/time/system/tests.rs index 7aca1d5c7..e7aa16b83 100644 --- a/nvx-rs/src/guest/time/system/tests.rs +++ b/nvx-rs/src/guest/time/system/tests.rs @@ -15,6 +15,26 @@ use crate::guest::time::NSEC_PER_MSEC; use crate::guest::time::events::CheckId; use crate::guest::time::state::HostClockStatus; +impl Suppression<'_> { + /// Runs the shared save without a caller deadline for the legacy cases below. + fn save(&self) -> Result<(), SaveError> { + self.save_with(sys::write_number) + } + + /// Runs the same save with the test's detector writer. + fn save_with( + &self, + write: impl FnMut(&CStr, i64) -> Result<(), Errno>, + ) -> Result<(), SaveError> { + self.save_until(None, write) + } + + /// Runs the shared restore without a caller deadline for the legacy cases below. + fn restore(&self, missing_ok: bool) -> Result<(), Vec> { + self.restore_until(missing_ok, None) + } +} + /// Privileged operations that record their requests and replay canned port input. pub(crate) struct FakeOps { /// The error with which the kernel refuses to grant the port permission, if any. @@ -909,3 +929,60 @@ fn a_save_waits_for_the_suppression_lock() { }); assert_eq!(test.settings(), settings([Some("1"), Some("0"), Some("0")])); } + +#[test] +fn capture_suppression_and_rollback_share_the_deadline_of_their_runtime_lock() { + let test = TestDetectors::new([Some("0"), Some("60"), Some("120")]); + let detectors = test.detectors(); + let suppression = test.suppression(&detectors); + let held = sys::lock_file(&test.lock, PRIVATE_FILE_MODE).unwrap(); + let deadline = Instant::now() + Duration::from_millis(20); + assert_eq!( + suppression.save_until(Some(deadline), sys::write_number), + Err(SaveError::Undone(Errno::ETIMEDOUT)) + ); + assert_eq!( + test.settings(), + settings([Some("0"), Some("60"), Some("120")]) + ); + assert_eq!(sys::check_exists(&test.record), Err(Errno::ENOENT)); + drop(held); + + suppression.save().unwrap(); + let held = sys::lock_file(&test.lock, PRIVATE_FILE_MODE).unwrap(); + let deadline = Instant::now() + Duration::from_millis(20); + assert!(suppression.restore_until(false, Some(deadline)).is_err()); + assert_eq!(test.settings(), settings([Some("1"), Some("0"), Some("0")])); + assert!(sys::read_file(&test.record).is_ok()); + drop(held); + suppression.restore(false).unwrap(); + assert_eq!( + test.settings(), + settings([Some("0"), Some("60"), Some("120")]) + ); +} + +#[test] +fn a_deadline_during_suppression_keeps_the_record_when_rollback_cannot_finish() { + let test = TestDetectors::new([Some("0"), Some("60"), Some("120")]); + let detectors = test.detectors(); + let suppression = test.suppression(&detectors); + let deadline = Instant::now() + Duration::from_millis(20); + let result = suppression.save_until(Some(deadline), |path, value| { + sys::write_number(path, value)?; + thread::sleep(Duration::from_millis(40)); + Ok(()) + }); + assert_eq!(result, Err(SaveError::Kept(Errno::ETIMEDOUT))); + assert_eq!( + test.settings(), + settings([Some("1"), Some("60"), Some("120")]) + ); + assert!(sys::read_file(&test.record).is_ok()); + assert!(suppression.restore_until(false, Some(deadline)).is_err()); + suppression.restore(false).unwrap(); + assert_eq!( + test.settings(), + settings([Some("0"), Some("60"), Some("120")]) + ); +} From bacf07f2436be1255b494fa20f599abe423cdbf7 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Fri, 6 Nov 2026 08:12:53 -0800 Subject: [PATCH 03/15] guest: preserve capture deadline and failure-stage semantics Keep caller timeouts distinct from conformance failures, retain the boot-check stage, and accept successful detector restoration without a post-completion timeout. Correct the deadline error conversion and make focused expiry tests force the relevant step deterministically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2610ed93-496e-4bc8-b990-ea9b49ab3462 --- nvx-rs/README.md | 35 ++++++++++++++- nvx-rs/src/guest/mod.rs | 3 +- nvx-rs/src/guest/snapshot.rs | 10 ++--- nvx-rs/src/guest/snapshot/tests.rs | 64 ++++++++++++++++++++++----- nvx-rs/src/guest/time/api.rs | 28 +++++++----- nvx-rs/src/guest/time/api/tests.rs | 39 +++++++++++++--- nvx-rs/src/guest/time/system.rs | 17 +++---- nvx-rs/src/guest/time/system/tests.rs | 6 ++- 8 files changed, 151 insertions(+), 51 deletions(-) diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 637a7f06c..97ad4a448 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -154,6 +154,7 @@ changes only when the module's purpose does. | [`nvx::guest::exec`](#nvxguestexec) | `Execution`: drives one program as an execution, for any guest init or agent | | [`nvx::guest::exec::binding`](#nvxguestexecbinding) | `Binding`: an `nvx::exec::Runner` bound to an `Execution`, the glue that a guest adapter would otherwise write | | [`nvx::guest::time`](#nvxguesttime) | x86_64: the steps of NVX time ABI v1 as typed calls in the caller's process, for any guest init or agent | +| [`nvx::guest::snapshot`](#nvxguestsnapshot) | x86_64: bounded Fresh capture, staged failure dispositions, and MMIO device mount checks, without execution control | | [`nvx::guest::activation`](#nvxguestactivation) | x86_64: fence-free activation, with the retained template capture and clone repair interface | | [`nvx::guest::reseed`](#nvxguestreseed) | A restore's fresh entropy, and the reseed of the kernel's random number generator with it | | [`nvx::guest::root`](#nvxguestroot) | The operations of a layered root: its bootstrap, assembly, shares, export, mappings, and teardown | @@ -1712,7 +1713,9 @@ A violation of the ABI stops the VM in enforcing mode, with the time ABI's status, as `nvx-time` does; in strict mode, `TimeAbi::strict`, which a caller that owns the VM's exit status and shutdown selects, the call fails with the `Violation`, whose `status` is the one with which enforcing mode stops the VM, -and the time daemon and a capture's preparation enforce the ABI all the same. +and the time daemon and the legacy activation capture's preparation enforce +the ABI all the same. The single-call snapshot capture below uses strict +preparation instead. A call takes the permission for OpenVMM's ports on the calling thread for each port transaction alone, and fails with `TimeError::PortRelease` if the thread @@ -1722,6 +1725,36 @@ did not start. `nvx-time` runs its steps through them. +### `nvx::guest::snapshot` + +x86_64: `capture(&Devices, Disks::Fresh, deadline)` composes the platform +template's mount checks, strict time checks, stall-detector suppression, +generation ID and capture request, without inspecting or freezing the +caller's executions. `Devices` names the distro and scratch devices by their +MMIO base values and the host filesystem by its virtio-fs tag. `Mounted`, +`Origin` and `MOUNT_TABLE_LIMIT` describe a mount refusal. + +Every step and wait before the request, including runtime-lock acquisition +and rollback, shares the caller's absolute deadline. The request itself has +no deadline while the host saves stopped processors. If no snapshot was +taken, `Captured::Continued` reports that the detectors were restored. + +`CaptureFailure` reports its `stage`, `cause` and `disposition`: `Resumed` +only when no suppression owned by this call remains, or `MustEnd { status }`. +A strict time violation preserves its actual status. A `PortRelease` remains +the cause even if detector rollback also fails. Request and release failures +are final. + +The time core performs its existing clock correction after a restore, but +this interface does not complete the clone's repair and returns `MustEnd`. +Fresh platform restore packets must carry no network settings; even an +otherwise empty network policy is explicitly refused. + +The caller owns its transport. After answering the capture request, send +nothing until `capture` returns. After `Continued` or a `Resumed` failure, +report on the same session. The activation module retains the legacy +Device-shaped capture interface for existing callers. + ### `nvx::guest::activation` x86_64: the preparation of a template's guest for a capture before its root's diff --git a/nvx-rs/src/guest/mod.rs b/nvx-rs/src/guest/mod.rs index 8c9085d9f..039d463fd 100644 --- a/nvx-rs/src/guest/mod.rs +++ b/nvx-rs/src/guest/mod.rs @@ -8,7 +8,8 @@ //! mounts, and its account of a host's mapping table; [`exec`], which drives each program as one //! execution, with its launch, its bounded output and input, its ending, and its outcome, alone //! or, through its [`binding`](exec::binding), under the [execution engine](crate::exec); and, -//! on x86-64, `activation`, through which a template's guest prepares for and requests a snapshot's +//! on x86-64, [`snapshot`], which composes a strictly checked Fresh capture by one caller deadline, +//! without execution control; and `activation`, through which a template's guest prepares for and requests a snapshot's //! capture before its root's assembly, behind the caller's fence of workload admission, with //! nothing mounted from the devices that its clones replace, and through which each clone that a //! restore of the snapshot starts repairs its restore: it activates the restore's memory target, diff --git a/nvx-rs/src/guest/snapshot.rs b/nvx-rs/src/guest/snapshot.rs index 859a43362..ef51aa607 100644 --- a/nvx-rs/src/guest/snapshot.rs +++ b/nvx-rs/src/guest/snapshot.rs @@ -9,8 +9,8 @@ //! sends nothing until this call returns. After [`Captured::Continued`] or a failure with //! [`Disposition::Resumed`], it reports on the same session. //! -//! This interface does not repair restored guests, which must end. A Fresh platform restore -//! that carries network settings is explicitly refused rather than silently dropping them. +//! The time core repairs a restore's clock, but this interface does not complete the clone's +//! repair, so a restored guest must end. Fresh restore network settings are explicitly refused. //! //! Each line of `/proc/self/mountinfo` describes one mount with fields separated by spaces: its //! ID, its parent's ID, the device number of its file system, `MAJ:MIN`, the root of the mount @@ -142,7 +142,7 @@ impl std::error::Error for CaptureFailure { /// Captures a Fresh template without freezing or inspecting the caller's executions. /// /// The deadline bounds pre-request work only. This call starts no helper, drives no transport, -/// repairs no restore, and never powers off the VM; failures return the caller's disposition. +/// completes no clone repair, and never powers off the VM; failures return its disposition. pub fn capture( devices: &Devices, disks: Disks, @@ -155,7 +155,7 @@ pub fn capture( CaptureCause::Time(error.into()), ) })?; - before(CaptureStage::BootChecks, deadline)?; + before(CaptureStage::Mounts, deadline)?; capture_with( activation::capture_system(Some(deadline)).as_mut(), devices, @@ -237,7 +237,7 @@ pub(super) fn check_mounts( devices: &Devices, deadline: Option, ) -> Result<(), CaptureCause> { - sys::FileLock::check_deadline(deadline).map_err(CaptureCause::Io)?; + sys::FileLock::check_deadline(deadline).map_err(|error| CaptureCause::Io(error.into()))?; let table = system.mount_table().map_err(CaptureCause::Io)?; if table.len() > MOUNT_TABLE_LIMIT { return Err(CaptureCause::Io(io::Error::new( diff --git a/nvx-rs/src/guest/snapshot/tests.rs b/nvx-rs/src/guest/snapshot/tests.rs index cc291a688..24dabb621 100644 --- a/nvx-rs/src/guest/snapshot/tests.rs +++ b/nvx-rs/src/guest/snapshot/tests.rs @@ -11,6 +11,8 @@ struct CaptureSystem { calls: Vec<&'static str>, /// The caller's deadline until the request context clears it. deadline: Option, + /// The original deadline, retained only to force expiry at a chosen test step. + original_deadline: Instant, /// The calling namespace's mount table. table: Vec, /// The next preparation's result and suppression ownership. @@ -21,10 +23,10 @@ struct CaptureSystem { outcome: Option>, /// Whether detector restoration fails. release_fails: bool, - /// Time spent reading the generation ID. - generation_delay: Duration, - /// Time spent in the request, which must not have a deadline. - request_delay: Duration, + /// Whether the generation read deliberately completes after the deadline. + expire_generation: bool, + /// Whether the unbounded request deliberately completes after the original deadline. + expire_request: bool, } impl CaptureSystem { @@ -33,13 +35,14 @@ impl CaptureSystem { Self { calls: Vec::new(), deadline: Some(deadline), + original_deadline: deadline, table: b"1 1 0:2 / / rw - rootfs rootfs rw\n".to_vec(), preparation: Some(Ok(())), generation: Some(Ok(GenerationId::from_bytes([7; GenerationId::LENGTH]))), outcome: Some(Ok(CaptureOutcome::Continued)), release_fails: false, - generation_delay: Duration::ZERO, - request_delay: Duration::ZERO, + expire_generation: false, + expire_request: false, } } } @@ -87,7 +90,9 @@ impl System for CaptureSystem { fn generation_id(&mut self) -> Result { self.calls.push("generation"); - thread::sleep(self.generation_delay); + if self.expire_generation { + expire(self.original_deadline); + } self.generation.take().expect("generation is read once") } @@ -101,11 +106,19 @@ impl System for CaptureSystem { ) ); self.calls.push("request"); - thread::sleep(self.request_delay); + if self.expire_request { + expire(self.original_deadline); + } self.outcome.take().expect("capture is requested once") } } +/// Forces expiry only after the operation under test has started. +fn expire(deadline: Instant) { + thread::sleep(deadline.saturating_duration_since(Instant::now()) + Duration::from_millis(1)); + assert!(Instant::now() >= deadline); +} + /// The canonical MMIO identities, independent of the legacy root Device type. fn capture_devices() -> Devices { Devices { @@ -117,9 +130,9 @@ fn capture_devices() -> Devices { #[test] fn fresh_capture_orders_the_existing_steps_and_does_not_deadline_the_request() { - let deadline = Instant::now() + Duration::from_millis(50); + let deadline = Instant::now() + Duration::from_secs(2); let mut system = CaptureSystem::new(deadline); - system.request_delay = Duration::from_millis(100); + system.expire_request = true; assert_eq!( capture_with(&mut system, &capture_devices(), deadline).unwrap(), Captured::Continued @@ -236,9 +249,9 @@ fn an_expired_pre_request_deadline_prevents_capture_and_does_not_extend_rollback assert_eq!(failure.disposition, Disposition::Resumed); assert!(system.calls.is_empty()); - let deadline = Instant::now() + Duration::from_millis(20); + let deadline = Instant::now() + Duration::from_secs(2); let mut system = CaptureSystem::new(deadline); - system.generation_delay = Duration::from_millis(40); + system.expire_generation = true; let failure = capture_with(&mut system, &capture_devices(), deadline).unwrap_err(); assert_eq!(failure.stage, CaptureStage::Release); assert_eq!( @@ -271,6 +284,33 @@ fn a_fresh_restore_refuses_even_an_otherwise_empty_network_policy() { assert_eq!(system.calls.last(), Some(&"release")); } +#[test] +fn a_pending_capture_is_not_released_and_a_restore_without_network_is_not_repaired() { + let deadline = Instant::now() + Duration::from_secs(2); + let mut system = CaptureSystem::new(deadline); + system.preparation = Some(Err((PreCaptureError::Pending, false))); + let failure = capture_with(&mut system, &capture_devices(), deadline).unwrap_err(); + assert!(matches!(failure.cause, CaptureCause::Pending)); + assert_eq!(failure.disposition, Disposition::Resumed); + assert!(!system.calls.contains(&"release")); + assert!(!system.calls.contains(&"request")); + + let mut system = CaptureSystem::new(deadline); + system.outcome = Some(Ok(CaptureOutcome::Restored(Restore::to( + GenerationId::from_bytes([8; GenerationId::LENGTH]), + )))); + let failure = capture_with(&mut system, &capture_devices(), deadline).unwrap_err(); + assert_eq!(failure.stage, CaptureStage::Capture); + assert_eq!( + failure.disposition, + Disposition::MustEnd { + status: status::TIME_REPAIR + } + ); + assert!(matches!(failure.cause, CaptureCause::Io(ref error) + if error.to_string().contains("does not repair"))); +} + #[test] fn request_and_release_failures_never_report_that_the_guest_resumed() { for release_fails in [false, true] { diff --git a/nvx-rs/src/guest/time/api.rs b/nvx-rs/src/guest/time/api.rs index 5904cb748..eed3d6916 100644 --- a/nvx-rs/src/guest/time/api.rs +++ b/nvx-rs/src/guest/time/api.rs @@ -88,8 +88,9 @@ impl TimeAbi { /// enforcing mode. /// /// The time daemon that [`start_daemon`](Self::start_daemon) starts enforces the ABI even - /// then, since no caller owns its failures, and so does the preparation of a capture that - /// `nvx::guest::clone` runs, as `nvx-time pre-capture` does. + /// then, since no caller owns its failures, and so does the legacy activation capture's + /// preparation, as `nvx-time pre-capture` does. The single-call + /// [`snapshot::capture`](crate::guest::snapshot::capture) uses strict preparation instead. pub fn strict(self) -> Self { Self { strict: !self.report_only, @@ -244,6 +245,7 @@ impl TimeAbi { released_preparation( pre_capture(&self.enforcing_runtime()), sys::check_released(), + None, ) } @@ -258,13 +260,8 @@ impl TimeAbi { let suppressed = result .as_ref() .map_or_else(PreCaptureError::holds_suppression, |()| true); - let result = match runtime.take_violation() { - Some(violation) => Err(PreCaptureError::CaptureChecks(TimeError::Violation( - violation, - ))), - None => result, - }; - released_preparation(result, sys::check_released()).map_err(|error| (error, suppressed)) + released_preparation(result, sys::check_released(), runtime.take_violation()) + .map_err(|error| (error, suppressed)) } /// Restores the stall detectors' settings that [`pre_capture`](Self::pre_capture) saved, and @@ -321,9 +318,15 @@ fn settled(runtime: &Runtime<'_>, result: Result) -> Result, released: Result<(), PortError>, + violation: Option, ) -> Result<(), PreCaptureError> { let Err(error) = released else { - return result; + return match violation { + Some(violation) if !matches!(result, Err(PreCaptureError::BootChecks(_))) => Err( + PreCaptureError::CaptureChecks(TimeError::Violation(violation)), + ), + _ => result, + }; }; let suppressed = result .as_ref() @@ -404,7 +407,7 @@ fn read_generation_id(runtime: &Runtime<'_>) -> Result } let mut id = [0_u8; GenerationId::LENGTH]; { - let _lock = system::lock_file_until(system::PORTB_LOCK_PATH, runtime.deadline) + let _lock = system::lock_file(system::PORTB_LOCK_PATH, runtime.deadline) .map_err(|error| TimeError::PortLock(error.into()))?; ports.write_u8(Port::PortbStatus, SELECT_GENERATION_ID); portb_read(&ports, Port::PortbData, &mut id); @@ -509,7 +512,8 @@ fn pre_capture(runtime: &Runtime<'_>) -> Result<(), PreCaptureError> { cpu / NSEC_PER_USEC, 0, 0, - ) { + ) && (error != Errno::ETIMEDOUT || sys::FileLock::check_deadline(runtime.deadline).is_ok()) + { checks.fail( runtime, CheckId::C10, diff --git a/nvx-rs/src/guest/time/api/tests.rs b/nvx-rs/src/guest/time/api/tests.rs index 2b3e94c45..656485a32 100644 --- a/nvx-rs/src/guest/time/api/tests.rs +++ b/nvx-rs/src/guest/time/api/tests.rs @@ -291,7 +291,7 @@ fn a_failed_revocation_fails_the_call_and_every_later_one_until_a_revocation_suc fn a_preparation_that_may_leave_port_permission_fails_with_its_suppression() { let revoke = Err(PortError::Revoke(Errno::ENOMEM)); // A preparation that suppressed the detectors holds their suppression. - let error = released_preparation(Ok(()), revoke).unwrap_err(); + let error = released_preparation(Ok(()), revoke, None).unwrap_err(); assert!( matches!( &error, @@ -305,14 +305,14 @@ fn a_preparation_that_may_leave_port_permission_fails_with_its_suppression() { assert!(error.holds_suppression()); // One that kept the record of the settings that it could not restore holds it too. let kept = Err(PreCaptureError::Unrestored(Errno::EIO)); - let error = released_preparation(kept, revoke).unwrap_err(); + let error = released_preparation(kept, revoke, None).unwrap_err(); assert!(error.holds_suppression(), "{error:?}"); // One that left the settings as they were holds nothing. for failed in [ PreCaptureError::Pending, PreCaptureError::Suppression(Errno::EIO), ] { - let error = released_preparation(Err(failed), revoke).unwrap_err(); + let error = released_preparation(Err(failed), revoke, None).unwrap_err(); assert!( matches!( error, @@ -326,18 +326,45 @@ fn a_preparation_that_may_leave_port_permission_fails_with_its_suppression() { assert!(!error.holds_suppression()); } // Without port permission, the preparation's result stands. - released_preparation(Ok(()), Ok(())).unwrap(); - let error = released_preparation(Err(PreCaptureError::Pending), Ok(())).unwrap_err(); + released_preparation(Ok(()), Ok(()), None).unwrap(); + let error = released_preparation(Err(PreCaptureError::Pending), Ok(()), None).unwrap_err(); assert!(matches!(error, PreCaptureError::Pending), "{error:?}"); assert!(!error.holds_suppression()); let kept = Err(PreCaptureError::Unrestored(Errno::EIO)); assert!( - released_preparation(kept, Ok(())) + released_preparation(kept, Ok(()), None) .unwrap_err() .holds_suppression() ); } +#[test] +fn strict_preparation_keeps_the_boot_failure_stage_and_suppression_ownership() { + let violation = Violation::of_missing_daemon(); + let boot = Err(PreCaptureError::BootChecks(TimeError::Violation( + violation.clone(), + ))); + let failure = released_preparation(boot, Ok(()), Some(violation)).unwrap_err(); + assert!( + matches!(failure, PreCaptureError::BootChecks(TimeError::Violation(ref violation)) + if violation.status() == status::TIME_CONFORMANCE) + ); + + let failure = released_preparation( + Ok(()), + Err(PortError::Revoke(Errno::ENOMEM)), + Some(Violation::of_missing_daemon()), + ) + .unwrap_err(); + assert!(matches!( + failure, + PreCaptureError::PortRelease { + suppressed: true, + .. + } + )); +} + /// Returns the outcome of a cold boot that accepted no sample. fn cold_boot() -> ColdBoot { ColdBoot { diff --git a/nvx-rs/src/guest/time/system.rs b/nvx-rs/src/guest/time/system.rs index db00cbd69..7909e5980 100644 --- a/nvx-rs/src/guest/time/system.rs +++ b/nvx-rs/src/guest/time/system.rs @@ -528,13 +528,8 @@ pub(crate) fn replace_file(path: &CStr, temporary: &CStr, text: &[u8]) -> Result file.rename(path) } -/// Opens or creates a runtime lock file and takes its exclusive advisory lock. -pub(crate) fn lock_file(path: &CStr) -> Result { - lock_file_until(path, None) -} - /// Takes a runtime lock using the caller's explicit pre-request deadline, if any. -pub(crate) fn lock_file_until(path: &CStr, deadline: Option) -> Result { +pub(crate) fn lock_file(path: &CStr, deadline: Option) -> Result { FileLock::check_deadline(deadline)?; make_runtime_directories(); FileLock::acquire(path, PRIVATE_FILE_MODE, deadline) @@ -561,12 +556,11 @@ pub(crate) fn state_apply_until( deadline: Option, update: impl FnOnce(&mut TimeState), ) -> Result<(), Errno> { - let _lock = lock_file_until(STATE_LOCK_PATH, deadline)?; + let _lock = lock_file(STATE_LOCK_PATH, deadline)?; let mut state = state_load().unwrap_or_default(); FileLock::check_deadline(deadline)?; update(&mut state); - state_store(&state)?; - FileLock::check_deadline(deadline) + state_store(&state) } /// Increments the persisted violation counter when state exists. @@ -736,8 +730,7 @@ impl Suppression<'_> { } FileLock::check_deadline(deadline).map_err(expired)?; sys::unlink(self.record) - .map_err(|error| describe(b"remove /run/nvx/time-suppression: ", error))?; - FileLock::check_deadline(deadline).map_err(expired) + .map_err(|error| describe(b"remove /run/nvx/time-suppression: ", error)) } } @@ -751,7 +744,7 @@ pub(crate) fn portb_read(ports: &PortAccess<'_>, port: Port, buffer: &mut [u8]) /// Locks the shared port-window transaction. pub(crate) fn lock_portb() -> Result { - lock_file(PORTB_LOCK_PATH) + lock_file(PORTB_LOCK_PATH, None) } /// Formats a failure of [`lock_portb`]: `portb lock: `, then the description of `error`. diff --git a/nvx-rs/src/guest/time/system/tests.rs b/nvx-rs/src/guest/time/system/tests.rs index e7aa16b83..462108047 100644 --- a/nvx-rs/src/guest/time/system/tests.rs +++ b/nvx-rs/src/guest/time/system/tests.rs @@ -967,10 +967,12 @@ fn a_deadline_during_suppression_keeps_the_record_when_rollback_cannot_finish() let test = TestDetectors::new([Some("0"), Some("60"), Some("120")]); let detectors = test.detectors(); let suppression = test.suppression(&detectors); - let deadline = Instant::now() + Duration::from_millis(20); + let deadline = Instant::now() + Duration::from_secs(2); let result = suppression.save_until(Some(deadline), |path, value| { sys::write_number(path, value)?; - thread::sleep(Duration::from_millis(40)); + thread::sleep( + deadline.saturating_duration_since(Instant::now()) + Duration::from_millis(1), + ); Ok(()) }); assert_eq!(result, Err(SaveError::Kept(Errno::ETIMEDOUT))); From 8c0e27c05f42c2a47c1a31e09b3f0b3894c71b5c Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Fri, 6 Nov 2026 08:25:54 -0800 Subject: [PATCH 04/15] guest: retain canonical Sys blocking-lock delegation Route the unbounded branch through the existing Sys facade, keeping its legacy blocking primitive in use without changing bounded acquisition or widening the seven-owner scope. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2610ed93-496e-4bc8-b990-ea9b49ab3462 --- nvx-rs/src/guest/sys/fd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nvx-rs/src/guest/sys/fd.rs b/nvx-rs/src/guest/sys/fd.rs index c1047d1f9..c6b7d501a 100644 --- a/nvx-rs/src/guest/sys/fd.rs +++ b/nvx-rs/src/guest/sys/fd.rs @@ -310,7 +310,7 @@ impl FileLock { deadline: Option, ) -> Result { let Some(deadline) = deadline else { - return lock_file(path, mode); + return super::lock_file(path, mode); }; Self::check_deadline(Some(deadline))?; let flags = OpenFlags::O_RDWR | OpenFlags::O_CREAT | OpenFlags::O_CLOEXEC; From 5d95bd658ac391c5d499000467b0b702582059fd Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Fri, 6 Nov 2026 08:42:04 -0800 Subject: [PATCH 05/15] guest: record fresh snapshot capture public API Record the repository-generated Linux API inventory for the new canonical capture types and TimeError::Deadline. The Windows inventory is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2610ed93-496e-4bc8-b990-ea9b49ab3462 --- nvx-rs/public-api/linux.txt | 45 +++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/nvx-rs/public-api/linux.txt b/nvx-rs/public-api/linux.txt index aaf095e64..b8df13f6d 100644 --- a/nvx-rs/public-api/linux.txt +++ b/nvx-rs/public-api/linux.txt @@ -1734,6 +1734,50 @@ variant nvx::guest::root::UpperKind::Scratch (hidden) variant nvx::guest::root::UpperKind::Tmpfs (hidden) fn nvx::guest::root::assemble (hidden) fn nvx::guest::root::bootstrap (hidden) +mod nvx::guest::snapshot +enum nvx::guest::snapshot::CaptureCause +variant nvx::guest::snapshot::CaptureCause::Io +field nvx::guest::snapshot::CaptureCause::Io::0 +variant nvx::guest::snapshot::CaptureCause::Mounted +field nvx::guest::snapshot::CaptureCause::Mounted::0 +variant nvx::guest::snapshot::CaptureCause::Pending +variant nvx::guest::snapshot::CaptureCause::Time +field nvx::guest::snapshot::CaptureCause::Time::0 +struct nvx::guest::snapshot::CaptureFailure +field nvx::guest::snapshot::CaptureFailure::cause +field nvx::guest::snapshot::CaptureFailure::disposition +field nvx::guest::snapshot::CaptureFailure::stage +enum nvx::guest::snapshot::CaptureStage +variant nvx::guest::snapshot::CaptureStage::BootChecks +variant nvx::guest::snapshot::CaptureStage::Capture +variant nvx::guest::snapshot::CaptureStage::CaptureChecks +variant nvx::guest::snapshot::CaptureStage::Fence +variant nvx::guest::snapshot::CaptureStage::GenerationId +variant nvx::guest::snapshot::CaptureStage::Mounts +variant nvx::guest::snapshot::CaptureStage::Release +variant nvx::guest::snapshot::CaptureStage::Suppression +enum nvx::guest::snapshot::Captured +variant nvx::guest::snapshot::Captured::Continued +struct nvx::guest::snapshot::Devices +field nvx::guest::snapshot::Devices::distro +field nvx::guest::snapshot::Devices::filesystem_tag +field nvx::guest::snapshot::Devices::scratch +enum nvx::guest::snapshot::Disks +variant nvx::guest::snapshot::Disks::Fresh +enum nvx::guest::snapshot::Disposition +variant nvx::guest::snapshot::Disposition::MustEnd +field nvx::guest::snapshot::Disposition::MustEnd::status +variant nvx::guest::snapshot::Disposition::Resumed +const nvx::guest::snapshot::MOUNT_TABLE_LIMIT +struct nvx::guest::snapshot::Mounted +method nvx::guest::snapshot::Mounted::mount_point +method nvx::guest::snapshot::Mounted::origin +enum nvx::guest::snapshot::Origin +variant nvx::guest::snapshot::Origin::Distro +variant nvx::guest::snapshot::Origin::Filesystem +variant nvx::guest::snapshot::Origin::Overlay +variant nvx::guest::snapshot::Origin::Scratch +fn nvx::guest::snapshot::capture mod nvx::guest::spawn struct nvx::guest::spawn::Barrier method nvx::guest::spawn::Barrier::release @@ -1848,6 +1892,7 @@ method nvx::guest::time::TimeAbi::restore_finish method nvx::guest::time::TimeAbi::start_daemon method nvx::guest::time::TimeAbi::strict enum nvx::guest::time::TimeError +variant nvx::guest::time::TimeError::Deadline variant nvx::guest::time::TimeError::EntropyFile field nvx::guest::time::TimeError::EntropyFile::path field nvx::guest::time::TimeError::EntropyFile::source From cee4660107e8b6c3e7c626927d8ac96063da8bc0 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Fri, 6 Nov 2026 09:16:27 -0800 Subject: [PATCH 06/15] guest: simplify redundant root mapping documentation links Apply only the six compiler-suggested redundant-link removals under scoped producer request bd218a74/84c50b3a. Production logic, imports, attributes, signatures and public API are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2610ed93-496e-4bc8-b990-ea9b49ab3462 --- nvx-rs/src/guest/root.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/nvx-rs/src/guest/root.rs b/nvx-rs/src/guest/root.rs index d769d3f0f..1c35b7e66 100644 --- a/nvx-rs/src/guest/root.rs +++ b/nvx-rs/src/guest/root.rs @@ -90,7 +90,7 @@ //! //! - is absolute and canonical: not `/`, without an empty, `.`, or `..` component, a trailing //! `/`, or a NUL, and of at most [`MAX_PATH_BYTES`](super::mappings::MAX_PATH_BYTES) bytes, as -//! the target of a [`Mapping`](super::mappings::Mapping) is; +//! the target of a [`Mapping`] is; //! - holds no `\`, `=`, `,`, or `"`; //! - is not where a container runtime mounts or binds in the root, as //! [`protocol::share`](crate::protocol::share) lists them for the host and the guest: `/proc`, @@ -105,18 +105,18 @@ //! overlay, or on the runtime's `/run` tmpfs once the root has mounted it, a directory with mode //! 0755, or a file child's file with mode 0644, whatever the umask or a default ACL; checks that //! the target is a directory, or a regular file for a child that is a host file; and attaches -//! each mount with its [`Access`](super::mappings::Access), `nosuid`, and `nodev` set already. +//! each mount with its [`Access`], `nosuid`, and `nodev` set already. //! The root mounts a share's device at `staging`, with them, then moves it onto its target, and //! mounts an aggregate's device at `shares`, then attaches a copy of each child's entry there. -//! [`Root::map`] applies a caller's host [`Mapping`](super::mappings::Mapping)s beneath the -//! overlay the same way, from the caller's [`SourceRoot`](super::mappings::SourceRoot). +//! [`Root::map`] applies a caller's host [`Mapping`]s beneath the +//! overlay the same way, from the caller's [`SourceRoot`]. //! //! The host export is the device of an aggregate whose children the root never binds itself: //! they are the sources of the caller's mappings, such as the mapping table of NVX's managed //! lifecycle, whose boot tokens [`protocol::export`](crate::protocol::export) reads. //! [`Root::mount_export`] mounts it at `hostfs/root`, out of the root, with the access of its //! [`ExportMode`](crate::protocol::export::ExportMode), `nosuid`, and `nodev`, and returns its -//! root, the [`SourceRoot`](super::mappings::SourceRoot) from which [`Root::map`] applies the +//! root, the [`SourceRoot`] from which [`Root::map`] applies the //! caller's mappings. A root has one export at most. //! //! The mounts are made in the caller's mount namespace, which must be recursively private, as a @@ -862,7 +862,7 @@ impl Root { /// /// Each unmount is attempted, whatever failed before, without detaching a busy mount except /// one retained after a failed attachment, and every failure names the mount it leaves in place. - /// A [`SourceRoot`](super::mappings::SourceRoot) of the export that the caller still holds + /// A [`SourceRoot`] of the export that the caller still holds /// keeps the export busy. /// /// The file system of an ext4 scratch is synced to the scratch device before the scratch is From cc0fb1eca09fe437c308a9d52f15549096507b16 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 7 Nov 2026 10:30:41 -0800 Subject: [PATCH 07/15] guest: bound the final capture preflight transaction Carry the caller deadline through the final port-B lock and clear it only at the snapshot-port write. Preserve bounded rollback and truthful pre-request failures, while keeping issued requests and their release unbounded and legacy callers unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2610ed93-496e-4bc8-b990-ea9b49ab3462 --- nvx-rs/src/guest/activation.rs | 23 ++++---- nvx-rs/src/guest/activation/tests.rs | 8 ++- nvx-rs/src/guest/snapshot.rs | 11 ++-- nvx-rs/src/guest/snapshot/tests.rs | 78 +++++++++++++++++++++++++--- nvx-rs/src/guest/time/api.rs | 26 ++++++++-- nvx-rs/src/guest/time/api/tests.rs | 36 +++++++++++-- 6 files changed, 147 insertions(+), 35 deletions(-) diff --git a/nvx-rs/src/guest/activation.rs b/nvx-rs/src/guest/activation.rs index 2c383cfae..4f7526013 100644 --- a/nvx-rs/src/guest/activation.rs +++ b/nvx-rs/src/guest/activation.rs @@ -491,7 +491,7 @@ impl CaptureBarrier { let request = CaptureRequest::in_memory(ScratchPolicy::Fresh, generation_id); let outcome = match self.system.capture(request) { Ok(outcome) => outcome, - Err(error) => { + Err((error, _)) => { // The fence is dropped closed. self.fence = None; let error = CaptureError::new(CaptureStage::Capture, CaptureCause::Time(error)); @@ -772,9 +772,6 @@ pub(super) trait System: Send { /// Prepares strictly by the caller deadline, retaining suppression ownership on failure. fn pre_capture_until(&mut self, deadline: Instant) -> Result<(), (PreCaptureError, bool)>; - /// Removes the pre-request deadline before requesting capture. - fn clear_deadline(&mut self); - /// Restores the stall detectors' settings, as [`TimeAbi::cancel_capture`] does. fn cancel_capture(&mut self) -> Result<(), Vec>; @@ -782,7 +779,7 @@ pub(super) trait System: Send { fn generation_id(&mut self) -> Result; /// Requests a capture, as [`TimeAbi::capture`] does. - fn capture(&mut self, request: CaptureRequest) -> Result; + fn capture(&mut self, request: CaptureRequest) -> Result; } /// The guest's kernel, at the paths of its interfaces, and its time ABI. @@ -860,10 +857,6 @@ impl System for LiveSystem { self.abi.pre_capture_until(deadline) } - fn clear_deadline(&mut self) { - self.deadline = None; - } - fn cancel_capture(&mut self) -> Result<(), Vec> { self.abi.cancel_capture_until(self.deadline) } @@ -872,8 +865,16 @@ impl System for LiveSystem { self.abi.generation_id_until(self.deadline) } - fn capture(&mut self, request: CaptureRequest) -> Result { - self.abi.capture(request) + fn capture(&mut self, request: CaptureRequest) -> Result { + let result = self.abi.capture_until(request, self.deadline); + if result + .as_ref() + .err() + .is_none_or(|(_, unbounded)| *unbounded) + { + self.deadline = None; + } + result } } diff --git a/nvx-rs/src/guest/activation/tests.rs b/nvx-rs/src/guest/activation/tests.rs index f5b411944..a592384d7 100644 --- a/nvx-rs/src/guest/activation/tests.rs +++ b/nvx-rs/src/guest/activation/tests.rs @@ -295,8 +295,6 @@ impl System for FakeSystem { result.map_err(|error| (error, held)) } - fn clear_deadline(&mut self) {} - fn generation_id(&mut self) -> Result { self.calls.push(Call::GenerationId); if self.generation_fails { @@ -306,15 +304,15 @@ impl System for FakeSystem { } } - fn capture(&mut self, request: CaptureRequest) -> Result { + fn capture(&mut self, request: CaptureRequest) -> Result { self.calls.push(Call::Capture(request)); match self.outcome { Outcome::Continued => Ok(CaptureOutcome::Continued), Outcome::Restored => Ok(CaptureOutcome::Restored( Restore::to(RESTORED).holding(seed()), )), - Outcome::Fails => Err(TimeError::StateUnavailable), - Outcome::PortRelease => Err(PortError::Revoke(Errno::ENOMEM).into()), + Outcome::Fails => Err((TimeError::StateUnavailable, true)), + Outcome::PortRelease => Err((PortError::Revoke(Errno::ENOMEM).into(), true)), } } } diff --git a/nvx-rs/src/guest/snapshot.rs b/nvx-rs/src/guest/snapshot.rs index ef51aa607..4f819f27e 100644 --- a/nvx-rs/src/guest/snapshot.rs +++ b/nvx-rs/src/guest/snapshot.rs @@ -189,10 +189,13 @@ fn capture_with( Ok(CaptureRequest::in_memory(ScratchPolicy::Fresh, generation)) })(); let request = prepared.map_err(|failure| failure.cleanup(system, suppressed))?; - system.clear_deadline(); - let outcome = system.capture(request).map_err(|error| { - CaptureFailure::new(CaptureStage::Capture, CaptureCause::Time(error)) - .cleanup(system, suppressed) + let outcome = system.capture(request).map_err(|(error, unbounded)| { + let stage = if unbounded { + CaptureStage::Capture + } else { + CaptureStage::CaptureChecks + }; + CaptureFailure::new(stage, CaptureCause::Time(error)).cleanup(system, suppressed) })?; match outcome { CaptureOutcome::Continued => { diff --git a/nvx-rs/src/guest/snapshot/tests.rs b/nvx-rs/src/guest/snapshot/tests.rs index 24dabb621..565ceae52 100644 --- a/nvx-rs/src/guest/snapshot/tests.rs +++ b/nvx-rs/src/guest/snapshot/tests.rs @@ -21,12 +21,16 @@ struct CaptureSystem { generation: Option>, /// The next request's outcome. outcome: Option>, + /// A failure before the request begins. + preflight_error: Option, /// Whether detector restoration fails. release_fails: bool, /// Whether the generation read deliberately completes after the deadline. expire_generation: bool, /// Whether the unbounded request deliberately completes after the original deadline. expire_request: bool, + /// Whether final capture preflight completes after the deadline. + expire_preflight: bool, } impl CaptureSystem { @@ -40,9 +44,11 @@ impl CaptureSystem { preparation: Some(Ok(())), generation: Some(Ok(GenerationId::from_bytes([7; GenerationId::LENGTH]))), outcome: Some(Ok(CaptureOutcome::Continued)), + preflight_error: None, release_fails: false, expire_generation: false, expire_request: false, + expire_preflight: false, } } } @@ -72,11 +78,6 @@ impl System for CaptureSystem { self.preparation.take().expect("preparation runs once") } - fn clear_deadline(&mut self) { - self.calls.push("unbounded"); - self.deadline = None; - } - fn cancel_capture(&mut self) -> Result<(), Vec> { self.calls.push("release"); sys::FileLock::check_deadline(self.deadline) @@ -96,8 +97,17 @@ impl System for CaptureSystem { self.generation.take().expect("generation is read once") } - fn capture(&mut self, request: CaptureRequest) -> Result { - assert_eq!(self.deadline, None); + fn capture(&mut self, request: CaptureRequest) -> Result { + assert_eq!(self.deadline, Some(self.original_deadline)); + if self.expire_preflight { + expire(self.original_deadline); + } + sys::FileLock::check_deadline(self.deadline).map_err(|_| (TimeError::Deadline, false))?; + if let Some(error) = self.preflight_error.take() { + return Err((error, false)); + } + self.calls.push("unbounded"); + self.deadline = None; assert_eq!( request, CaptureRequest::in_memory( @@ -109,7 +119,10 @@ impl System for CaptureSystem { if self.expire_request { expire(self.original_deadline); } - self.outcome.take().expect("capture is requested once") + self.outcome + .take() + .expect("capture is requested once") + .map_err(|error| (error, true)) } } @@ -178,6 +191,55 @@ fn a_pre_request_failure_resumes_only_after_its_detector_restoration_succeeds() } } +#[test] +fn final_capture_preflight_keeps_the_deadline_and_resumes_only_after_rollback() { + for release_fails in [false, true] { + let deadline = Instant::now() + Duration::from_secs(1); + let mut system = CaptureSystem::new(deadline); + system.preflight_error = Some(TimeError::PortLock(io::Error::from_raw_os_error( + Errno::EACCES as i32, + ))); + system.release_fails = release_fails; + let failure = capture_with(&mut system, &capture_devices(), deadline).unwrap_err(); + assert_eq!(system.deadline, Some(deadline)); + assert!(!system.calls.contains(&"request")); + assert_eq!(system.calls.last(), Some(&"release")); + if release_fails { + assert_eq!(failure.stage, CaptureStage::Release); + assert_eq!( + failure.disposition, + Disposition::MustEnd { + status: status::TIME_REPAIR + } + ); + } else { + assert_eq!(failure.stage, CaptureStage::CaptureChecks); + assert_eq!(failure.disposition, Disposition::Resumed); + assert!(matches!( + failure.cause, + CaptureCause::Time(TimeError::PortLock(_)) + )); + } + } +} + +#[test] +fn final_capture_preflight_expiry_never_requests_or_extends_rollback() { + let deadline = Instant::now() + Duration::from_millis(20); + let mut system = CaptureSystem::new(deadline); + system.expire_preflight = true; + let failure = capture_with(&mut system, &capture_devices(), deadline).unwrap_err(); + assert_eq!(system.deadline, Some(deadline)); + assert!(!system.calls.contains(&"request")); + assert_eq!(failure.stage, CaptureStage::Release); + assert_eq!( + failure.disposition, + Disposition::MustEnd { + status: status::TIME_REPAIR + } + ); +} + #[test] fn strict_time_status_is_retained_when_detector_rollback_also_fails() { for violation in [ diff --git a/nvx-rs/src/guest/time/api.rs b/nvx-rs/src/guest/time/api.rs index eed3d6916..70a0c403c 100644 --- a/nvx-rs/src/guest/time/api.rs +++ b/nvx-rs/src/guest/time/api.rs @@ -204,8 +204,21 @@ impl TimeAbi { /// [`Restore::finished`] tells whether the call finished the restore, and how it reports a /// failed handoff of repair step 7. pub fn capture(&self, request: CaptureRequest) -> Result { - let runtime = self.runtime(); - settled(&runtime, capture(&runtime, request)) + self.capture_until(request, None) + .map_err(|(error, _)| error) + } + + /// Bounds capture preflight and reports whether cleanup has entered the unbounded phase. + pub(crate) fn capture_until( + &self, + request: CaptureRequest, + deadline: Option, + ) -> Result { + let mut runtime = self.runtime(); + runtime.deadline = deadline; + let result = capture(&mut runtime, request); + let unbounded = runtime.deadline.is_none(); + settled(&runtime, result).map_err(|error| (error, unbounded)) } /// Finishes a restore that [`capture`](Self::capture) left unfinished, after the caller @@ -780,7 +793,11 @@ const ENTROPY_FLAGS: OpenFlags = OpenFlags::O_WRONLY .union(OpenFlags::O_CLOEXEC); /// The capture request and repair of [`TimeAbi::capture`]. -fn capture(runtime: &Runtime<'_>, request: CaptureRequest) -> Result { +fn capture( + runtime: &mut Runtime<'_>, + request: CaptureRequest, +) -> Result { + sys::FileLock::check_deadline(runtime.deadline).map_err(|_| TimeError::Deadline)?; let CaptureRequest { scratch, generation_id, @@ -819,13 +836,14 @@ fn capture(runtime: &Runtime<'_>, request: CaptureRequest) -> Result EntropySink::Seed, }; - let _lock = match system::lock_portb() { + let _lock = match system::lock_file(system::PORTB_LOCK_PATH, runtime.deadline) { Ok(lock) => lock, Err(error) => { entropy.discard(); return Err(TimeError::PortLock(error.into())); } }; + runtime.deadline = None; ports.write_u8(Port::Snapshot, scratch.request()); let start = clock_ns(sys::Clock::Monotonic); let read = (ports.read_u8(Port::PortbStatus) & PORTB_PACKET_AVAILABLE != 0) diff --git a/nvx-rs/src/guest/time/api/tests.rs b/nvx-rs/src/guest/time/api/tests.rs index 656485a32..de5281b81 100644 --- a/nvx-rs/src/guest/time/api/tests.rs +++ b/nvx-rs/src/guest/time/api/tests.rs @@ -406,13 +406,13 @@ fn the_daemon_does_not_start_while_the_thread_may_hold_port_permission() { #[test] fn a_capture_refuses_an_entropy_path_with_a_nul_byte_before_its_transaction() { let ops = FakeOps::new([]); - let runtime = Runtime::new(&ops, true); + let mut runtime = Runtime::new(&ops, true); let request = CaptureRequest::new( ScratchPolicy::Fresh, GenerationId::from_bytes([1; GenerationId::LENGTH]), PathBuf::from(OsStr::from_bytes(b"restore\0entropy")), ); - let error = capture(&runtime, request).unwrap_err(); + let error = capture(&mut runtime, request).unwrap_err(); assert!( matches!(&error, TimeError::EntropyFile { source, .. } if source.kind() == io::ErrorKind::InvalidInput), "{error:?}" @@ -613,13 +613,43 @@ fn capture_in_each_mode() { system::make_runtime_directories(); state_store(&TimeState::default()).unwrap(); let previous = GenerationId::from_bytes([1; GenerationId::LENGTH]); + let held = system::lock_portb().unwrap(); + let holder = std::thread::spawn(move || { + std::thread::sleep(std::time::Duration::from_secs(1)); + drop(held); + }); + let ops = FakeOps::new([0]); + let mut runtime = Runtime::new(&ops, true); + let deadline = Instant::now() + std::time::Duration::from_millis(250); + runtime.deadline = Some(deadline); + let result = capture( + &mut runtime, + CaptureRequest::in_memory(ScratchPolicy::Fresh, previous), + ); + holder.join().unwrap(); + assert!( + matches!(result, Err(TimeError::PortLock(ref error)) + if error.kind() == io::ErrorKind::TimedOut), + "{result:?}" + ); + assert_eq!(runtime.deadline, Some(deadline)); + assert_eq!(ops.transactions(), 1); + assert!( + !ops.output + .lock() + .unwrap() + .iter() + .any(|(port, _)| *port == Port::Snapshot) + ); + assert!(sys::check_released().is_ok()); + eprintln!("FINAL_CAPTURE_PORTB_DEADLINE_REGRESSION_EXERCISED"); let entropy: [u8; Seed::LENGTH] = std::array::from_fn(|index| index as u8 + 2); let secret = &entropy[GenerationId::LENGTH..]; let path = Path::new("/run/nvx/restore-entropy"); // Returns the capture's outcome, and what it wrote to the kernel log and the console. let capture_with = |input: Vec, request| { let ops = FakeOps::new(input); - let outcome = capture(&Runtime::new(&ops, true), request).unwrap(); + let outcome = capture(&mut Runtime::new(&ops, true), request).unwrap(); let mut logged = ops.kernel_log.lock().unwrap().clone(); logged.extend_from_slice(&ops.console.lock().unwrap()); (outcome, logged) From 791699c065eb1b91fbd5f88f302eaf00376406ad Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 7 Nov 2026 11:09:27 -0800 Subject: [PATCH 08/15] guest: emit the capture regression witness through the existing test diagnostic style Keep the actual privileged final-lock witness without print_stderr, matching the existing out-of-line guest-test writers. The bounded-lock assertions and production source remain unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2610ed93-496e-4bc8-b990-ea9b49ab3462 --- nvx-rs/src/guest/time/api/tests.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/nvx-rs/src/guest/time/api/tests.rs b/nvx-rs/src/guest/time/api/tests.rs index de5281b81..2447b8f09 100644 --- a/nvx-rs/src/guest/time/api/tests.rs +++ b/nvx-rs/src/guest/time/api/tests.rs @@ -1,5 +1,6 @@ use std::ffi::OsStr; use std::fs; +use std::io::Write; use std::os::unix::fs::PermissionsExt; use std::path::Path; use std::sync::atomic::Ordering; @@ -642,7 +643,11 @@ fn capture_in_each_mode() { .any(|(port, _)| *port == Port::Snapshot) ); assert!(sys::check_released().is_ok()); - eprintln!("FINAL_CAPTURE_PORTB_DEADLINE_REGRESSION_EXERCISED"); + writeln!( + io::stderr(), + "FINAL_CAPTURE_PORTB_DEADLINE_REGRESSION_EXERCISED" + ) + .unwrap(); let entropy: [u8; Seed::LENGTH] = std::array::from_fn(|index| index as u8 + 2); let secret = &entropy[GenerationId::LENGTH..]; let path = Path::new("/run/nvx/restore-entropy"); From 0cca77d3ab29eec90f33d9feefab2767d3063be1 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 7 Nov 2026 15:46:39 -0800 Subject: [PATCH 09/15] fix: retry transient Windows performance artifact publication Keep verified artifact publication atomic and retry only Windows access/sharing denials, within the existing fetch deadline and a one-second bound. Preserve permanent failures and destination guards; add focused regressions without weakening existing tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2610ed93-496e-4bc8-b990-ea9b49ab3462 --- scripts/nvx_tools/performance_history.py | 22 +++++- scripts/test_performance.py | 88 ++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/scripts/nvx_tools/performance_history.py b/scripts/nvx_tools/performance_history.py index 55a39b08e..1cc6ea1e6 100644 --- a/scripts/nvx_tools/performance_history.py +++ b/scripts/nvx_tools/performance_history.py @@ -74,6 +74,8 @@ # The persistence reads GitHub for at most this long, well within its job's # timeout, and stops at the first run that it can't fetch in time. _FETCH_BUDGET_SECONDS = 15 * 60.0 +_RENAME_RETRY_SECONDS = 1.0 +_RENAME_RETRY_INTERVAL_SECONDS = 0.05 _DIGEST = re.compile(r"sha256:([0-9a-f]{64})") _REPOSITORY = re.compile(r"[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+") _RESULT_FILE = re.compile(r"[A-Za-z0-9][A-Za-z0-9_.-]*\.csv") @@ -426,7 +428,25 @@ def _fetch( results = staging / "results" results.mkdir() _extract(archive, results) - results.rename(directory) + # Windows can briefly deny a directory rename while a child file is open. + rename_deadline = min(deadline, time.monotonic() + _RENAME_RETRY_SECONDS) + while True: + try: + results.rename(directory) + except PermissionError as error: + remaining = rename_deadline - time.monotonic() + if ( + getattr(error, "winerror", None) not in (5, 32) + or remaining <= 0 + or not results.is_dir() + or directory.exists() + ): + raise + time.sleep(min(_RENAME_RETRY_INTERVAL_SECONDS, remaining)) + if time.monotonic() >= rename_deadline: + raise + else: + break return directory diff --git a/scripts/test_performance.py b/scripts/test_performance.py index 0fb111d73..3b11d5728 100644 --- a/scripts/test_performance.py +++ b/scripts/test_performance.py @@ -4812,6 +4812,12 @@ def test_gate_requires_a_new_hypervisor_build_to_reset_its_series(self): SERIES_FILE = f"{KVM_SERIES}-microvm-v2-1vcpu.csv" +class _WindowsRenameDenial(PermissionError): + def __init__(self, winerror: int) -> None: + super().__init__("access denied") + self.winerror = winerror + + class _RunsApi: """Serves the CI runs of dev pushes, newest first and `page_size` to a page, at most `limit` for a search of their creation times, as GitHub @@ -5035,6 +5041,88 @@ def persist(self) -> performance_history.Persistence: def commits(self, path: str = SERIES_FILE) -> list[str]: return [row.commit for row in performance.read_results(self.history / path)] + def test_transient_windows_rename_denials_retry_only_publication(self): + b = self.commit("b") + self.record(1, b) + denials = [_WindowsRenameDenial(5), _WindowsRenameDenial(32)] + original_rename = Path.rename + attempts = 0 + + def rename(source: Path, destination: Path) -> Path: + nonlocal attempts + attempts += 1 + if attempts <= len(denials): + raise denials[attempts - 1] + return original_rename(source, destination) + + with patch.object(Path, "rename", autospec=True, side_effect=rename): + outcome = self.persist() + + self.assertFalse(outcome.failed) + self.assertEqual([run.sha for run in outcome.persisted], [b]) + self.assertEqual(self.commits(), ["seed", b]) + self.assertEqual(self.api.downloads, [1001]) + self.assertGreaterEqual(attempts, 3) + + def test_persistent_windows_rename_denial_stops_before_the_next_run(self): + b = self.commit("b") + c = self.commit("c") + self.record(1, b) + self.record(2, c) + error = _WindowsRenameDenial(5) + + with patch.object(Path, "rename", side_effect=error) as rename: + outcome = self.persist() + + self.assertTrue(outcome.failed) + self.assertIn("access denied", outcome.stopped or "") + self.assertEqual(outcome.persisted, []) + self.assertEqual(self.commits(), ["seed"]) + self.assertEqual(self.api.downloads, [1001]) + self.assertGreater(rename.call_count, 1) + + def test_windows_rename_denial_does_not_extend_the_fetch_deadline(self): + b = self.commit("b") + self.record(1, b) + error = _WindowsRenameDenial(5) + + with ( + patch.object(Path, "rename", side_effect=error) as rename, + self.assertRaises(PermissionError) as refused, + ): + performance_history._fetch( + self.api, + REPOSITORY, + performance_history.RecordedRun(1, b, ""), + self.api.artifacts[1][0], + self.downloads, + time.monotonic(), + ) + + self.assertIs(refused.exception, error) + rename.assert_called_once() + self.assertEqual(self.commits(), ["seed"]) + + def test_windows_rename_denial_does_not_replace_an_existing_destination(self): + b = self.commit("b") + self.record(1, b) + error = _WindowsRenameDenial(5) + + def rename(source: Path, destination: Path) -> Path: + destination.mkdir() + (destination / "preserved").write_bytes(b"another download") + raise error + + with patch.object(Path, "rename", autospec=True, side_effect=rename) as publish: + outcome = self.persist() + + self.assertTrue(outcome.failed) + self.assertEqual(self.commits(), ["seed"]) + publish.assert_called_once() + self.assertEqual( + (self.downloads / "1" / "preserved").read_bytes(), b"another download" + ) + def test_runs_newer_than_the_history_persist_oldest_first(self): a = self.commit("a") self.write(self.history, ["seed", a], KVM_RUNNER_HOST) From 0e7fb2b3ae1790dbd9d5f5864bb81bbda01c9497 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 7 Nov 2026 19:40:43 -0800 Subject: [PATCH 10/15] guest: bound capture failure diagnostics by the caller deadline Keep bounded capture failure diagnostics nonblocking, including the kernel log, console and reopened stderr, without changing shared stream flags. Preserve blocking legacy diagnostics and the original strict failure status when output or its reporting lock cannot proceed. Cover full owned pipes, unchanged stream flags, expiry, legacy writes and reporting-lock contention. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2610ed93-496e-4bc8-b990-ea9b49ab3462 --- nvx-rs/src/guest/sys/fd.rs | 32 +++++++ nvx-rs/src/guest/sys/fd/tests.rs | 37 ++++++++ nvx-rs/src/guest/time/system.rs | 118 ++++++++++++++++++++------ nvx-rs/src/guest/time/system/tests.rs | 114 ++++++++++++++++++++++++- 4 files changed, 271 insertions(+), 30 deletions(-) diff --git a/nvx-rs/src/guest/sys/fd.rs b/nvx-rs/src/guest/sys/fd.rs index c6b7d501a..b2f7ba2fc 100644 --- a/nvx-rs/src/guest/sys/fd.rs +++ b/nvx-rs/src/guest/sys/fd.rs @@ -75,6 +75,38 @@ impl FileDescriptor { Ok(()) } + /// Writes through a nonblocking pipe or character device by the caller deadline, without + /// waiting for flow control. Regular files ignore nonblocking mode. Legacy writes are unchanged. + pub(crate) fn write_all_until( + &self, + mut bytes: &[u8], + deadline: Option, + ) -> Result<(), Errno> { + let Some(deadline) = deadline else { + return self.write_all(bytes); + }; + FileLock::check_deadline(Some(deadline))?; + let flags = fcntl(&self.0, FcntlArg::F_GETFL).map(OpenFlags::from_bits_retain)?; + if !flags.contains(OpenFlags::O_NONBLOCK) { + return Err(Errno::EINVAL); + } + let kind = + SFlag::from_bits_truncate(nix::sys::stat::fstat(&self.0)?.st_mode) & SFlag::S_IFMT; + if !matches!(kind, SFlag::S_IFIFO | SFlag::S_IFCHR) { + return Err(Errno::EOPNOTSUPP); + } + while !bytes.is_empty() { + FileLock::check_deadline(Some(deadline))?; + match super::signal::write_without_sigpipe(self, bytes) { + Ok(0) => return Err(Errno::EIO), + Ok(count) => bytes = &bytes[count..], + Err(Errno::EINTR) => {} + Err(error) => return Err(error), + } + } + FileLock::check_deadline(Some(deadline)) + } + /// Reads at `offset` once, preserving the C cast from `u64` to `off_t`. pub(crate) fn pread(&self, bytes: &mut [u8], offset: u64) -> Result { nix::sys::uio::pread(&self.0, bytes, offset as _) diff --git a/nvx-rs/src/guest/sys/fd/tests.rs b/nvx-rs/src/guest/sys/fd/tests.rs index 5cdd015e4..0a4cd0c53 100644 --- a/nvx-rs/src/guest/sys/fd/tests.rs +++ b/nvx-rs/src/guest/sys/fd/tests.rs @@ -43,6 +43,43 @@ impl FileDescriptor { } } +#[test] +fn deadline_writes_refuse_blocking_descriptors_and_keep_legacy_writes() { + let (reader, writer) = create_pipe().unwrap(); + assert_eq!( + writer.write_all_until(b"bounded", Some(Instant::now() + Duration::from_secs(1))), + Err(Errno::EINVAL) + ); + writer.write_all_until(b"legacy", None).unwrap(); + let mut bytes = [0; 6]; + reader.read_exact(&mut bytes).unwrap(); + assert_eq!(&bytes, b"legacy"); + let file = FileDescriptor::from_owned(tempfile::tempfile().unwrap().into()); + file.set_nonblocking().unwrap(); + assert_eq!( + file.write_all_until(b"bounded", Some(Instant::now() + Duration::from_secs(1))), + Err(Errno::EOPNOTSUPP) + ); +} + +#[test] +fn an_expired_write_deadline_leaves_the_pipe_unchanged() { + let (reader, writer) = create_pipe().unwrap(); + writer.set_nonblocking().unwrap(); + reader.set_nonblocking().unwrap(); + assert_eq!( + writer.write_all_until(b"late", Some(Instant::now())), + Err(Errno::ETIMEDOUT) + ); + assert_eq!(reader.read(&mut [0; 4]), Err(Errno::EAGAIN)); + writer + .write_all_until(b"on time", Some(Instant::now() + Duration::from_secs(1))) + .unwrap(); + let mut bytes = [0; 7]; + reader.read_exact(&mut bytes).unwrap(); + assert_eq!(&bytes, b"on time"); +} + /// Sets the soft limit of the process's open descriptors to `limit`, and returns the limit that /// it replaced, for a test that runs out of descriptors in a process of its own. /// diff --git a/nvx-rs/src/guest/time/system.rs b/nvx-rs/src/guest/time/system.rs index 7909e5980..95683405d 100644 --- a/nvx-rs/src/guest/time/system.rs +++ b/nvx-rs/src/guest/time/system.rs @@ -4,8 +4,9 @@ use std::ffi::{CStr, CString}; use std::fmt::{self, Display}; use std::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering}; -use std::sync::{Mutex, PoisonError}; -use std::time::Instant; +use std::sync::{Mutex, MutexGuard, PoisonError, TryLockError}; +use std::thread; +use std::time::{Duration, Instant}; use super::Phase; use super::api::Violation; @@ -155,10 +156,10 @@ pub(crate) trait PrivilegedOps: Sync { /// Stops the VM with `status`, which in production does not return. fn power_off(&self, status: PowerOffStatus); /// Appends `record` to the kernel log, when the process may write it. - fn write_kernel_log(&self, record: &[u8]); + fn write_kernel_log(&self, record: &[u8], deadline: Option); /// Writes `separator` and then `line` to the console, or to standard error when the console /// does not open. - fn write_console(&self, separator: &[u8], line: &[u8]); + fn write_console(&self, separator: &[u8], line: &[u8], deadline: Option); } /// An MSR device, or the token of a test's fake. @@ -218,25 +219,50 @@ impl PrivilegedOps for RealOps { sys::power_off(status.code()) } - fn write_kernel_log(&self, record: &[u8]) { - let flags = OpenFlags::O_WRONLY | OpenFlags::O_CLOEXEC; - if let Ok(log) = sys::open(kernel::KERNEL_LOG_DEVICE, flags, Mode::empty()) { - let _ = log.write_all(record); + fn write_kernel_log(&self, record: &[u8], deadline: Option) { + let flags = OpenFlags::O_WRONLY + | OpenFlags::O_CLOEXEC + | deadline.map_or(OpenFlags::empty(), |_| OpenFlags::O_NONBLOCK); + if FileLock::check_deadline(deadline).is_ok() + && let Ok(log) = sys::open(kernel::KERNEL_LOG_DEVICE, flags, Mode::empty()) + { + let _ = log.write_all_until(record, deadline); } } - fn write_console(&self, separator: &[u8], line: &[u8]) { - let flags = OpenFlags::O_WRONLY | OpenFlags::O_NOCTTY | OpenFlags::O_CLOEXEC; - match sys::open(kernel::CONSOLE_DEVICE, flags, Mode::empty()) { - Ok(console) => { - let _ = console.write_all(separator); - let _ = console.write_all(line); - } - Err(_) => { + fn write_console(&self, separator: &[u8], line: &[u8], deadline: Option) { + write_console_at(kernel::CONSOLE_DEVICE, separator, line, deadline); + } +} + +/// Best-effort duplicate diagnostics cannot delay the structured failure returned to the caller. +fn write_console_at(path: &CStr, separator: &[u8], line: &[u8], deadline: Option) { + if FileLock::check_deadline(deadline).is_err() { + return; + } + let flags = OpenFlags::O_WRONLY + | OpenFlags::O_NOCTTY + | OpenFlags::O_CLOEXEC + | deadline.map_or(OpenFlags::empty(), |_| OpenFlags::O_NONBLOCK); + match sys::open(path, flags, Mode::empty()) { + Ok(console) => { + let _ = console.write_all_until(separator, deadline); + let _ = console.write_all_until(line, deadline); + } + Err(_) => match deadline { + None => { sys::write_standard_error(separator); sys::write_standard_error(line); } - } + Some(_) => { + // Reopen rather than duplicate: changing a shared stderr description's flags + // would also change other writers. + if let Ok(stderr) = sys::open(c"/proc/self/fd/2", flags, Mode::empty()) { + let _ = stderr.write_all_until(separator, deadline); + let _ = stderr.write_all_until(line, deadline); + } + } + }, } } @@ -369,8 +395,31 @@ impl<'a> Runtime<'a> { /// starts nothing, as [`fork_process`] checks. pub(crate) fn log_release_failure(&self, error: PortError) { let record = format!("nvx-time: {error}\n"); - self.ops - .write_kernel_log(&[DIAGNOSTIC_PRIORITY, record.as_bytes()].concat()); + self.ops.write_kernel_log( + &[DIAGNOSTIC_PRIORITY, record.as_bytes()].concat(), + self.deadline, + ); + } + + fn lock_reports(&self) -> Result>, Errno> { + let Some(deadline) = self.deadline else { + return Ok(self.report_lock.lock().ok()); + }; + loop { + FileLock::check_deadline(Some(deadline))?; + match self.report_lock.try_lock() { + Ok(guard) => { + FileLock::check_deadline(Some(deadline))?; + return Ok(Some(guard)); + } + Err(TryLockError::Poisoned(_)) => return Ok(None), + Err(TryLockError::WouldBlock) => thread::sleep( + deadline + .saturating_duration_since(Instant::now()) + .min(Duration::from_millis(1)), + ), + } + } } /// Emits one serialized violation record to every ABI destination. @@ -381,8 +430,9 @@ impl<'a> Runtime<'a> { phase: Phase, detail: &[u8], ) { - let _guard = self.report_lock.lock().ok(); - self.emit_event_unlocked(code, source, phase, detail); + if let Ok(_guard) = self.lock_reports() { + self.emit_event_unlocked(code, source, phase, detail); + } } /// Emits an event while the caller already owns `report_lock`. @@ -402,13 +452,20 @@ impl<'a> Runtime<'a> { clock_ns(sys::Clock::Boottime), detail, ); - self.ops - .write_kernel_log(&[KERNEL_LOG_PRIORITY, line.as_bytes()].concat()); - if let Ok(ports) = self.open_ports() { + self.ops.write_kernel_log( + &[KERNEL_LOG_PRIORITY, line.as_bytes()].concat(), + self.deadline, + ); + if FileLock::check_deadline(self.deadline).is_ok() + && let Ok(ports) = self.open_ports() + { if self.async_output.load(Ordering::Relaxed) { ports.write_u8(Port::PortbData, b'\n'); } for byte in line.bytes() { + if FileLock::check_deadline(self.deadline).is_err() { + break; + } ports.write_u8(Port::PortbData, byte); } // The event goes on after a failure, which keeps the thread from starting anything, @@ -422,15 +479,20 @@ impl<'a> Runtime<'a> { } else { b"" }; - self.ops.write_console(separator, line.as_bytes()); + self.ops + .write_console(separator, line.as_bytes(), self.deadline); let _ = count_violation(self.deadline); } /// Reports a conformance failure, and powers off unless report-only or strict. pub(crate) fn conformance_failure(&self, code: ViolationCode, phase: Phase, detail: &[u8]) { - let _guard = self.report_lock.lock().ok(); - self.emit_event_unlocked(code, EventSource::Conformance, phase, detail); - self.stop(PowerOffStatus::Conformance, code, detail); + match self.lock_reports() { + Ok(_guard) => { + self.emit_event_unlocked(code, EventSource::Conformance, phase, detail); + self.stop(PowerOffStatus::Conformance, code, detail); + } + Err(_) => self.stop(PowerOffStatus::Conformance, code, detail), + } } /// Emits a fatal event and requests its typed power-off status, unless report-only or diff --git a/nvx-rs/src/guest/time/system/tests.rs b/nvx-rs/src/guest/time/system/tests.rs index 462108047..2a139e074 100644 --- a/nvx-rs/src/guest/time/system/tests.rs +++ b/nvx-rs/src/guest/time/system/tests.rs @@ -55,6 +55,10 @@ pub(crate) struct FakeOps { pub(crate) kernel_log: Mutex>, /// Console output, in order. pub(crate) console: Mutex>, + /// A test-owned console path that exercises the production output instead. + console_device: Option, + /// Caller deadlines passed to kernel-log and console output, respectively. + diagnostic_deadlines: Mutex)>>, /// What the frequency MSR reads: the declared TSC rate, or an error. pub(crate) tsc_hz: Result, /// The next TSC read, which each read advances by [`FakeOps::TSC_STEP`]. @@ -85,6 +89,8 @@ impl FakeOps { power: AtomicUsize::new(0), kernel_log: Mutex::new(Vec::new()), console: Mutex::new(Vec::new()), + console_device: None, + diagnostic_deadlines: Mutex::new(Vec::new()), tsc_hz: Ok(Self::TSC_HZ), tsc: AtomicU64::new(Self::TSC_START), realtime_ns: Self::REALTIME_NS, @@ -208,11 +214,23 @@ impl PrivilegedOps for FakeOps { .store(usize::from(status.code()), Ordering::Relaxed); } - fn write_kernel_log(&self, record: &[u8]) { + fn write_kernel_log(&self, record: &[u8], deadline: Option) { + self.diagnostic_deadlines + .lock() + .unwrap() + .push((true, deadline)); self.kernel_log.lock().unwrap().extend_from_slice(record); } - fn write_console(&self, separator: &[u8], line: &[u8]) { + fn write_console(&self, separator: &[u8], line: &[u8], deadline: Option) { + self.diagnostic_deadlines + .lock() + .unwrap() + .push((false, deadline)); + if let Some(path) = &self.console_device { + write_console_at(path, separator, line, deadline); + return; + } let mut console = self.console.lock().unwrap(); console.extend_from_slice(separator); console.extend_from_slice(line); @@ -683,6 +701,98 @@ fn a_strict_runtime_keeps_its_first_violation_and_never_powers_off() { ); } +#[test] +fn a_full_console_pipe_does_not_delay_or_change_a_strict_capture_failure() { + use std::os::fd::AsRawFd; + + let (reader, writer) = sys::create_pipe().unwrap(); + let path = CString::new(format!("/proc/self/fd/{}", writer.borrowed().as_raw_fd())).unwrap(); + let flags = OpenFlags::O_WRONLY | OpenFlags::O_CLOEXEC | OpenFlags::O_NONBLOCK; + let filling = sys::open(&path, flags, Mode::empty()).unwrap(); + let bytes = [0_u8; 4096]; + loop { + match filling.write(&bytes) { + Ok(count) => assert_ne!(count, 0), + Err(Errno::EINTR) => {} + Err(Errno::EAGAIN) => break, + Err(error) => panic!("fill pipe: {error}"), + } + } + let status_flags = || { + nix::fcntl::fcntl(writer.borrowed(), nix::fcntl::FcntlArg::F_GETFL) + .map(OpenFlags::from_bits_retain) + .unwrap() + }; + let original = status_flags(); + assert!(!original.contains(OpenFlags::O_NONBLOCK)); + let (returned, waiting) = mpsc::channel(); + let (ready, started) = mpsc::channel(); + let observer = thread::spawn(move || { + ready.send(()).unwrap(); + let returned = waiting.recv_timeout(Duration::from_secs(2)).is_ok(); + if !returned { + reader.read(&mut [0; 4096]).unwrap(); + } + returned + }); + started.recv().unwrap(); + let mut ops = FakeOps::new([]); + ops.console_device = Some(path); + let mut runtime = Runtime::new(&ops, false).strict(); + let deadline = Instant::now() + Duration::from_secs(1); + runtime.deadline = Some(deadline); + runtime.conformance_failure( + ViolationCode::Conformance(CheckId::C12), + Phase::Capture, + b"blocked diagnostics", + ); + returned.send(()).unwrap(); + assert!( + observer.join().unwrap(), + "diagnostic output blocked until the pipe was drained" + ); + assert_eq!(status_flags(), original); + assert_eq!( + *ops.diagnostic_deadlines.lock().unwrap(), + [(true, Some(deadline)), (false, Some(deadline))] + ); + let violation = runtime.take_violation().unwrap(); + assert_eq!( + (violation.status(), violation.code(), violation.detail()), + (193, "G_CONFORMANCE_C12", "blocked diagnostics") + ); + assert_eq!(ops.power.load(Ordering::Relaxed), 0); +} + +#[test] +fn a_busy_report_lock_keeps_the_strict_failure_without_waiting_past_its_deadline() { + let ops = FakeOps::new([]); + let mut runtime = Runtime::new(&ops, false).strict(); + runtime.deadline = Some(Instant::now() + Duration::from_millis(20)); + let held = runtime.report_lock.lock().unwrap(); + let (finished, received) = mpsc::channel(); + thread::scope(|scope| { + let worker = scope.spawn(|| { + runtime.conformance_failure( + ViolationCode::Conformance(CheckId::C12), + Phase::Capture, + b"busy diagnostics", + ); + finished.send(()).unwrap(); + }); + let returned = received.recv_timeout(Duration::from_secs(1)).is_ok(); + drop(held); + worker.join().unwrap(); + assert!(returned, "the reporting lock outlived the caller deadline"); + }); + let violation = runtime.take_violation().unwrap(); + assert_eq!( + (violation.status(), violation.code(), violation.detail()), + (193, "G_CONFORMANCE_C12", "busy diagnostics") + ); + assert_eq!(ops.power.load(Ordering::Relaxed), 0); +} + #[test] fn modes_of_the_runtime_files_match_the_c_program() { assert_eq!(DIRECTORY_MODE.bits(), 0o755); From 9d681a149757cbea1df5218a58300230a26df6a0 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 7 Nov 2026 20:08:57 -0800 Subject: [PATCH 11/15] guest: document the bounded diagnostic reporting lock Satisfy the existing private-item documentation lint without changing the deadline, output or strict-failure logic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2610ed93-496e-4bc8-b990-ea9b49ab3462 --- nvx-rs/src/guest/time/system.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nvx-rs/src/guest/time/system.rs b/nvx-rs/src/guest/time/system.rs index 95683405d..ab53ebec0 100644 --- a/nvx-rs/src/guest/time/system.rs +++ b/nvx-rs/src/guest/time/system.rs @@ -401,6 +401,8 @@ impl<'a> Runtime<'a> { ); } + /// Acquires the reporting lock by the caller deadline. A poisoned lock keeps the legacy + /// guardless reporting behavior. fn lock_reports(&self) -> Result>, Errno> { let Some(deadline) = self.deadline else { return Ok(self.report_lock.lock().ok()); From 3a0ae6905c3433e93bf6c4d363b7afe9dd40a63d Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 7 Nov 2026 22:06:41 -0800 Subject: [PATCH 12/15] guest: retain the first capture port-release failure Keep the first revocation errno in each Runtime before later diagnostic transactions can clear the current thread marker. Strict preparation prioritizes that original failure over a strict violation, including failed grant rollback and sample-release paths. Preserve the existing deadline, suppression and legacy call behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2610ed93-496e-4bc8-b990-ea9b49ab3462 --- nvx-rs/src/guest/time/api.rs | 2 +- nvx-rs/src/guest/time/api/tests.rs | 121 ++++++++++++++++++++++++++ nvx-rs/src/guest/time/system.rs | 35 +++++++- nvx-rs/src/guest/time/system/tests.rs | 18 +++- 4 files changed, 171 insertions(+), 5 deletions(-) diff --git a/nvx-rs/src/guest/time/api.rs b/nvx-rs/src/guest/time/api.rs index 70a0c403c..b32fd18c5 100644 --- a/nvx-rs/src/guest/time/api.rs +++ b/nvx-rs/src/guest/time/api.rs @@ -273,7 +273,7 @@ impl TimeAbi { let suppressed = result .as_ref() .map_or_else(PreCaptureError::holds_suppression, |()| true); - released_preparation(result, sys::check_released(), runtime.take_violation()) + released_preparation(result, runtime.check_released(), runtime.take_violation()) .map_err(|error| (error, suppressed)) } diff --git a/nvx-rs/src/guest/time/api/tests.rs b/nvx-rs/src/guest/time/api/tests.rs index 2447b8f09..5ec239443 100644 --- a/nvx-rs/src/guest/time/api/tests.rs +++ b/nvx-rs/src/guest/time/api/tests.rs @@ -7,6 +7,7 @@ use std::sync::atomic::Ordering; use super::*; use crate::guest::sys::{ClockAdjustment, FrequencyHold}; +use crate::guest::time::NSEC_PER_MSEC; use crate::guest::time::decoders::{HostClock, PACKET_DNS_SIZE}; use crate::guest::time::state::HostClockStatus; use crate::guest::time::system::PORTB_TIME_WINDOW; @@ -339,6 +340,126 @@ fn a_preparation_that_may_leave_port_permission_fails_with_its_suppression() { ); } +#[test] +fn strict_preparation_keeps_its_first_diagnostic_revocation_after_a_successful_release() { + let ops = FakeOps::new([]); + ops.revoke_errors.lock().unwrap().extend([ + Some(Errno::ENOMEM), + None, + Some(Errno::EIO), + None, + None, + None, + ]); + let mut runtime = Runtime::new(&ops, false).strict(); + runtime.deadline = Some(Instant::now() + std::time::Duration::from_secs(1)); + for (check, detail) in [ + (CheckId::C12, b"first".as_slice()), + (CheckId::C13, b"second".as_slice()), + (CheckId::C14, b"third".as_slice()), + ] { + runtime.conformance_failure( + ViolationCode::Conformance(check), + super::super::Phase::Capture, + detail, + ); + } + assert_eq!(ops.transactions(), 3); + assert_eq!(sys::check_released(), Ok(())); + let violation = runtime.take_violation().unwrap(); + assert_eq!( + (violation.status(), violation.code(), violation.detail()), + (193, "G_CONFORMANCE_C12", "first") + ); + let error = + released_preparation(Ok(()), runtime.check_released(), Some(violation)).unwrap_err(); + assert!( + matches!( + &error, + PreCaptureError::PortRelease { + error: TimeError::PortRelease(source), + suppressed: true, + } if has_errno(source, Errno::ENOMEM) + ), + "{error:?}" + ); + assert_eq!(ops.power.load(Ordering::Relaxed), 0); +} + +#[test] +fn strict_preparation_keeps_a_partial_grants_failed_rollback_after_a_successful_release() { + let ops = FakeOps::new([]); + ops.grant_errors + .lock() + .unwrap() + .extend([None, Some(Errno::EPERM)]); + ops.revoke_errors + .lock() + .unwrap() + .extend([Some(Errno::ENOMEM), None]); + let mut runtime = Runtime::new(&ops, false).strict(); + runtime.deadline = Some(Instant::now() + std::time::Duration::from_secs(1)); + runtime.conformance_failure( + ViolationCode::Conformance(CheckId::C12), + super::super::Phase::Capture, + b"partial grant", + ); + assert_eq!(sys::check_released(), Err(PortError::Revoke(Errno::ENOMEM))); + runtime.conformance_failure( + ViolationCode::Conformance(CheckId::C13), + super::super::Phase::Capture, + b"later release", + ); + assert_eq!(sys::check_released(), Ok(())); + let error = released_preparation(Ok(()), runtime.check_released(), runtime.take_violation()) + .unwrap_err(); + assert!( + matches!( + &error, + PreCaptureError::PortRelease { + error: TimeError::PortRelease(source), + suppressed: true, + } if has_errno(source, Errno::ENOMEM) + ), + "{error:?}" + ); + assert_eq!(ops.power.load(Ordering::Relaxed), 0); +} + +#[test] +fn strict_preparation_keeps_a_samples_failed_revocation_after_successful_diagnostics() { + let ops = FakeOps::new([u32::from(PORTB_TIME_WINDOW)]); + ops.revoke_errors + .lock() + .unwrap() + .extend([Some(Errno::ENOMEM), None]); + let mut runtime = Runtime::new(&ops, false).strict(); + runtime.deadline = Some(Instant::now() + std::time::Duration::from_secs(1)); + assert_eq!( + system::best_sample(&runtime, 0, NSEC_PER_MSEC, 0).unwrap_err(), + system::SampleError::Ports(PortError::Revoke(Errno::ENOMEM)) + ); + runtime.conformance_failure( + ViolationCode::Conformance(CheckId::C12), + super::super::Phase::Capture, + b"sample release", + ); + assert_eq!(sys::check_released(), Ok(())); + let error = released_preparation(Ok(()), runtime.check_released(), runtime.take_violation()) + .unwrap_err(); + assert!( + matches!( + &error, + PreCaptureError::PortRelease { + error: TimeError::PortRelease(source), + suppressed: true, + } if has_errno(source, Errno::ENOMEM) + ), + "{error:?}" + ); + assert_eq!(ops.power.load(Ordering::Relaxed), 0); +} + #[test] fn strict_preparation_keeps_the_boot_failure_stage_and_suppression_ownership() { let violation = Violation::of_missing_daemon(); diff --git a/nvx-rs/src/guest/time/system.rs b/nvx-rs/src/guest/time/system.rs index ab53ebec0..5edda0692 100644 --- a/nvx-rs/src/guest/time/system.rs +++ b/nvx-rs/src/guest/time/system.rs @@ -283,6 +283,8 @@ pub(crate) struct Runtime<'a> { strict: bool, /// The first violation that the call reported in strict mode, which fails the call. violation: Mutex>, + /// The first diagnostic revocation errno, or `u32::MAX` when none occurred. + release_failure: AtomicU32, /// Current VM generation. generation: AtomicU32, /// The declared TSC rate, `F`, in Hz, once known, or zero. @@ -303,6 +305,7 @@ impl<'a> Runtime<'a> { report_only, strict: false, violation: Mutex::new(None), + release_failure: AtomicU32::new(u32::MAX), generation: AtomicU32::new(0), tsc_hz: AtomicU64::new(0), async_output: AtomicBool::new(false), @@ -385,7 +388,31 @@ impl<'a> Runtime<'a> { /// Takes the calling thread's permission for the time ABI's ports, for one transaction, /// which [`PortAccess::release`] ends. pub(crate) fn open_ports(&self) -> Result, PortError> { - self.ops.open_ports() + self.ops + .open_ports() + .inspect_err(|&error| self.retain_release_failure(error)) + } + + /// Reports the first diagnostic revocation failure, otherwise the thread's current + /// permission state. A later successful release cannot erase the call's original failure. + pub(crate) fn check_released(&self) -> Result<(), PortError> { + match self.release_failure.load(Ordering::Relaxed) { + u32::MAX => sys::check_released(), + errno => Err(PortError::Revoke(Errno::from_raw(errno as i32))), + } + } + + /// Keeps the first revocation failure before diagnostics or later transactions can replace + /// the calling thread's current error. + fn retain_release_failure(&self, error: PortError) { + if let PortError::Revoke(errno) = error { + let _ = self.release_failure.compare_exchange( + u32::MAX, + errno as u32, + Ordering::Relaxed, + Ordering::Relaxed, + ); + } } /// Writes `error`, the failure of a transaction's end, to the kernel log, such as @@ -394,6 +421,7 @@ impl<'a> Runtime<'a> { /// are on the null device. Until a later transaction revokes the permission, the thread /// starts nothing, as [`fork_process`] checks. pub(crate) fn log_release_failure(&self, error: PortError) { + self.retain_release_failure(error); let record = format!("nvx-time: {error}\n"); self.ops.write_kernel_log( &[DIAGNOSTIC_PRIORITY, record.as_bytes()].concat(), @@ -952,7 +980,10 @@ pub(crate) fn best_sample( let rates = clock_rates(runtime)?; let ports = runtime.open_ports().map_err(SampleError::Ports)?; let selected = select_sample(&ports, runtime.ops, attempts, bound_ns, generation, rates); - ports.release().map_err(SampleError::Ports)?; + ports + .release() + .inspect_err(|&error| runtime.retain_release_failure(error)) + .map_err(SampleError::Ports)?; selected } diff --git a/nvx-rs/src/guest/time/system/tests.rs b/nvx-rs/src/guest/time/system/tests.rs index 2a139e074..4d7b73fb2 100644 --- a/nvx-rs/src/guest/time/system/tests.rs +++ b/nvx-rs/src/guest/time/system/tests.rs @@ -39,8 +39,12 @@ impl Suppression<'_> { pub(crate) struct FakeOps { /// The error with which the kernel refuses to grant the port permission, if any. pub(crate) grant_error: Option, + /// Per-range grant results used before falling back to `grant_error`. + pub(crate) grant_errors: Mutex>>, /// The error with which the kernel refuses to revoke the port permission, if any. pub(crate) revoke_error: Option, + /// Per-range revocation results used before falling back to `revoke_error`. + pub(crate) revoke_errors: Mutex>>, /// Changes of the port permission, as `(first, count, enable)`, in order. pub(crate) permissions: Mutex>, /// Values that successive port reads return. @@ -81,7 +85,9 @@ impl FakeOps { pub(crate) fn new(input: impl IntoIterator) -> Self { Self { grant_error: None, + grant_errors: Mutex::new(VecDeque::new()), revoke_error: None, + revoke_errors: Mutex::new(VecDeque::new()), permissions: Mutex::new(Vec::new()), input: Mutex::new(input.into_iter().collect()), output: Mutex::new(Vec::new()), @@ -156,9 +162,17 @@ impl PortBackend for FakeOps { .unwrap() .push((first, count, enable)); let error = if enable { - self.grant_error + self.grant_errors + .lock() + .unwrap() + .pop_front() + .unwrap_or(self.grant_error) } else { - self.revoke_error + self.revoke_errors + .lock() + .unwrap() + .pop_front() + .unwrap_or(self.revoke_error) }; error.map_or(Ok(()), Err) } From 34b3b4296e5d7b1ae27a1fa6fa2bcbdbe4d7beba Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 7 Nov 2026 22:24:17 -0800 Subject: [PATCH 13/15] guest: use existing checks in port-release regressions The first focused musl test build failed before executing because the new test referred to C13 and C14, while the ABI defines C1 through C12. Use C11 and C10 for the later diagnostic events; retain all original error, status, suppression and successful-revocation assertions. No production behavior changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2610ed93-496e-4bc8-b990-ea9b49ab3462 --- nvx-rs/src/guest/time/api/tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nvx-rs/src/guest/time/api/tests.rs b/nvx-rs/src/guest/time/api/tests.rs index 5ec239443..e5ccc63ef 100644 --- a/nvx-rs/src/guest/time/api/tests.rs +++ b/nvx-rs/src/guest/time/api/tests.rs @@ -355,8 +355,8 @@ fn strict_preparation_keeps_its_first_diagnostic_revocation_after_a_successful_r runtime.deadline = Some(Instant::now() + std::time::Duration::from_secs(1)); for (check, detail) in [ (CheckId::C12, b"first".as_slice()), - (CheckId::C13, b"second".as_slice()), - (CheckId::C14, b"third".as_slice()), + (CheckId::C11, b"second".as_slice()), + (CheckId::C10, b"third".as_slice()), ] { runtime.conformance_failure( ViolationCode::Conformance(check), @@ -406,7 +406,7 @@ fn strict_preparation_keeps_a_partial_grants_failed_rollback_after_a_successful_ ); assert_eq!(sys::check_released(), Err(PortError::Revoke(Errno::ENOMEM))); runtime.conformance_failure( - ViolationCode::Conformance(CheckId::C13), + ViolationCode::Conformance(CheckId::C11), super::super::Phase::Capture, b"later release", ); From 9659385535721dd49b47211e6f719f784c1c6ea8 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 7 Nov 2026 22:49:41 -0800 Subject: [PATCH 14/15] guest: keep capture revocation causes through diagnostics Retain a pending thread revocation before a later grant can clear it. For bounded Fresh capture, prefer that original cause at the actual preparation and capture finalizers, even after the request cleared its deadline. Preserve legacy capture behavior and unbounded post-request cleanup. Exercise dropped permission followed by repair diagnostics and the real preparation finalizer; replace unnecessary sample-specific instrumentation with the common pre-grant guard. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2610ed93-496e-4bc8-b990-ea9b49ab3462 --- nvx-rs/src/guest/time/api.rs | 38 +++++++++++++++++----- nvx-rs/src/guest/time/api/tests.rs | 46 +++++++++++++++++++++++---- nvx-rs/src/guest/time/system.rs | 12 +++---- nvx-rs/src/guest/time/system/tests.rs | 7 ++++ 4 files changed, 82 insertions(+), 21 deletions(-) diff --git a/nvx-rs/src/guest/time/api.rs b/nvx-rs/src/guest/time/api.rs index b32fd18c5..7561f8d38 100644 --- a/nvx-rs/src/guest/time/api.rs +++ b/nvx-rs/src/guest/time/api.rs @@ -217,8 +217,7 @@ impl TimeAbi { let mut runtime = self.runtime(); runtime.deadline = deadline; let result = capture(&mut runtime, request); - let unbounded = runtime.deadline.is_none(); - settled(&runtime, result).map_err(|error| (error, unbounded)) + finished_capture(&runtime, result, deadline.is_some()) } /// Finishes a restore that [`capture`](Self::capture) left unfinished, after the caller @@ -269,12 +268,7 @@ impl TimeAbi { ) -> Result<(), (PreCaptureError, bool)> { let mut runtime = self.runtime(); runtime.deadline = Some(deadline); - let result = pre_capture(&runtime); - let suppressed = result - .as_ref() - .map_or_else(PreCaptureError::holds_suppression, |()| true); - released_preparation(result, runtime.check_released(), runtime.take_violation()) - .map_err(|error| (error, suppressed)) + finished_preparation(&runtime, pre_capture(&runtime)) } /// Restores the stall detectors' settings that [`pre_capture`](Self::pre_capture) saved, and @@ -325,6 +319,34 @@ fn settled(runtime: &Runtime<'_>, result: Result) -> Result, + result: Result, + bounded: bool, +) -> Result { + let unbounded = runtime.deadline.is_none(); + if bounded { + runtime + .check_released() + .map_err(|error| (TimeError::from(error), unbounded))?; + } + settled(runtime, result).map_err(|error| (error, unbounded)) +} + +/// Ends bounded preparation with its original revocation cause and suppression ownership. +fn finished_preparation( + runtime: &Runtime<'_>, + result: Result<(), PreCaptureError>, +) -> Result<(), (PreCaptureError, bool)> { + let suppressed = result + .as_ref() + .map_or_else(PreCaptureError::holds_suppression, |()| true); + released_preparation(result, runtime.check_released(), runtime.take_violation()) + .map_err(|error| (error, suppressed)) +} + /// Ends a preparation of a capture with `result`, unless the calling thread may still hold port /// permission, as `released` tells, which the preparation reports instead, with whether it holds /// the stall detectors' suppression. diff --git a/nvx-rs/src/guest/time/api/tests.rs b/nvx-rs/src/guest/time/api/tests.rs index e5ccc63ef..cbe4c7520 100644 --- a/nvx-rs/src/guest/time/api/tests.rs +++ b/nvx-rs/src/guest/time/api/tests.rs @@ -366,13 +366,13 @@ fn strict_preparation_keeps_its_first_diagnostic_revocation_after_a_successful_r } assert_eq!(ops.transactions(), 3); assert_eq!(sys::check_released(), Ok(())); - let violation = runtime.take_violation().unwrap(); + let violation = runtime.peek_violation().unwrap(); assert_eq!( (violation.status(), violation.code(), violation.detail()), (193, "G_CONFORMANCE_C12", "first") ); - let error = - released_preparation(Ok(()), runtime.check_released(), Some(violation)).unwrap_err(); + let (error, suppressed) = finished_preparation(&runtime, Ok(())).unwrap_err(); + assert!(suppressed); assert!( matches!( &error, @@ -411,8 +411,8 @@ fn strict_preparation_keeps_a_partial_grants_failed_rollback_after_a_successful_ b"later release", ); assert_eq!(sys::check_released(), Ok(())); - let error = released_preparation(Ok(()), runtime.check_released(), runtime.take_violation()) - .unwrap_err(); + let (error, suppressed) = finished_preparation(&runtime, Ok(())).unwrap_err(); + assert!(suppressed); assert!( matches!( &error, @@ -445,8 +445,8 @@ fn strict_preparation_keeps_a_samples_failed_revocation_after_successful_diagnos b"sample release", ); assert_eq!(sys::check_released(), Ok(())); - let error = released_preparation(Ok(()), runtime.check_released(), runtime.take_violation()) - .unwrap_err(); + let (error, suppressed) = finished_preparation(&runtime, Ok(())).unwrap_err(); + assert!(suppressed); assert!( matches!( &error, @@ -460,6 +460,38 @@ fn strict_preparation_keeps_a_samples_failed_revocation_after_successful_diagnos assert_eq!(ops.power.load(Ordering::Relaxed), 0); } +#[test] +fn a_bounded_capture_keeps_a_dropped_revocation_before_repair_diagnostics() { + for bounded in [true, false] { + let ops = FakeOps::new([]); + ops.revoke_errors + .lock() + .unwrap() + .extend([Some(Errno::ENOMEM), None]); + let runtime = Runtime::new(&ops, false).strict(); + let ports = runtime.open_ports().unwrap(); + drop(ports); + assert_eq!(sys::check_released(), Err(PortError::Revoke(Errno::ENOMEM))); + let failure = repair_failed(&runtime, ViolationCode::RepairPacket, "invalid packet"); + assert_eq!(sys::check_released(), Ok(())); + let (error, unbounded) = finished_capture(&runtime, Err(failure), bounded).unwrap_err(); + assert!(unbounded); + if bounded { + assert!( + matches!(&error, TimeError::PortRelease(source) if has_errno(source, Errno::ENOMEM)), + "{error:?}" + ); + } else { + assert!( + matches!(&error, TimeError::Violation(violation) + if violation.status() == status::TIME_REPAIR && violation.code() == "G_REPAIR_PACKET"), + "{error:?}" + ); + } + assert_eq!(ops.power.load(Ordering::Relaxed), 0); + } +} + #[test] fn strict_preparation_keeps_the_boot_failure_stage_and_suppression_ownership() { let violation = Violation::of_missing_daemon(); diff --git a/nvx-rs/src/guest/time/system.rs b/nvx-rs/src/guest/time/system.rs index 5edda0692..c1a770ecb 100644 --- a/nvx-rs/src/guest/time/system.rs +++ b/nvx-rs/src/guest/time/system.rs @@ -283,7 +283,7 @@ pub(crate) struct Runtime<'a> { strict: bool, /// The first violation that the call reported in strict mode, which fails the call. violation: Mutex>, - /// The first diagnostic revocation errno, or `u32::MAX` when none occurred. + /// The first port-revocation errno, or `u32::MAX` when none occurred. release_failure: AtomicU32, /// Current VM generation. generation: AtomicU32, @@ -388,12 +388,15 @@ impl<'a> Runtime<'a> { /// Takes the calling thread's permission for the time ABI's ports, for one transaction, /// which [`PortAccess::release`] ends. pub(crate) fn open_ports(&self) -> Result, PortError> { + if let Err(error) = sys::check_released() { + self.retain_release_failure(error); + } self.ops .open_ports() .inspect_err(|&error| self.retain_release_failure(error)) } - /// Reports the first diagnostic revocation failure, otherwise the thread's current + /// Reports the first port-revocation failure, otherwise the thread's current /// permission state. A later successful release cannot erase the call's original failure. pub(crate) fn check_released(&self) -> Result<(), PortError> { match self.release_failure.load(Ordering::Relaxed) { @@ -980,10 +983,7 @@ pub(crate) fn best_sample( let rates = clock_rates(runtime)?; let ports = runtime.open_ports().map_err(SampleError::Ports)?; let selected = select_sample(&ports, runtime.ops, attempts, bound_ns, generation, rates); - ports - .release() - .inspect_err(|&error| runtime.retain_release_failure(error)) - .map_err(SampleError::Ports)?; + ports.release().map_err(SampleError::Ports)?; selected } diff --git a/nvx-rs/src/guest/time/system/tests.rs b/nvx-rs/src/guest/time/system/tests.rs index 4d7b73fb2..4ca81339a 100644 --- a/nvx-rs/src/guest/time/system/tests.rs +++ b/nvx-rs/src/guest/time/system/tests.rs @@ -15,6 +15,13 @@ use crate::guest::time::NSEC_PER_MSEC; use crate::guest::time::events::CheckId; use crate::guest::time::state::HostClockStatus; +impl Runtime<'_> { + /// Reads the strict violation without consuming the production finalizer's input. + pub(crate) fn peek_violation(&self) -> Option { + self.violation.lock().unwrap().clone() + } +} + impl Suppression<'_> { /// Runs the shared save without a caller deadline for the legacy cases below. fn save(&self) -> Result<(), SaveError> { From 3a7bd293fbc3f997c1999e5c29000e2c13eeacae Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Mon, 9 Nov 2026 01:10:01 -0800 Subject: [PATCH 15/15] guest: address bounded capture review feedback Stream the mount table's fixed and optional fields without collecting them while retaining its byte-level grammar and owned values. Describe time deadline expiry without restricting it to capture preparation. Put artifact publication retries behind one deadline condition without retrying downloads, preserving the initial rename and original failure. Use signature-checked rename fixtures and cover an overslept retry delay. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c65af75-7c5d-43dc-ab6d-1e9356ed0982 --- nvx-rs/src/guest/snapshot.rs | 47 +++++++++++++++--------- nvx-rs/src/guest/time/api.rs | 4 +- scripts/nvx_tools/performance_history.py | 17 ++++++--- scripts/test_performance.py | 39 +++++++++++++++++++- 4 files changed, 79 insertions(+), 28 deletions(-) diff --git a/nvx-rs/src/guest/snapshot.rs b/nvx-rs/src/guest/snapshot.rs index 4f819f27e..3d8f4c2dc 100644 --- a/nvx-rs/src/guest/snapshot.rs +++ b/nvx-rs/src/guest/snapshot.rs @@ -312,32 +312,43 @@ impl Line { /// Parses one line of the mount table, without its newline, or returns `None` for a line of /// another form than the kernel's. fn parse(line: &[u8]) -> Option { - let fields = line.split(|&byte| byte == b' ').collect::>(); - if fields.len() < FIXED_FIELDS { - return None; - } - parse_decimal(fields[0])?; - parse_decimal(fields[1])?; - let device = DeviceNumber::parse(fields[2])?; - let separator = FIXED_FIELDS - + fields[FIXED_FIELDS..] - .iter() - .position(|&field| field == SEPARATOR)?; - let [filesystem, source, ..] = &fields[separator + 1..] else { + let mut fields = line.split(|&byte| byte == b' '); + let fixed: [Option<&[u8]>; FIXED_FIELDS] = std::array::from_fn(|_| fields.next()); + let [ + Some(id), + Some(parent), + Some(device), + Some(root), + Some(mount_point), + Some(options), + ] = fixed + else { return None; }; + parse_decimal(id)?; + parse_decimal(parent)?; + let device = DeviceNumber::parse(device)?; // The kernel writes each field with a byte at least, but the source, which is empty when // the mounter named an empty one. - if fields[..separator] - .iter() - .chain([filesystem]) - .any(|field| field.is_empty()) - { + if root.is_empty() || mount_point.is_empty() || options.is_empty() { + return None; + } + for field in fields.by_ref() { + if field == SEPARATOR { + break; + } + if field.is_empty() { + return None; + } + } + let filesystem = fields.next()?; + let source = fields.next()?; + if filesystem.is_empty() { return None; } Some(Self { device, - mount_point: PathBuf::from(OsString::from_vec(unescape(fields[4]))), + mount_point: PathBuf::from(OsString::from_vec(unescape(mount_point))), filesystem: unescape(filesystem), source: unescape(source), }) diff --git a/nvx-rs/src/guest/time/api.rs b/nvx-rs/src/guest/time/api.rs index 7561f8d38..e0a1850a2 100644 --- a/nvx-rs/src/guest/time/api.rs +++ b/nvx-rs/src/guest/time/api.rs @@ -1395,7 +1395,7 @@ impl Display for Violation { #[derive(Debug)] #[non_exhaustive] pub enum TimeError { - /// The caller's absolute pre-request deadline expired. + /// The caller's absolute deadline expired. Deadline, /// The calling thread could not take the permission for OpenVMM's ports, which needs /// `CAP_SYS_RAWIO`. @@ -1439,7 +1439,7 @@ impl From for TimeError { impl Display for TimeError { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::Deadline => formatter.write_str("the capture preparation deadline expired"), + Self::Deadline => formatter.write_str("the caller deadline expired"), Self::PortAccess(error) => write!(formatter, "ioperm: {}", sys::describe(error)), Self::PortRelease(error) => { write!(formatter, "ioperm revoke: {}", sys::describe(error)) diff --git a/scripts/nvx_tools/performance_history.py b/scripts/nvx_tools/performance_history.py index 1cc6ea1e6..1aff1efca 100644 --- a/scripts/nvx_tools/performance_history.py +++ b/scripts/nvx_tools/performance_history.py @@ -430,23 +430,28 @@ def _fetch( _extract(archive, results) # Windows can briefly deny a directory rename while a child file is open. rename_deadline = min(deadline, time.monotonic() + _RENAME_RETRY_SECONDS) - while True: + first_attempt = True + rename_error: PermissionError | None = None + while first_attempt or time.monotonic() < rename_deadline: + first_attempt = False try: results.rename(directory) except PermissionError as error: - remaining = rename_deadline - time.monotonic() if ( getattr(error, "winerror", None) not in (5, 32) - or remaining <= 0 or not results.is_dir() or directory.exists() ): raise - time.sleep(min(_RENAME_RETRY_INTERVAL_SECONDS, remaining)) - if time.monotonic() >= rename_deadline: - raise + rename_error = error + remaining = rename_deadline - time.monotonic() + if remaining > 0: + time.sleep(min(_RENAME_RETRY_INTERVAL_SECONDS, remaining)) else: break + else: + assert rename_error is not None + raise rename_error return directory diff --git a/scripts/test_performance.py b/scripts/test_performance.py index 3b11d5728..7ed336f8f 100644 --- a/scripts/test_performance.py +++ b/scripts/test_performance.py @@ -5071,7 +5071,7 @@ def test_persistent_windows_rename_denial_stops_before_the_next_run(self): self.record(2, c) error = _WindowsRenameDenial(5) - with patch.object(Path, "rename", side_effect=error) as rename: + with patch.object(Path, "rename", autospec=True, side_effect=error) as rename: outcome = self.persist() self.assertTrue(outcome.failed) @@ -5087,7 +5087,7 @@ def test_windows_rename_denial_does_not_extend_the_fetch_deadline(self): error = _WindowsRenameDenial(5) with ( - patch.object(Path, "rename", side_effect=error) as rename, + patch.object(Path, "rename", autospec=True, side_effect=error) as rename, self.assertRaises(PermissionError) as refused, ): performance_history._fetch( @@ -5103,6 +5103,41 @@ def test_windows_rename_denial_does_not_extend_the_fetch_deadline(self): rename.assert_called_once() self.assertEqual(self.commits(), ["seed"]) + def test_windows_rename_retry_stops_when_sleep_overruns_the_deadline(self): + b = self.commit("b") + self.record(1, b) + error = _WindowsRenameDenial(5) + clock = 0.0 + + def oversleep(_: float) -> None: + nonlocal clock + clock = 2.0 + + with ( + patch.object(Path, "rename", autospec=True, side_effect=error) as rename, + patch.object( + performance_history.time, "monotonic", side_effect=lambda: clock + ), + patch.object( + performance_history.time, "sleep", side_effect=oversleep + ) as sleep, + self.assertRaises(PermissionError) as refused, + ): + performance_history._fetch( + self.api, + REPOSITORY, + performance_history.RecordedRun(1, b, ""), + self.api.artifacts[1][0], + self.downloads, + 10.0, + ) + + self.assertIs(refused.exception, error) + rename.assert_called_once() + sleep.assert_called_once() + self.assertEqual(self.api.downloads, [1001]) + self.assertEqual(self.commits(), ["seed"]) + def test_windows_rename_denial_does_not_replace_an_existing_destination(self): b = self.commit("b") self.record(1, b)