Skip to content

openvmm: pin unit contracts and hot-plugged PCIe restore - #1450

Closed
Pedro Henrique Penna (ppenna) wants to merge 2 commits into
devfrom
agents/vm-contract-state-ideas-report
Closed

Pedro Henrique Penna (ppenna) wants to merge 2 commits into
devfrom
agents/vm-contract-state-ideas-report

Conversation

@ppenna

@ppenna Pedro Henrique Penna (ppenna) commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

What and why

OpenVMM's saved state holds the state units that have state to save, by name.
The chain's position 017, which microsoft/openvmm#4624 proposes upstream, adds
the names of all the units, so that a restore refuses a VM with extra or
missing units. A name says nothing of what a unit is to the guest, though:
whether the VM's configuration created it or it was hot-plugged, or whether the
device behind the same name differs, such as in its BARs or PCI IDs. A snapshot
of a VM with a device hot-plugged onto a PCIe port therefore cannot be restored
in a new process, and a different device behind the same name goes unnoticed.

This pins an alternative, microsoft/openvmm#5014. Each state unit records in
saved state the part of the VM's contract that it owns, generically, so that a
restore can check what the guest saw and can re-create the PCIe devices that
were hot-plugged, for any VM profile with hot-pluggable ports. It has three
commits on 6ddd6ca50, the head of feature/microvm/dev, which dev pins:

  • 021d269b8, state_unit: record a contract for each state unit. Each unit
    declares a contract when it is added, which holds:

    • a stable, guest-visible identity, such as pcie-port:<port> for a PCIe
      device;
    • a kind;
    • a lifetime: static, dynamic, or derived;
    • descriptors, which vmotherboard derives from a chipset device's
      registrations and PCI configuration space.

    A save records the contracts of exactly the units that it saved, from the
    same pass over the units. A restore checks them by name, whatever the
    registration order, and reports missing, unexpected, and changed units. The
    saved state records the contracts in place of position 017's names, and the
    saved-state schema moves to version 2.

  • 07b4281df, openvmm: re-create hot-plugged PCIe devices on restore. On
    --restore-snapshot, the devices that the command line places on the ports
    that the snapshot records as hot-plugged go to the VM worker. The worker
    re-creates them on their ports, without a hotplug notification, before it
    applies saved state. The commit also adds the add-pcie-device and
    rm-pcie-device console commands.

  • 3e7809ba4, vmm_tests: save and restore a hot-plugged PCIe device.

microsoft/openvmm#5015 proposes the same change to upstream main, in place of
#4624.

Commits:

  • openvmm: pin unit contracts and hot-plugged PCIe restore moves the gitlink
    from 6ddd6ca50 to 3e7809ba4343ba630f10841052f35f86b521d231, the head of
    [DO NOT REVIEW] state_unit: record unit contracts and re-create hot-plugged PCIe devices openvmm#5014, with the changes that the new saved-state schema
    needs:

    • nvx::contract::DEVICE_STATE_SCHEMA_VERSION moves to 2. That gives
      templates a new compatibility key, so the template store captures them
      again rather than restore those of version 1, which OpenVMM refuses.
    • The template manifest fixtures are written again with the pinned
      OpenVMM's code, at the new schema, and with the state units in the name
      order that OpenVMM now records.
    • The manifest tests encode the new version.

    The control-session vectors, source excerpts, and CPU profiles that nvx-rs
    copies from OpenVMM match the revision's.

  • doc: map the state-unit contracts in the OpenVMM roadmap maps positions
    243-245, which are not yet assigned to PRs. The pin is 114 commits past the
    mapped tip. The commits conflict with upstream main in 21 of their 37 files,
    which is why state_unit: record unit contracts and re-create hot-plugged PCIe devices openvmm#5015 ports them instead.

Scope and prerequisites:

  • No prerequisite beyond 6ddd6ca50, which dev pins. This is the first
    revision of [DO NOT REVIEW] state_unit: record unit contracts and re-create hot-plugged PCIe devices openvmm#5014, so it replaces no reviewed revision and
    needs no equivalence evidence.
  • Only devices on PCIe ports are re-created. A restore whose snapshot records
    another dynamic unit, such as a VPCI device, still fails. NVX's microVMs have
    no PCIe ports, so for them this changes only the saved-state schema and the
    check before a restore.
  • Templates captured before this change are captured again, since their key
    changes.

