[Pedro Henrique Penna (ppenna)]

Summary

encode_outer, in nvx-rs/src/protocol/outer.rs, and nvxs::encode_record, in nvx-rs/src/protocol/nvxs/record.rs, each wrote the 44-byte header of an NVXS record in the same eight statements:

output.extend_from_slice(&OUTER_MAGIC);
output.extend_from_slice(&OUTER_VERSION.to_le_bytes());
output.push(record_type.code());
output.push(0);
output.extend_from_slice(instance_id);
output.extend_from_slice(&epoch.to_le_bytes());
output.extend_from_slice(&sequence.to_le_bytes());
output.extend_from_slice(&length.to_le_bytes());

So the wire layout of the header, which OpenVMM's broker parses, had a definition in each encoder. A private function of outer.rs, encode_header(record_type, instance_id, epoch, sequence, length), now returns the header as a [u8; OUTER_HEADER_LEN] array, with each field at the offset at which decode_outer_header and the record parser read it, and both encoders append it before the payload.

This is the twenty-ninth nvx-rs-quality/ pull request. The 19 from #603 to #637 merged, as did #678, and #679 through #685 and #689 are open. Each had a Copilot overview review that recommended approval with no comment to follow, #636 and #637 also have approvals without comments to follow, and none was closed without merging. This run surveyed the crate by reading, and with a throwaway script, kept outside the repository, that reports every window of six or more whitespace-normalized lines that repeats in src and tests, ignoring blank lines, comments, and lines of punctuation, exactly and with names and literals abstracted. It skipped every file that an open pull request to dev changed when the run started, among them README.md, Cargo.toml, src/lib.rs, host/mod.rs, guest/mod.rs, most of the managed agent, guest/sys, guest/time, guest/time_probe, the guest programs' modules, guest/nvxs.rs, guest/spawn.rs, host/instance.rs, host/process.rs, host/filesystem.rs, host/platform, host/store/{mod,tests}.rs, host/sandbox/{mod,mount,options,distro,golden}.rs, host/cli/session.rs, host/cli/sandbox/{lifecycle,mod,state}.rs, host/network/{lowering,policy/mod}.rs, host/python/golden.rs, tests/cli.rs, tests/managed_lifecycle.rs, and tests/supervisor.rs.

No pull request has changed outer.rs or nvxs/record.rs since the NVXS session was added, and this candidate gives the most production code one definition per changed line among those in the remaining files: 44 changed lines for the wire layout that two public encoders write. The other candidates either repeat only test fixtures, such as the policy and native helpers of the sandbox tests, the kernel_address helper of the ingress tests, and the nvx-ingress test DIRECTORY command lines of tests/guest_tools.rs, or share a few statements behind a helper whose signature costs about as many lines as it saves, such as the port selectors of EgressRule's builders, the container loops of host/python/dumps.rs, and the Windows stat and lstat of host/python/fs.rs. Follow-ups lists them.

Where the choice was open, I took the conservative option:

• encode_header is pub(super), visible in protocol alone, and checks nothing. Each encoder keeps its own checks, which differ: encode_outer checks only the payload length of a record of any type byte, which tests use to send malformed records, and encode_record checks everything that RecordParser checks. So neither encoder calls the other: encode_record cannot take a record of any type byte, and calling encode_outer from encode_record would allocate a second vector for every record.
• It returns an array rather than appending to the caller's vector: rustfmt lays a six-argument call out on eight lines, which would have cost each site as many lines as the statements it replaces, while each call of encode_header fits one line. Writing each field at its offset also mirrors the decoders. Both encoders still reserve the same capacity, then append the header and the payload.
• The two header decoders keep their reads: they check the fields in different orders and fail with different errors, ProtocolError messages and RecordError variants, so a shared reader wants a change of its own. Follow-ups lists it.

Changes

All changes are in nvx-rs/src/protocol/outer.rs and nvx-rs/src/protocol/nvxs/record.rs: 28 insertions and 16 deletions.

• encode_header(record_type, instance_id, epoch, sequence, length) -> [u8; OUTER_HEADER_LEN], pub(super) in protocol::outer, with a doc comment: the magic at offset 0, the version at 4, the record type at 6, a zero flags byte at 7, the instance ID at 8, the epoch at 24, the sequence number at 32, and the payload length at 40, with the integers little-endian.
• The sites that now share it:
  • encode_outer in protocol/outer.rs (nvx::protocol::encode_outer), which appends the header and then the payload to the vector that it returns;
  • encode_record in protocol/nvxs/record.rs (nvx::protocol::nvxs::encode_record), which reserves room in output and appends the header and then the payload to it.

No public item, message, error, protocol byte, fixture, or other file changes.

Behavior

Unchanged:

