[Pedro Henrique Penna (ppenna)]

Summary

nvx-console-pending, nvx-mmio-write, and nvx-reseed report each failed system call with a reporter and then fail with CommandError::Operation. Their reporters returned nothing, so ten call sites repeated both steps in a block of four lines, and reseed() wrote read_seed's nvx-reseed: open PATH: REASON diagnostic a second time, inline, for /dev/random.

Each reporter now returns the CommandError::Operation with which its caller fails, as ingress::usage and managed_agent::fatal already return their failures, so each site is one expression: .map_err(|error| perror(err, b"open /dev/mem", error))?, or return Err(file_failure(...)). In reseed.rs, a function, file_failure, replaces read_seed's closure and also reports reseed()'s failure to open /dev/random, so that diagnostic has one definition. The three programs lose 21 lines.

This is the ninth nvx-rs-quality/ pull request. #603, #605, #606, #607, #608, #609, #611, and #613 merged, #613 while this run was underway; all had approving Copilot reviews and no other feedback to follow, and none was closed without merging. Those runs changed protocol/exec.rs, the managed agent's exec.rs, loopback.rs, and seam.rs, host/filesystem.rs, the time tools, guest/sys/socket.rs and spawn.rs, and testing/fake_openvmm.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 to dev changed when it started: #613 (nvx-rs/src/testing/fake_openvmm.rs), #610 (aci_edge_sandboxes/README.md and doc/project-structure.md), and #585 (aci_edge_sandboxes/src/async_api.rs and tests/async_api.rs).

Of the candidates that it found, this one, a follow-up that #609, #611, and #613 listed, simplifies production code of three guest programs that no earlier run touched, and brings them to a convention that two other programs already follow. The argument-vector builders of the program tests, under Follow-ups, would remove somewhat more per changed line, but only from tests; where the choice was close, I took the change to the shipped programs. The other candidates are listed under Follow-ups too.

Changes

All changes are in nvx-rs/src/guest/: 34 insertions and 55 deletions in three files.

• console_pending.rs: the fail closure returns CommandError::Operation. The sites that now call it in one expression are the open of the device, its FIONREAD ioctl through sys::console_pending, and its close.
• mmio_write.rs: perror returns CommandError::Operation. The sites are the open, mmap, munmap, and close of /dev/mem.
• reseed.rs: file_failure(err, operation, path, reason), which reports nvx-reseed: OPERATION PATH: REASON and returns CommandError::Operation, replaces read_seed's fail closure. The sites that now share it are:
  • reseed()'s open of /dev/random, which wrote the same diagnostic inline;
  • read_seed's open of the seed file, and its end-of-file and read-error failures.

reseed()'s random ioctl failure, whose diagnostic names no path, and the diagnostics that do not report an errno, such as the page-boundary check, keep their own code. No signature, public item, message, error kind, exit status, protocol byte, fixture, or other file changes.

Behavior

• Each site writes the same bytes in the same single sys::report write, which writes parts.concat() at once, and fails with the same CommandError::Operation, exit status 1. reseed()'s open failure wrote [b"nvx-reseed: open ", path, b": ", text, b"\n"] and now writes [b"nvx-reseed: ", b"open", b" ", path, b": ", reason, b"\n"], the same bytes.

• A reporter still runs only on its failure path, and nothing runs between the report and the return.

• Only the guest programs, which build for Linux with the guest feature, change: no host code, no code that the host and the guest share, and nothing in the --no-default-features build. The command lines, exit statuses, and byte output of every guest binary are unchanged.

• Tests that cover the change:

  • guest::console_pending::tests::reports_failures_with_the_errno_description: the open and FIONREAD failures;
  • guest::mmio_write::tests::reports_failures_as_perror: the open and mmap failures;
  • guest::reseed::tests::reports_seeds_that_cannot_be_read: the seed file's open, end-of-file, and read failures;
  • guest::reseed::tests::reports_a_failed_reseed: the failure to open /dev/random, with the expected-message helper of the seed file's failures;
  • in tests/guest_tools.rs, on the built binaries, console_pending_reports_the_unread_bytes, an open failure, and mmio_write_reports_an_inaccessible_dev_mem, which checks the open failure where /dev/mem is inaccessible, as it is to an unprivileged user.

  They ran and passed in the Linux test run with all features on prometheus32 and in its musl run of check-guest-binaries, and the guest_tools tests also against the guest binaries built from TREE on prometheus30. The close and munmap failures have no test, as before, since a test cannot make them fail.

Follow-ups

The survey turned these up, and this change leaves them alone. I found no suspected bug.

