Skip to content

Remove the unused nvx-port-io helper - #574

Merged
Pedro Henrique Penna (ppenna) merged 1 commit into
devfrom
agents/remove-nvx-port-io
Oct 16, 2026
Merged

Pedro Henrique Penna (ppenna) merged 1 commit into
devfrom
agents/remove-nvx-port-io

Conversation

@ppenna

@ppenna Pedro Henrique Penna (ppenna) commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #528. This PR removes nvx-port-io, which nothing runs any more.

  • Why it's unused: the snapshot path reads the VM generation ID with nvx-reseed, and a microVM test asserts that the snapshot script no longer touches /dev/port. Yet every guest image still installs the C program in /sbin, and Port the small guest tools to Rust #528 ported it to Rust.
  • Removed: the C program and its documentation. Its entries in the initramfs helper list, the Azure Linux helper build in docker/Dockerfile, and the Rust guest-binary list. The Rust port and its tests.
  • System layer: it drops the helpers that only nvx-port-io used: FileLock and lock_exclusive, create_directory, and retry.
  • Snapshot-script test: its stale nvx-port-io assertion goes too; its /dev/port check stays.
  • In-place rebuilds: build-guest-binaries --output DIRECTORY still updates the output of an earlier build in place. It accepts the retired nvx-port-io executable when that build's manifest.json records it, removes it during installation, and still refuses any other file. Two regression tests cover the rebuild and the refusals.

Impact on the other #358 PRs

Base and CI

#528 has merged, and this PR targets dev, rebased onto c7aa4e24e. CI run 38815237446 passed on 2dd9774d8, including the guest builds and the microVM tests on KVM, MSHV, and WHP.

Validation

On 2dd9774d8:

  • Windows: Ruff check and format, Pyright for Linux and Windows, compileall, and the validate-nvx suite (1,079 tests, 119 skipped). The check-nvx-rs formatting, Clippy, tests with all and with default features, and rustdoc, plus Clippy for the guest feature on x86_64-unknown-linux-musl. The aci_edge_sandboxes formatting, Clippy, and tests.
  • CI: everything else, including check-guest-binaries, build-guest-binaries, and test-guest-binaries on Linux, the guest image builds, and the microVM tests.
  • Guest binaries: the five remaining Rust guest binaries keep the digests of dev's CI build at 576baf7f, because only unused code was removed. The new source digest is c2c6670a2b15e45822bbe8e15d7557bd2a2150ee0101e38c219a68039df364ab.

Earlier, on #528's head 0525d6ff with the original change, every check-nvx-rs command and the three guest-binary commands also passed on WSL Linux.

Base automatically changed from agents/fix-issue-358-validate-changes-ci to dev October 15, 2026 18:06
Copilot AI balanced review requested due to automatic review settings October 15, 2026 19:38

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.

🟡 Changes recommended

The reduced inventory rejects existing six-binary output directories, blocking in-place rebuilds.

1 open finding
What changed in this PR

Removes the unused nvx-port-io helper from NVX, reducing the guest-tool migration scope established by #528.

Changes:

  • Deletes the C and Rust implementations and their tests.
  • Removes build entries and system helpers used only by this tool.
  • Updates documentation while preserving the snapshot test’s /dev/port check.
File Description
scripts/​test_microvm_tests.py Removes the obsolete helper assertion.
scripts/​nvx_tools/​build_constants.py Removes Rust and initramfs build entries.
nvx-rs/​tests/​guest_tools.rs Removes helper integration tests.
nvx-rs/​src/​guest/​sys/​mod.rs Removes unused exports, retry logic, and tests.
nvx-rs/​src/​guest/​sys/​fd.rs Removes unused locking and directory helpers.
nvx-rs/​src/​guest/​port_io.rs Deletes the Rust implementation.
nvx-rs/​src/​guest/​mod.rs Removes the module export.
nvx-rs/​src/​bin/​nvx-port-io.rs Deletes the Rust entry point.
nvx-rs/​README.md Removes the helper documentation.
nvx-rs/​Cargo.toml Removes the binary target.
guest/​common/​nvx-port-io.c Deletes the C implementation.
docker/​Dockerfile Removes Azure Linux helper compilation.
doc/​usage.md Updates executable counts to five.
doc/​build.md Removes the packaged-helper description.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/nvx_tools/build_constants.py
Copilot AI balanced review requested due to automatic review settings October 15, 2026 21:58

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 focused removal and retired-output handling have no identified blocking issues; guest-image validation remains pending CI.

1 open finding

🧠 Review effort: Balanced

Nothing runs nvx-port-io any more: the snapshot path reads the VM
generation ID with nvx-reseed, and a microVM test asserts that the
snapshot script no longer touches /dev/port. Yet every guest image still
installs the C program in /sbin, and #528 ported it to Rust.

Remove the C program, its build and packaging entries, its
documentation, and the Rust port with its tests. The guest system layer
drops the helpers that only nvx-port-io used: the file lock, directory
creation, and the EINTR retry loop.

build-guest-binaries still updates the output of an earlier build in
place: it accepts the retired nvx-port-io executable when the build
manifest records it, removes it during installation, and refuses any
other file.

Part of #358.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ppenna
Pedro Henrique Penna (ppenna) merged commit 75333da into dev Oct 16, 2026
33 checks passed
@ppenna
Pedro Henrique Penna (ppenna) deleted the agents/remove-nvx-port-io branch October 16, 2026 01:57
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