[Pedro Henrique Penna (ppenna)]

Summary

HostSession::send, GuestSession::send, and GuestSession::credit, the NVXS state machines' encoders of DATA and CREDIT in nvx-rs/src/protocol/nvxs, each encoded a record at the next sequence number of its direction in the same steps, such as the host's:

let next = next_sequence(self.send_sequence, RecordType::Data)?;
let taken = encode_data(&identity, self.send_sequence, data, output)?;
self.send_sequence = next;
Ok(taken)

That is: take the number after the current one with next_sequence, which fails at the last one, encode the record at the current number, and store the next number only once the record has encoded. The two sends spelled those steps around encode_data, which took the sequence number by value, and credit spelled them around encode_record. So the rule by which a direction numbers its records, which OpenVMM's broker checks, had three definitions. A private function of protocol/nvxs/mod.rs, encode_next, now takes those steps for a record of any type, with the direction's sequence number by reference, and encode_data encodes its DATA record with it.

This is the thirty-first nvx-rs-quality/ pull request. Of the earlier thirty, #603 through #705 merged and #742 is open; each had a Copilot overview review that recommended approval with no comment to follow, #636 and #637 also have approvals without comments, and none was closed without merging. Five of them, #678, #683, #685, #689, and #705, listed this change as a follow-up, and none has touched protocol/nvxs/{mod,guest,host}.rs.

This run surveyed the crate by reading, by the follow-ups of the earlier pull requests, and with a throwaway script, kept outside the repository, that reports every window of six or more whitespace-normalized lines that repeats in nvx-rs/src and nvx-rs/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, protocol/mod.rs, guest/mod.rs, guest/sys/{mod,fd,filesystem,ioctl}.rs, guest/cgroup.rs, guest/mappings.rs, guest/time/checks.rs, guest/managed_agent/seam.rs, host/artifacts.rs, host/filesystem.rs, host/instance.rs, host/process.rs, host/platform, host/python/{mod,fs,windows}.rs, host/store/{mod,files,tests}.rs, host/supervisor/mod.rs, host/sandbox/mount.rs, host/cli/{mod,args}.rs, host/cli/sandbox, testing/fake_openvmm.rs, tests/cli.rs, and tests/fake_openvmm.rs. Among the remaining candidates, this one gives a protocol rule of the shared host and guest code one definition for 84 changed lines, about a third of them a new test. The others repeat test fixtures, or share a few statements behind a helper whose signature and documentation cost about as many lines as it saves; Follow-ups lists them.

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

• encode_next and encode_data stay private to protocol::nvxs, beside next_sequence, and take the sequence number by reference rather than the session, so that both roles, whose states differ, call them with the field that they keep.
• GuestSession::acknowledge keeps encoding its ACK with encode_record at sequence number 0. A resetting guest's sequence number is always 0, so encode_next would number the ACK the same way, but the protocol fixes the ACK at 0, and the explicit number says so.
• Each caller keeps its own checks before the shared steps: the state, and for a credit, the outstanding bytes.

Changes

All changes are in nvx-rs/src/protocol/nvxs: 57 insertions and 27 deletions in four files.

• mod.rs: encode_next(record_type, identity, sequence: &mut u64, payload, output), a private function with a doc comment, encodes a record of record_type with payload and the session's identity at the number that sequence holds, and then advances sequence past it. A record that would take the direction's last sequence number, or that would be malformed, fails without encoding anything or advancing sequence. encode_data now takes sequence: &mut u64 too: it takes at most OUTER_MAX_PAYLOAD bytes from the front of data, as before, encodes them in a DATA record with encode_next, and returns how many it took.
• The sites that now share it:
  • HostSession::send in host.rs (nvx::protocol::nvxs::HostSession::send): one call of encode_data after its state check;
  • GuestSession::send in guest.rs (nvx::protocol::nvxs::GuestSession::send): the same;
  • GuestSession::credit in guest.rs (nvx::protocol::nvxs::GuestSession::credit): encode_next with its CREDIT after its state and credit checks, then the decrease of the outstanding bytes.
