Repository navigation
guest: delete the old capture steps and make their surface private (SL6.11c) - #1504
Draft
Pedro Henrique Penna (ppenna) wants to merge 2 commits into
Draft
Pedro Henrique Penna (ppenna) wants to merge 2 commits into
Pedro Henrique Penna (ppenna) wants to merge 2 commits into
Conversation
Contributor
This pull request changes existing testsIt modifies or removes 46 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
Relocated unchanged: 0; added: 2. Compared the merge base |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The activation example swallows capture failures that require VM shutdown, losing the required fatal status instead of passing it to the caller’s shutdown path.
1 open issue
What changed in this PR
This PR removes the obsolete guest capture API after the runtime adapter’s deletion, leaving guest::snapshot::capture as the public capture-and-repair entry point in the library-slimming workstream.
Changes:
- Deletes activation’s legacy capture steps and makes restore repair private to snapshot.
- Makes the time ABI’s capture and restore interfaces crate-private while retaining their internal consumers.
- Updates tests, documentation, and the Linux public-API snapshot to reflect the reduced surface.
| File | Description |
|---|---|
nvx-rs/src/lib.rs |
Documents snapshot capture and activation separately. |
nvx-rs/src/guest/time/mod.rs |
Narrows re-exports and simplifies the public example. |
nvx-rs/src/guest/time/api/tests.rs |
Adapts CPU-set assertions to removed accessors. |
nvx-rs/src/guest/time/api.rs |
Restricts capture and restore visibility; removes unused accessors. |
nvx-rs/src/guest/snapshot/tests.rs |
Adds migrated preparation-failure coverage. |
nvx-rs/src/guest/snapshot/repair.rs |
Makes repair internal and removes public wrappers. |
nvx-rs/src/guest/snapshot.rs |
Clarifies capture’s process effects and mount-check documentation. |
nvx-rs/src/guest/mod.rs |
Updates guest API and compatibility-alias documentation. |
nvx-rs/src/guest/activation/tests.rs |
Retires legacy-step tests and retains mount-check coverage. |
nvx-rs/src/guest/activation/state/apply.rs |
Redirects a documentation import to snapshot devices. |
nvx-rs/src/guest/activation.rs |
Deletes legacy capture types and rewrites the example. |
nvx-rs/README.md |
Describes the reduced public capture and time interfaces. |
nvx-rs/public-api/linux.txt |
Records the removed public API entries. |
doc/guest-agent-guidelines.md |
Updates guidance on capture’s process-starting behavior. |
doc/acceptance-case-matrix.md |
Updates doctest locations and migration notes. |
Review effort: Balanced
guest::snapshot::capture has replaced the activation's capture steps: one call checks the mounts, prepares, requests the capture and repairs a restore (SL7.4a, SL7.4b). Their last other users, the runtime adapter's guest worker and its tests, left with SL5.2. Delete the steps from guest::activation: the caller's AdmissionFence, CapturePreparation with prepare, and the Device-shaped Devices that only it took, CaptureBarrier with capture and cancel, the generic Captured and Restored with into_parts, and CaptureError with into_cause. The capture's stages, causes, mount checks and live seam, which guest::snapshot uses, stay where they are. The seam's pre_capture, which only the preparation called, goes. Make the repair private to guest::snapshot: guest::activation no longer re-exports Repair and repair, Repair and Repair::run become pub(super), and repair and Repair::new, which only the public API reached, go. The repair's implementation and its tests stay. The activation tests of the fence, the barrier and a restored clone's parts retire. The mount-check tests check snapshot::check_mounts directly, the test of the error names checks CaptureFailure, and the test of a failed time preparation's stages moves to the snapshot tests, on the one-call path. Update the rustdoc, the nvx-rs README, the case matrix's row of the activation doc test, which now captures through snapshot::capture, and the public-API snapshot, which drops 56 lines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
TimeAbi::capture and TimeAbi::restore_finish, and the types that only they take and return, were public so that a guest agent could run a capture's steps itself. guest::snapshot::capture runs them in one call now, and nothing outside the crate needs them. Make them crate-private, with all their methods: TimeAbi::capture and TimeAbi::restore_finish, CaptureRequest, CaptureOutcome, Restore with network, RestoreNetwork, RestoreFinish, MemoryRange, CpuSet and ScratchPolicy. guest::snapshot and nvx-time keep using them, unchanged. CpuSet's contains, len, is_empty and iter, which only the public API reached, go; its test checks the same through its mask. guest::time's example keeps the cold-boot step and the daemon, and its docs, TimeAbi's, the guest module's and snapshot::capture's say that the capture request and the finish are private. Update the README, the guest-agent guidelines, the case matrix's row of the time doc test, and the public-API snapshot, which drops 40 lines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pedro Henrique Penna (ppenna)
force-pushed
the
agents/slim-sl6-11c-capture-steps
branch
from
November 12, 2026 00:40
94f5f08 to
8f088d8
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Part of #1120 (SL6.11c, "The old capture steps are gone"). Parent #1087, umbrella #1076.
Stacked on #1499 (SL5.2): its deletion of the runtime adapter removes the last users of the old capture steps. This pull request's base is SL5.2's branch,
agents/slim-sl5-2-remove-adapter(a2fadde7), so it merges only after #1499.What and why
guest::snapshot::capturehas replaced the activation's capture steps (SL7.4a and SL7.4b, #1394): one bounded call checks the mounts, prepares, requests the capture and repairs a restore. With the runtime adapter gone (#1499), nothing uses the steps any more, and the repair and the time ABI's capture and restore surface have no user outsideguest::snapshotandnvx-time. So:guest::activation(first commit):AdmissionFence;CapturePreparation, withnewandprepare;CaptureBarrier, withcaptureandcancel;Restored, withrestoreandinto_parts; andCaptureError, withstage,causeandinto_cause. With them go the genericactivation::Captured<F>, which onlyCaptureBarrier::capturereturned, andactivation::Devices, which onlyCapturePreparation::newtook (see Judgment calls). Whatguest::snapshotuses stays where it is:CaptureStage,CaptureCause, and the capture's seam of the kernel and the time ABI (System,LiveSystem,capture_system,live_abi,at). The seam'spre_capture, which only the deleted preparation called, goes;TimeAbi::pre_capturestays fornvx-time pre-capture.guest::snapshot(first commit).guest::activation, and so its aliasguest::clone, no longer re-exportsRepairandrepair, andRepairandRepair::runbecomepub(super).repairandRepair::new, which only the public API reached, go, since the dead-code check that CI denies finds them once they are private. The repair's implementation and its internal tests don't change.Repaired,RepairError,RepairCause,RepairStage,RangeFaultandRepairProgressstay public, sincesnapshot::capturereturns them.TimeAbi::captureandTimeAbi::restore_finish, andCaptureRequest,CaptureOutcome,Restore(withnetwork),RestoreNetwork,RestoreFinish,MemoryRange,CpuSetandScratchPolicy, with all their methods, becomepub(crate).guest::snapshotandnvx-time'scaptureandrestore-finishcommands use them as before. NoTimeAbimethod is deleted.CpuSet'scontains,len,is_emptyanditer, which nothing in the crate calls, go, for the same dead-code reason.guest::activation, whose example now captures throughsnapshot::captureand then makes theActivation; ofguest::time, whose example keeps the cold boot and the daemon; and ofguest, the crate root,TimeAbi,snapshot::captureand the repair's types. The nvx-rs README'snvx::guest::time,nvx::guest::snapshotandnvx::guest::activationsections, the module-table rows oftimeandactivation, whose purposes change, and the guest programs' paragraph. The guest-agent guidelines' note on the one call that starts processes. The case matrix's rows of the doc tests that this rewrites or moves (below).nvx-rs/public-api/linux.txtdrops 96 lines: 56 in the first commit, the items above underguest::activationand its aliasguest::clone, and 40 in the second, underguest::time. The Windows snapshot doesn't change.No behavior changes:
snapshot::capture, its failure dispositions and its repair, andnvx-timerun the same code.BOOT_CONTRACT_VERSIONandCONTROL_CONTRACT_REVISIONdon't change.Judgment calls
These calls were approved for Pedro Henrique Penna (@ppenna):
activation::Devicesandactivation::Captured<F>go with the steps. The Scope names neither, but each existed only for them.Devices, withguest::root::Devicefields, wasCapturePreparation::new's parameter: SL7.4a moved the capture'sDevicestoguest::snapshot, naming the devices by their MMIO bases, and kept this one for the old caller alone.Captured<F>wasCaptureBarrier::capture's result, which holds aRestored<F>. Keeping them would leave public types that no call takes or returns. DeletingDevicesforces one import edit: the doc link of the hiddenRootTemplate::filesystem_tagnow namesguest::snapshot::Devices(apply.rs).CpuSet'scontains,len,is_emptyanditergo. The Scope makesCpuSetcrate-private with all its methods, and nothing in the crate calls these four, so they are code that the change leaves unused, which CI's warning-denied dead-code check refuses.CpuSet::newandCpuSet::with, which the repair calls, stay.CaptureStage::Fencestays. No call produces it any more, butCaptureStageisguest::snapshot::CaptureFailure's, which "Not in this PR" protects. A later PR can drop the variant.guest::clonealias stays. SL5.2 reports that nothing uses it any more, but this Scope doesn't name it. Its doc comment no longer says that it carries a legacy capture interface.scripts/nvx_tools/kernel_features.pykeeps namingnvx::guest::clone::repairamong the consumers of its processor and memory-hotplug features. The inventory is an input of the guest kernel's cache key (KERNEL_INPUT_HASHinbuild-guest-artifacts), so editing it rebuilds the kernels. The follow-up that SL5.2 started for this file updates its stale entries once, with a single rebuild, after SL6.11b, and now coversnvx::guest::clone::repairtoo: the repair that the entries mean runs insidenvx::guest::snapshot::capture.Not in this PR
As the issue says: changing
guest::snapshot::capture, its failure disposition, or its one-call capture and repair; redesigning repair or time synchronization; deleting theTimeAbimethods thatnvx-timeuses (capture,restore_finish,pre_captureandcancel_capture); and adding public capture-step types or compatibility aliases in their place.Size
Measured against the stack base
a2fadde7: 5 core lines in 2 counted files (estimate 30 and 4, stop 60).guest/activation.rsroot,snapshotandtimethat the deletion narrows; it deletes 227 lines of codeguest/activation/state/apply.rsDevices, fromguest::snapshot, which rustfmt moves with its#[cfg(doc)]These don't count, as the rules' exception for files whose items a PR only makes crate-private, with the edits that this forces, says. Counted too, they would make 45 core lines in 5 files, still under the stop:
guest/time/api.rs: 34 lines that make the itemspub(crate), 5 of them the signature ofCaptureRequest::new, which rustfmt wraps once it ispub(crate). It deletes the four deadCpuSetmethods.guest/time/mod.rs: 4 lines of re-exports, split betweenpub(crate)andpub.guest/snapshot/repair.rs: 2 lines that makeRepairandRepair::runpub(super). It deletes the deadrepairandRepair::new.guest/mod.rs,guest/snapshot.rsandlib.rschange only docs. Tests, docs, the case matrix and the public-API snapshot don't count.Tests of the area
guest::activation::tests, 19 tests before, 8 after:the_fence_closes_before_the_checks_and_the_detectors_are_suppressed_before_the_capture,a_fence_that_fails_to_close_stops_the_preparation,a_preparation_behind_a_fence_that_another_holds_closed_fails_and_leaves_it_closed,a_held_suppression_that_a_failed_preparation_cannot_restore_is_a_release_error,a_port_permission_that_the_thread_may_hold_outranks_a_failed_restore,cancel_and_drop_open_the_fence_then_restore_the_detectors,a_failed_restoration_of_the_detectors_is_reported_after_the_fence_opens,a_restored_clone_gets_the_fence_closed_with_the_detectors_suppressed,a_failed_capture_restores_the_detectors_and_opens_the_fence_only_before_its_requestanda_failed_capture_whose_detectors_cannot_be_restored_is_a_release_error. What they checked of the capture apart from the fence, the barrier and the restored clone's parts, the one call's tests check: its order (fresh_capture_orders_the_existing_steps_and_does_not_deadline_the_request), the rollback after a failure before the request and its failure (a_pre_request_failure_resumes_only_after_its_detector_restoration_succeeds,strict_time_status_is_retained_when_detector_rollback_also_fails), the precedence of a port permission that the thread may hold (port_release_is_fatal_and_keeps_its_cause_at_each_time_stage_despite_rollback_failure), the request's and the release's failures (request_and_release_failures_never_report_that_the_guest_resumed), and a restore's seed (a_pending_capture_is_not_released_and_an_unseeded_restore_must_end; the repair'scapture_returns_the_repaired_generation_resources_and_acknowledgement_after_the_finish);a_failed_time_preparation_names_its_stage_and_opens_the_fencebecomesguest::snapshot::tests::a_failed_time_preparation_names_its_stage_and_restores_the_detectors_that_it_holds, on the one call;each_mount_from_the_template_s_devices_blocks_a_capture,the_first_blocking_mount_of_the_table_is_reported,mounts_from_other_devices_and_tags_block_no_capture,a_device_without_a_window_has_nothing_mounted_from_itanda_mount_table_that_cannot_be_checked_blocks_a_capture, anderrors_name_their_stage_and_their_cause;the_live_system_finds_windows_and_block_devices_in_procfs_and_sysfsandthe_capture_and_the_repair_take_the_time_abi_in_strict_mode.guest::snapshot::tests: every test kept; the fixture loses itspre_capture, and the test above is added.guest::snapshot::repair::tests: kept unchanged. They build theRepairwithwith_system, and capture through the one call.guest::time::api::tests:cpu_sets_hold_the_cpus_that_the_abi_coversrewritten; every other test kept unchanged.guest::time::commands::tests,guest::init's andguest::managed_agent's tests: kept unchanged.guest::activation's andguest::time's rewritten,guest's moved a line up (below).The case matrix
doc/acceptance-case-matrix.mdnames doc tests by file and line. Three of its rows change, and its counts don't:nvx-rs/src/guest/activation.rs - guest::activation, now at line 158: this PR rewrites it onguest::snapshot::capture, as its row said the PR that removes the fence would. It stays kept.nvx-rs/src/guest/time/mod.rs - guest::time, now at line 9: it loses its capture and restore finish, which became crate-private. It stays kept.nvx-rs/src/guest/mod.rs - guest, now at line 40: unchanged, one line up, since the paragraph above it is one line shorter.The matrix names none of the unit tests that this PR retires, moves or rewrites (
git grepof each name).Validation
feature/nvx-refactorhas noscripts/local_gates.py.dev's copy, given this change's files, selects test-changes, markdown-links, test-layout, the nvx-rs gates and nvx-cli; they ran from this branch's own action definitions, with the pinned Rust 1.95.0 (MSRV 1.89) andNVX_BUNDLE=skip.Windows (local), on this head:
cargo fmt --all --check-D warningsRUSTDOCFLAGS="-D warnings" cargo doc --locked --no-deps --all-featurescargo test --locked --all-features --lib error::tests, which reads the READMEx86_64-unknown-linux-gnu(--all-targets --all-features) and forx86_64-unknown-linux-musl(--no-default-features --features guest --all-targets), and rustdoc for Linux, each with warnings deniedpython scripts/public_api.py --updatecheck_markdown_links.py,check_test_layout.pyLinux, on
nvx-mshv-test(linux-mshv, virtual machine, Azure Linux 3, kernel 6.18), in a temporary worktree of this branch:cargo fmt --all --check, check-nvx-rs's six Clippy runs, and rustdoc with-D warningscargo test --locked --all-features --lib -- guest::activation guest::snapshot guest::time guest::init guest::managed_agentcargo test --locked --all-features --docguest::activation (line 158),guest::time (line 9),guest (line 40),guest::snapshot (line 25)andlib.rs (line 58)cargo test --locked --all-features,cargo test --locked,cargo build --locked --lib --no-default-features,cargo test --locked --lib --no-default-featurespython3 scripts/public_api.py(both snapshots),check_test_layout.pycargo +1.89 check --locked --all-targets --all-featurescargo check --locked --all-targets --all-features --target aarch64-apple-darwinpython3 scripts/nvx.py check-guest-binaries --toolchain 1.95.0(musl Clippy and tests)cc65ae8c, which differs from6148ad32only in theguest::activationexample that Copilot's review fixed, and whose doc test compiles at this head: fmt, Clippy (all targets and features,--lib,--lib --no-default-features,cli), rustdoc, the guest unit tests and doc tests above,public_api.py,check-guest-binariesMicroVMs, on
nvx-mshv-test. OpenVMM, built withnvx.py build-openvmmat this branch's pin (14dc4116), and the kernel and the Alpine initramfs, built withnvx.py build-guest --nativefrom this head, whose guest binariesnvx-timeandnvx-initit compiles.python3 scripts/nvx.py test-microvm --backend mshv --scenario snapshot-core --scenario platform-restore-processors --scenario restore-memory --scenario time-abi-conformance --scenario untiered-network-policy --processors 1 2passed all five:snapshot-core(12.5 s),platform-restore-processors(3.9 s),restore-memory(3.2 s),untiered-network-policy(12.7 s), whose guest'snvx-time captureconsumes each restore packet and prints its network settings, andtime-abi-conformance(0.4 s; capture, restore and boot markers within budget). Their guests capture and restore throughnvx-snapshot, which runsnvx-time captureandnvx-time restore-finish, the commands ofTimeAbi::captureandTimeAbi::restore_finish, which this change makes crate-private.Left to CI: the bundled-artifacts test, which nothing here reaches; the guest binaries' release build and static-ELF check; the KVM, MSHV and WHP microVM suites; and the capture and restore smoke tests of SL5.1c (#1118), which this branch doesn't have yet.
Changes to existing tests
python3 scripts/check_test_changes.py --base origin/agents/slim-sl5-2-remove-adapter,dev's script, lists 46 changes to existing tests, hence thetests-modifiedlabel; it also finds 2 added and none relocated unchanged. All are inguest::activation::tests, but for the two noted.Modified, 16:
enum Calldrops the calls of the fence and of the time ABI (Close,Open,PreCapture,CancelCapture,GenerationId,Capture): the mount checks look up only the mount table, the windows and the block devices.struct FakeSystemand itsnewdrop the fake time ABI's fields (preparation,restoration_fails,generation_fails,outcome) and their defaults.FakeSystem'spre_capture_until,cancel_capture,generation_idandcapture(4) are nowunreachable!, since the mount checks ask nothing of the time ABI; the trait still requires them.fn devicesreturnsguest::snapshot::Devices, the MMIO bases, in place of the deletedactivation::Devices.SeedandPortError, and importguest::snapshot(asmounts),CaptureFailure,DevicesandDispositiondirectly, sinceguest::activationno longer importssnapshotunder that name.snapshot::check_mountsthrough the new helpercheck, in place of a preparation. They assert the same mounts, origins, mount points, error kinds and texts, and lookups. They no longer assert the fence's and the time ABI's calls around the checks, nor the stage's prefix,failed to check the mount table:, whichCaptureFailureadds; the one call'sa_mount_refusal_never_prepares_or_requests_capturechecks both the stage and that a refusal prepares nothing.errors_name_their_stage_and_their_causechecks theDisplayform and thesourceofCaptureFailure, which are those of the deletedCaptureError, in its place, and dropsinto_cause. Its stage and origin names don't change.cpu_sets_hold_the_cpus_that_the_abi_covers(guest::time::api::tests) checks the set's CPUs, count, membership and emptiness through its mask,set.0, sinceCpuSet's owniter,len,containsandis_emptygo.Removed, 30:
GENERATION,RESTORED,FakeFencewith itsimpl,handle,is_closed, and itsAdmissionFenceimplementation'scloseandopen,Preparation,Outcome,FakeSystem'spre_capture,fence,prepare,prepared,preparation_calls,platform_requestandseed.CaptureSystem'spre_capture(guest::snapshot::tests), which asserted that the one call never used the enforcing preparation: the trait method is gone.Added, 2: the helper
check, andguest::snapshot::tests::a_failed_time_preparation_names_its_stage_and_restores_the_detectors_that_it_holds, the moved test. It checks the same stages, causes and error numbers for each failure of the time ABI's preparation, through the one call, which also reports them asResumedand restores the stall detectors only when the failure holds their suppression.The
openvmmgitlink doesn't move.