Repository navigation
nvx-rs: build the plain spawn descriptors from the managed ones - #611
Merged
Pedro Henrique Penna (ppenna) merged 1 commit intoOct 19, 2026
Merged
Conversation
SpawnDescriptors, in guest/sys/spawn.rs, has two constructors: managed, with which the managed agent spawns every workload, and plain, with which only the loopback test spawns its workloads without privileges. plain repeated managed's construction of all seven descriptor fields and differed from it only in inherit_config, so a field added to the struct had to be added to both, and the one difference hid among the repeated fields. plain now takes managed's action set and overrides inherit_config with struct update syntax, and its comment names that difference: the child closes the configuration descriptor instead of keeping it across exec. The two constructors can no longer drift apart. Behavior is unchanged: managed is untouched, and plain returns the same value in every field as before. plain is compiled only into test builds with the host feature, so no library, CLI, or guest binary build changes. The loopback test that spawns its workloads through plain, control_session_broker_and_real_agent_loop_back, and the spawn tests, which use managed, cover both constructors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The narrow, test-only refactor preserves descriptor values and configuration-close behavior, with no unresolved findings.
No open issues 🎉
What changed in this PR
Simplifies test-only spawn descriptor construction by sharing the managed constructor while preserving plain-child behavior.
Changes:
- Reuses
managedthrough struct update syntax, retaininginherit_config: false. - Clarifies the configuration descriptor’s handling in the doc comment.
| File | Description |
|---|---|
nvx-rs/src/guest/sys/spawn.rs |
Removes duplicated descriptor initialization from plain. |
Review effort: Balanced
Pedro Henrique Penna (ppenna)
deleted the
nvx-rs-quality/plain-spawn-descriptors
branch
October 19, 2026 23:26
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
SpawnDescriptors, innvx-rs/src/guest/sys/spawn.rs, tells the forked child what to do with each descriptor of a workload. It has two constructors:managed, with which the managed agent spawns every workload;plain, compiled only into test builds with thehostfeature, with which the loopback test spawns its workloads without privileges.plainrepeatedmanaged's construction of all seven descriptor fields and differed from it only ininherit_config. It is nowmanaged's action set withinherit_config: false, written with struct update syntax, and its comment names that one difference: the child closes the configuration descriptor instead of keeping it acrossexec. A field added to the struct now needs one constructor updated, and the difference no longer hides among the repeated fields.This is the seventh
nvx-rs-quality/pull request. #603, #605, #606, #607, and #608 merged, #608 while this run was underway, and #609 is open; all had approving Copilot reviews and no other feedback to follow, and none was closed without merging. Those runs changedprotocol/exec.rs, the managed agent'sexec.rs,loopback.rs, andseam.rs,host/filesystem.rs, the time tools, andguest/sys/socket.rs. This run surveyed the crate by reading and with a throwaway duplicate-window script, exact and with names and literals abstracted, and skipped every file that an open pull request todevchanged when it started: #608 (guest/time/commands.rs,daemon.rs, andsystem.rs), #609 (guest/sys/socket.rs), #522 (tests/cli.rs), and #585 (aci_edge_sandboxes/src/async_api.rsandtests/async_api.rs). Of the candidates that it found, this one, a follow-up that #609 listed, removes the most duplicated code per changed line, in a file that no earlier run touched. The host-side candidates, in the part of the crate that earlier runs covered least, removed less per changed line; they are listed under Follow-ups.Changes
All changes are in
nvx-rs/src/guest/sys/spawn.rs: 4 insertions and 8 deletions.SpawnDescriptors::plainreturnsSelf { inherit_config: false, ..Self::managed(config, stdout, stderr, launch) }instead of listing every field, and its doc comment says how it differs frommanaged.managed's construction of the descriptor fields:SpawnDescriptors::managed, whichguest/managed_agent/launch.rscalls for every managed workload, as do the tests ofguest/sys/spawn.rs;SpawnDescriptors::plain, whichspawn_unprivilegedinguest/managed_agent/loopback.rscalls.No signature, public item, message, error kind, protocol byte, fixture, or other file changes.
Behavior
managedis unchanged.plainreturns the same value in every field as before: the same seven descriptors from the same arguments, andinherit_config: false.child_exectherefore still closes the configuration descriptor in a plain child, and still clears its close-on-exec flag in a managed one.plainis#[cfg(all(test, feature = "host"))], so no library, CLI, or guest binary build includes it. The command lines, exit statuses, and byte output of every guest binary are unchanged, and the--no-default-featuresbuild does not change.Tests that cover the change:
guest::managed_agent::loopback::tests::control_session_broker_and_real_agent_loop_backspawns its workloads throughplain;guest::sys::spawn::testsbuild their descriptors withmanaged.Both ran and passed in the Linux test run with all features on prometheus32, and the spawn tests also in the musl run of
check-guest-binaries.Follow-ups
The survey turned these up, and this change leaves them alone. I found no suspected bug.
cgroup::tests::setupanddispatch::tests::direct_systemare identical,dispatch::tests::mapping_systembuilds the host-export tree ofmappings::tests::setup, anddispatch::tests::maps_payloadandmappings::tests::payloadare the sameMAPSencoder. Constructors next toTestSysteminseam.rscould serve them, in about 160 changed lines.mainof each of the nine guest programs restoresSIGPIPEand converts its outcome withExitStatus::from(...).into(); four of them are identical. One entry helper inguest/status.rscould hold that convention.host/:process::tests::a_process_runs_until_its_last_thread_exitsandcli::sandbox::identity::tests::a_vm_runs_until_its_last_thread_exitsrepeat the fixture of a Python process whose main thread exits first, with identicalReapedguards and wait loops.session.rs,features,map_host_paths, andnext_exec_eventrepeat the request-ID check,map_host_pathsandnext_exec_eventtruncate an error category to 64 characters the same way, and the tests'ScriptedandRespondertransports have identicalreadmethods.filesystem.rs,network/mod.rs, andsandbox/options/tests.rseach define the same option-values test helper.testing/fake_openvmm.rs,sandbox_sharesandmapped_childrenparse--mount-childvalues with the same code.Instance::runtimeandInstance::launchinhost/instance.rs;console_pending.rsandmmio_write.rs;run_fileandrun_networkinguest/device_io.rs;device_iohelper intests/guest_tools.rs, which repeats the spawn and output collection ofrun;daemon.rs, and the port-window lock failures thatcmd_sampleandcmd_generation_idincommands.rsreport with the same four lines.Validation
BASE:
77f84a4d89d7dc9996c76ed71a5d1b2cc6f917e0, which wasorigin/devwhen the run started: the merge of Repin OpenVMM to microVM v2.2 #600, which moved the OpenVMM pin. Before this pull request was opened,origin/devhad advanced by the merge of nvx-rs: report time-ABI repair failures through one Runtime method #608 and a performance-baseline update, neither of which changesnvx-rs/src/guest/sys/spawn.rs, and no open pull request todevchanged it.TREE:
76b62eaccfcd0ecbcac0b735678890916c3a5de7, 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.77f84a4d89d7, BASE's own release. The newest release when the run started predated Repin OpenVMM to microVM v2.2 #600's pin, so the hosts waited until CI published this one. Its OpenVMM,d0bdbed8f6e3b86271277dc22658109e2e633b2d, is the pinned revision.Each host rebuilt
nvx-rsfrom TREE and recorded its provenance, sincenvx.pyruns thenvx-rs/target/release/nvx-rsthat it finds, and pre-built themanaged_lifecycleandopenvmm_e2etest binaries, so that no Cargo build overlapped its hypervisor tests.Guest artifacts were rebuilt from TREE on prometheus30, with these steps:
build-guest-binaries --output;test-guest-binaries --binaries, where 34 tests passed;build-initramfswithNVX_GUEST_BINARIES.initramfs.cpio.gz(SHA-2565b5c7d8aada348d371b3321e5794cd9c90e39348f3f0c8e22e8f97ce1923a080), 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:
-D warnings: forx86_64-unknown-linux-gnu, which builds theguestmodule, with all targets and features and for the library without default features; for the guest build onx86_64-unknown-linux-musl, ascheck-guest-binarieslints it; and natively, in both configurations;aarch64-apple-darwin) checks, for bothnvx-rsandaci_edge_sandboxes.test-aci-edge-sandboxestest-microvm, default suitecheck-guest-binaries(318 tests), 607nvx-rstests with all features and 263 with the default ones, artifact bundling, 157 and 85aci_edge_sandboxestests, and agent bundling; the macOS checks ran locallynvx-rstests with all features and 263 with the default ones, artifact bundling, 157 and 85aci_edge_sandboxestests, and agent bundling; the MSRV checks ran locallyThe Linux gates and tests ran in the foreground of their SSH sessions, with every signal at its default disposition (
SigIgn: 0), so the CLI's Ctrl-C tests ran as intended. Every gate and test passed at its first attempt, no host fell back to a host CPU profile, and each host's time-ABI evidence line counted no boot, capture, or restore over its CPU-time budget.The run lock and prometheus28's host lock held the ID of an earlier run that had stopped without releasing them, which its operator confirmed. Both locks were younger than the four hours after which a run takes one over, so I renamed them aside and took them over only after checking that no process ran from that host's worktree and that the other two hosts held no lock.
Left to CI:
test-openvmm-unitandtest-openvmm;smp-lapic,managed-exec-config, andsandbox-lifecycle;