• tests/record.rs: a new unit test, a_direction_advances_its_sequence_only_past_the_records_that_it_encodes, which the module documentation now mentions. It encodes a DATA record at sequence number 7 and checks that the number advances to 8, and that empty DATA, a malformed record, and DATA at u64::MAX, the last sequence number, fail, with a RecordError and with SequenceOverflow(Data), without encoding anything or advancing the number.

Behavior

Unchanged:

• Each role encodes the same records as before, with the same bytes, at the same sequence numbers: ACK at 0, then CREDIT and DATA from 1 in the guest's direction, and DATA from 0 in the host's.
• The checks run in the same order: the state, then for a credit its bytes, then next_sequence, whose SequenceOverflow comes before any RecordError of encode_record, as when each caller called next_sequence before it encoded. A failure leaves output and the sequence number as they were, since encode_record checks a record before it writes it and the number advances only after it; credit still decreases the outstanding bytes only once its record has encoded.
• send still returns the bytes that it took, at most OUTER_MAX_PAYLOAD. Both functions are private: no public item, message, error, protocol byte, or golden fixture changes. The guest images change only because mod.rs, guest.rs, and host.rs are among their sources; no guest program's command line, exit status, or output changes.
• Tests that cover it, none of them removed, skipped, or weakened:
  • the session tests of both roles in protocol/nvxs/tests/: the_host_attaches_after_waiting_and_exchanges_data and the_host_sends_a_long_message_in_records_of_the_largest_payload, which also checks that empty DATA encodes nothing and takes no sequence number, the_guest_attaches_acknowledges_receives_and_credits, where ACK, CREDIT, and DATA share the guest's numbers, and the_guest_credits_only_bytes_that_it_received, where refused credits change nothing;
  • the NVXS integration tests of tests/nvxs, which drive GuestConnection and HostConnection, the connections over which the guest's and the host's roles send, through a broker that relays records as OpenVMM's does, with messages that span records; the managed agent's dispatch tests; and the scenarios of tests/cli.rs and tests/fake_openvmm.rs;
  • the new unit test, for the last sequence number, which no test can reach through a role;
  • 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.

• protocol/nvxs/guest.rs and host.rs: receive still repeats the same nine lines after its state check in both roles; sharing them needs a trait over each role's parser, accept, and fail. acknowledge could encode its ACK with encode_next; see Summary.
• host/python/dumps.rs: write_value repeats its bracket, comma, and indentation loop for lists and dicts, and its empty-container arms; one container writer would give json.dumps's layout one definition.
• net::is_valid_guest_network and guest_subnet in host/network/mod.rs both parse an IPv4 ADDRESS/PREFIX and compute its mask, with prefix limits of 1 to 30 and 1 to 32; the host-loopback check of openvmm_arguments reads only the prefix, which a shared parser must keep.
• guest/ingress/encode.rs's ListenerTable::add and guest/ingress/diag.rs's note_wildcard_tcp repeat a bounded find-or-insert merge.
• guest/time_probe/latency.rs and warp.rs repeat the check that fails with unknown TSC rate; pass --tsc-hz and their out of memory report.
• host/cli/session.rs: features, exec, and acknowledged each draw a request ID, set the deadline, and send the request.
• Test fixtures: the maps_payload helper of guest/managed_agent/dispatch.rs and the payload helper of guest/managed_agent/mappings.rs, which are identical; the guest programs' args test helpers; the shell command helpers of guest/exec/tests.rs, guest/spawn.rs, and guest/process.rs; and tests/nvxs/broker.rs's parse loops.
• The follow-ups of earlier runs that wait on files that open pull requests change remain, among them host/python/fs.rs's Windows stat and lstat, write_private in host/instance.rs and host/store/files.rs, and Checks::load's callers in guest/time.

Validation

