Skip to content

nvx-rs: report time-ABI repair failures through one Runtime method - #608

Merged
Pedro Henrique Penna (ppenna) merged 1 commit into
devfrom
nvx-rs-quality/repair-failure
Oct 19, 2026
Merged

Pedro Henrique Penna (ppenna) merged 1 commit into
devfrom
nvx-rs-quality/repair-failure

Conversation

@ppenna

Copy link
Copy Markdown
Contributor

Summary

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 (195) unless the time tools run report-only. The guest's time tools reported it in two ways:

  • commands::repair_failed passed that status, source, and phase to Runtime::fail_fatal for the eleven repair failures of nvx-time capture and nvx-time restore-finish;
  • guest/time/daemon.rs spelled all four arguments out at six sites, each with G_REPAIR_SUPPRESSION, and restore_work inlined an exact copy of release_suppression.

This change adds Runtime::repair_failure(code, detail), beside Runtime::conformance_failure, and routes all of them through it, so the status, source, and phase of a repair failure live in one place. restore_work now calls release_suppression.

#605, #606, and #607 each listed this refactor as a follow-up that waited for a test seam: Runtime::emit_event opened /dev/kmsg and /dev/console itself, so no unit test could observe an event. This change adds that seam to PrivilegedOps, the trait that already lets the tests' FakeOps stand in for the time tools' ports, MSRs, clock, and power-off. RealOps keeps the device writes, FakeOps records 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 to dev changed when it started, including those of #585, #597, #598, and #607 in the two crates. It picked guest/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:
    • PrivilegedOps gains write_kernel_log(record) and write_console(separator, line). RealOps implements them with the code that Runtime::emit_event and its private console_write ran, which move there unchanged.
    • Runtime::emit_event calls them in the order in which it wrote before: the kernel log, then port B, then the console.
    • Runtime::repair_failure(code, detail) calls fail_fatal with PowerOffStatus::Repair, EventSource::Repair, and Phase::Restore.
    • The tests' FakeOps records 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.
  • Sites that now share Runtime::repair_failure:
    • commands.rs: repair_failed, which the nine repair failures of repair_restore (run by capture) and the two of restore-finish call, now calls it and still returns CommandError::Operation;
    • daemon.rs: restore_work's failed grace-period release, start_restore's unreadable restore record and failed fork, release_suppression, and reap_children's abnormal end of the restore worker.
  • daemon.rs: restore_work calls release_suppression instead of an inline copy of its body, and the comment of release_suppression now covers all three of its uses.

Behavior

  • Every repair failure passes the same status, code, source, phase, and detail as before, so every event line, power-off status, and command exit status is unchanged.
  • An event writes the same bytes, with the same open flags and the same standard-error fallback, to the same three destinations in the same order. It reads the asynchronous-output flag at the same two points: before the port writes and before the console write. The only difference is internal: the kernel log record is built before /dev/kmsg is opened instead of after.
  • restore_work's inline copy and release_suppression were identical, so the call runs the same code.
  • FakeOps and the new test are test-only. RealOps is the only production implementation of PrivilegedOps.

Follow-ups

These turned up in the survey; this change leaves them alone.

  • daemon.rs still passes PowerOffStatus::Violation, EventSource::Watcher, and Phase::Runtime to fail_fatal at its three watcher sites in watch_kernel_log and discipline_poll. A Runtime::watcher_violation(code, detail) could serve them, and the new seam lets unit tests reach it and Checks::fail.
  • These follow-ups of earlier runs remain:
    • the managed-agent test fixtures, which can share constructors next to TestSystem in seam.rs now that nvx-rs: replace the loopback System wrapper with a spawn hook #607 has landed;
    • SpawnDescriptors::managed and the test-only SpawnDescriptors::plain in guest/sys/spawn.rs, which differ only in inherit_config;
    • the port-window lock failures that cmd_sample and cmd_generation_id in guest/time/commands.rs report with the same four lines;
    • the device_io helper in tests/guest_tools.rs, which repeats the spawn and output collection of run;
    • Instance::runtime and Instance::launch in host/instance.rs;
    • the repeated datagram socket creation in guest/sys/socket.rs;
    • run_file and run_network in guest/device_io.rs;
    • the program tests' argument-vector helpers, which can wait until the open pull requests that change reseed.rs, time/mod.rs, and time_probe/mod.rs land.

I found no suspected bug in the code that this change touches.

Validation

  • BASE: da399442504e5a46d0064a0b95ae340d0dc688ed, which was origin/dev when the run started: the merge of nvx-rs: compute the outermost export paths with one function #606. Before this pull request was opened, origin/dev had 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 to dev changed them.

  • TREE: 23b0be07194f64ef4695245be6f91ad1b8e82204, the tree of this commit. Every host applied the patch to BASE and checked that git write-tree equals TREE.

  • Release on every host: v0.1.0-dev.7ae815488b27. BASE has no release yet, so download took 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-user works, with these steps:

    • build-guest-binaries --output;
    • test-guest-binaries --binaries, where 34 tests passed;
    • the Alpine build-initramfs with NVX_GUEST_BINARIES.

    initramfs.cpio.gz (SHA-256 58752c75c699bc46508d4201fafb6b054d859c74d719acd145219e1fefc23664), 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:

    • rustfmt;
    • Clippy with -D warnings for the guest build (x86_64-unknown-linux-musl, --no-default-features --features guest, all targets), and for x86_64-unknown-linux-gnu with all targets and features and for its library without default features;
    • rustdoc with -D warnings and the MSRV (1.89) check for x86_64-unknown-linux-gnu;
    • the steps that no host could run, because the hosts lack the toolchain or target: the Windows MSRV (1.89) checks and the macOS (aarch64-apple-darwin) checks, for both nvx-rs and aci_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.

Host profile Backend Host type check-nvx-rs and check-aci-edge-sandboxes without a hypervisor Guest artifacts test-aci-edge-sandboxes test-microvm, default suite
prometheus32 kvm baremetal Pass: every Linux step, including MSRV 1.89, check-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 locally Copied in, digests match Pass (18/18) Pass (41 scenario runs)
prometheus30 mshv baremetal None; prometheus32 and prometheus28 ran the gates Built and tested here Pass (18/18) Pass (41 scenario runs)
prometheus28 whp baremetal Pass: every Windows step, including 314 library tests with all features, artifact bundling, and agent bundling; the MSRV checks ran locally Copied in, digests match Pass (18/18) Pass (41 scenario runs)

Each host's time-ABI evidence line counted 18 restore markers, with no restore over its CPU-time budget, so restore_work and release_suppression ran 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:

  • the OpenVMM unit and VMM suites, test-openvmm-unit and test-openvmm;
  • the microVM scenarios that run only when named: smp-lapic, managed-exec-config, and sandbox-lifecycle;
  • the debug-kernel restore scenarios, the Ubuntu and Azure Linux guests, and guest determinism;
  • the benchmarks;
  • the virtual-machine runners;
  • check-quality and validate-nvx, whose inputs this change does not touch.

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>
Copilot AI balanced review requested due to automatic review settings October 19, 2026 17:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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_failure and routes command and daemon failures through it.
  • Reuses release_suppression for 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants