[Pedro Henrique Penna (ppenna)]

Keeps a later lifecycle request from panicking on a state unit in a terminal state, for microsoft/nvx#1148. A unit whose task does not answer its start request is left in the terminal StartUncertain state, which only a later start refused: a later stop, reset, save, restore, or time advance that reached such a unit panicked the VM worker on run_op's state assertion, and a worker restart whose recovery start failed kept serving a VM that it marked running over stopped units, so that its next pause, or its teardown, panicked on StateUnits::stop's running assertion.

Two commits on 4c2697aef, the head of feature/microvm/dev. microsoft/nvx#1435 pins this head.

Handle a unit in a terminal state instead of panicking (78e752de1)

• Cause. The start rollback of the chain's position 015 leaves a unit whose task does not answer its start, or its start confirmation, in the terminal StartUncertain state, and StateUnits::start refuses a set that holds one. The other state changes still assumed that every unit was Stopped or Running: run_op asserts that a unit that is not in the operation's old state is already in its new state. So a stop of a running set that holds such a unit, as after a hot-add whose start is uncertain, panicked with "unit ... in StartUncertain state, should be Running or Stopped", and a reset, save, restore, or time advance after a failed start with "should be Stopped or Stopped".
• Change. As the later revisions of state_unit: roll back a partial start instead of panicking #4872, which proposes position 015 upstream, do:
  • reset, save, restore, and advance_time fail with a StateTransitionError that names each unit in a terminal state, before they send any unit a request (check_no_terminal_units). Unlike state_unit: roll back a partial start instead of panicking #4872, they check this before they check that the units are stopped, so that they fail rather than panic even while such a unit keeps the units running, as a later partial-start rollback that keeps a possibly running unit's dependencies running would.
  • stop also stops the units whose start is uncertain, since they may be running, in the same reverse dependency order as the others, and leaves them in StartUncertain, so that a later start still fails.
• Scope. A unit whose task has ended still panics the stop that it does not answer (run_op's UnitRecvError), as any unit's does; openvmm: stop, save, reset or restore panics if a state unit's task has exited nvx#1363 makes that an error. state_unit: roll back a partial start instead of panicking #4872's other later changes, which keep the units running after an uncertain start, block the rollback's stop of a dependency of a unit that was not stopped, and add stop_for_teardown, are not part of this pull request.
• Tests. In vmm_core/state_unit/src/tests/terminal.rs, a_stop_stops_a_unit_whose_start_is_uncertain adds a unit to a running set, as a hot-add does, that depends on a running unit and whose task starts it but drops its start request unanswered, and requires a stop to stop it before its dependency and to leave it terminal. requests_fail_after_a_start_leaves_a_terminal_unit and requests_fail_while_a_terminal_unit_keeps_the_set_running require a reset, a save, a restore, and a time advance each to fail, naming only that unit, without sending any unit a request, after a start that leaves a unit terminal, and while such a unit keeps the units running. Without the change, the stop and the first reset panic on the state assertion, and the reset of a running set on its running assertion.

Tear the VM down when a restart cannot resume it (6ddd6ca50)

• Cause. A worker restart stops the state units of a running VM before it saves the VM, and starts them again when the save fails. When that start failed too, the worker reported the failure and kept serving the VM, which it still marked running although its state units were stopped, or left terminal by the failed start. The next pause, reset, or pulse save and restore, and the worker's teardown, then stopped units that did not run, and panicked on the running assertion of StateUnits::stop.
• Change. As state_unit: roll back a partial start instead of panicking #4872 does, the worker tears the VM down after such a restart, since the VM cannot resume, and its teardown stops the state units when they run (StateUnits::is_running), rather than when the VM is marked running.
• Scope. The microVM profile refuses worker restarts, so no NVX microVM reaches this path.
• Tests. None: no harness of the VM worker can fail a unit's start after a failed save in a running worker, and state_unit: roll back a partial start instead of panicking #4872 has none either.

Validation

At this head, 6ddd6ca50, with Rust 1.97.1. It is 46545c262 on 46bfe02ed, which NVX's first validation tested, rebased onto 4c2697aef; git range-diff shows both patches identical, and the commits from 46bfe02ed to 4c2697aef change none of their files.

• Windows: cargo clippy --all-targets -p state_unit -p openvmm_core -- -D warnings and cargo doc --no-deps for both with RUSTDOCFLAGS=-D warnings, without warnings; cargo nextest run --profile agent -p state_unit -p openvmm_core, 148 passed; cargo xtask fmt --fix, clean.
• Linux (NVX's KVM and MSHV test VMs, Azure Linux 3.0, glibc and musl builds): the same clippy, rustdoc, and nextest commands, without warnings, 148 passed.
• Without the change, the three new tests alone on 4c2697aef fail, as described above, on both Linux VMs, as they did on 46bfe02ed there and on Windows.
• End to end, NVX's microVM lifecycle, managed-lifecycle, sandbox-lifecycle, snapshot-core, and snapshot-rollback scenarios passed with OpenVMM built from this head on KVM, MSHV, and WHP (NVX's test VMs), as they did at 46545c262. No NVX microVM reaches the panics that this fixes.

Land by fast-forwarding feature/microvm/dev to this head, 6ddd6ca505a7212d92e2a8e561a19635f7185d4a, since this repository's merge button squashes and would leave NVX's pin off the branch.

[github-actions]

2 Petri tests failed (1 unstable)