Repository navigation
nvx-rs: report time-ABI repair failures through one Runtime method - #608
Merged
Merged
Conversation
A failure of restore-time repair is a fatal time-ABI event: it has the repair source and the restore phase, and it stops the VM with the repair power-off status unless the tools run report-only. commands::repair_failed passed those three to Runtime::fail_fatal, and daemon.rs spelled them out again at six sites, each with G_REPAIR_SUPPRESSION. restore_work also inlined the body of release_suppression. No unit test reached any of them, because Runtime::emit_event opened /dev/kmsg and /dev/console itself, so #605, #606, and #607 left this refactor until a test seam existed. Runtime::repair_failure, beside Runtime::conformance_failure, now passes the repair status, source, and phase. repair_failed and the six daemon sites call it, and restore_work calls release_suppression. Two new PrivilegedOps methods, write_kernel_log and write_console, are the seam. RealOps runs the code with which emit_event wrote an event's kernel log record and console line, and the FakeOps of the unit tests records their bytes, as it already records port writes and power-off requests. Behavior is unchanged. Every repair failure passes the status, code, source, phase, and detail that it passed before. An event writes the same bytes, with the same open flags, to the kernel log, port B, and the console, in that order, and reads the asynchronous-output flag at the same points. A new unit test checks a repair failure's event line at all three destinations and its power-off status, with and without report-only mode, in the glibc and musl test runs. The microVM restore scenarios run restore_work, and so release_suppression, in guests on KVM, MSHV, and WHP. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused refactor preserves existing behavior, adds regression coverage, and has no identified blocking issues.
No open issues 🎉
What changed in this PR
Centralizes guest time-ABI repair failure reporting while preserving existing event output and power-off behavior.
Changes:
- Adds
Runtime::repair_failureand routes command and daemon failures through it. - Reuses
release_suppressionfor restore cleanup. - Adds testable logging operations and regression coverage for normal and report-only modes.
| File | Description |
|---|---|
nvx-rs/src/guest/time/system.rs |
Adds shared repair reporting, logging seams, and regression tests. |
nvx-rs/src/guest/time/daemon.rs |
Consolidates repair failures and suppression cleanup. |
nvx-rs/src/guest/time/commands.rs |
Delegates repair failures to the shared runtime method. |
Review effort: Balanced
Pedro Henrique Penna (ppenna)
deleted the
nvx-rs-quality/repair-failure
branch
October 19, 2026 21:51
This was referenced Oct 19, 2026
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.
Summary
A failure of restore-time repair is a fatal time-ABI event: it has the
repairsource and therestorephase, and it stops the VM with the repair power-off status (195) unless the time tools run report-only. The guest's time tools reported it in two ways:commands::repair_failedpassed that status, source, and phase toRuntime::fail_fatalfor the eleven repair failures ofnvx-time captureandnvx-time restore-finish;guest/time/daemon.rsspelled all four arguments out at six sites, each withG_REPAIR_SUPPRESSION, andrestore_workinlined an exact copy ofrelease_suppression.This change adds
Runtime::repair_failure(code, detail), besideRuntime::conformance_failure, and routes all of them through it, so the status, source, and phase of a repair failure live in one place.restore_worknow callsrelease_suppression.#605, #606, and #607 each listed this refactor as a follow-up that waited for a test seam:
Runtime::emit_eventopened/dev/kmsgand/dev/consoleitself, so no unit test could observe an event. This change adds that seam toPrivilegedOps, the trait that already lets the tests'FakeOpsstand in for the time tools' ports, MSRs, clock, and power-off.RealOpskeeps the device writes,FakeOpsrecords them, and a new unit test pins what a repair failure reports.Earlier
nvx-rs-quality/runs opened #603, #605, #606, and #607, which all merged, #607 while this run was underway; their reviews approved them without feedback to follow. This run surveyed the crate by reading and with a throwaway duplicate-window script, and skipped every file that an open pull request todevchanged when it started, including those of #585, #597, #598, and #607 in the two crates. It pickedguest/time, which no earlier run touched.Changes
All changes are in
nvx-rs/src/guest/time/: 101 insertions and 71 deletions in three files. No public item, message, protocol byte, fixture, or other file changes.system.rs:PrivilegedOpsgainswrite_kernel_log(record)andwrite_console(separator, line).RealOpsimplements them with the code thatRuntime::emit_eventand its privateconsole_writeran, which move there unchanged.Runtime::emit_eventcalls them in the order in which it wrote before: the kernel log, then port B, then the console.Runtime::repair_failure(code, detail)callsfail_fatalwithPowerOffStatus::Repair,EventSource::Repair, andPhase::Restore.FakeOpsrecords the kernel log and console bytes, and a new unit test,repair_failures_report_restore_events_and_power_off_unless_report_only, checks a repair failure's event line at all three destinations and its power-off status, with and without report-only mode.Runtime::repair_failure:commands.rs:repair_failed, which the nine repair failures ofrepair_restore(run bycapture) and the two ofrestore-finishcall, now calls it and still returnsCommandError::Operation;daemon.rs:restore_work's failed grace-period release,start_restore's unreadable restore record and failed fork,release_suppression, andreap_children's abnormal end of the restore worker.daemon.rs:restore_workcallsrelease_suppressioninstead of an inline copy of its body, and the comment ofrelease_suppressionnow covers all three of its uses.Behavior
/dev/kmsgis opened instead of after.restore_work's inline copy andrelease_suppressionwere identical, so the call runs the same code.FakeOpsand the new test are test-only.RealOpsis the only production implementation ofPrivilegedOps.Follow-ups
These turned up in the survey; this change leaves them alone.
daemon.rsstill passesPowerOffStatus::Violation,EventSource::Watcher, andPhase::Runtimetofail_fatalat its three watcher sites inwatch_kernel_loganddiscipline_poll. ARuntime::watcher_violation(code, detail)could serve them, and the new seam lets unit tests reach it andChecks::fail.TestSysteminseam.rsnow that nvx-rs: replace the loopback System wrapper with a spawn hook #607 has landed;SpawnDescriptors::managedand the test-onlySpawnDescriptors::plaininguest/sys/spawn.rs, which differ only ininherit_config;cmd_sampleandcmd_generation_idinguest/time/commands.rsreport with the same four lines;device_iohelper intests/guest_tools.rs, which repeats the spawn and output collection ofrun;Instance::runtimeandInstance::launchinhost/instance.rs;guest/sys/socket.rs;run_fileandrun_networkinguest/device_io.rs;reseed.rs,time/mod.rs, andtime_probe/mod.rsland.I found no suspected bug in the code that this change touches.
Validation
BASE:
da399442504e5a46d0064a0b95ae340d0dc688ed, which wasorigin/devwhen the run started: the merge of nvx-rs: compute the outermost export paths with one function #606. Before this pull request was opened,origin/devhad advanced only by the merge of nvx-rs: replace the loopback System wrapper with a spawn hook #607 and a performance-baseline update, neither of which changes the three files, and no open pull request todevchanged them.TREE:
23b0be07194f64ef4695245be6f91ad1b8e82204, the tree of this commit. Every host applied the patch to BASE and checked thatgit write-treeequals TREE.Release on every host:
v0.1.0-dev.7ae815488b27. BASE has no release yet, sodownloadtook the one from two first-parent commits earlier. Its OpenVMM,5ab8a84621524f1777b2cb5e65a70dab42c8b453, is the pinned revision.Guest artifacts were rebuilt from TREE on prometheus30, the Linux host where
unshare --user --map-root-userworks, with these steps:build-guest-binaries --output;test-guest-binaries --binaries, where 34 tests passed;build-initramfswithNVX_GUEST_BINARIES.initramfs.cpio.gz(SHA-25658752c75c699bc46508d4201fafb6b054d859c74d719acd145219e1fefc23664), its packages manifest, and its provenance went to the other two hosts, where the SHA-256 digests matched.Local checks on Windows with Rust 1.95.0, cross-checked for Linux because the guest code builds only there:
-D warningsfor the guest build (x86_64-unknown-linux-musl,--no-default-features --features guest, all targets), and forx86_64-unknown-linux-gnuwith all targets and features and for its library without default features;-D warningsand the MSRV (1.89) check forx86_64-unknown-linux-gnu;aarch64-apple-darwin) checks, for bothnvx-rsandaci_edge_sandboxes.The new unit test ran in both Linux test runs on prometheus32: the glibc run with all features and the musl run of
check-guest-binaries.An earlier tree of this change,
bf58263e9c81cabc2645f52be4e356029c530227, had the refactor without the seam and the test. It passed the same no-hypervisor gates on prometheus32 and prometheus28 before I added the seam that the earlier runs asked for. Every gate and test below ran again on the final TREE.test-aci-edge-sandboxestest-microvm, default suitecheck-guest-binaries(279 musl library tests and 34 guest-binary tests), 545 library tests with all features, artifact bundling, and agent bundling; the macOS checks ran locallyEach host's time-ABI evidence line counted 18 restore markers, with no restore over its CPU-time budget, so
restore_workandrelease_suppressionran in guests on every backend. On prometheus32 the line also counted 1 of 58 boot checks over KVM's CPU-time budget, as it did for #606 on this host. The suite reports that count but never gates on it, and the boot checks do not reach the changed code unless they fail.Left to CI:
test-openvmm-unitandtest-openvmm;smp-lapic,managed-exec-config, andsandbox-lifecycle;