• BASE: 2817f4eb79911ef8380dc2b580b9d5c77e6b5b21, origin/dev when the run started and still when this pull request was opened.

• TREE: 0eb2bfbc105c1de3dcb4013b4724fd1384b26e2c, the tree of this commit. Every host applied the patch to BASE with git apply --index and checked that git write-tree equals TREE.

• Release on every host: v0.1.0-dev.e714acbd47bf, which download chose because BASE and its parent, two [skip ci] baseline updates that change only data/, have no release. Its OpenVMM, 9f04d05c5441a9b31f7f81effa0ce7c4fc66a0d3, 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 (source_sha256 ca621c9931225e7c7a7fc219e098016e1cd9ca5aeff1e1def9d165a74a0a3d48), so each host removed it, rebuilt it from TREE with the Cargo command with which nvx.py builds it, and recorded its provenance with record-nvx-rs-provenance. Each host checked that the record names TREE's sources (source_sha256 a0a60f0ee17d00331a6dd3eecd58cab0fe482171b089cddce9216477eb924acd, which this machine also computes for TREE) 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, nine static executables;
  • test-guest-binaries --binaries, where 34 tests passed;
  • the Alpine build-initramfs with NVX_GUEST_BINARIES.

  initramfs.cpio.gz (SHA-256 0482761d8ef555f2ec469a27efeaa4d7bc721a1487f8e53104f9c0f0b7a7628a), its packages manifest (2763252b80b0aa36e8c0a59ead9251cc0555563be73d6634fde74af12d3223ae), and its provenance (11c0331b4cc7972bef769d8a1fa3c31878fafb31a81681b81d1b53356cc94485) went through this machine to the other two hosts, where the SHA-256 digests matched.

• 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 and with the default ones; and rustdoc with -D warnings. On WSL Ubuntu, where the guest module compiles: Clippy with -D warnings with all features and on the library without default features, and the NVXS unit tests, the new test among them.

• This machine also ran the steps for which 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, since prometheus28 has no 1.89; 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.

• 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, with RUSTUP_AUTO_INSTALL=0; 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: 907 in 11 test targets with all features and 424 in 9 with the default ones; artifact bundling (15); check-guest-binaries (508 in 9 test targets); 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 (42 scenario runs)
prometheus30	mshv	baremetal	None; prometheus32 and prometheus28 ran the gates	Built and tested here (34 tests)	Pass (18/18)	Pass (42 scenario runs)
prometheus28	whp	baremetal	Pass: every Windows step, 21 commands; nvx tests: 503 in 11 test targets with all features and 426 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 (42 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 no signal (SigIgn: 0), 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. The time-ABI evidence lines counted no boot, capture, or restore over its CPU-time budget on any host, and the exhaustive check reported ok/8/0 on each. 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 VMM suite boots the Alpine image, which CI rebuilds because mod.rs, guest.rs, and host.rs of protocol/nvxs are among its sources;
• 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 validation order, encoding, and failure behavior, with regression coverage and no unresolved issues.

No open issues 🎉

What changed in this PR

This PR consolidates NVXS record sequencing in nvx-rs, giving host and guest DATA sends and guest CREDIT records one shared encoding path without changing public APIs or wire behavior.

Changes:

• Adds private encode_next to advance sequence numbers only after successful encoding.
• Routes both roles’ DATA sends and guest CREDIT encoding through the shared helper.
• Adds a regression test for successful advancement, malformed DATA, and sequence overflow.

File	Description
nvx-rs/​src/​protocol/​nvxs/​tests/​record.rs	Tests sequence advancement and failure behavior; updates module documentation.
nvx-rs/​src/​protocol/​nvxs/​mod.rs	Introduces encode_next and delegates DATA encoding to it.
nvx-rs/​src/​protocol/​nvxs/​host.rs	Simplifies host DATA sends using the shared sequencing logic.
nvx-rs/​src/​protocol/​nvxs/​guest.rs	Shares sequencing logic for guest DATA and CREDIT records.

_{Review effort: Balanced}