Changes to existing tests, for the tests-modified label (all follow the
schema's move to version 2):

  • host::template::tests, ManifestSpec's template helper: the manifest's
    saved-state schema version, 1 to 2, the version that NVX now accepts.
  • host::template::manifest::tests::the_fields_of_a_manifest_need_not_be_written_in_their_order:
    the encoded schema version, field 11, 1 to 2, for the same reason.
  • host::template::manifest::tests::other_versions_of_the_format_are_unsupported:
    it refused version 2 and now refuses versions 1 and 3, the earlier schema and
    a later one.
  • nvx-rs/tests/fixtures/template/generate.rs: sorts the state-unit names, as
    OpenVMM now records them.
  • nvx-rs/tests/fixtures/template/platform-kvm/manifest.bin and
    platform-mshv-network/manifest.bin: written again by generate.rs with the
    pinned OpenVMM's code. Writing them again at this pin gives identical bytes.

Landing order: land microsoft/openvmm#5014 first, by fast-forwarding
feature/microvm/dev to the exact pinned revision,
3e7809ba4343ba630f10841052f35f86b521d231, since OpenVMM's merge button
squashes and would leave the pin off the branch; then merge this pull request.

Validation

Locally, on Windows:

  • OpenVMM at 3e7809ba4, with Rust 1.97.1:
    • cargo check and cargo clippy --all-targets for state_unit,
      vmotherboard, vmm_core, openvmm_defs, openvmm_helpers,
      openvmm_core, openvmm_entry, petri, vpci_relay, and vmm_tests:
      no warnings.
    • cargo doc --no-deps for the same packages except vmm_tests: no
      warnings.
    • cargo nextest run --profile agent for the same packages except
      vmm_tests: 699 passed.
    • cargo xtask fmt --fix: clean.
  • python scripts\local_gates.py --base origin/dev --run with every selected
    gate but nvx-cli:
  • python scripts\openvmm_pin.py check-roadmap --worktree,
    python scripts\openvmm_pin.py check-branch --target origin/dev,
    python scripts\check_markdown_links.py, git diff --check, and
    python scripts\nvx.py verify: passed.

On NVX's test VMs (nvx-kvm-test, nvx-mshv-test, and nvx-whp-test), at this
head, with OpenVMM built from 3e7809ba4 (build-openvmm, whose provenance
names it) and the guest artifacts of v0.1.0-dev.fdd1985f50d4, which this
branch's guest code matches:

  • python3 scripts/nvx.py test-microvm --backend <kvm|mshv|whp>, the default
    suite: passed on all three, with 57, 57, and 58 scenario runs and none
    skipped.

  • python3 scripts/nvx.py test-runtime-adapter --backend <kvm|mshv|whp>: all
    161 scenarios of 11 targets passed on each.

  • On KVM, cargo xflowey vmm-tests-run --filter 'test(/openvmm_linux_x64_/)'
    in openvmm/: 40 passed, including pcie_hotplug_save_restore,
    pcie_hotplug, and pcie_save_restore.

  • A cross-process save and restore of a hot-plugged virtio-rng device, driven
    through OpenVMM's console with the OpenVMM test kernel on two hot-pluggable
    root ports:

    • Hot-add the device to a port, save-snapshot, and restore in a new process
      with --virtio-rng --virtio-rng-pcie-port: the device produces entropy,
      and can be removed and added again.
    • A restore without the device, with it on the other port, or with a device
      on the port of one removed before the save fails with an inventory error
      that names the contract.
    • A device removed before the save, and a cold-plugged one, restore.

    All 9 cases passed on KVM and MSHV. On WHP they passed only with
    save-snapshot patched to open its RAM file writable. That is a separate,
    existing bug: vm_controller opens the file read-only, then calls
    sync_all, which Windows refuses. This pull request does not change it.

Before the rebase onto 6ddd6ca50, the same three OpenVMM commits, as
ad47c5688 on 46bfe02ed, had passed the same runs on all three VMs. The
rebase added only the terminal-unit check.

Left to CI: the full matrix, since the gitlink is an input of every suite,
including both OpenVMM suites, the nvx-cli gate, and the benchmarks.

Pin OpenVMM 3e7809ba4, whose three new chain commits, which
microsoft/openvmm#5014 proposes, replace the flat state-unit inventory of
position 017 with a contract for each state unit: its stable, guest-visible
identity, kind, lifetime, and descriptors, which vmotherboard derives from a
chipset device's registrations and PCI configuration space. A snapshot
records the contracts with the units' saved state, and a restore checks them
before applying that state, and re-creates the devices that were hot-plugged
onto PCIe ports from the devices that the restore supplies for those ports.

The saved state records the contracts in place of the names of the state
units, so OpenVMM's saved-state schema, which a snapshot manifest records,
moves to version 2. Move nvx::contract::DEVICE_STATE_SCHEMA_VERSION with it,
which gives templates a new compatibility key, so that the template store
captures them again rather than restore those of version 1, which OpenVMM
refuses. Write the template manifest fixtures again with the pinned
OpenVMM's code, at the new schema and with the state units in the name order
that OpenVMM now records, and update the manifest tests that encode the
schema version.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Map positions 243-245, the three commits that the pin 3e7809ba4 adds after
position 242, which microsoft/openvmm#5014 proposes. Position 243 replaces
the flat state-unit inventory of PR09's position 017 with a contract for
each state unit that a snapshot records and a restore checks, and moves the
saved-state schema to version 2; position 244 re-creates on restore the
devices that were hot-plugged onto PCIe ports; and position 245 tests a
hot-plugged device across a save and restore. They conflict with upstream
main, to which microsoft/openvmm#5015 proposes them instead of
microsoft/openvmm#4624. The pin is 114 commits past the mapped tip, and
positions 243-245 are not yet assigned to PRs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 10, 2026 14:56
@ppenna Pedro Henrique Penna (ppenna) added the tests-modified Changes existing tests, test data, or test wiring label Nov 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This pull request changes existing tests

It modifies or removes 6 existing test asset(s), listed below. Review each with care: a change to a test can hide a regression that the test caught. See doc/contribute.md#changes-to-existing-tests.

Test code

Change What Before After
modified helper fn template in host::template::tests, impl ManifestSpec`` nvx-rs/src/host/template/tests.rs:618 nvx-rs/src/host/template/tests.rs:618
modified test fn the_fields_of_a_manifest_need_not_be_written_in_their_order in host::template::manifest::tests nvx-rs/src/host/template/manifest/tests.rs:231 nvx-rs/src/host/template/manifest/tests.rs:231
modified test fn other_versions_of_the_format_are_unsupported in host::template::manifest::tests nvx-rs/src/host/template/manifest/tests.rs:256 nvx-rs/src/host/template/manifest/tests.rs:256

Test data

Change What Before After
modified file nvx-rs/tests/fixtures/template/generate.rs nvx-rs/tests/fixtures/template/generate.rs nvx-rs/tests/fixtures/template/generate.rs
modified file nvx-rs/tests/fixtures/template/platform-kvm/manifest.bin nvx-rs/tests/fixtures/template/platform-kvm/manifest.bin nvx-rs/tests/fixtures/template/platform-kvm/manifest.bin
modified file nvx-rs/tests/fixtures/template/platform-mshv-network/manifest.bin nvx-rs/tests/fixtures/template/platform-mshv-network/manifest.bin nvx-rs/tests/fixtures/template/platform-mshv-network/manifest.bin

Relocated unchanged: 0; added: 0. Compared the merge base fdd1985f50d4 with 6905c055ab03.

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 required OpenVMM pin conflicts with the advanced upstream branch, and forcing it would discard landed lost-unit fixes.

1 open issue
What changed in this PR

This PR updates NVX for an OpenVMM revision that validates per-unit saved-state contracts and re-creates hot-plugged PCIe devices during restore. For NVX microVMs, the main effects are stricter restore checks and a new saved-state schema.

Changes:

  • Advances NVX’s accepted device-state schema to version 2.
  • Updates manifest tests and fixture generation for the new schema and sorted unit names.
  • Maps the pinned OpenVMM commits and their upstream relationships.
File Description
nvx-rs/​tests/​fixtures/​template/​README.md Documents the new fixture schema, ordering, and provenance.
nvx-rs/​tests/​fixtures/​template/​generate.rs Sorts state-unit names to match OpenVMM.
nvx-rs/​src/​host/​template/​tests.rs Uses schema version 2 in manifest test data.
nvx-rs/​src/​host/​template/​manifest/​tests.rs Tests acceptance of version 2 and rejection of versions 1 and 3.
nvx-rs/​src/​contract.rs Advances the device-state schema constant to version 2.
doc/​openvmm-upstream-roadmap.md Maps the new pin, its three commits, and upstream proposals.

Review effort: Balanced

Comment on lines +13 to +14
head of microsoft/openvmm#5014, whose three commits, positions 243-245, follow position 242,
`6ddd6ca50`, the head of `feature/microvm/dev`, which microsoft/openvmm#5012 merged with
@ppenna

Copy link
Copy Markdown
Contributor Author

Closing as not planned, together with microsoft/openvmm#5014, which this pull request pins.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests-modified Changes existing tests, test data, or test wiring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants