From ac572dd46f4cb5d3af14b66cddfd9b4c0a70ae4a Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Thu, 12 Nov 2026 21:24:27 -0800 Subject: [PATCH 1/4] guest: drop guest::clone, the alias of guest::activation SL7.3 renamed guest::clone to guest::activation and kept the old name as an alias until SL5.2 deleted the test adapter, its last user (#1105). SL5.2 (#1499) found that nothing used the alias any more, and the slices after it left it in place, since their scopes didn't name it, so the breaking release would have published it as API. The alias goes, with its 144 lines of the Linux public-API snapshot; the Windows snapshot has no guest module. The crate's README no longer calls it a compatibility alias, the migration guide maps guest::clone to guest::activation without keeping the old name, the changelog lists the rename among the breaking changes, and protocol::status names the repair's error at guest::snapshot, where the repair runs. Part of #1076. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 1 + doc/migrate-from-alpha.7.md | 2 +- nvx-rs/README.md | 4 +- nvx-rs/public-api/linux.txt | 144 ---------------------------------- nvx-rs/src/guest/mod.rs | 3 - nvx-rs/src/protocol/status.rs | 2 +- 6 files changed, 5 insertions(+), 151 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ce7da428..c438c3e43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ changed item to its replacement, or to what its caller now does. - **No exec.** The exec engine, `nvx::exec`, and the guest's process modules, `nvx::guest::{exec, spawn, process, cgroup, entry}`, are gone. A guest's init runs its executions itself. +- **`nvx::guest::clone` is `nvx::guest::activation`.** The old name is gone. - **The guest's space is private.** nvx-init's space, `nvx::guest::{root, gpt}` and the mapper of `nvx::guest::mappings`, is private to `nvx`. `resolve_guest_path` is gone, and `ExportPlan` holds paths only. diff --git a/doc/migrate-from-alpha.7.md b/doc/migrate-from-alpha.7.md index aec4ea8e9..81daa0a2a 100644 --- a/doc/migrate-from-alpha.7.md +++ b/doc/migrate-from-alpha.7.md @@ -252,7 +252,7 @@ keeps its path and its signature. | alpha.7 | The release | | --- | --- | -| `guest::clone` | `guest::activation`, which `guest::clone` still names. Its `CaptureCause` and `CaptureStage` have a new `Repair` variant | +| `guest::clone` | `guest::activation`: the old name is gone. Its `CaptureCause` and `CaptureStage` have a new `Repair` variant | | `guest::clone`'s `CapturePreparation`, `CaptureBarrier`, `AdmissionFence`, `CaptureError`, `Captured`, `Devices`, `Repair`, `Restored`, and `repair` | `guest::snapshot::capture(&devices, disks, deadline)`, one call that checks the devices, captures, and, in a VM that a restore started, repairs the restore before it returns `Captured::Restored`. A `CaptureFailure` says whether the guest resumed or must end. The init stops its own executions first, as [Coordinate a capture](guest-agent-guidelines.md#coordinate-a-capture) describes | | `guest::clone::Activation::new`, `guest::clone::Activation::with_resources` | Take no admission fence: `guest::activation::Activation::new(&repaired, timeout, now)` | | `guest::clone`'s `apply`, `Applied`, `ApplyError`, `AppliedTeardownError`, `NetworkError`, and `RootTemplate` | No longer public: the init applies a restore's activation record in its own space, as [Complete a restore before Ready](guest-agent-guidelines.md#complete-a-restore-before-ready) describes | diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 97516172a..a325c6a56 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -882,8 +882,8 @@ the activation record. x86_64: the activation of each clone that a restore of a snapshot starts, once `nvx::guest::snapshot::capture` repaired it, for a guest init or agent, whether -or not it is PID 1. `nvx::guest::clone` remains a compatibility alias. -Activation itself holds no admission fence and starts no process. +or not it is PID 1. Activation itself holds no admission fence and starts no +process. In a clone, the capture repairs the restore before it returns: it checks that the online CPUs are a prefix that the restore's processor target does not diff --git a/nvx-rs/public-api/linux.txt b/nvx-rs/public-api/linux.txt index 45ba2cbfb..ff293bc44 100644 --- a/nvx-rs/public-api/linux.txt +++ b/nvx-rs/public-api/linux.txt @@ -420,150 +420,6 @@ method nvx::guest::activation::Repaired::acknowledged_at method nvx::guest::activation::Repaired::cpus method nvx::guest::activation::Repaired::generation_id method nvx::guest::activation::Repaired::memory -mod nvx::guest::clone -struct nvx::guest::clone::Activation -method nvx::guest::clone::Activation::complete -method nvx::guest::clone::Activation::deadline -method nvx::guest::clone::Activation::established -method nvx::guest::clone::Activation::input -method nvx::guest::clone::Activation::new -method nvx::guest::clone::Activation::start -method nvx::guest::clone::Activation::state -method nvx::guest::clone::Activation::tick -method nvx::guest::clone::Activation::with_resources -enum nvx::guest::clone::ActivationAction -variant nvx::guest::clone::ActivationAction::Acknowledge -variant nvx::guest::clone::ActivationAction::Apply -field nvx::guest::clone::ActivationAction::Apply::0 -variant nvx::guest::clone::ActivationAction::Continue -variant nvx::guest::clone::ActivationAction::Join -variant nvx::guest::clone::ActivationAction::Ready -field nvx::guest::clone::ActivationAction::Ready::0 -variant nvx::guest::clone::ActivationAction::Serve -enum nvx::guest::clone::ActivationCause -variant nvx::guest::clone::ActivationCause::Changed -variant nvx::guest::clone::ActivationCause::Cold -variant nvx::guest::clone::ActivationCause::Completion -variant nvx::guest::clone::ActivationCause::Expired -variant nvx::guest::clone::ActivationCause::Failed -field nvx::guest::clone::ActivationCause::Failed::0 -variant nvx::guest::clone::ActivationCause::Record -field nvx::guest::clone::ActivationCause::Record::0 -variant nvx::guest::clone::ActivationCause::Request -variant nvx::guest::clone::ActivationCause::Reset -variant nvx::guest::clone::ActivationCause::Resources -field nvx::guest::clone::ActivationCause::Resources::actual -field nvx::guest::clone::ActivationCause::Resources::expected -variant nvx::guest::clone::ActivationCause::Start -variant nvx::guest::clone::ActivationCause::Transaction -field nvx::guest::clone::ActivationCause::Transaction::accepted -field nvx::guest::clone::ActivationCause::Transaction::offered -struct nvx::guest::clone::ActivationFailure -method nvx::guest::clone::ActivationFailure::cause -method nvx::guest::clone::ActivationFailure::state -enum nvx::guest::clone::ActivationInput -variant nvx::guest::clone::ActivationInput::Activate -field nvx::guest::clone::ActivationInput::Activate::0 -variant nvx::guest::clone::ActivationInput::Flow -variant nvx::guest::clone::ActivationInput::Request -variant nvx::guest::clone::ActivationInput::Reset -variant nvx::guest::clone::ActivationInput::TooLong -field nvx::guest::clone::ActivationInput::TooLong::0 -enum nvx::guest::clone::ActivationState -variant nvx::guest::clone::ActivationState::Activating -variant nvx::guest::clone::ActivationState::Bootstrap -variant nvx::guest::clone::ActivationState::Failed -variant nvx::guest::clone::ActivationState::Ready -variant nvx::guest::clone::ActivationState::Restricted -variant nvx::guest::clone::ActivationState::Started -enum nvx::guest::clone::CaptureCause -variant nvx::guest::clone::CaptureCause::Io -field nvx::guest::clone::CaptureCause::Io::0 -variant nvx::guest::clone::CaptureCause::Mounted -field nvx::guest::clone::CaptureCause::Mounted::0 -variant nvx::guest::clone::CaptureCause::Pending -variant nvx::guest::clone::CaptureCause::Repair -field nvx::guest::clone::CaptureCause::Repair::0 -variant nvx::guest::clone::CaptureCause::Time -field nvx::guest::clone::CaptureCause::Time::0 -enum nvx::guest::clone::CaptureStage -variant nvx::guest::clone::CaptureStage::BootChecks -variant nvx::guest::clone::CaptureStage::Capture -variant nvx::guest::clone::CaptureStage::CaptureChecks -variant nvx::guest::clone::CaptureStage::Fence -variant nvx::guest::clone::CaptureStage::GenerationId -variant nvx::guest::clone::CaptureStage::Mounts -variant nvx::guest::clone::CaptureStage::Release -variant nvx::guest::clone::CaptureStage::Repair -variant nvx::guest::clone::CaptureStage::Suppression -const nvx::guest::clone::MACHINE_ID_FILE -const nvx::guest::clone::MOUNT_TABLE_LIMIT -struct nvx::guest::clone::Mounted -method nvx::guest::clone::Mounted::mount_point -method nvx::guest::clone::Mounted::origin -enum nvx::guest::clone::Origin -variant nvx::guest::clone::Origin::Distro -variant nvx::guest::clone::Origin::Filesystem -variant nvx::guest::clone::Origin::Overlay -variant nvx::guest::clone::Origin::Scratch -const nvx::guest::clone::RESOLVER_FILE -enum nvx::guest::clone::RangeFault -variant nvx::guest::clone::RangeFault::Aperture -variant nvx::guest::clone::RangeFault::Extra -variant nvx::guest::clone::RangeFault::Gap -variant nvx::guest::clone::RangeFault::Misaligned -variant nvx::guest::clone::RangeFault::Overlap -enum nvx::guest::clone::RepairCause -variant nvx::guest::clone::RepairCause::Activation -field nvx::guest::clone::RepairCause::Activation::online -field nvx::guest::clone::RepairCause::Activation::target -variant nvx::guest::clone::RepairCause::BlockSize -field nvx::guest::clone::RepairCause::BlockSize::0 -variant nvx::guest::clone::RepairCause::CapturedRam -variant nvx::guest::clone::RepairCause::Finished -variant nvx::guest::clone::RepairCause::Incomplete -variant nvx::guest::clone::RepairCause::Io -field nvx::guest::clone::RepairCause::Io::0 -variant nvx::guest::clone::RepairCause::Missing -field nvx::guest::clone::RepairCause::Missing::0 -variant nvx::guest::clone::RepairCause::NoSeed -variant nvx::guest::clone::RepairCause::Offline -field nvx::guest::clone::RepairCause::Offline::block -field nvx::guest::clone::RepairCause::Offline::state -variant nvx::guest::clone::RepairCause::OnlineCpus -field nvx::guest::clone::RepairCause::OnlineCpus::0 -variant nvx::guest::clone::RepairCause::ProcessorTarget -field nvx::guest::clone::RepairCause::ProcessorTarget::online -field nvx::guest::clone::RepairCause::ProcessorTarget::target -variant nvx::guest::clone::RepairCause::Range -field nvx::guest::clone::RepairCause::Range::fault -field nvx::guest::clone::RepairCause::Range::index -variant nvx::guest::clone::RepairCause::Reseed -field nvx::guest::clone::RepairCause::Reseed::0 -variant nvx::guest::clone::RepairCause::Time -field nvx::guest::clone::RepairCause::Time::0 -variant nvx::guest::clone::RepairCause::UntargetedRanges -struct nvx::guest::clone::RepairError -method nvx::guest::clone::RepairError::cause -method nvx::guest::clone::RepairError::progress -method nvx::guest::clone::RepairError::stage -method nvx::guest::clone::RepairError::status -struct nvx::guest::clone::RepairProgress -method nvx::guest::clone::RepairProgress::added -method nvx::guest::clone::RepairProgress::finish_started -method nvx::guest::clone::RepairProgress::onlined -method nvx::guest::clone::RepairProgress::reseeded -enum nvx::guest::clone::RepairStage -variant nvx::guest::clone::RepairStage::Finish -variant nvx::guest::clone::RepairStage::Memory -variant nvx::guest::clone::RepairStage::Processors -variant nvx::guest::clone::RepairStage::Reseed -variant nvx::guest::clone::RepairStage::Restore -struct nvx::guest::clone::Repaired -method nvx::guest::clone::Repaired::acknowledged_at -method nvx::guest::clone::Repaired::cpus -method nvx::guest::clone::Repaired::generation_id -method nvx::guest::clone::Repaired::memory mod nvx::guest::console_pending (hidden) fn nvx::guest::console_pending::main (hidden) mod nvx::guest::device_io (hidden) diff --git a/nvx-rs/src/guest/mod.rs b/nvx-rs/src/guest/mod.rs index ed45d3c64..ec29932ee 100644 --- a/nvx-rs/src/guest/mod.rs +++ b/nvx-rs/src/guest/mod.rs @@ -65,9 +65,6 @@ pub use generation::GenerationId; #[cfg(target_arch = "x86_64")] pub mod activation; -/// Compatibility name for the guest's activation. -#[cfg(target_arch = "x86_64")] -pub use activation as clone; #[doc(hidden)] pub mod console_pending; #[cfg(target_arch = "x86_64")] diff --git a/nvx-rs/src/protocol/status.rs b/nvx-rs/src/protocol/status.rs index 45c54a57d..325cdd928 100644 --- a/nvx-rs/src/protocol/status.rs +++ b/nvx-rs/src/protocol/status.rs @@ -1,7 +1,7 @@ //! The exit statuses with which a clone's guest ends its VM once it fails fast, and from which //! the host classifies a clone's end. The guest library's errors select them: the time ABI's //! violations, `nvx::guest::time::Violation::status`, and a clone's repair errors, -//! `nvx::guest::clone::RepairError::status`. +//! `nvx::guest::snapshot::RepairError::status`. //! //! - [`TIME_CONFORMANCE`], 193, [`TIME_VIOLATION`], 194, and [`TIME_REPAIR`], 195: the time ABI's //! own statuses, which keep their meanings. In enforcing mode, the time ABI stops the VM with From 782453f68fc92dc42b30d357ae1e4cd7f19a6c6c Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Thu, 12 Nov 2026 21:24:53 -0800 Subject: [PATCH 2/4] guest: delete what only the exec core's tests used nvx-init's plain-process core (SL7.5c, #1115) carried expectations of dead code, outside tests, until SL7.6a (#1116) used it. SL7.6a merged, but the expectations stayed, and hid what the core's only user, the managed agent, never uses: - AccountMode::Existing, the check that an execution's IDs name an account of the root's passwd and group databases: the agent always starts its workloads by their numeric IDs, so AccountMode and ExecRequest::account go with it. - StartFailure::into_parts: the agent reads the failure's fields. - Outcome's cause and discarded: the agent decides an execution's outcome from its own cancellation, timeout and session flags, and the terminal status, so Event::Ended carries the status alone. - The discard path: reset() closes both outputs as it marks them discarded, so no read ever discarded, and only Outcome counted the bytes. The marks, the counts and Event::Discarded go. - The streams of Closed and ReadFailed, and ReadFailed's error, which the agent maps to events of its own without them. - Exec::output_hung_up and sys::hung_up, which only a test called. Nothing that runs changes. The test-layout allowlist records the two test hooks that init.rs keeps. Tests: the exec tests read the execution's cause where they read its outcome's. only_the_collectors_terminal_status_settles_the_owned_leader_and_output counts the closed outputs, and no longer asks whether a closed output hung up. ending_failures_are_reported_and_discarding_preserves_the_first_cause becomes ending_failures_are_reported_and_the_first_cause_is_kept: it checks the same kill failures, cause and status, with both outputs delivered. invalid_requests_fail_before_a_child_starts drops the missing-account case, and privileged_exec_checks_account_mode_and_enters_cwd_after_dropping_credentials becomes privileged_exec_needs_no_account_and_enters_cwd_after_dropping_credentials, without the account databases' fixtures. The launch tests no longer check the account mode, and the restore tests import io::Read. Part of #1076. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nvx-rs/src/guest/init.rs | 144 +----------- nvx-rs/src/guest/init/tests/exec.rs | 213 +++++------------- nvx-rs/src/guest/init/tests/restore.rs | 2 +- nvx-rs/src/guest/managed_agent/exec.rs | 4 +- nvx-rs/src/guest/managed_agent/launch.rs | 3 +- .../src/guest/managed_agent/launch/tests.rs | 10 +- nvx-rs/src/guest/sys/mod.rs | 2 +- nvx-rs/src/guest/sys/poll.rs | 10 - scripts/test-layout-allowlist.json | 2 +- 9 files changed, 83 insertions(+), 307 deletions(-) diff --git a/nvx-rs/src/guest/init.rs b/nvx-rs/src/guest/init.rs index 37bc326d4..e8176e77a 100644 --- a/nvx-rs/src/guest/init.rs +++ b/nvx-rs/src/guest/init.rs @@ -41,7 +41,7 @@ //! Outside PID 1, `nvx-init` changes nothing: it fails with status 2 and a message. use std::ffi::{CStr, CString, OsStr}; -use std::io::{self, Read}; +use std::io; use std::net::Ipv4Addr; use std::os::fd::BorrowedFd; use std::os::unix::ffi::OsStrExt; @@ -336,17 +336,6 @@ pub(crate) fn report_activation(failure: impl std::fmt::Display) { ); } -/// Whether execution requires an account in the shared root's databases. -#[cfg_attr( - not(test), - expect(dead_code, reason = "SL7.6a selects an execution account mode") -)] -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(crate) enum AccountMode { - Existing, - Numeric, -} - /// One output pipe of a plain execution. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum Stream { @@ -399,24 +388,12 @@ pub(crate) struct StartFailure { pub(crate) cleanup: Option, } -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "SL7.6a settles failed starts before returning idle" - ) -)] impl StartFailure { /// Checks without collecting. The owner hands over queued terminal statuses first; `false` /// without one means the child's status was lost, including automatic kernel collection. pub(crate) fn is_pending(&self) -> Result { self.process.map_or(Ok(false), ProcessId::is_unreaped) } - - /// Transfers the first error, owned child, and cleanup diagnostic to their control owner. - pub(crate) fn into_parts(self) -> (io::Error, Option, Option) { - (self.error, self.process, self.cleanup) - } } impl From for StartFailure { @@ -446,32 +423,16 @@ impl std::fmt::Display for LaunchFailure { } } -/// The terminal status and any requested ending, after launch and output have settled. -#[cfg_attr( - not(test), - expect(dead_code, reason = "SL7.6a consumes execution outcomes") -)] -#[derive(Clone, Copy, Debug)] -pub(crate) struct Outcome { - pub(crate) status: ProcessStatus, - pub(crate) cause: Option, - pub(crate) discarded: [u64; 2], -} - /// One nonblocking step of the init-owned execution. -#[cfg_attr( - not(test), - expect(dead_code, reason = "SL7.6a consumes execution events") -)] #[derive(Debug)] pub(crate) enum Event<'a> { Launch(io::Result>), Output(Stream, &'a [u8]), - Discarded(Stream, usize), - Closed(Stream), - ReadFailed(Stream, io::Error), + Closed, + ReadFailed, KillFailed(KillTarget, io::Error), - Ended(Outcome), + /// The terminal status, after launch and output have settled. + Ended(ProcessStatus), } /// The inputs of one plain-process execution in the VM's shared root. @@ -488,8 +449,6 @@ pub(crate) struct ExecRequest { pub(crate) uid: u32, /// Real, effective, and saved group ID. pub(crate) gid: u32, - /// Whether the image must contain this account. - pub(crate) account: AccountMode, /// How long the leader may run. pub(crate) timeout: Option, /// A pipe for streaming input, or the null device. @@ -509,63 +468,7 @@ impl ExecRequest { { return Err(Errno::EINVAL.into()); } - match self.account { - AccountMode::Numeric => Ok(()), - AccountMode::Existing => { - let root = sys::open( - c"/", - OpenFlags::O_PATH | OpenFlags::O_CLOEXEC, - Mode::empty(), - )?; - let read = |path| -> io::Result> { - let file = sys::open_beneath( - &root, - path, - OpenFlags::O_RDONLY | OpenFlags::O_NONBLOCK | OpenFlags::O_NOCTTY, - Mode::empty(), - true, - )?; - let (regular, length) = sys::file_size(&file)?; - if !regular || length > 1 << 20 { - return Err(Errno::EINVAL.into()); - } - let mut bytes = Vec::new(); - file.take((1 << 20) + 1).read_to_end(&mut bytes)?; - if bytes.len() > 1 << 20 { - return Err(Errno::EFBIG.into()); - } - Ok(bytes) - }; - let passwd = read(c"etc/passwd")?; - let group = read(c"etc/group")?; - let mut matching = passwd.split(|&byte| byte == b'\n').filter_map(|line| { - if matches!(line.trim_ascii_start().first(), None | Some(b'#')) { - return None; - } - let fields: Vec<_> = line.split(|&byte| byte == b':').collect(); - (parse_decimal(fields.get(2)?) == Some(self.uid.into())).then_some(fields) - }); - let fields = matching.next().ok_or(Errno::ENOENT)?; - if matching.next().is_some() - || fields.len() != 7 - || fields[0].is_empty() - || parse_decimal(fields[3]) != Some(self.gid.into()) - || fields[5].first() != Some(&b'/') - || !group.split(|&byte| byte == b'\n').any(|line| { - if matches!(line.trim_ascii_start().first(), None | Some(b'#')) { - return false; - } - line.split(|&byte| byte == b':') - .nth(2) - .and_then(parse_decimal) - == Some(self.gid.into()) - }) - { - return Err(Errno::EINVAL.into()); - } - Ok(()) - } - } + Ok(()) } } @@ -606,8 +509,6 @@ pub(crate) struct Exec { launch: Option, input: Option, outputs: [Option; 2], - discarding: [bool; 2], - discarded: [u64; 2], turn: usize, status: Option, status_lost: bool, @@ -617,10 +518,6 @@ pub(crate) struct Exec { ended: bool, } -#[cfg_attr( - not(test), - expect(dead_code, reason = "SL7.6a drives nvx-init's exec core") -)] impl Exec { /// Forks the program in its own process group, resetting its priority before taking its /// identity, with empty groups and capabilities and `no_new_privs`, before entering its cwd. @@ -705,8 +602,6 @@ impl Exec { }), input, outputs, - discarding: [false; 2], - discarded: [0; 2], turn: 0, status: None, status_lost: false, @@ -777,18 +672,14 @@ impl Exec { match reader.read(buffer) { Ok(0) => { self.outputs[index] = None; - return Ok(Some(Event::Closed(stream))); - } - Ok(length) if self.discarding[index] => { - self.discarded[index] = self.discarded[index].saturating_add(length as u64); - return Ok(Some(Event::Discarded(stream, length))); + return Ok(Some(Event::Closed)); } Ok(length) => return Ok(Some(Event::Output(stream, &buffer[..length]))), Err(Errno::EINTR) => self.turn = index, Err(Errno::EAGAIN) => {} - Err(error) => { + Err(_) => { self.outputs[index] = None; - return Ok(Some(Event::ReadFailed(stream, error.into()))); + return Ok(Some(Event::ReadFailed)); } } } @@ -797,11 +688,7 @@ impl Exec { && !self.has_open_output() { self.ended = true; - return Ok(Some(Event::Ended(Outcome { - status, - cause: self.cause, - discarded: self.discarded, - }))); + return Ok(Some(Event::Ended(status))); } Ok(None) } @@ -862,10 +749,9 @@ impl Exec { self.end_with(Cause::Cancelled); } - /// Ends the execution on session reset, killing its group and closing discarded output, + /// Ends the execution on session reset, killing its group and closing its output unread, /// including pipes that an escaped process keeps open. pub(crate) fn reset(&mut self) { - self.discarding = [true; 2]; self.outputs = [None, None]; self.end_with(Cause::Disconnected); } @@ -878,14 +764,6 @@ impl Exec { self.outputs.iter().any(Option::is_some) } - pub(crate) fn output_hung_up(&self, stream: Stream) -> io::Result { - self.outputs[stream as usize] - .as_ref() - .map_or(Ok(true), |reader| { - sys::hung_up(reader.borrowed()).map_err(Into::into) - }) - } - pub(crate) fn kill_failure(&mut self) -> Option> { self.failures .iter_mut() diff --git a/nvx-rs/src/guest/init/tests/exec.rs b/nvx-rs/src/guest/init/tests/exec.rs index c5bb40474..58d53f469 100644 --- a/nvx-rs/src/guest/init/tests/exec.rs +++ b/nvx-rs/src/guest/init/tests/exec.rs @@ -1,5 +1,6 @@ //! The plain-process core, including privileged checks run explicitly in a Linux guest. +use std::io::Read; use std::os::unix::ffi::OsStrExt; use std::os::unix::process::ExitStatusExt; use std::process::ExitStatus; @@ -34,7 +35,7 @@ impl Exec { } } -/// One execution's observed streams, launch, outcome, and collected child IDs. +/// One execution's observed streams, launch, terminal status, and collected child IDs. #[derive(Default)] struct Collected { /// Standard output. @@ -43,13 +44,13 @@ struct Collected { stderr: Vec, /// The launch result, once it arrives. launch: Option>, - /// The execution's final outcome. - outcome: Option, + /// The execution's terminal status. + status: Option, /// The children reaped by this PID-1-style collector. reaped: Vec, } -/// Describes a shell in numeric-account mode, with no inherited environment. +/// Describes a shell, with no inherited environment. pub(super) fn request(script: &str) -> ExecRequest { ExecRequest { program: c"/bin/sh".to_owned(), @@ -62,7 +63,6 @@ pub(super) fn request(script: &str) -> ExecRequest { directory: c"/".to_owned(), uid: 0, gid: 0, - account: AccountMode::Numeric, timeout: None, input: true, search_path: None, @@ -101,9 +101,9 @@ fn step(execution: &mut Exec, collected: &mut Collected) { Some(Event::Launch(report)) => collected.launch = Some(report.unwrap()), Some(Event::Output(Stream::Stdout, bytes)) => collected.stdout.extend_from_slice(bytes), Some(Event::Output(Stream::Stderr, bytes)) => collected.stderr.extend_from_slice(bytes), - Some(Event::Ended(outcome)) => collected.outcome = Some(outcome), + Some(Event::Ended(status)) => collected.status = Some(status), Some(Event::KillFailed(target, error)) => panic!("kill {target:?}: {error}"), - Some(Event::ReadFailed(stream, error)) => panic!("read {stream:?}: {error}"), + Some(Event::ReadFailed) => panic!("an output read failed"), Some(_) | None => {} } } @@ -121,11 +121,9 @@ fn until(execution: &mut Exec, collected: &mut Collected, ready: impl Fn(&Collec } } -/// Drives the execution to its final outcome. +/// Drives the execution to its terminal status. fn finish(execution: &mut Exec, collected: &mut Collected) { - until(execution, collected, |collected| { - collected.outcome.is_some() - }); + until(execution, collected, |collected| collected.status.is_some()); } #[test] @@ -227,8 +225,6 @@ fn pipe_execution(stdout: &[u8], stderr: &[u8]) -> Exec { launch: Some(report), input: None, outputs: [output(stdout), output(stderr)], - discarding: [false; 2], - discarded: [0; 2], turn: 0, status: None, status_lost: true, @@ -258,7 +254,7 @@ fn only_the_collectors_terminal_status_settles_the_owned_leader_and_output() { assert!(!execution.observe(43, ExitStatus::from_raw(7 << 8))); assert!(!execution.observe(42, ExitStatus::from_raw((19 << 8) | 0x7f))); let mut buffer = [0; 2]; - let mut closed = Vec::new(); + let mut closed = 0; let mut collected = [Vec::new(), Vec::new()]; let deadline = Instant::now() + Duration::from_secs(5); while execution.launch.is_some() || execution.has_open_output() { @@ -267,39 +263,36 @@ fn only_the_collectors_terminal_status_settles_the_owned_leader_and_output() { Some(Event::Output(stream, bytes)) => { collected[stream as usize].extend_from_slice(bytes) } - Some(Event::Closed(stream)) => closed.push(stream), + Some(Event::Closed) => closed += 1, Some(Event::Launch(report)) => assert_eq!(report.unwrap(), None), None => std::thread::sleep(Duration::from_millis(1)), event => panic!("unexpected {event:?}"), } } assert_eq!(collected, [b"out".to_vec(), b"err".to_vec()]); - assert_eq!(closed.len(), 2); + assert_eq!(closed, 2); assert!(!execution.has_open_output()); - assert!(execution.output_hung_up(Stream::Stdout).unwrap()); assert!(execution.status().is_none()); assert!(execution.observe(42, ExitStatus::from_raw(7 << 8))); assert!(!execution.observe(42, ExitStatus::from_raw(0))); assert!(matches!(execution.next_event(&mut buffer).unwrap(), Some(Event::KillFailed(KillTarget::ProcessGroup, error)) if error.raw_os_error() == Some(Errno::EBADF as i32))); - let Some(Event::Ended(outcome)) = execution.next_event(&mut buffer).unwrap() else { - panic!("no outcome"); + let Some(Event::Ended(status)) = execution.next_event(&mut buffer).unwrap() else { + panic!("no terminal status"); }; - assert_eq!(outcome.status.code(), Some(7)); - assert_eq!(outcome.cause, None); - assert_eq!(outcome.discarded, [0; 2]); + assert_eq!(status.code(), Some(7)); + assert_eq!(execution.cause, None); assert!(execution.next_event(&mut buffer).unwrap().is_none()); } #[test] -fn ending_failures_are_reported_and_discarding_preserves_the_first_cause() { - let name = "guest::init::tests::exec::ending_failures_are_reported_and_discarding_preserves_the_first_cause"; +fn ending_failures_are_reported_and_the_first_cause_is_kept() { + let name = "guest::init::tests::exec::ending_failures_are_reported_and_the_first_cause_is_kept"; if isolate_process_test("NVX_INIT_EXEC_ENDING_TEST", name) { return; } - let mut execution = pipe_execution(b"drop", b"keep"); + let mut execution = pipe_execution(b"out", b"err"); execution.status_lost = false; - execution.discarding[Stream::Stdout as usize] = true; execution.cancel(); execution.end(); assert!(matches!( @@ -312,25 +305,23 @@ fn ending_failures_are_reported_and_discarding_preserves_the_first_cause() { )); assert!(execution.observe(42, ExitStatus::from_raw(9 | 0x80))); let mut buffer = [0; 2]; - let mut dropped = 0; - let mut retained = Vec::new(); + let mut collected = [Vec::new(), Vec::new()]; loop { match execution.next_event(&mut buffer).unwrap() { - Some(Event::Discarded(Stream::Stdout, length)) => dropped += length, - Some(Event::Output(Stream::Stderr, bytes)) => retained.extend_from_slice(bytes), - Some(Event::Ended(outcome)) => { - assert_eq!(outcome.cause, Some(Cause::Cancelled)); - assert_eq!(outcome.status.signal(), Some(9)); - assert!(outcome.status.core_dumped()); - assert_eq!(outcome.discarded, [4, 0]); + Some(Event::Output(stream, bytes)) => { + collected[stream as usize].extend_from_slice(bytes) + } + Some(Event::Ended(status)) => { + assert_eq!(status.signal(), Some(9)); + assert!(status.core_dumped()); break; } - Some(Event::ReadFailed(_, error)) => panic!("{error}"), + Some(Event::ReadFailed) => panic!("an output read failed"), _ => {} } } - assert_eq!(dropped, 4); - assert_eq!(retained, b"keep"); + assert_eq!(execution.cause, Some(Cause::Cancelled)); + assert_eq!(collected, [b"out".to_vec(), b"err".to_vec()]); } #[test] @@ -349,10 +340,6 @@ fn invalid_requests_fail_before_a_child_starts() { let mut invalid = request("exit 0"); invalid.uid = u32::MAX; assert!(Exec::start(invalid).is_err()); - let mut missing = request("exit 0"); - missing.uid = 4_000_000_000; - missing.account = AccountMode::Existing; - assert!(Exec::start(missing).is_err()); } /// Forces real failures at both post-fork descriptor boundaries while init remains the reaper. @@ -536,7 +523,11 @@ fn a_failed_start_preserves_its_first_errno_and_the_cleanup_failure() { .err() .unwrap(); assert!(failure.is_pending().unwrap()); - let (error, owned, cleanup) = failure.into_parts(); + let StartFailure { + error, + process: owned, + cleanup, + } = failure; assert_eq!(owned, Some(process)); assert_eq!(error.raw_os_error(), Some(Errno::EBADF as i32)); assert_eq!(cleanup, Some(Errno::EPERM)); @@ -630,7 +621,7 @@ fn host_launch_tests_use_the_syscall_seam_without_mocking_the_execution_core() { let mut collected = Collected::default(); finish(&mut execution, &mut collected); assert_eq!(collected.stdout, b"native"); - assert_eq!(collected.outcome.unwrap().status.code(), Some(9)); + assert_eq!(collected.status.unwrap().code(), Some(9)); let mut execution = Exec::start_simulated(request("exit 99"), Some(Errno::EPERM)).unwrap(); let mut collected = Collected::default(); finish(&mut execution, &mut collected); @@ -641,7 +632,7 @@ fn host_launch_tests_use_the_syscall_seam_without_mocking_the_execution_core() { error: Errno::EPERM as i32 })) ); - assert_eq!(collected.outcome.unwrap().status.code(), Some(125)); + assert_eq!(collected.status.unwrap().code(), Some(125)); }); } @@ -699,7 +690,7 @@ fn procfs_workload_cwd_checks(simulated: bool) { None => { assert_eq!(collected.launch, Some(None)); assert_eq!(collected.stdout, expected.as_bytes()); - assert!(collected.outcome.unwrap().status.success()); + assert!(collected.status.unwrap().success()); } Some(error) => { assert_eq!( @@ -710,7 +701,7 @@ fn procfs_workload_cwd_checks(simulated: bool) { })) ); assert!(collected.stdout.is_empty()); - assert_eq!(collected.outcome.unwrap().status.code(), Some(125)); + assert_eq!(collected.status.unwrap().code(), Some(125)); } } } @@ -771,8 +762,8 @@ fn a_stolen_or_auto_reaped_status_is_reported_once_and_never_reclassified_by_an_ assert!(execution.observe(execution.id(), ExitStatus::from_raw(status))); let mut collected = Collected::default(); finish(&mut execution, &mut collected); - assert_eq!(collected.outcome.unwrap().status.code(), Some(7)); - assert_eq!(collected.outcome.unwrap().cause, None); + assert_eq!(collected.status.unwrap().code(), Some(7)); + assert_eq!(execution.cause, None); } }); } @@ -867,7 +858,7 @@ fn privileged_exec_resets_priority_credentials_and_signals_before_exec() { let mut collected = Collected::default(); finish(&mut execution, &mut collected); assert_eq!(collected.launch, Some(None)); - assert!(collected.outcome.unwrap().status.success()); + assert!(collected.status.unwrap().success()); let output = String::from_utf8(collected.stdout).unwrap(); for expected in [ format!("Uid:\t{uid}\t{uid}\t{uid}\t{uid}\n"), @@ -935,7 +926,7 @@ fn privileged_exec_streams_pipes_and_reports_its_status() { assert_eq!(collected.launch, Some(None)); assert_eq!(collected.stdout, b"out:hello\n"); assert_eq!(collected.stderr, b"err:hello\n"); - assert_eq!(collected.outcome.unwrap().status.code(), Some(7)); + assert_eq!(collected.status.unwrap().code(), Some(7)); assert!(execution.descriptors().next().is_none()); assert!(!execution.observe(execution.id(), ExitStatus::from_raw(0))); let mut values = request("if read -r line; then exit 99; fi; pwd"); @@ -951,7 +942,7 @@ fn privileged_exec_streams_pipes_and_reports_its_status() { let mut collected = Collected::default(); finish(&mut execution, &mut collected); assert_eq!(collected.stdout, b"/\n"); - assert!(collected.outcome.unwrap().status.success()); + assert!(collected.status.unwrap().success()); let mut values = request(""); values.program = c"env".to_owned(); values.arguments = vec![c"env".to_owned()]; @@ -968,30 +959,25 @@ fn privileged_exec_streams_pipes_and_reports_its_status() { .lines() .any(|line| line == "PWD=/tmp") ); - assert!(collected.outcome.unwrap().status.success()); + assert!(collected.status.unwrap().success()); }, ); } #[test] #[ignore = "requires a guest's root credentials"] -fn privileged_exec_checks_account_mode_and_enters_cwd_after_dropping_credentials() { +fn privileged_exec_needs_no_account_and_enters_cwd_after_dropping_credentials() { isolated( - "privileged_exec_checks_account_mode_and_enters_cwd_after_dropping_credentials", + "privileged_exec_needs_no_account_and_enters_cwd_after_dropping_credentials", || { - for account in [AccountMode::Existing, AccountMode::Numeric] { - let mut values = request("pwd"); - values.account = account; - if account == AccountMode::Numeric { - values.uid = 4_000_000_000; - values.gid = 4_000_000_000; - } - let mut execution = Exec::start(values).unwrap(); - let mut collected = Collected::default(); - finish(&mut execution, &mut collected); - assert_eq!(collected.launch, Some(None)); - assert_eq!(collected.stdout, b"/\n"); - } + let mut values = request("pwd"); + values.uid = 4_000_000_000; + values.gid = 4_000_000_000; + let mut execution = Exec::start(values).unwrap(); + let mut collected = Collected::default(); + finish(&mut execution, &mut collected); + assert_eq!(collected.launch, Some(None)); + assert_eq!(collected.stdout, b"/\n"); let directory = tempfile::tempdir().unwrap(); sys::chmod( &sys::path_c_string(directory.path()).unwrap(), @@ -1008,7 +994,7 @@ fn privileged_exec_checks_account_mode_and_enters_cwd_after_dropping_credentials let failure = collected.launch.unwrap().unwrap(); assert_eq!(failure.stage, 18); assert_eq!(failure.error, Errno::EACCES as i32); - assert_eq!(collected.outcome.unwrap().status.code(), Some(125)); + assert_eq!(collected.status.unwrap().code(), Some(125)); let path = sys::path_c_string(directory.path()).unwrap(); sys::chown(&path, 65534, 65534).unwrap(); let mut values = request("exit 99"); @@ -1023,81 +1009,11 @@ fn privileged_exec_checks_account_mode_and_enters_cwd_after_dropping_credentials error: Errno::EACCES as i32 })) ); - assert_eq!(collected.outcome.unwrap().status.code(), Some(125)); - let root = tempfile::tempdir().unwrap(); - let etc = root.path().join("etc"); - std::fs::create_dir(&etc).unwrap(); - let passwd = "root:x:0:0::/:/bin/sh\n"; - let group = "root:x:0:\n"; - for (users, groups, expected) in [ - (passwd.to_owned(), group, Ok(())), - ( - format!("# ignored:x:0:0::/:/bin/sh\n{passwd}"), - group, - Ok(()), - ), - (format!("{passwd}{passwd}"), group, Err(Errno::EINVAL)), - ( - "other:x:1:0::/:/bin/sh\n".to_owned(), - group, - Err(Errno::ENOENT), - ), - ("root:x:0:0::/\n".to_owned(), group, Err(Errno::EINVAL)), - (":x:0:0::/:/bin/sh\n".to_owned(), group, Err(Errno::EINVAL)), - ( - "root:x:0:1::/:/bin/sh\n".to_owned(), - group, - Err(Errno::EINVAL), - ), - ( - "root:x:0:0::relative:/bin/sh\n".to_owned(), - group, - Err(Errno::EINVAL), - ), - (passwd.to_owned(), "other:x:1:\n", Err(Errno::EINVAL)), - ] { - std::fs::write(etc.join("passwd"), users).unwrap(); - std::fs::write(etc.join("group"), groups).unwrap(); - check_account_fixture(root.path(), expected); - } - std::fs::write(etc.join("group"), group).unwrap(); - std::fs::write(etc.join("users"), passwd).unwrap(); - std::fs::remove_file(etc.join("passwd")).unwrap(); - std::os::unix::fs::symlink("users", etc.join("passwd")).unwrap(); - check_account_fixture(root.path(), Err(Errno::ELOOP)); - std::fs::remove_file(etc.join("passwd")).unwrap(); - nix::unistd::mkfifo(&etc.join("passwd"), nix::sys::stat::Mode::S_IRWXU).unwrap(); - check_account_fixture(root.path(), Err(Errno::EINVAL)); - std::fs::remove_file(etc.join("passwd")).unwrap(); - std::fs::File::create(etc.join("passwd")) - .unwrap() - .set_len((1 << 20) + 1) - .unwrap(); - check_account_fixture(root.path(), Err(Errno::EINVAL)); + assert_eq!(collected.status.unwrap().code(), Some(125)); }, ); } -/// Reads fixture account files only in a guest test's forked child; production never changes root. -fn check_account_fixture(root: &std::path::Path, expected: Result<(), Errno>) { - match sys::fork().unwrap() { - sys::ForkResult::Child => { - nix::unistd::chroot(root).unwrap(); - nix::unistd::chdir("/").unwrap(); - let mut values = request("exit 99"); - values.account = AccountMode::Existing; - assert_eq!( - values.check().map_err(|error| error.raw_os_error()), - expected.map_err(|error| Some(error as i32)) - ); - sys::exit_immediately(0); - } - sys::ForkResult::Parent(child) => { - assert_eq!(sys::wait(child), Some(ChildStatus::Exited(0))); - } - } -} - #[test] #[ignore = "requires a guest's root credentials"] fn privileged_every_ending_kills_the_process_group_including_normal_completion() { @@ -1142,9 +1058,7 @@ fn privileged_every_ending_kills_the_process_group_including_normal_completion() Some(Event::KillFailed(target, error)) => { panic!("kill {target:?}: {error}") } - Some(Event::ReadFailed(stream, error)) => { - panic!("read {stream:?}: {error}") - } + Some(Event::ReadFailed) => panic!("an output read failed"), _ => {} } } @@ -1163,7 +1077,7 @@ fn privileged_every_ending_kills_the_process_group_including_normal_completion() until(&mut execution, &mut collected, |collected| { collected.reaped.contains(&descendant) }); - let outcome = collected.outcome.unwrap(); + let status = collected.status.unwrap(); let expected = [ None, Some(Cause::Terminated), @@ -1171,11 +1085,11 @@ fn privileged_every_ending_kills_the_process_group_including_normal_completion() Some(Cause::TimedOut), Some(Cause::Disconnected), ][ending]; - assert_eq!(outcome.cause, expected); + assert_eq!(execution.cause, expected); if ending == 0 { - assert_eq!(outcome.status.code(), Some(7)); + assert_eq!(status.code(), Some(7)); } else { - assert_eq!(outcome.status.signal(), Some(nix::libc::SIGKILL)); + assert_eq!(status.signal(), Some(nix::libc::SIGKILL)); } assert!(!sys::process_exists( pid(i32::try_from(descendant).unwrap()) @@ -1281,10 +1195,7 @@ fn privileged_capture_sweep_kills_setsid_escapees_and_spares_both_helpers() { held.reset(); finish(&mut held, &mut held_output); assert!(sys::process_exists(held_id)); - assert_eq!( - held_output.outcome.unwrap().cause, - Some(Cause::Disconnected) - ); + assert_eq!(held.cause, Some(Cause::Disconnected)); #[allow(clippy::zombie_processes)] let unrelated = std::process::Command::new("sleep") .arg("30") diff --git a/nvx-rs/src/guest/init/tests/restore.rs b/nvx-rs/src/guest/init/tests/restore.rs index 86cf3a741..7d5aef517 100644 --- a/nvx-rs/src/guest/init/tests/restore.rs +++ b/nvx-rs/src/guest/init/tests/restore.rs @@ -1,7 +1,7 @@ //! The restore-only announcer step and its existing helper supervision. use std::cell::RefCell; -use std::io::{Seek, SeekFrom, Write}; +use std::io::{Read, Seek, SeekFrom, Write}; use std::os::unix::fs::PermissionsExt; use std::path::Path; diff --git a/nvx-rs/src/guest/managed_agent/exec.rs b/nvx-rs/src/guest/managed_agent/exec.rs index f8cc787d7..3e0859645 100644 --- a/nvx-rs/src/guest/managed_agent/exec.rs +++ b/nvx-rs/src/guest/managed_agent/exec.rs @@ -299,11 +299,11 @@ impl Workload { init::Event::Launch(report) => { Event::Launch(report.map(|failure| failure.map(LaunchFailure::Direct))) } - init::Event::ReadFailed(..) => Event::ReadFailed, + init::Event::ReadFailed => Event::ReadFailed, init::Event::KillFailed(init::KillTarget::ProcessGroup, error) => { Event::DirectKillFailed(error) } - init::Event::Ended(outcome) => Event::Ended(outcome.status), + init::Event::Ended(status) => Event::Ended(status), _ => Event::Other, }) }), diff --git a/nvx-rs/src/guest/managed_agent/launch.rs b/nvx-rs/src/guest/managed_agent/launch.rs index 3b2740097..8f0fe0038 100644 --- a/nvx-rs/src/guest/managed_agent/launch.rs +++ b/nvx-rs/src/guest/managed_agent/launch.rs @@ -1,6 +1,6 @@ use std::ffi::{CStr, CString}; -use crate::guest::init::{AccountMode, ExecRequest}; +use crate::guest::init::ExecRequest; use crate::guest::parse::parse_decimal; use crate::guest::sys::{Errno, c_string}; use crate::protocol::GuestExecPayload; @@ -33,7 +33,6 @@ pub(crate) fn direct_request( directory, uid: parse_id(config.uid.as_bytes())?, gid: parse_id(config.gid.as_bytes())?, - account: AccountMode::Numeric, timeout: None, input: false, search_path: None, diff --git a/nvx-rs/src/guest/managed_agent/launch/tests.rs b/nvx-rs/src/guest/managed_agent/launch/tests.rs index 05b42e9cc..da59a8cce 100644 --- a/nvx-rs/src/guest/managed_agent/launch/tests.rs +++ b/nvx-rs/src/guest/managed_agent/launch/tests.rs @@ -51,7 +51,6 @@ fn direct_launch_uses_the_plain_process_core_without_a_helper_or_cgroup() { assert!(environment.contains(&expected), "{expected:?}"); } assert!(!plan.input); - assert_eq!(plan.account, AccountMode::Numeric); } #[test] @@ -216,13 +215,12 @@ fn privileged_direct_launch_hardens_credentials_and_resets_priority() { Some(Event::Launch(report)) => assert!(report.unwrap().is_none()), Some(Event::Output(Stream::Stdout, bytes)) => output.extend_from_slice(bytes), Some(Event::Output(Stream::Stderr, bytes)) => errors.extend_from_slice(bytes), - Some(Event::Ended(outcome)) => { - assert_eq!(outcome.status.code(), Some(0)); + Some(Event::Ended(status)) => { + assert_eq!(status.code(), Some(0)); break; } - Some(Event::ReadFailed(_, error) | Event::KillFailed(_, error)) => { - panic!("{error}"); - } + Some(Event::ReadFailed) => panic!("an output read failed"), + Some(Event::KillFailed(_, error)) => panic!("{error}"), Some(_) => {} None => { let descriptors = workload.descriptors().map(|(_, fd)| fd).collect::>(); diff --git a/nvx-rs/src/guest/sys/mod.rs b/nvx-rs/src/guest/sys/mod.rs index 56d49f625..9618bcaef 100644 --- a/nvx-rs/src/guest/sys/mod.rs +++ b/nvx-rs/src/guest/sys/mod.rs @@ -110,7 +110,7 @@ pub(crate) use ioctl::{ pub(crate) use mapping::MemoryMapping; #[cfg(target_arch = "x86_64")] pub(crate) use msr::{MsrAccess, MsrFile}; -pub(crate) use poll::{Pollable, hung_up, poll, readable_or_hung_up, ready}; +pub(crate) use poll::{Pollable, poll, readable_or_hung_up, ready}; pub(crate) use port::{PORT_DEVICE, Port}; #[cfg(target_arch = "x86_64")] pub(crate) use port::{PortAccess, PortError, check_released, power_off, probe_port}; diff --git a/nvx-rs/src/guest/sys/poll.rs b/nvx-rs/src/guest/sys/poll.rs index a30346532..fe43111b9 100644 --- a/nvx-rs/src/guest/sys/poll.rs +++ b/nvx-rs/src/guest/sys/poll.rs @@ -47,16 +47,6 @@ pub(crate) fn ready( .collect()) } -/// Returns whether `descriptor` hung up, without waiting: for the read end of a pipe, whether -/// every writer closed it, whatever bytes it still holds. -pub(crate) fn hung_up(descriptor: BorrowedFd<'_>) -> Result { - let mut polls = [PollFd::new(descriptor, PollFlags::empty())]; - nix::poll::poll(&mut polls, PollTimeout::ZERO)?; - Ok(polls[0] - .revents() - .is_some_and(|events| events.contains(PollFlags::POLLHUP))) -} - /// A descriptor owner and the readiness that [`poll`] waits for. #[derive(Clone, Copy, Debug)] pub(crate) enum Pollable<'a> { diff --git a/scripts/test-layout-allowlist.json b/scripts/test-layout-allowlist.json index f139492dd..0d7771a17 100644 --- a/scripts/test-layout-allowlist.json +++ b/scripts/test-layout-allowlist.json @@ -2,7 +2,7 @@ "schema": "nvx-test-layout-allowlist", "schema_version": 2, "seams": { - "nvx-rs/src/guest/init.rs": 7, + "nvx-rs/src/guest/init.rs": 2, "nvx-rs/src/guest/init/boot.rs": 1, "nvx-rs/src/guest/reseed.rs": 1, "nvx-rs/src/guest/sys/spawn.rs": 5, From 80f072e6d8caac78563bae798cb3658be0065613 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Thu, 12 Nov 2026 21:25:07 -0800 Subject: [PATCH 3/4] guest: hold the switched layered root without a dead-code expectation cold_boot returns the switched root in Layered, beside what the managed agent takes, and an expectation of dead code that named SL7.6b (#1116) covered Layered, since nothing reads the root's field: serve_boot moves the other fields out, and the root stays in the partly moved boot until serve_boot returns. SL7.6b merged, but the root must still live as long as the service: it holds the lock of its runtime directory and the descriptors of its view and live shares. serve_boot now moves the root into a binding of its own, which a comment explains and which drops at the same point, so the expectation goes and nothing that runs changes. The test-layout allowlist drops boot.rs, which keeps no test hook. Part of #1076. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nvx-rs/src/guest/init.rs | 3 +++ nvx-rs/src/guest/init/boot.rs | 7 ------- scripts/test-layout-allowlist.json | 1 - 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/nvx-rs/src/guest/init.rs b/nvx-rs/src/guest/init.rs index e8176e77a..9683d76e9 100644 --- a/nvx-rs/src/guest/init.rs +++ b/nvx-rs/src/guest/init.rs @@ -1007,6 +1007,9 @@ fn serve_boot(runtime: &mut Runtime) -> Result { ); } let boot = boot::cold_boot(&command_line).map_err(|error| error.to_string())?; + // Keeps the switched root, with its runtime directory's lock and the descriptors of its view + // and live shares, until the VM's service ends. + let _root = boot.root; runtime.system.report( format!( "NVX-INIT-ROOT: switched mappings={}\n", diff --git a/nvx-rs/src/guest/init/boot.rs b/nvx-rs/src/guest/init/boot.rs index 372412102..316ad0061 100644 --- a/nvx-rs/src/guest/init/boot.rs +++ b/nvx-rs/src/guest/init/boot.rs @@ -29,13 +29,6 @@ use crate::{Error, Result}; mod account; /// The real switched root and the descriptors and account defaults its control caller inherits. -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "SL7.6b takes the initialized layered control handoff" - ) -)] pub(super) struct Layered { pub(super) root: Root, pub(super) command_line: Vec, diff --git a/scripts/test-layout-allowlist.json b/scripts/test-layout-allowlist.json index 0d7771a17..d7ce7e98a 100644 --- a/scripts/test-layout-allowlist.json +++ b/scripts/test-layout-allowlist.json @@ -3,7 +3,6 @@ "schema_version": 2, "seams": { "nvx-rs/src/guest/init.rs": 2, - "nvx-rs/src/guest/init/boot.rs": 1, "nvx-rs/src/guest/reseed.rs": 1, "nvx-rs/src/guest/sys/spawn.rs": 5, "nvx-rs/src/guest/time/system.rs": 3, From c251d842d5e6ecba0b306856b0668881b58a0e06 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Thu, 12 Nov 2026 21:25:15 -0800 Subject: [PATCH 4/4] host: drop the VM directory's copies of the snapshot names vm's KEY and DESCRIPTION, a finished capture's key.bin and snapshot.json, carried expectations of dead code until SL2.12b and SL2.14 (#1102) used them. Those slices named the files with snapshot::KEY_FILE and snapshot::DESCRIPTION_FILE instead, which finish_capture writes into the VM's directory, so only vm's tests used the copies. They go, and the tests check that the directory reserves snapshot's names and that a cold capture VM's directory holds none of them. Process::wait_bool also stays from the old lifecycle: it kept a boolean wait "for the legacy lifecycle callers", which phase 4 of #1076 deleted, and wait was its only caller. wait now waits itself. The test-layout allowlist drops vm.rs, which keeps no test hook. Part of #1076. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nvx-rs/src/host/process.rs | 14 +++----------- nvx-rs/src/host/vm.rs | 12 ------------ nvx-rs/src/host/vm/tests.rs | 7 ++++--- scripts/test-layout-allowlist.json | 3 +-- 4 files changed, 8 insertions(+), 28 deletions(-) diff --git a/nvx-rs/src/host/process.rs b/nvx-rs/src/host/process.rs index d5e03a54e..9ea3f07f5 100644 --- a/nvx-rs/src/host/process.rs +++ b/nvx-rs/src/host/process.rs @@ -269,21 +269,13 @@ impl Process { /// process, or with [`WaitMethod::Polling`], checks its identity at a short interval. An /// error means that the wait cannot determine whether the process exited. pub fn wait(&self, deadline: Instant) -> io::Result> { - Ok(if self.wait_bool(deadline)? { - Some(Exited(())) - } else { - None - }) - } - - /// Keeps the boolean wait result for the legacy lifecycle callers. - pub(crate) fn wait_bool(&self, deadline: Instant) -> io::Result { - wait_handle( + let exited = wait_handle( &self.handle, self.reference.pid, self.reference.start_time, deadline, - ) + )?; + Ok(exited.then_some(Exited(()))) } /// Returns whether the process has exited, without waiting. diff --git a/nvx-rs/src/host/vm.rs b/nvx-rs/src/host/vm.rs index de8d9af3f..38717c619 100644 --- a/nvx-rs/src/host/vm.rs +++ b/nvx-rs/src/host/vm.rs @@ -50,18 +50,6 @@ use crate::{Error, Result}; /// OpenVMM's snapshot files: a capture's destination and a restore's snapshot view. const OPENVMM: &str = "openvmm"; -/// A finished capture's key. -#[cfg_attr( - not(test), - expect(dead_code, reason = "SL2.12b and SL2.14 use it (#1102)") -)] -const KEY: &str = "key.bin"; -/// A finished capture's informational snapshot description. -#[cfg_attr( - not(test), - expect(dead_code, reason = "SL2.12b and SL2.14 use it (#1102)") -)] -const DESCRIPTION: &str = "snapshot.json"; /// The sparse placeholder image that a capture VM attaches. const CAPTURE_IMAGE: &str = "capture-image.raw"; /// The dormant scratch that a capture VM attaches. diff --git a/nvx-rs/src/host/vm/tests.rs b/nvx-rs/src/host/vm/tests.rs index cef6dbb99..540d0f9fe 100644 --- a/nvx-rs/src/host/vm/tests.rs +++ b/nvx-rs/src/host/vm/tests.rs @@ -19,6 +19,7 @@ use crate::host::network::{ #[cfg(any(target_os = "linux", windows))] use crate::host::process::Opened; use crate::host::runtime::{ArtifactIdentity, LaunchOptions, RuntimeIdentity}; +use crate::host::snapshot::{DESCRIPTION_FILE, KEY_FILE}; use crate::host::vm_directory::{LOG, OUTCOME, RESTORE_READY, tests::vm_dir}; use crate::hypervisor::Hypervisor; use crate::net::Ipv4Cidr; @@ -69,8 +70,8 @@ fn the_reserved_names_are_the_documented_ones_and_leave_the_openvmm_prefix_to_op CONSOLE, RESTORE_READY, OPENVMM, - KEY, - DESCRIPTION, + KEY_FILE, + DESCRIPTION_FILE, CAPTURE_IMAGE, CAPTURE_SCRATCH, ]; @@ -323,7 +324,7 @@ fn capture_creation_records_sparse_disks_devices_and_a_complete_serde_context() assert!(argument_values(arguments, option).is_empty(), "{option}"); } assert_eq!(fs::read_dir(vm.path()).unwrap().count(), 3); - for name in [OPENVMM, KEY, DESCRIPTION] { + for name in [OPENVMM, KEY_FILE, DESCRIPTION_FILE] { assert!(!vm.path().join(name).exists(), "{name}"); } } diff --git a/scripts/test-layout-allowlist.json b/scripts/test-layout-allowlist.json index d7ce7e98a..39768d534 100644 --- a/scripts/test-layout-allowlist.json +++ b/scripts/test-layout-allowlist.json @@ -12,7 +12,6 @@ "nvx-rs/src/host/platform/windows/identity.rs": 7, "nvx-rs/src/host/platform/windows/mod.rs": 2, "nvx-rs/src/host/platform/windows/process.rs": 2, - "nvx-rs/src/host/platform/windows/spawn.rs": 1, - "nvx-rs/src/host/vm.rs": 2 + "nvx-rs/src/host/platform/windows/spawn.rs": 1 } }