nvx-rs: track open workload output pipes by their Option alone

[Pedro Henrique Penna (ppenna)]

Summary

The managed agent's exec loop, run in nvx-rs/src/guest/managed_agent/exec.rs, recorded each workload output pipe twice: as an Option<FileDescriptor> that it set to None at end of file, and as a stdout_open or stderr_open flag that it cleared at the same moment. Each check consulted the flag and then unwrapped the Option with an expect that could not fail, and the 15-line block that reads a ready pipe, streaming its output to the host or draining it once the session is lost, appeared once per pipe. This change makes the Option the only record of an open pipe and reads both pipes through one closure, so the loop loses 35 lines and no flag can disagree with its pipe.

This is the second nvx-rs-quality/ run. The first pull request, #603, is open, with an approving Copilot review and no feedback to follow; it changed protocol/exec.rs. This run surveyed the rest of the crate, by reading and with a throwaway duplicate-window script, skipped every file that an open pull request to dev changes (#585, #596, #597, #598, and #603), and picked the candidate with the most value per changed line among those that existing tests cover.

Changes

All changes are in run in nvx-rs/src/guest/managed_agent/exec.rs: 10 insertions and 45 deletions. No public item, test, fixture, or other file changes.

• stdout_open and stderr_open are gone. The loop condition and the straggler-kill condition test stdout_pipe.is_some() and stderr_pipe.is_some(), and the poll set passes stdout_pipe.as_ref() and stderr_pipe.as_ref() directly.

• One closure, read_output(pipe, readable, kind), replaces the two read-or-drain blocks. Both pipes now share it:

  • stdout, with ready[0] and APP_STDOUT;
  • stderr, with ready[1] and APP_STDERR.

  It matches on the Option instead of calling expect("stdout reader is open") and expect("stderr reader is open").

Behavior

• Each flag was true exactly while its pipe was Some: both started open, and the only writes set a pipe to None and its flag to false together. Every condition and the poll set therefore evaluate as before.
• The closure makes the same choice as the blocks that it replaces. A pipe that is open and that poll reported ready is drained once the session is lost, and otherwise streamed with the same request ID, record kind, and shared output_bytes budget; any other pipe yields StreamResult::Open and stays open. stdout is still read before stderr, so the order of output records and the output-limit accounting are unchanged.
• The removed expect calls could not fire, so no reachable panic path changes. nvx-managed-agent keeps its command line, exit statuses, and protocol bytes.
• The loopback test of the real agent, control_session_broker_and_real_agent_loop_back, covers the loop on Linux: it streams stdout and stderr, and ends workloads by exit, signal, real-time signal, timeout, output limit, cancellation, and a session reset, after which the agent drains the pipes. The managed microVM lifecycle scenario and test-aci-edge-sandboxes cover it on every backend.

Follow-ups

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

• guest/time/daemon.rs repeats runtime.fail_fatal(PowerOffStatus::Repair, ViolationCode::RepairSuppression, EventSource::Repair, Phase::Restore, ...) six times, and restore_work inlines the body of release_suppression. A Runtime::repair_failure, beside Runtime::conformance_failure, could serve those calls and commands::repair_failed. No unit test reaches these paths, because Runtime::emit_event writes to /dev/kmsg and /dev/console, so the refactor wants a test seam first.

• The managed-agent tests repeat fixtures:

  • cgroup::tests::setup and dispatch::tests::direct_system are identical;
  • mappings::tests::payload and dispatch::tests::maps_payload are identical;
  • mappings::tests::setup builds the same host-export tree as dispatch::tests::mapping_system.

  Shared constructors next to TestSystem in seam.rs could serve all of them.

• loopback::tests::LoopbackSystem forwards every System method to TestSystem except spawn; a spawn hook on TestSystem would remove about 50 lines.

• The unit tests of console_pending, exit, device_io, mmio_write, reseed, time, and time_probe each build a program's argument vector of CStrings the same way. A helper in guest::sys::tests could serve them once the open pull requests that change reseed.rs, time/mod.rs, and time_probe/mod.rs land.

Validation

• BASE: 42c962e47ad4a64dd7511f78969b28745697fbc8, which was origin/dev when the run started. Before this pull request was opened, origin/dev had advanced only by the merge of sandbox: accept only the distro layer and scratch #576 and a performance-baseline update, neither of which changes nvx-rs or aci_edge_sandboxes, and no open pull request to dev changed nvx-rs/src/guest/managed_agent/exec.rs.

• TREE: cd604cd3cc6bb275b90b9130c76889a4d9eebb9a, 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.f7e8e3e46859. BASE, a [skip ci] data commit, has no release, so download took the one from the first-parent commit before it. Its OpenVMM, 5ab8a84621524f1777b2cb5e65a70dab42c8b453, is the pinned revision.

• Guest artifacts were rebuilt from TREE on prometheus30, 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 84c49b429a0ff5c2ae6d56b0fd4ddb0da3730840609a0dcf912f9d75b6da8b3f), 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:

  • rustfmt;
  • Clippy with -D warnings for the guest binaries' configuration (x86_64-unknown-linux-musl, --no-default-features --features guest, all targets), for x86_64-unknown-linux-gnu with all targets and features and for its library without default features, and for Windows likewise;
  • cargo test --all-features on Windows;
  • 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.

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 (277 + 34 musl tests), the loopback agent test among 492 library tests, artifact bundling, and agent bundling; the macOS checks ran locally	Copied in, digests match	Pass (18/18)	Pass (42 scenario runs)
prometheus30	mshv	baremetal	None; prometheus32 and prometheus28 ran the gates	Built and tested here	Pass (18/18)	Pass (42 scenario runs)
prometheus28	whp	baremetal	Pass: every Windows step, including artifact bundling and agent bundling; the MSRV checks ran locally	Copied in, digests match	Pass (18/18)	Pass (42 scenario runs)

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 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.

[Copilot]

🟢 Approval recommended

The focused refactor preserves existing behavior, with no blocking issues identified; tests were not rerun.

No open issues 🎉

What changed in this PR

Simplifies the managed agent’s execution loop without changing output handling or termination behavior.

Changes:

• Uses each pipe’s Option as the sole open-state indicator.
• Shares stdout and stderr read-or-drain logic while preserving output order and limit accounting.

File	Description
nvx-rs/​src/​guest/​managed_agent/​exec.rs	Removes redundant pipe-state flags and consolidates output handling.

_{Review effort: Balanced}