Skip to content

nvx-rs: convert OSErrors without a filename to CLI exits in one place - #617

Open
Pedro Henrique Penna (ppenna) wants to merge 1 commit into
devfrom
nvx-rs-quality/cli-os-errors
Open

Pedro Henrique Penna (ppenna) wants to merge 1 commit into
devfrom
nvx-rs-quality/cli-os-errors

Conversation

@ppenna

Copy link
Copy Markdown
Contributor

Summary

The nvx-rs CLI reports an OSError as nvx.py did: as str(error), which names a file only when Python's OSError had one. For the OSErrors that name no file, 21 call sites spelled out Exit::error(os_error_message(&error, None)), mostly inside a map_err closure, and output::stream_error wrapped the same call for 5 more. This change adds Exit::from_os_error, which builds that exit once, so that most of the 26 sites now read .map_err(Exit::from_os_error), and removes output::stream_error. The CLI now says in one place how an OSError without a filename reads, and new code has one obvious way to write it.

Changes

All of the changes are in nvx-rs/src/host/cli/.

  • Exit::from_os_error (new, mod.rs) returns Exit::Error(os_error_message(&error, None)). It sits next to Exit::error and Exit::from_stop, and takes the io::Error by value so that it can be passed to map_err directly. These sites now share it:
    • sandbox/lifecycle.rs (13): prepare_state_directory, outcome_destination (2), write_new_json, launch_and_connect (2), end_failed_start (5), wait_interruptibly, and hold_directory.
    • sandbox/lock.rs (4): open_lock_file, try_lock_file, LifecycleLock::acquire, and LifecycleLock::remove.
    • sandbox/mod.rs (3): the output of run's exec and one-shot run operations, and poll.
    • session.rs (2): Session::connect and Session::failure.
    • sandbox/state.rs (1): resolved.
    • artifacts.rs (1) and egress.rs (1): run.
    • mod.rs (1): Exit::report, for help text that cannot be written.
  • output::stream_error (removed) returned os_error_message(error, None). Its five callers, in artifacts.rs, egress.rs, sandbox/mod.rs (2), and Exit::report, are among the sites above.
  • os_errors_without_a_file_read_as_python_formats_them (new test, mod.rs) pins the message for a missing path, [Errno 2] … or [WinError 3] … without the path or Rust's (os error N), and for an error without an OS code, which prints its own text. No existing test reached these messages: the golden CLI scenarios record only an OSError that names a file.
  • session.rs and output.rs no longer import os_error_message.

Behavior

  • Same values. Every site builds the same Exit as before: Exit::Error of os_error_message(&error, None). Exit::error(message) only moved its String into Exit::Error, and stream_error returned exactly os_error_message(error, None). Messages, exit statuses, and output are unchanged, on POSIX and on Windows.
  • Same evaluation. Each map_err still converts only on failure, and no site gained or lost a check, a system call, or an early return.
  • Untouched conversions. The OSErrors that name a file keep their own conversions: lifecycle.rs's os_error, lock.rs's open_error, state.rs's os_error_with_two_paths and its open-coded ones, and sandbox/mod.rs's spawn_error. So does identity.rs, which builds a String for its own error type.
  • Unchanged interfaces. Only the cli feature compiles these modules, so the library without it, aci_edge_sandboxes, and the guest binaries build as before. The public API, the protocol bytes, the golden fixtures, and the guest binaries' command lines, statuses, and output are unchanged.

Follow-ups

  • Five sites open-code lifecycle.rs's private os_error(error, path), the conversion of an OSError that names a file: state.rs's write_json (2) and set_private, and lock.rs's LifecycleLock::acquire and LifecycleLock::remove. Sharing that helper, for instance beside Exit::from_os_error, would complete this consolidation. This change leaves them alone to stay one reviewable step.
  • Other duplicates that this survey found, for later runs:
    • The managed agent's test fixtures, which nvx-rs: decode the MAPS entries once for both payload decoders #616 also lists: cgroup.rs's setup repeats dispatch.rs's direct_system, which loopback.rs builds inline too; mappings.rs's payload repeats dispatch.rs's maps_payload; and both files carry the same host-export fixture.
    • The args test helpers that build a guest program's argument vector in device_io.rs, exit.rs, time/mod.rs, and time_probe/mod.rs, and in console_pending.rs, mmio_write.rs, and reseed.rs, which nvx-rs: make the guest failure reporters return their CommandError #614 changes.
    • lifecycle.rs removes the runtime files in the same order in start_locked and stop, and reads and checks the runtime record the same way in load_running and deprovision.
    • The tests' option_values (host/filesystem.rs), rules (host/network/mod.rs), and values (host/sandbox/options/tests.rs) are one helper.