• Both encoders write the same bytes as before, in the same order: the 44 header bytes, then the payload, into a vector of the same capacity, OUTER_HEADER_LEN + payload.len().
• Each encoder runs the same checks in the same order before it writes anything, and fails with the same errors, so encode_record still leaves output unchanged on an error. encode_header cannot fail.
• Tests that cover it, none of them added, removed, skipped, or weakened:
  • the golden bytes of encode_outer in protocol/tests.rs: host_attach_matches_the_python_client, data_record_matches_the_python_client, golden_outer_headers_decode_as_the_broker_sends_them, and guest_outer_records_match_golden_bytes;
  • the golden bytes of encode_record: records_match_the_brokers_golden_vectors in protocol/nvxs/tests/record.rs, with every record type, and the golden attachments of the_guest_attaches_acknowledges_receives_and_credits and the_host_attaches_after_waiting_and_exchanges_data in the session tests, besides the encoder's checks in record.rs's other tests;
  • each encoder against the other's decoder: the managed agent's dispatch tests, whose harness writes records with encode_outer to the agent's GuestSession and reads its records with decode_outer_header; the tests of host/session.rs; and the scenarios of tests/cli.rs and tests/fake_openvmm.rs, in which the host's HostSession talks to the fake OpenVMM, which encodes and decodes with encode_outer and decode_outer_header;
  • on each hypervisor, the managed agent in the rebuilt initramfs and the host's nvx-rs and aci_edge_sandboxes, all built from TREE, exchanged NVXS records through OpenVMM's own broker in test-aci-edge-sandboxes, the managed-lifecycle scenario, and the other managed scenarios of test-microvm.

Follow-ups

Not in this pull request, to keep it to one change. I found no suspected bug.

• decode_outer_header in outer.rs and the record parser's parse_header in nvxs/record.rs still each read the header's fields at the same offsets. A reader of the raw fields beside encode_header would give the layout one definition in both directions, while each decoder kept its own checks, their order, and its errors.
• outer.rs: encode_ack_payload and decode_ack_payload repeat their window check and message, as encode_credit_payload and decode_credit_payload repeat theirs, and check_payload in nvxs/record.rs checks the same ranges with errors of its own.
• protocol/nvxs/guest.rs and host.rs: each role's send repeats the check of the next sequence number, the DATA encoding, and the advance, and the guest's credit does the same for CREDIT; encode_data could take the sequence number by reference and advance it.
• Test fixtures: the policy and native helpers of host/sandbox/tests.rs and host/sandbox/mount/tests.rs, the kernel_address helper of guest/ingress/address.rs and collect.rs, and the five nvx-ingress test DIRECTORY command lines of tests/guest_tools.rs.
• EgressRule::on_port, on_port_range, and on_protocol in host/network/mod.rs each build and push a PortSelector; host/python/dumps.rs's write_value repeats its bracket, comma, and indentation loop for lists and dicts; and host/python/fs.rs's Windows stat and lstat build the same Stat.
• The other follow-ups that nvx-rs: lay out execution-helper configurations with one function #689 and earlier pull requests list remain, among them receive in both NVXS roles, the bounded merges of guest/ingress, the IPv4 ADDRESS/PREFIX parsing of net and host/network/mod.rs, and those that wait on files that open pull requests change.

Validation

• BASE: 8b585d722d4ade4146291ed7d4e123889d8bbbd2, origin/dev when the run started. Before this pull request was opened, origin/dev advanced to 74c56f430 by the merges of ci: recover Windows runners after VMSS image upgrades #669 and tests: define the guest command once #677 and two baseline updates, which change scripts/, tests/runtime-adapter/, and data/, none of this pull request's files.

• TREE: ac6777dcf885a5dfa9527cf87c18cc5e21500c09, 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.8b585d722d4a, built from BASE itself. Its OpenVMM, d0bdbed8f6e3b86271277dc22658109e2e633b2d, is the pinned revision, as each host checked in build/openvmm.provenance.json.

• nvx-rs: download installed the release's nvx-rs, which BASE's sources built, so each host removed it and rebuilt it from TREE with the Cargo command with which nvx.py builds it. Cargo recompiled nvx, the executable's digest changed, and record-nvx-rs-provenance recorded it; each host checked that the record names TREE's sources (source_sha256 07c3a52dba58608a34600178bd7430feca6d7f66fca4791dc669ed0a30f5218b) and the new executable. Each host then built the openvmm_e2e and managed_lifecycle test binaries, after its other Cargo work and before its hypervisor tests, whose logs show no compilation.

• Guest artifacts were rebuilt from TREE on prometheus30, whose user namespaces let Alpine's apk run its scripts (prometheus32 denies them):

  • 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 c7d2859c829b0da62a3ad677525cd90d3c41e805c20af73b061f97dc0bd23c12), its packages manifest (b5d0268f2e22ff6c6d67d883c79a330e86afd5536be893915bfd9fe0270f2572), and its provenance (9f7ea99e6c085999b091a97f536b21cd13ab28af914acb8bcb3a1a1901dc082f) went through this machine to the other two hosts, where the SHA-256 digests matched. On every host, the 191 source files that the provenance records matched the worktree's, the two changed files with TREE's digests, and on prometheus30 the guest binaries' build manifest names TREE's guest-binary sources (035d2b528a60b49e5ef4ef8801b35cf4aa9058c3161a93b252709d32e73761ce).