• The diagnostics that do not report an errno still pair sys::report with return Err(CommandError::Operation): mmio_write's invalid page size and its two page-boundary checks, console_pending's failed write, and, in reseed.rs, the random ioctl failure, the three generation-ID checks, and the output and getrandom failures. A reporter in guest/status.rs that returns the failure could serve them, and the time tools' similar sites.
• The unit tests of console_pending, exit, device_io, mmio_write, reseed, time, and time_probe build a program's argument vector of CStrings in the same way, in about 50 lines. A helper in guest::sys::tests could serve them all now that Ship nvx-rs in NVX releases #598, which changed reseed.rs, time/mod.rs, and time_probe/mod.rs, has landed.
• The main of each of the nine guest programs restores SIGPIPE and converts its outcome with ExitStatus::from(...).into(); four of them are identical. One entry helper in guest/status.rs could hold that convention.
• In host/:
  • in sandbox/mount.rs, MountAccess, MountOwner, and MountKind repeat from_name with the same message form, and sandbox/mount/tests.rs and sandbox/tests.rs define the same policy test helper;
  • in session.rs, features, map_host_paths, and next_exec_event repeat the request-ID check, map_host_paths and next_exec_event truncate an error category to 64 characters the same way, ping and stop differ only in their request and reply kinds and their message, and the tests' Scripted and Responder transports have identical read methods;
  • process::tests::a_process_runs_until_its_last_thread_exits and cli::sandbox::identity::tests::a_vm_runs_until_its_last_thread_exits repeat the fixture of a Python process whose main thread exits first;
  • filesystem.rs, network/mod.rs, and sandbox/options/tests.rs each define the same option-values test helper;
  • Instance::runtime and Instance::launch in instance.rs read a record and refuse another STATE_FORMAT in two shapes.
• These follow-ups of earlier runs also remain:
  • the managed-agent test fixtures: cgroup::tests::setup and dispatch::tests::direct_system are identical, dispatch::tests::maps_payload and mappings::tests::payload are the same MAPS encoder, and dispatch::tests::mapping_system builds the host-export tree of mappings::tests::setup;
  • run_file and run_network in guest/device_io.rs, and the device_io helper in tests/guest_tools.rs, which repeats the spawn and output collection of run;
  • the time tools' three watcher-violation sites in daemon.rs, and the port-window lock failures that cmd_sample and cmd_generation_id in commands.rs report with the same four lines;
  • the two MAPS decoders of protocol/maps.rs, whose differences nvx-rs: decode the EXEC header once for both payload decoders #603 described, which want a decision before they share code.

Validation

• BASE: 131786652a20048ed4b951580cc025bdae40238c, which was origin/dev when the run started: a performance-baseline update on top of the merge of nvx-rs: build the plain spawn descriptors from the managed ones #611. Before this pull request was opened, origin/dev had advanced by the merge of nvx-rs: parse fake OpenVMM --mount-child values with one function #613 and another baseline update, neither of which changes the three files, and no open pull request to dev changes them.

• TREE: d3ebd6da782964648e30b282e1050c4a9b6d40db, 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.5fd538e726d2, from the merge of setup: integrate reusable runner setup and Azure VMSS bootstrap #522, the nearest first-parent ancestor of BASE that had packages. Its OpenVMM, d0bdbed8f6e3b86271277dc22658109e2e633b2d, is the pinned revision.

• download did not install the release's nvx-rs, whose sources differ, so each host built nvx-rs from TREE with Cargo and recorded its provenance. Each also pre-built the managed_lifecycle and openvmm_e2e test binaries, so that no Cargo build overlapped its hypervisor tests.

• Guest artifacts were rebuilt from TREE on prometheus30, whose user namespaces let Alpine's apk run its scripts, 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 4ef90a58ebbc2abfaf47474147972eb2d0cfe57d087c4d7af58bcecff4207a00), 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:

  • rustfmt;
  • Clippy with -D warnings: for x86_64-unknown-linux-gnu, which builds the guest module, with all targets and features and for the library without default features, and for the guest build on x86_64-unknown-linux-musl, as check-guest-binaries lints it;
  • the 15 unit tests of the three modules, in WSL;
  • 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 (320 tests), 609 nvx-rs tests with all features and 263 with the default ones, artifact bundling, 157 and 85 aci_edge_sandboxes tests, and agent bundling; the macOS checks ran locally	Copied in, digests match	Pass (18/18)	Pass (31 scenarios)
prometheus30	mshv	baremetal	None; prometheus32 and prometheus28 ran the gates	Built and tested here	Pass (18/18)	Pass (31 scenarios)
prometheus28	whp	baremetal	Pass: every Windows step, including 331 nvx-rs tests with all features and 263 with the default ones, artifact bundling, 157 and 85 aci_edge_sandboxes tests, and agent bundling; the MSRV checks ran locally	Copied in, digests match	Pass (18/18)	Pass (31 scenarios)

The 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 outside the cpu-profile-fallback scenario, which forces that fallback. The time-ABI evidence lines counted no capture or restore over its CPU-time budget, and no boot over it on prometheus30 and prometheus28. On prometheus32, one boot of 58, that of the filesystem-read-only scenario at 1 vCPU, spent 8,593 µs in nvx-time's boot check against KVM's 6,000 µs budget; the suite reports that count without gating on it, and this change does not touch the time tools.

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

[Copilot]

🟢 Approval recommended

The focused refactor preserves failure behavior and cleanup order, with no unresolved blocking issues.

No open issues 🎉

What changed in this PR

Simplifies failure handling in three NVX guest tools while preserving diagnostics, returned errors, and exit statuses.

Changes:

• Makes failure reporters return CommandError::Operation, shortening ten call sites.
• Shares reseed file-error reporting between seed reads and opening /dev/random.

File	Description
nvx-rs/​src/​guest/​reseed.rs	Extracts a shared file-failure reporter.
nvx-rs/​src/​guest/​mmio_write.rs	Returns errors directly from perror.
nvx-rs/​src/​guest/​console_pending.rs	Returns errors directly from the failure closure.

_{Review effort: Balanced}