Validation

  • BASE: 830536ed210e0d8bef0ca9166c13fb9636acb614 (origin/dev when this run started). origin/dev has since gained only openvmm: track feature/microvm/dev on microsoft/openvmm #610, which touches none of these files.
  • TREE: 23e709da2f91f9e129cf639b367ffecdf51c3c8e, the tree of this pull request's commit 787fdd0ca0705cf9612c8cad274862b934ea5962.
  • Release and OpenVMM: the hosts installed v0.1.0-dev.6c4171aa3166 with nvx.py download --hypervisor BACKEND. Its OpenVMM provenance is d0bdbed8f6e3b86271277dc22658109e2e633b2d, which matches the pinned gitlink.
  • nvx-rs CLI: each host rebuilt it from TREE with cargo build --release --locked --features cli and recorded its provenance, with the same source digest (618eb99f…) on all three, so the microVM scenarios that run nvx-rs ran this change rather than the release's binary.
  • Guest artifacts were rebuilt from TREE on prometheus30 with build-guest-binaries --output and test-guest-binaries --binaries (34 passed). The Alpine initramfs was then built with NVX_GUEST_BINARIES=… build-initramfs. The initramfs (54364e73…), its package manifest (97eae955…), and its provenance (1eb6186e…) were copied to the other two hosts with matching SHA-256 digests. No guest binary compiles the changed modules.
  • Local checks (Windows, Rust 1.95.0) on TREE:
    • rustfmt, Clippy with -D warnings (all features, and the library without default features), the --all-features and default tests, including the new test, and rustdoc with -D warnings.
    • MSRV 1.89 check for nvx-rs and aci_edge_sandboxes, on behalf of the WHP host, which has no 1.89 toolchain.
    • The aarch64-apple-darwin check for both crates, on behalf of the KVM host, which lacks that target.
Host profile Backend Host type check-nvx-rs (no hypervisor) check-aci-edge-sandboxes (no hypervisor) Guest binaries + initramfs test-aci-edge-sandboxes test-microvm (default suite)
prometheus32 kvm baremetal Pass: Linux steps, including MSRV and check-guest-binaries (320 passed); tests 611 + 263 passed, 1 ignored each; bundling 15 passed Pass: Linux steps, including MSRV and agent bundling; tests 157 + 85 passed Not run (unshare --user is denied) Pass, 18/18 Pass, 35 scenarios (41 runs)
prometheus30 mshv baremetal Not run (the Linux steps run on the KVM host) Not run Pass: builder (build, 34 tests, initramfs) Pass, 18/18 Pass, 35 scenarios (41 runs)
prometheus28 whp baremetal Pass: Windows steps, including artifact bundling (15 passed); tests 333 + 263 passed, 1 ignored each Pass: Windows steps, including agent bundling; tests 157 + 85 passed Not run Pass, 18/18 Pass, 35 scenarios (41 runs)

The macOS checks for the KVM host's steps and the MSRV checks for the WHP host's steps ran locally, as listed above. No step failed or was rerun.

Left to CI:

  • The virtual-machine runner matrix.
  • test-openvmm-unit and test-openvmm.
  • The test-microvm scenarios that run only when named: smp-lapic, managed-exec-config, and sandbox-lifecycle. The last drives the lifecycle commands whose code this change touches most; the golden CLI scenarios replay those lifecycles locally against the fake OpenVMM.
  • The Ubuntu and Azure Linux guests.
  • The Docker guest build and guest determinism.
  • Benchmarks and performance gates.
  • check-quality and validate-nvx. This change touches no script, Markdown, or file layout.

The nvx-rs CLI reports an OSError as nvx.py did, as str(error), which
names the file of the failed operation only if Python's OSError had
one. For the OSErrors that name no file, 21 sites in lifecycle.rs,
lock.rs, state.rs, sandbox/mod.rs, and session.rs each spelled out
Exit::error(os_error_message(&error, None)), mostly in a map_err
closure, and output::stream_error wrapped the same call for the five
sites that report a failed write to a standard stream, in
artifacts.rs, egress.rs, sandbox/mod.rs, and Exit::report.

Exit::from_os_error now builds that exit, next to Exit::error and
Exit::from_stop. It takes the io::Error by value, so that most sites
become .map_err(Exit::from_os_error), and the CLI says in one place
how an OSError without a filename reads. output::stream_error, which
nothing else used, goes away.

Behavior is unchanged: from_os_error returns
Exit::Error(os_error_message(&error, None)), the value that every site
built before, so each message and exit status stays the same. The
OSErrors that name a file keep their own conversions. A new unit test
pins the message for a missing path and for an error without an OS
code, which no test reached; the golden CLI scenarios and the sandbox
scenarios of the microVM suite run the changed functions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 20, 2026 03:41

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 narrow refactor preserves existing behavior, adds focused coverage, and has no unresolved findings.

No open issues 🎉

What changed in this PR

Centralizes filename-free OS error conversion in the NVX Rust CLI while preserving messages, exit statuses, and control flow.

Changes:

  • Adds Exit::from_os_error and replaces repeated conversions.
  • Removes output::stream_error and unused imports.
  • Adds a test for Python-style error formatting.
File Description
nvx-rs/​src/​host/​cli/​session.rs Uses shared session error conversion.
nvx-rs/​src/​host/​cli/​sandbox/​state.rs Uses shared path-resolution error conversion.
nvx-rs/​src/​host/​cli/​sandbox/​mod.rs Updates output and process-polling conversions.
nvx-rs/​src/​host/​cli/​sandbox/​lock.rs Updates lock error conversions.
nvx-rs/​src/​host/​cli/​sandbox/​lifecycle.rs Updates lifecycle error conversions.
nvx-rs/​src/​host/​cli/​output.rs Removes the redundant formatting helper.
nvx-rs/​src/​host/​cli/​mod.rs Defines the shared conversion and tests its formatting.
nvx-rs/​src/​host/​cli/​egress.rs Updates policy-output error conversion.
nvx-rs/​src/​host/​cli/​artifacts.rs Updates artifact-output error conversion.

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