• Local checks with Rust 1.95.0 on Windows: rustfmt; Clippy with -D warnings on all targets with all features and on the library without default features; cargo test with all features (425 tests in 11 test targets) and with the default ones (354 in 9); and rustdoc with -D warnings. This machine also ran the steps that no host could run, because the hosts lack the toolchain or target: on Windows, the MSRV (1.89) checks of nvx, aci_edge_sandboxes, and the runtime adapter's protocol-only and host-only configurations; and, on WSL Ubuntu, the macOS (aarch64-apple-darwin) checks of nvx and aci_edge_sandboxes and the MSRV check of the runtime adapter's guest-only configuration, which ran as the action defines it, starting with its rustup target add, which gave the local 1.89 the musl target. I removed that target again afterwards. A first attempt at that last check ran nothing: my script read the MSRV as empty from this machine's CRLF checkout, and rustup refused the empty toolchain name.

• The change adds, moves, and deletes no file and changes no Markdown, so the CLI tests of validate-nvx and the link check had nothing to cover.

• The hosts ran each action's steps with the toolchains and targets that they have, skipping the actions' installation commands; nothing was installed on them.

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, 29 commands, including the MSRV (1.89) checks; nvx tests: 755 in 11 test targets with all features and 355 in 9 with the default ones, besides the runs of single tests that some tests make of their own binary; artifact bundling (15); check-guest-binaries (412); the runtime adapter's protocol-only, host-only, and musl guest-only configurations (13, 14, and 21 tests); 158 and 85 aci_edge_sandboxes tests over the two feature sets; agent bundling (4). The macOS checks and the guest-only MSRV check ran locally	Copied in, digests match	Pass (18/18)	Pass (41 scenario runs)
prometheus30	mshv	baremetal	None; prometheus32 and prometheus28 ran the gates	Built and tested here (34 tests)	Pass (18/18)	Pass (41 scenario runs)
prometheus28	whp	baremetal	Pass: every Windows step, 21 commands; nvx tests: 425 in 11 test targets with all features and 354 in 9 with the default ones; artifact bundling (15); the runtime adapter's protocol-only and host-only configurations (13 and 14 tests); 158 and 85 aci_edge_sandboxes tests over the two feature sets; agent bundling (4). The MSRV checks ran locally	Copied in, digests match	Pass (18/18)	Pass (41 scenario runs)

Every gate and hypervisor test passed at its first run. The Linux gates ran in the foreground of their SSH session, whose shell ignored only SIGQUIT (SigIgn: 0x4), not SIGINT, so the CLI's Ctrl-C tests ran as intended. Each suite's managed-lifecycle scenario ran the managed_lifecycle test with Cargo, which passed. OpenVMM warned of a host CPU-profile fallback only in the cpu-profile-fallback scenario, which forces one. The time-ABI evidence lines counted no capture or restore over its CPU-time budget on any host, and no boot on prometheus32 and prometheus28; on prometheus30, they counted 1 of 58 boots, which the evidence reports without failing the suite, in nvx-time's boot checks, which encode no NVXS record. On every host, one openvmm_e2e test printed skipped: the host has no second volume for a check that needs one, and on the Linux hosts the filesystem caller-ownership scenario reported that OpenVMM cannot assume other host identities, so it checked that guest root and foreign guest callers fail with EPERM; both are properties of the hosts, not of this change.

Left to CI:

• the OpenVMM unit and VMM suites, test-openvmm-unit and test-openvmm, since OpenVMM is unchanged;
• the test-microvm scenarios that run only when named: smp-lapic, and managed-exec-config and sandbox-lifecycle, which need artifacts that packages omit;
• the debug-kernel scenarios, the Ubuntu and Azure Linux guests, the Docker guest build, and verify-guest-determinism;
• the benchmarks and performance gates;
• the virtual-machine runners;
• check-quality and validate-nvx, which no Python, shell, or Markdown change touches.

[Copilot]

🟢 Approval recommended

The focused refactor preserves encoding behavior and validation, with existing golden-vector coverage and no unresolved findings.

No open issues 🎉

What changed in this PR

Centralizes NVXS record-header encoding so both encoders share one wire-layout implementation.

Changes:

• Adds a protocol-scoped helper that returns the 44-byte header.
• Updates both encoders while preserving validation, byte order, and payload handling.

File	Description
nvx-rs/​src/​protocol/​outer.rs	Defines the shared header helper and uses it in encode_outer.
nvx-rs/​src/​protocol/​nvxs/​record.rs	Uses the shared helper in encode_record.

_{Review effort: Balanced}