Repository navigation
nvx-rs: run the guest programs' main through one shared function - #679
Merged
Merged
Conversation
Eight of the nine guest programs repeated the same main: restore the SIGPIPE disposition that the process inherited, collect the process's arguments, run the program, and turn its outcome into the exit code that ExitStatus selects. Four copies were identical, and the others differed only in the devices and output streams that they pass on. status::run_main now holds that sequence, next to the ExitStatus and CommandError that it maps. Each program's main hands it a closure that calls the program's run function with what is the program's own, such as /dev/port, /dev/mem, or /dev/random and its output streams, so that main states only that, and no program can skip a step or map its outcome another way. The test modules that convert outcomes themselves now import ExitStatus, which their parents no longer use. Behavior is unchanged: run_main makes the same calls in the same order, restore_sigpipe, then args, then the program's run function with the same arguments, and converts the outcome with the same From implementations. The output adapters that the closures create now drop before the outcome converts, but none of them buffers anything or acts when dropped. nvx-managed-agent keeps its main, because an open pull request changes its module. The guest_tools integration tests run each built program and cover the entry point: usage errors exit with 2, failures with 1, and nvx-console-pending and nvx-device-io end on SIGPIPE at a closed pipe unless the disposition was ignored across exec. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused extraction preserves call order, arguments, and exit-code mapping, with no unresolved findings.
No open issues 🎉
What changed in this PR
Centralizes startup and exit-code handling for eight NVX guest programs without changing their behavior.
Changes:
- Adds
status::run_mainto restore SIGPIPE, collect arguments, and map outcomes to exit codes. - Replaces duplicated entry-point logic while preserving each program’s devices and streams.
- Moves
ExitStatusimports into tests where needed.
| File | Description |
|---|---|
nvx-rs/src/guest/time/mod.rs |
Delegates startup to run_main. |
nvx-rs/src/guest/time_probe/mod.rs |
Uses shared startup and updates test imports. |
nvx-rs/src/guest/status.rs |
Defines and documents run_main. |
nvx-rs/src/guest/reseed.rs |
Uses shared startup, retaining /dev/random. |
nvx-rs/src/guest/mmio_write.rs |
Uses shared startup, retaining /dev/mem. |
nvx-rs/src/guest/ingress/mod.rs |
Delegates startup to run_main. |
nvx-rs/src/guest/exit.rs |
Uses shared startup, retaining PORT_DEVICE. |
nvx-rs/src/guest/device_io.rs |
Uses shared startup, retaining direct output. |
nvx-rs/src/guest/console_pending.rs |
Uses shared startup and updates test imports. |
Review effort: Balanced
This was referenced Oct 21, 2026
Pedro Henrique Penna (ppenna)
deleted the
nvx-rs-quality/guest-run-main
branch
October 21, 2026 14:54
This was referenced Oct 21, 2026
Closed
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
Eight of the nine guest programs repeated the same
main: restore theSIGPIPEdisposition that the process inherited, collect the process's arguments, run the program, and turn itsResult<(), CommandError>into the exit code thatExitStatusselects. Four copies were identical nine-line blocks, and the other four differed only in the devices and output streams that they pass on.This adds
status::run_main, which holds that sequence once, next to theExitStatusandCommandErrorthat it maps. Each program'smainnow hands it a closure that calls the program'srunwith what is the program's own, somainstates only that, and no program can skip a step or map its outcome another way.Changes
nvx-rs/src/guest/status.rs: addsrun_main(run: impl FnOnce(&[CString]) -> Result<(), CommandError>) -> ExitCode, which callssys::restore_sigpipe(), passessys::args()torun, and returnsExitStatus::from(outcome).into(). The module documentation names it.console_pending.rs,ingress/mod.rs,time/mod.rs, andtime_probe/mod.rs: their identicalmainbodies becomerun_main(|args| run(args, &mut sys::Stdout::new(), &mut io::stderr())).reseed.rs: the same, withc"/dev/random"first.exit.rs(PORT_DEVICE),device_io.rs(&mut sys::DirectStdout), andmmio_write.rs(c"/dev/mem"and&mut io::stderr()): their one-line bodies callrun_mainthe same way.console_pending,device_io,exit,mmio_write,reseed, andtime_probeconvert outcomes withExitStatusthemselves, so they import it now that their parent modules no longer use it.git diff --shortstat: 9 files changed, 41 insertions(+), 51 deletions(-).Behavior
Unchanged:
run_mainmakes the same calls in the same order as eachmaindid:sys::restore_sigpipe(), thensys::args(), then the program'srunwith the same arguments and streams, then the sameFrom<Result<(), CommandError>> for ExitStatusandFrom<ExitStatus> for ExitCodeconversions. Command lines, exit statuses, and output are those of BASE.sys::Stdout,sys::DirectStdout, andio::Stderrnow drop when the closure returns, before the outcome converts, rather than at the end ofmain's statement. None of them buffers anything or acts when dropped (sys::Stdoutonly records whether standard output is a terminal, and standard error is unbuffered), so nothing observable moves.nvx-rsCLI is built with--features cli, which does not compile theguestmodule, so it is unaffected.tests/guest_tools.rsruns each built program and covers the entry point: usage errors exit with 2 and failures with 1, andnvx-console-pendingandnvx-device-ioend onSIGPIPEat a closed pipe unless the disposition was ignored acrossexec. A unit test ofrun_mainwould reset the test process's ownSIGPIPEdisposition, so it has none; those integration tests cover it.Follow-ups
nvx-managed-agent'smaininguest/managed_agent/mod.rscan callrun_maintoo once nvx-rs: add caller-owned guest cgroup and child-status APIs #655, which changes that file, has merged. ItsmaincreatesRealSystembefore it reads the arguments, so that change should confirm that creating it inside the closure is harmless.[program, arguments...]vector ofCStrings with a helper of their own (argsindevice_io,exit,time, andtime_probe, and inline inconsole_pending,mmio_write, andreseed's fixture). One test helper besidesys::argscould replace them;guest/sys/mod.rsis changed by open pull requests (nvx-rs: add caller-owned guest cgroup and child-status APIs #655 and nvx-rs: hold the guest port permission for one transaction at a time #662), so this one leaves them.nvx-time-probe'slatency,warp, andclockname themselves in 11 calls ofbad_option(b"COMMAND", ..., err);Argumentscould carry its command name and report an invalid option or value itself.Validation
BASE:
916a61f0297900db13d7d9d04d170f01ef9ed71f,origin/devwhen the run started and still when this pull request was opened: a performance-baseline update after the merge of nvx-rs: run the control session on the shared NVXS session's host role #641. No open pull request todevchanges the files of this one.TREE:
f9932126516d0fa6cf0cac0ebc940cee07a18a9e, 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.dad3e41d7f05, from the merge of nvx-rs: run the control session on the shared NVXS session's host role #641, the nearest first-parent ancestor of BASE that had packages, two commits earlier. Its OpenVMM,d0bdbed8f6e3b86271277dc22658109e2e633b2d, is the pinned revision.downloadinstalled the release'snvx-rs, whose provenance names BASE'snvx-rssources. TREE changes only theguestmodule, which the CLI'sclifeature does not compile. Each host built theopenvmm_e2eandmanaged_lifecycletest binaries after its other Cargo work, and its hypervisor test logs show no compilation.Guest artifacts were rebuilt from TREE on prometheus30, whose user namespaces let Alpine's apk run its scripts:
build-guest-binaries --output;test-guest-binaries --binaries, where 34 tests passed;build-initramfswithNVX_GUEST_BINARIES.initramfs.cpio.gz(SHA-256e4fb1eb5ccff8de2584849992bbcaca7d18ea08db6b27a19bfbce588443fa749), its packages manifest, and its provenance went through this machine to the other two hosts, where the SHA-256 digests matched.Local checks on Windows with Rust 1.95.0:
-D warnings, with all targets and features, for Windows,x86_64-unknown-linux-gnu, andx86_64-unknown-linux-musl, and on the library with onlyguestfor musl;nvx,aci_edge_sandboxes, and the runtime adapter's protocol-only and host-only configurations; the macOS (aarch64-apple-darwin) checks ofnvxandaci_edge_sandboxes; and the MSRV check of the runtime adapter's guest-only configuration, which this machine ran as the action defines it, starting with itsrustup target add, which gave the local 1.89 the musl target; I removed the target again afterwards.The change adds, moves, and deletes no file and changes no Markdown, so the CLI tests of validate-nvx and the link check had nothing to cover.
One deviation, now undone: my first probe of prometheus28 listed the targets of
1.89, which that host did not have, and rustup installed it to answer. I uninstalled it at once, before any gate ran, so the host keeps only its own toolchains, and its gates skipped the MSRV steps, which ran on this machine.test-aci-edge-sandboxestest-microvm, default suitecheck-guest-binaries(398 tests), 754nvxtests with all features,guest_toolsamong them, and 362 with the default ones, artifact bundling (15), the runtime adapter's protocol-only, host-only, and musl guest-only configurations (13, 14, and 21 tests), 158 and 85aci_edge_sandboxestests, and agent bundling (4). The macOS checks and the guest-only MSRV check ran locallynvxtests with all features and 358 with the default ones, artifact bundling (15), the runtime adapter's protocol-only and host-only configurations (13 and 14 tests), 158 and 85aci_edge_sandboxestests, and agent bundling (4). The MSRV checks ran locallyEvery gate and test passed at its first attempt. The Linux gates ran in the foreground of their SSH sessions with no signal ignored (
SigIgn: 0), so the CLI's Ctrl-C tests ran as intended. Each suite'smanaged-lifecyclescenario ran themanaged_lifecycletest with Cargo, which passed. OpenVMM warned of a host CPU-profile fallback only in thecpu-profile-fallbackscenario, which forces one.The time-ABI evidence lines counted no capture or restore check over its CPU-time budget on any host, and no boot check over budget on prometheus32 or prometheus28. On prometheus30, one of 58 boot checks, in the capture of the 4-vCPU
smp-snapshotscenario, used 6,114 µs of CPU against its 6,000 µs budget; CI reports these counts and gates on none of them. Rerunning that case on the same host measured 4,089 µs, and the change adds no work tonvx-time's checks: itsmainmakes the same calls as before.Left to CI:
test-openvmm-unitandtest-openvmm;smp-lapic,managed-exec-config, andsandbox-lifecycle;