Skip to content

nvx-rs: drop the guest::clone alias and the code that stale dead-code expectations hid - #1566

Merged
Pedro Henrique Penna (ppenna) merged 4 commits into
feature/nvx-refactorfrom
agents/slim-drop-clone-alias-and-dead-code
Nov 13, 2026
Merged

nvx-rs: drop the guest::clone alias and the code that stale dead-code expectations hid#1566
Pedro Henrique Penna (ppenna) merged 4 commits into
feature/nvx-refactorfrom
agents/slim-drop-clone-alias-and-dead-code

Conversation

@ppenna

@ppenna Pedro Henrique Penna (ppenna) commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Part of #1076. It removes scaffolding that the slices of #1105 (SL7.3), #1116 (SL7.6a, SL7.6b), #1102 (SL2.12b, SL2.14) and #1100 (SL2.9a) left behind after they merged.

What and why

A check of #1076's issues against this branch's head, 367f7fe4, found every planned pull request merged. It also found two kinds of scaffolding still in place that the issues said must end. This PR removes both before the breaking release.

The guest::clone alias. SL7.3 renamed guest::clone to guest::activation, and kept an alias until SL5.2 deleted the test adapter (#1105). SL5.2 (#1499) found that nothing used the alias any more, but SL6.11c, SL6.11d, SL6.13b and SL6.10g each left it, because their scopes didn't name it. 0.2.0-alpha.1 would have published it: 144 lines of API that #1076's planned surface doesn't hold, and whose later removal would break the release's users a second time.

  • pub use activation as clone; goes, and public_api.py --update drops its 144 lines from nvx-rs/public-api/linux.txt. The Windows snapshot has no guest module.
  • nvx-rs/README.md no longer calls it a compatibility alias. doc/migrate-from-alpha.7.md maps guest::clone to guest::activation and drops the old name. CHANGELOG.md lists the rename among the breaking changes, and protocol::status names guest::snapshot::RepairError::status.

Stale dead-code expectations. Eight #[cfg_attr(not(test), expect(dead_code, reason = "…"))] attributes named callers in slices that had already merged: SL7.6a five times in guest/init.rs, SL7.6b in guest/init/boot.rs, and SL2.12b and SL2.14 in host/vm.rs. Turned into warnings, they let the compiler report 11 items that nothing outside tests uses, and #1076's PR rules don't allow such items in production files:

  • nvx-init's exec core (SL7.5c, which SL7.6a put to use):
    • AccountMode::Existing, which checks that an execution's IDs name an account in the root's passwd and group. The managed agent, the core's only user, always starts workloads by numeric ID, so AccountMode and ExecRequest::account go too.
    • StartFailure::into_parts. The agent reads the fields directly.
    • Outcome's cause and discarded. The agent decides an outcome from its own cancellation, timeout and session flags plus the terminal status, so Event::Ended now carries only the status.
    • The streams of Closed and ReadFailed, and ReadFailed's error. The agent maps these events to its own events without them.
    • Exec::output_hung_up, with sys::hung_up, which only a test called.
  • The discard path, whose only output was the unread discarded count. reset() closes both outputs as it marks them discarded, so no read ever discarded anything. The marks, the counts and Event::Discarded go.
  • Layered::root (SL7.6b) is not dead. serve_boot moves the other fields out, and the switched root stays in the partly moved boot until the service ends, holding its runtime directory's lock and the descriptors of its view and live shares. serve_boot now moves it into a _root binding of its own, with a comment, which drops at the same point.
  • vm::KEY and vm::DESCRIPTION (SL2.12b, SL2.14). Those slices named the files snapshot::KEY_FILE and snapshot::DESCRIPTION_FILE, which finish_capture writes into the VM's directory, so only vm's tests used the copies. The tests now check snapshot's names.
  • Process::wait_bool (SL2.9a) kept a boolean wait "for the legacy lifecycle callers", which phase 4 deleted. wait, its only caller, now waits itself.

Runtime behavior doesn't change: no production path read what goes. scripts/test-layout-allowlist.json records the test hooks that remain, via check_test_layout.py --tighten: 2 in init.rs, and none in boot.rs or vm.rs.

Decisions

  1. Delete what production ignored, rather than make it read it. Having the agent report a read failure's error or an ending's cause would change what it does, and it already derives each outcome from its own state.
  2. Exec::cause stays. deadline() and end_with use it. The tests that read an outcome's cause now read the execution's, so they check the same thing.
  3. Layered::root stays held, not removed. Dropping it at the end of cold_boot would release the runtime directory's lock and close the view's and the shares' descriptors early.

Tests

python3 scripts/check_test_changes.py --base origin/feature/nvx-refactor lists 30 changes to existing tests and 1 addition, so the PR has the tests-modified label:

  • The exec tests' helpers:
    • Collected, step and finish collect the terminal status instead of the outcome.
    • request drops the account mode, and pipe_execution drops the removed fields.
    • procfs_workload_cwd_checks, host_launch_tests_use_the_syscall_seam_without_mocking_the_execution_core, privileged_exec_resets_priority_credentials_and_signals_before_exec and privileged_exec_streams_pipes_and_reports_its_status read collected.status.unwrap() where they read collected.outcome.unwrap().status.
  • only_the_collectors_terminal_status_settles_the_owned_leader_and_output counts the closed outputs and reads the execution's cause. It no longer checks the discarded counts, which are gone. It also no longer asks whether a closed output hung up, which the preceding check that no output is open already covers.
  • 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, and since no output is discarded any more, both outputs are delivered.
  • invalid_requests_fail_before_a_child_starts drops its missing-account case.
  • privileged_exec_checks_account_mode_and_enters_cwd_after_dropping_credentials becomes privileged_exec_needs_no_account_and_enters_cwd_after_dropping_credentials; the tool lists it as removed and added. It keeps the run under numeric IDs that no account names, and the working-directory checks. It drops the account databases' fixtures and their helper, check_account_fixture.
  • a_failed_start_preserves_its_first_errno_and_the_cleanup_failure destructures the StartFailure instead of calling into_parts, with the same assertions.
  • a_stolen_or_auto_reaped_status_is_reported_once_and_never_reclassified_by_an_ending, privileged_every_ending_kills_the_process_group_including_normal_completion and privileged_capture_sweep_kills_setsid_escapees_and_spares_both_helpers read each cause from the execution.
  • direct_launch_uses_the_plain_process_core_without_a_helper_or_cgroup no longer checks the account mode, and privileged_direct_launch_hardens_credentials_and_resets_priority matches the new events.
  • the_reserved_names_are_the_documented_ones_and_leave_the_openvmm_prefix_to_openvmm and capture_creation_records_sparse_disks_devices_and_a_complete_serde_context name snapshot::KEY_FILE and snapshot::DESCRIPTION_FILE.
  • The eight removed hooks are the expectations themselves.

Size

  • Core lines: 17 added or changed, in 5 Rust files outside tests: guest/init.rs (10), host/process.rs (3), guest/managed_agent/exec.rs (2), guest/managed_agent/launch.rs (1) and guest/sys/mod.rs (1). It deletes 166 lines of code.
  • 10 Rust files outside tests change. Besides those 5, guest/mod.rs, guest/init/boot.rs, guest/sys/poll.rs and host/vm.rs only lose lines, and protocol/status.rs changes a doc comment's path. Almost all of it is code that leaves.

Commits

  1. guest: drop guest::clone, the alias of guest::activation
  2. guest: delete what only the exec core's tests used
  3. guest: hold the switched layered root without a dead-code expectation
  4. host: drop the VM directory's copies of the snapshot names, which also folds Process::wait_bool into wait

Each commit passes check_test_layout.py, and Clippy with warnings denied both for x86_64-unknown-linux-gnu with every feature and for x86_64-unknown-linux-musl with the guest feature alone.

Not in this PR

  • guest::snapshot::CaptureStage::Fence, which no capture produces any more. It is public API, and SL6.11c left its removal to a later PR.
  • host::nvxs, the compatibility path of host::transport's HostConnection and HostConnectionError, which no issue of nvx-rs slimming workstream: make nvx a library of mechanisms and value types #1076 names.
  • BOOT_CONTRACT_VERSION and CONTROL_CONTRACT_REVISION, which nothing here changes.

Validation

All checks ran on the head, c251d842, with Rust 1.95.0 and the locked dependencies. This branch has no local_gates.py, so the commands are those of its check-nvx-rs and check-quality actions that the change selects.

Windows (x86_64-pc-windows-msvc):

  • cargo fmt --all --check: passed.
  • cargo clippy -- -D warnings: passed with --all-targets --all-features; --lib; --lib --no-default-features; --lib --no-default-features --features serde; --lib --bins --features cli; and --lib --bins --no-default-features --features testing.
  • cargo clippy --target x86_64-unknown-linux-gnu -- -D warnings: passed with --all-targets --all-features, and for the library with default features, without them, with serde, cli and testing alone.
  • check-guest-binaries' lint, cargo clippy --target x86_64-unknown-linux-musl --no-default-features --features guest --all-targets -- -D warnings: passed.
  • cargo test --locked --all-features, cargo test --locked and cargo test --locked --lib --no-default-features: passed, with 1065, 518 and 191 library tests.
  • RUSTDOCFLAGS="-D warnings" cargo doc --locked --no-deps --all-features: passed, also for x86_64-unknown-linux-musl.
  • The minimum supported Rust version, cargo +1.89 check --locked --all-targets --all-features: passed.
  • cargo check --locked --all-targets --all-features --target aarch64-apple-darwin: passed. Its one warning, about two test helpers of host::snapshot that the macOS build doesn't use, predates this PR.
  • python scripts/public_api.py: passed. --update changed only linux.txt, by the alias's 144 lines.
  • python scripts/check_test_layout.py and python scripts/check_markdown_links.py: passed.
  • python -m unittest scripts.test_crate_release scripts.test_public_api scripts.test_test_layout scripts.test_test_changes: passed.

Linux (WSL 2, Ubuntu, kernel 6.18):

  • cargo test --locked --all-features: passed, with 1768 library tests passed and 59 ignored.
  • cargo test --locked and cargo test --locked --lib --no-default-features: passed.
  • check-guest-binaries' cargo test --locked --target x86_64-unknown-linux-musl --no-default-features --features guest: passed, with 855 library tests passed and 32 ignored.
  • The isolated exec tests ran their bodies in their child processes, the renamed ending_failures_are_reported_and_the_first_cause_is_kept included.
  • Every changed #[ignore]d privileged test, run as root: privileged_exec_needs_no_account_and_enters_cwd_after_dropping_credentials, privileged_every_ending_kills_the_process_group_including_normal_completion (all five endings), privileged_capture_sweep_kills_setsid_escapees_and_spares_both_helpers, privileged_exec_resets_priority_credentials_and_signals_before_exec, privileged_exec_streams_pipes_and_reports_its_status and privileged_direct_launch_hardens_credentials_and_resets_priority: all passed.
  • cargo +1.89 check --locked --all-targets --all-features: passed.

Left to CI:

  • The microVM jobs on KVM, MSHV and WHP, which boot nvx-init and the managed agent with the changed core.
  • The NVX CLI's Python suites, validate-nvx, whose inputs this PR doesn't change.

@ppenna
guest: drop guest::clone, the alias of guest::activation
ac572dd 
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>
@ppenna
guest: delete what only the exec core's tests used
782453f 
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>
@ppenna
guest: hold the switched layered root without a dead-code expectation
80f072e 
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>
@ppenna
host: drop the VM directory's copies of the snapshot names
c251d84 
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>
@github-actions github-actions Bot added the tests-modified Changes existing tests, test data, or test wiring label Nov 13, 2026
@github-actions

github-actions Bot commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

This pull request changes existing tests

It modifies or removes 30 existing test asset(s), listed below. Review each with care: a change to a test can hide a regression that the test caught. See doc/contribute.md#changes-to-existing-tests.

Test code

Change What Before After
modified helper struct Collected in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:39 nvx-rs/src/guest/init/tests/exec.rs:40
modified helper fn request in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:53 nvx-rs/src/guest/init/tests/exec.rs:54
modified helper fn step in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:93 nvx-rs/src/guest/init/tests/exec.rs:93
modified helper fn finish in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:125 nvx-rs/src/guest/init/tests/exec.rs:125
modified helper fn pipe_execution in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:210 nvx-rs/src/guest/init/tests/exec.rs:208
modified helper fn procfs_workload_cwd_checks in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:660 nvx-rs/src/guest/init/tests/exec.rs:651
modified test fn only_the_collectors_terminal_status_settles_the_owned_leader_and_output in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:243 nvx-rs/src/guest/init/tests/exec.rs:239
modified test fn ending_failures_are_reported_and_discarding_preserves_the_first_cause in guest::init::tests::exec (renamed to ending_failures_are_reported_and_the_first_cause_is_kept) nvx-rs/src/guest/init/tests/exec.rs:295 nvx-rs/src/guest/init/tests/exec.rs:289
modified test fn invalid_requests_fail_before_a_child_starts in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:337 nvx-rs/src/guest/init/tests/exec.rs:328
modified test fn a_failed_start_preserves_its_first_errno_and_the_cleanup_failure in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:515 nvx-rs/src/guest/init/tests/exec.rs:502
modified test fn host_launch_tests_use_the_syscall_seam_without_mocking_the_execution_core in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:621 nvx-rs/src/guest/init/tests/exec.rs:612
modified test fn a_stolen_or_auto_reaped_status_is_reported_once_and_never_reclassified_by_an_ending in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:729 nvx-rs/src/guest/init/tests/exec.rs:720
modified test fn privileged_exec_resets_priority_credentials_and_signals_before_exec in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:842 nvx-rs/src/guest/init/tests/exec.rs:833
modified test fn privileged_exec_streams_pipes_and_reports_its_status in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:918 nvx-rs/src/guest/init/tests/exec.rs:909
modified test fn privileged_every_ending_kills_the_process_group_including_normal_completion in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:1103 nvx-rs/src/guest/init/tests/exec.rs:1019
modified test fn privileged_capture_sweep_kills_setsid_escapees_and_spares_both_helpers in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:1190 nvx-rs/src/guest/init/tests/exec.rs:1104
modified test fn direct_launch_uses_the_plain_process_core_without_a_helper_or_cgroup in guest::managed_agent::launch::tests nvx-rs/src/guest/managed_agent/launch/tests.rs:38 nvx-rs/src/guest/managed_agent/launch/tests.rs:38
modified test fn privileged_direct_launch_hardens_credentials_and_resets_priority in guest::managed_agent::launch::tests nvx-rs/src/guest/managed_agent/launch/tests.rs:167 nvx-rs/src/guest/managed_agent/launch/tests.rs:166
modified test fn the_reserved_names_are_the_documented_ones_and_leave_the_openvmm_prefix_to_openvmm in host::vm::tests nvx-rs/src/host/vm/tests.rs:64 nvx-rs/src/host/vm/tests.rs:65
modified test fn capture_creation_records_sparse_disks_devices_and_a_complete_serde_context in host::vm::tests nvx-rs/src/host/vm/tests.rs:197 nvx-rs/src/host/vm/tests.rs:198
removed helper fn check_account_fixture in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:1082
removed hook the test hooks of enum AccountMode in guest::init nvx-rs/src/guest/init.rs:345
removed hook the test hooks of impl impl StartFailure in guest::init nvx-rs/src/guest/init.rs:409
removed hook the test hooks of struct Outcome in guest::init nvx-rs/src/guest/init.rs:455
removed hook the test hooks of enum Event in guest::init nvx-rs/src/guest/init.rs:467
removed hook the test hooks of impl impl Exec in guest::init nvx-rs/src/guest/init.rs:624
removed hook the test hooks of struct Layered in guest::init::boot nvx-rs/src/guest/init/boot.rs:39
removed hook the test hooks of const KEY in host::vm nvx-rs/src/host/vm.rs:58
removed hook the test hooks of const DESCRIPTION in host::vm nvx-rs/src/host/vm.rs:64
removed test fn privileged_exec_checks_account_mode_and_enters_cwd_after_dropping_credentials in guest::init::tests::exec nvx-rs/src/guest/init/tests/exec.rs:978

Relocated unchanged: 0; added: 1. Compared the merge base 367f7fe4d0d5 with c251d842d5e6.

@ppenna
Pedro Henrique Penna (ppenna) merged commit ccd2384 into feature/nvx-refactor Nov 13, 2026
45 checks passed
@ppenna
Pedro Henrique Penna (ppenna) deleted the agents/slim-drop-clone-alias-and-dead-code branch November 13, 2026 06:10
@ppenna Pedro Henrique Penna (ppenna) mentioned this pull request Nov 13, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@ppenna Pedro Henrique Penna (ppenna)

Labels

tests-modified Changes existing tests, test data, or test wiring

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ppenna