Repository navigation
nvx-rs: split four of the host's oversized files into directory modules - #921
Open
Pedro Henrique Penna (ppenna) wants to merge 6 commits into
Open
Pedro Henrique Penna (ppenna) wants to merge 6 commits into
Pedro Henrique Penna (ppenna) wants to merge 6 commits into
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
Static review found no blocking issues in the mechanical extraction or compatibility adjustments; builds and tests were not independently run.
No open issues 🎉
What changed in this PR
This PR advances #898 and #896 by splitting four oversized nvx::host files into focused directory modules while preserving behavior, public API paths, test names, and documentation link targets.
Changes:
- Separates network handling, artifact discovery, sandbox CLI parsing and execution, and mount planning into smaller modules.
- Adds re-exports and adjusts internal visibility and conditional imports for the new module boundaries.
- Relocates tests and updates their imports without changing their module paths.
| File | Description |
|---|---|
nvx-rs/src/host/sandbox/mount/tests.rs |
Updates imports for extracted mount helpers. |
nvx-rs/src/host/sandbox/mount/request.rs |
Holds mount parsing, validation, and request types. |
nvx-rs/src/host/sandbox/mount/mod.rs |
Retains shared types, constants, and re-exports. |
nvx-rs/src/host/sandbox/mount/host_paths.rs |
Isolates host-path overlap and policy checks. |
nvx-rs/src/host/sandbox/mount/boot.rs |
Holds boot arguments and share tokens. |
nvx-rs/src/host/sandbox/mount/aggregate.rs |
Holds aggregate naming and mount planning. |
nvx-rs/src/host/network/rule.rs |
Extracts egress rule types and parsing. |
nvx-rs/src/host/network/proxy.rs |
Extracts proxy types and parsing. |
nvx-rs/src/host/network/loopback.rs |
Extracts loopback access and forwarding checks. |
nvx-rs/src/host/network/arguments.rs |
Isolates network argument generation. |
nvx-rs/src/host/cli/sandbox/tests.rs |
Updates imports for sandbox CLI tests. |
nvx-rs/src/host/cli/sandbox/options.rs |
Extracts option parsing and network configuration. |
nvx-rs/src/host/cli/sandbox/one_shot.rs |
Extracts foreground process and interruption handling. |
nvx-rs/src/host/cli/sandbox/mod.rs |
Retains command orchestration and compatibility re-exports. |
nvx-rs/src/host/artifacts/tests.rs |
Relocates artifact tests under their existing module path. |
nvx-rs/src/host/artifacts/require.rs |
Isolates artifact existence checks. |
nvx-rs/src/host/artifacts/mod.rs |
Retains discovery, constants, and public re-exports. |
nvx-rs/src/host/artifacts/identity.rs |
Extracts metadata reading and identity helpers. |
nvx-rs/src/host/artifacts/guest.rs |
Extracts guest descriptors and artifact names. |
nvx-rs/src/host/artifacts/dir.rs |
Extracts artifact layouts and directory operations. |
Review effort: Balanced
host/network/mod.rs held, in 1,916 lines, the network posture of a VM with its parts and the parsers of their native forms, the translation of a posture into OpenVMM options, and 988 lines of inline tests. Split it along those sections into new files beside it, as #898 lays them out: - rule.rs: Access, and the egress rules, Protocol through EgressRules, with the parsing of their native form, and parse_rules and parse_rule, which turn the rules of an action into those that the lowering takes; - loopback.rs: HostLoopback, ForwardProtocol and LoopbackForward, and check_loopback_forwards; - proxy.rs: ProxyScheme and NetworkProxy; - arguments.rs: openvmm_arguments, proxy_arguments and check_proxy_only, and guest_subnet; - tests.rs: the inline tests, which keep their module path, host::network::tests, and so their names. NetworkPolicy and the constants stay in mod.rs, beside the address, lowering, and policy submodules; policy holds the egress policy files, not NetworkPolicy. The code moves unchanged. The new modules are private, and mod.rs re-exports their public items, so every path under nvx::host::network resolves as before. The helpers that the new files share, parse_port, parse_rules, check_loopback_forwards, and guest_subnet, become pub(super). rule.rs and loopback.rs import what only the cli feature's parsers use under that feature, so that builds without it import nothing unused. The tests import Ipv4Addr, which they took from mod.rs's imports through their glob. Without their indentation, rustfmt joins two of their statements that now fit on one line, and the continuations of two strings move left with the code around them, which leaves the strings as they were, since a continuation skips the leading whitespace of its line. Five links of the moved documentation named items that mod.rs has in scope but their new files do not: policy, twice, NetworkProxy, EgressRules::name, and MAX_RULES. Each gets a link definition with its crate path, so that the moved lines stay as they were and the links keep their targets (#896, invariant 6). Validation at this commit, on Windows and on Linux under WSL 2: rustdoc's JSON lists the same public API as on dev, for Windows (9,632 entries) and Linux (13,779), and, with private items, every intra-doc link of the crate keeps its target, with the items of the new modules named as their parent's. The lists of tests, with all features, with default features, and of the library without default features, are those of dev on Windows (841, 695, and 201) and Linux (1,440, 681, and 201). Clippy with warnings denied passes for Windows and Linux, and cargo check for macOS, the host-module guard passes, and the library tests with all features pass on Windows (700) and Linux (1,237). Part of #898. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
host/artifacts.rs holds, in 1,667 lines, the discovery of the artifacts that a launch runs, the guests and the layouts that hold their artifacts, the identities of those artifacts, the checks that they exist, and 727 lines of inline tests. The next commit splits it into a directory module along those sections, as #898 lays them out. Move it first, unchanged, to host/artifacts/mod.rs, with git mv, in a commit of its own, so that git log --follow and git blame follow it through the split. Its module path, host::artifacts, stays the same, and with it the paths of its items and the names of its tests. Validation at this commit, on Windows and on Linux under WSL 2: rustdoc's JSON lists the same public API as on dev, for Windows (9,632 entries) and Linux (13,779), and, with private items, every intra-doc link of the crate keeps its target. The lists of tests, with all features, with default features, and of the library without default features, are those of dev on Windows (841, 695, and 201) and Linux (1,440, 681, and 201). Clippy with warnings denied passes for Windows and Linux, and cargo check for macOS, the host-module guard passes, and the library tests with all features pass on Windows (700) and Linux (1,237). Part of #898. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
host/artifacts/mod.rs, which the previous commit moved from host/artifacts.rs, held the discovery of the artifacts that a launch runs, the guests and the directories that hold their artifacts, the identities that the install metadata of those directories names, the checks that artifacts exist, and the inline tests. Split it along those sections, as #898 lays them out: - guest.rs: Guest, with its Display; - dir.rs: Layout and ArtifactDir, and ArtifactPaths and GuestPaths, which ArtifactDir::paths returns; - identity.rs: sha256_identity, read_metadata, and provenance_identity; - require.rs: required, and require_artifact and require_existing, which #897 moved here from host::sandbox; - tests.rs: the inline tests, which keep their module path, host::artifacts::tests, so that the artifact-bundling step of check-nvx-rs, cargo test --lib --features bundled artifacts, still selects them by name. Artifacts and its discovery, from MANIFEST_NAME through check_manifest, and the constants of the layouts and their metadata stay in mod.rs. The code moves unchanged. The new modules are private, and mod.rs re-exports their public items, so every path under nvx::host::artifacts resolves as before. It also re-exports, under their pub(crate) visibility, require_artifact and require_existing, which other modules of the crate import from host::artifacts. Only the CLI's run and sandbox commands import require_artifact, so its re-export takes the cli feature, as the crate's other re-exports for the CLI alone do. The helpers that dir.rs takes from its siblings, sha256_identity, read_metadata, provenance_identity, and required, become pub(super), and rustfmt wraps the signature of provenance_identity, which the visibility makes too long for one line. The tests import ArtifactIdentity, which they took from mod.rs's imports through their glob. Without their indentation, rustfmt reflows two of their statements, and the lines of their raw string, a compatible SOURCE-MANIFEST.json, keep their bytes. Four links in the moved documentation of ArtifactDir name ErrorCode::ArtifactMissing, which dir.rs does not import. Each gets a link definition with its crate path, so that the moved lines stay as they were and the links keep their targets (#896, invariant 6). Validation at this commit, on Windows and on Linux under WSL 2: rustdoc's JSON lists the same public API as on dev, for Windows (9,632 entries) and Linux (13,779), and, with private items, every intra-doc link of the crate keeps its target, with the items of the new modules named as their parent's. The lists of tests, with all features, with default features, and of the library without default features, are those of dev on Windows (841, 695, and 201) and Linux (1,440, 681, and 201). Clippy with warnings denied passes for Windows and Linux, and cargo check for macOS, the host-module guard passes, and the library tests with all features pass on Windows (700) and Linux (1,237). The artifact-bundling step passes on Windows, with the same 18 tests. Part of #898. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
host/cli/sandbox/mod.rs held, in 888 lines, the parser of nvx-rs sandbox, from its usage and help through the network that its network options describe, the operations that run with the parsed options, and the OpenVMM process of a one-shot run. Split the parser and that process into new files beside it, as #898 lays them out: - options.rs: the parser, from SYNTAX through sandbox_network, with Options and its Default and Receiver implementations; - one_shot.rs: run_openvmm, OneShot, end_interrupted, and spawn_error. Artifacts, error_with_cause, run, run_plan, and prepare_run stay in mod.rs, beside the lifecycle, lock, state, and tests submodules. The code moves unchanged. The items that cli, the parent module, takes from its sandbox module, SYNTAX, HYPERVISORS, MOUNT_OWNERS, MAX_MEMORY_MIB, OPTIONS, parse, hypervisor, path_text, and run_openvmm, were pub(super) there, so they become pub(in crate::host::cli), which names the same module from their new files, and mod.rs re-exports them, pub(super), at their paths. Only cli's tests take SYNTAX and OPTIONS, since parse, which reads them in production, moves with them, so they are re-exported for tests alone, as the crate's other re-exports for tests are. mod.rs also re-exports Options and path_string under their pub(crate) visibility, and imports sandbox_network, which run calls, and which becomes pub(super). rustfmt wraps the signatures of sandbox_network and run_openvmm, which their visibilities make too long for one line. spawn_error keeps its pub(crate) visibility; only OneShot, beside it, calls it. The tests of the sandbox command import SandboxLayer, PolicyKind, and PolicyPathOption, which they took from mod.rs's imports through their glob, and which only the parser uses now. The moved documentation links only to items that its new files import, or by crate paths, so its links keep their targets (#896, invariant 6). Validation at this commit, on Windows and on Linux under WSL 2: rustdoc's JSON lists the same public API as on dev, for Windows (9,632 entries) and Linux (13,779), and, with private items, every intra-doc link of the crate keeps its target, with the items of the new modules named as their parent's. The lists of tests, with all features, with default features, and of the library without default features, are those of dev on Windows (841, 695, and 201) and Linux (1,440, 681, and 201). Clippy with warnings denied passes for Windows and Linux, and cargo check for macOS, the host-module guard passes, and the library tests with all features pass on Windows (700) and Linux (1,237). Part of #898. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
host/sandbox/mount.rs holds, in 1,142 lines, the mounts of a sandbox: the types of their access, owner, kind, and policy paths, the parsing and checks of their values and targets, SandboxMount, the queries of their host paths, the aggregation of several shares into one virtio-fs device, and the shares that a launch hands the guest at boot. Its tests already live in host/sandbox/mount/tests.rs. The next commit splits it into that directory along those sections, as #898 lays them out. Move it first, unchanged, to host/sandbox/mount/mod.rs, with git mv, in a commit of its own, so that git log --follow and git blame follow it through the split. Its module path, host::sandbox::mount, stays the same, and with it the paths of its items and the names of its tests, whose module still reads tests.rs beside it. Validation at this commit, on Windows and on Linux under WSL 2: rustdoc's JSON lists the same public API as on dev, for Windows (9,632 entries) and Linux (13,779), and, with private items, every intra-doc link of the crate keeps its target. The lists of tests, with all features, with default features, and of the library without default features, are those of dev on Windows (841, 695, and 201) and Linux (1,440, 681, and 201). Clippy with warnings denied passes for Windows and Linux, and cargo check for macOS, the host-module guard passes, and the library tests with all features pass on Windows (700) and Linux (1,237). Part of #898. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
host/sandbox/mount/mod.rs, which the previous commit moved from host/sandbox/mount.rs, held the mounts of a sandbox: the types of their access, owner, kind, and policy paths, the requests of shares, the checks of their host paths, the planning of their export, and the shares that a launch hands the guest at boot. Split it along those sections, beside its tests, as #898 lays them out: - request.rs: the parsing and checks of mount values and targets, parse_mount_value through os_error, and SandboxMount, with absolute_host_path, command_line_fragment, and validate_mounts, which follow it; - host_paths.rs: HostQueries through check_distinct_host_paths; - aggregate.rs: aggregate_child_name through plan_mounts_with; - boot.rs: BootShares, boot_shares, and the command-line fragments, path_text through aggregate_fragment. The constants and the access, owner, kind, and policy types stay in mod.rs, with setting_from_name, which the from_name functions of three of those types share. The code moves unchanged. The new modules are private, and mod.rs re-exports their public items, so every path under nvx::host::sandbox::mount resolves as before. It also re-exports, under their pub(crate) visibility, the helpers that other modules of the crate take from it, os_error, has_forbidden_character, has_irregular_component, targets_overlap, HostQueries, check_distinct_export_roots, and export_command_line_fragment, and, for the sandbox tests alone, validate_mount_host_paths_with. The fields and helpers that the new files share become pub(super): the guest_target, host_path, access, owner, and kind fields of SandboxMount, which plan_mounts_with and validate_mount_host_paths_with read, and its kind_refusal; command_line_fragment, which the boot fragments call; and path_text, which check_distinct_export_roots calls. The policy field, which only SandboxMount reads, stays private. The tests import Path, PathBuf, Owner, ReservedTargets, and paths, which they took from mod.rs's imports through their glob, and plan_mounts_with and require_mount_owner_supported_on, crate-visible helpers that only they call from outside the files of those helpers. Links in the moved documentation, and in the module's own, name items that their files no longer import: RESERVED_MOUNT_TARGETS and RESERVED_EXACT_MOUNT_TARGETS in request.rs, plan_mounts in host_paths.rs, and filesystem::plan, ExportPlan, and GUEST_EXPORT in mod.rs. Each gets a link definition with its crate path, so that the documentation stays as it was and the links keep their targets (#896, invariant 6). Validation at this commit, on Windows and on Linux under WSL 2: rustdoc's JSON lists the same public API as on dev, for Windows (9,632 entries) and Linux (13,779), and, with private items, every intra-doc link of the crate keeps its target, with the items of the new modules named as their parent's; the one new link is that of request.rs's module documentation. The lists of tests, with all features, with default features, and of the library without default features, are those of dev on Windows (841, 695, and 201) and Linux (1,440, 681, and 201). Clippy with warnings denied passes for Windows and Linux, and cargo check for macOS, the host-module guard passes, and the library tests with all features pass on Windows (700) and Linux (1,237). The other steps of check-nvx-rs and check-aci-edge-sandboxes pass too, on both: the tests with all features (830 on Windows, 1,418 on Linux) and default features (685 and 669), the artifact bundling (18 and 19), rustdoc with warnings denied, the Rust 1.89 check, the runtime adapter's configurations, the guest binaries' checks on Linux, and aci_edge_sandboxes's lints, tests, agent bundling, and documentation. Part of #898. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pedro Henrique Penna (ppenna)
force-pushed
the
agents/issue-898-fix-and-commit-structure
branch
from
October 27, 2026 01:57
fb22789 to
0c083d6
Compare
This branch has not been deployed
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.
Part of #898. Part of #896.
This splits the four oversized files of
nvx::hostthat no open pull request edits,network/mod.rs,artifacts.rs,cli/sandbox/mod.rs, andsandbox/mount.rs, into directory modules along their existing sections, as #898 lays them out. The code moves unchanged. The public API, the names of the tests, and the targets of the intra-doc links stay the same.The other seven files of #898 wait for the open pull requests that edit them, as the issue's coordination rule asks:
filesystem.rs(#876, #885),platform/windows.rsandplatform/linux.rs(#881, #885, #887),instance.rs(#886, #917),cli/sandbox/lifecycle.rs(#886),store/mod.rs(#881, #885, #886, #888), andsupervisor/mod.rs(#886, #887, #888). #914, which edited the module documentation at the top ofartifacts.rs, merged in the meantime. This branch is rebased on it, and that documentation is now at the top ofartifacts/mod.rs. #919 rewrites the doc comments ofrequire_artifactandrequire_existing, which commit 3 moves toartifacts/require.rs, so whichever of the two lands second carries those lines over. #919's extended guard passes on this branch. #876 adds cases tosandbox/mount/tests.rsfar from the imports that commit 6 adds there, so the two merge cleanly.Commits
Each commit splits one file. A file that becomes
X/mod.rsfirst moves there withgit mv, in a commit of its own, so thatgit log --followandgit blamefollow it.network/mod.rs(1,916 lines) becomesrule.rs,loopback.rs,proxy.rs,arguments.rs, andtests.rs.NetworkPolicyand the constants stay inmod.rs.artifacts.rsmoves toartifacts/mod.rsunchanged.artifacts/mod.rsbecomesguest.rs,dir.rs,identity.rs,require.rs, which holdsrequiredand the helpers that nvx-rs: make the host module's dependency graph acyclic #897 moved here, andtests.rs. The tests keep their module path,host::artifacts::tests, so theartifactsfilter of check-nvx-rs's bundling step still selects them by name: 19 tests on Linux and 18 on Windows, as before.cli/sandbox/mod.rsbecomesoptions.rs, which holds the parser fromSYNTAXthroughsandbox_network, andone_shot.rs, which holdsrun_openvmm,OneShot,end_interrupted, andspawn_error.sandbox/mount.rsmoves tosandbox/mount/mod.rs, beside itstests.rs, unchanged.sandbox/mount/mod.rsbecomesrequest.rs,host_paths.rs,aggregate.rs, andboot.rs. The constants and the access, owner, kind, and policy types stay inmod.rs.What changes besides the moved lines
Each commit message lists these changes in detail:
mod.rsre-exports its public items withpub use, and the crate-visible items that other modules import withpub(crate) use, so every path keeps resolving. The private helpers and fields that the new sibling files share becomepub(super), as nvx-rs: split the host's oversized files into directory modules #898 asks. The items ofcli::sandboxthat werepub(super), so visible incli, and that now sit one level deeper, becomepub(in crate::host::cli), which names the same module.cli::sandboxre-exports them at their old paths withpub(super).cfg, as the crate's other such re-exports do:artifacts::require_artifacttakesfeature = "cli", andcli::sandbox::{SYNTAX, OPTIONS}andsandbox::mount::validate_mount_host_paths_withtaketest. Imports that onlyclicode uses takecfg(feature = "cli"), and the one that only non-Windows code uses takescfg(not(windows)).mod.rs's imports throughuse super::*now import those names themselves.crate::path, so the documentation lines stay as they were. There are 5 such definitions innetwork, 4 inartifacts, and 6 inmount. Three ofmount's are for its own module documentation, whose imports moved with the code that uses them.cli::sandboxneeded none. No new module shares a name with an item of its parent, so no link needsfn@.\continuations, whose leading whitespace the string skips, move left with the code around them.Validation
I ran the checks on Windows 11 and on Linux under WSL 2 (Ubuntu, kernel 6.18), with Rust 1.95.0 and the 1.89 MSRV. No check needs a hypervisor. I validated each commit on
devat2af6eb15. The branch is now rebased on2b2b47ea. Innvx-rs, the new base adds only #914's documentation and aCargo.tomlcomment, and that documentation now sits inartifacts/mod.rs, so each commit's code is the code that I validated. After rebasing on236fd4e0, which changed the runtime adapter, I re-ran the steps of check-nvx-rs and check-aci-edge-sandboxes on the last commit. On2b2b47ea, I re-ran rustfmt, Clippy, the macOS check, the guard, rustdoc with-D warnings, and the artifacts tests.At each of the six commits, on both platforms:
RUSTC_BOOTSTRAP=1 cargo rustdoc --lib --all-features -- -Z unstable-options --output-format json) forx86_64-pc-windows-msvcandx86_64-unknown-linux-gnu. I walked it from the crate root through public paths, re-exports, fields, variants, inherent methods, and trait impls, synthetic and blanket ones included. Each commit has 0 differences fromdev, over 9,632 entries on Windows and 13,779 on Linux.cargo test -- --listlists exactly the tests ofdevwith all features, with default features, and for the library without default features: 841, 695, and 201 on Windows, and 1,440, 681, and 201 on Linux. The inline tests that moved totests.rskeep their names, and the bundling step'sartifactsfilter selects the same tests.--document-private-items, I resolved every intra-doc link of every item, private ones included, to the definition path of its target, and named the items of the new modules as their parent's. Every link keeps its target: 1,533 links on Windows and 2,145 on Linux. The only new link is in the new module documentation ofmount/request.rs. rustdoc reports no new warning, with or without private items.tests/host_modules.rspasses. At the last commit, nvx-rs: follow the whole crate in the host module test #919's extended guard passes too (22 tests) and reports the same layers asdev.-D warningspasses for Windows and Linux on all targets with all features, on the library, and on the library without default features. On Windows, it also passes on all targets with default features and on the library with each ofcli,testing, andbundled.cargo checkpasses for macOS (aarch64-apple-darwin). The library tests with all features pass on Windows (700) and Linux (1,237).At the last commit, the steps of check-nvx-rs and check-aci-edge-sandboxes pass:
-D warnings; the 1.89 check; the runtime adapter's protocol-only (39 tests) and host-only (84 tests) configurations with their 1.89 checks; and aci_edge_sandboxes's formatting, Clippy, tests (168 and 89), agent bundling (4), rustdoc, and 1.89 check.The test counts are each test binary's own totals. Some tests run their binary again as a child, and the child's totals aren't counted.
I left the hypervisor-backed jobs to CI: the CLI scenario tests and the microVM and sandbox tests on KVM, MSHV, and WHP.