[Pedro Henrique Penna (ppenna)]

Keeps a state unit whose task has ended from panicking the VM worker, for microsoft/nvx#1363. StateUnits::run_op, which stop, reset, save, restore, advance_time, and the rollback of a failed start use, panicked with UnitRecvError { .., source: Channel(Closed) } when a unit did not answer its request, as when its task has exited, by a stated policy that unit failure is not recoverable. OpenVMM's profiles abort on panic, so the process ended without reporting the failure or tearing the VM down. Only start already failed instead, leaving the unit in the terminal StartUncertain state.

Three commits on 6ddd6ca50, the head of feature/microvm/dev. microsoft/nvx#1448 pins this head.

Roll back a partial start without panicking on a lost unit (2fcd1573d)

• Cause. The rollback of a failed start, which the chain's position 015 adds, stops the units that did start with run_op, so a started unit whose task then exits panics it. It also stops a started unit while a unit that depends on it may still run, such as a unit whose start is uncertain or, in a hot-add, a unit that already ran.
• Change. As the later revisions of state_unit: roll back a partial start instead of panicking #4872 do, the rollback stops the started units in reverse dependency order: a started unit is not sent a stop request while a unit that depends on it is not stopped, unless that unit was removed, and a started unit whose stop is blocked or not answered is left in StartUncertain. Such a unit may still run, so the units then stay running, and a later StateUnits::stop stops it. A unit that failed to confirm its start is still stopped with the units that started, and can start again.
• Tests. In tests/start.rs, a dependency of a unit whose start is uncertain, a started unit whose task exits before the rollback stops it, a new dependency of a running unit, and a unit that drops the rollback's stop request each fail the start and stay running, and a later stop stops them. At 6ddd6ca50, two of them panic in run_op with UnitRecvError, and the rollback stops the dependency in the other two.

Fail a stop instead of panicking when a unit is lost (5a750a862)

• Cause. StateUnits::stop returned () and panicked in run_op on a unit that did not answer. A microVM stops its units for every snapshot quiesce, pause, and teardown.
• Change. As state_unit: fail a stop instead of panicking when a unit is lost #4937 proposes upstream, the stop uses the rollback's ordered stop, which moves to a module that both share, and returns anyhow::Result<()>. A unit whose stop request is not answered is left in a new terminal state, StopUncertain, and the units that it depends on are not sent a stop request, since it may still use them; the units then stay running, so that a later stop can stop them. A unit removed during the stop, even while it waits behind a unit whose stop is not answered, is not a failure and no longer keeps its dependencies running. A stop also stops a unit in a terminal state, which keeps its marker.
• Callers.
  • LoadedVm::pause returns the error, leaving the VM paused with state units running. A reset, a pulse save and restore, a VMRS dump, a pause, a worker restart, and PCI resource assignment report it, and the worker then tears the VM down, since those units can never start again: it does so whenever state units run while the VM does not, as a failed stop, or a start whose rollback did not stop every unit, leaves them.
  • VmRpc::Pause becomes failable, as this chain made VmRpc::Resume, so that the REPL, the ttrpc PauseVm handler, the VM controller's disk snapshot, and petri report a failed pause. During a microVM snapshot boundary it fails, as a resume does, rather than being dropped.
  • A microVM snapshot quiesce whose units do not all stop fails with the new SnapshotQuiesceError::Unrecoverable, and the VM controller terminates the source, which can neither roll back nor resume, with nothing published.
  • The teardown, a failed snapshot boundary, a failed restore readiness event, and an expired restore gate log a failed stop, since the VM ends anyway.
  • OpenHCL reports a failed stop to the requester of a pause, which UhVmRpc::Pause now allows, a diagnostic save, a worker restart, or a servicing request, and keeps serving, as after a failed start; state_unit: fail a stop instead of panicking when a unit is lost #4937 tears it down instead.
• Tests. In the new tests/stop.rs, a lost unit fails the stop, keeps its dependency running, and fails a later start, reset, save, restore, and time advance, while the units run and after another stop; a unit whose stop is not answered stops when asked again and stays terminal; and a unit removed during a stop does not keep its dependency running, even while it waits behind a unit whose stop is not answered. At the previous commit, the first two, and the removal behind an unanswered stop, panic in run_op with UnitRecvError. The existing tests that stop units, among them [DO NOT REVIEW] state_unit: handle a unit in a terminal state instead of panicking #5012's in tests/terminal.rs, unwrap the result, and the snapshot filter's tests count Pause among the failable requests.

Fail a reset, save, restore, or time advance on a lost unit (a8a0e3517)

• Cause. reset, save, restore, and advance_time reach the same panic in run_op. A microVM saves its state units for every snapshot, and restores them and advances their time for every restore.
• Change. run_op fails the operation instead, naming the unit in the operation's StateTransitionError, and leaves the unit in the operation's terminal state, ResetUncertain, SaveUncertain, RestoreUncertain, or AdvanceTimeUncertain, since its state is unknown, so that a later start, reset, save, restore, or time advance fails too. A unit removed during the operation still does not fail it. run_op now serves only these failable requests.
• Tests. In the new tests/lost.rs, a unit built with UnitBuilder::build whose receiver is dropped, as in the issue's reproduction, fails a save, a reset, a restore, and a time advance, naming the unit, while the other units complete it, and stays terminal; a unit removed during a save does not fail it. At the previous commit, the first four panic in run_op with UnitRecvError { name: "lost", source: Channel(Closed) }.

Decisions

• Each operation leaves a unit that does not answer in a terminal state of its own, rather than in one shared state, so that the state names the request that went unanswered.
• VmRpc::Pause becomes failable, like this chain's VmRpc::Resume, rather than dropping its reply or panicking on the failed stop, as state_unit: fail a stop instead of panicking when a unit is lost #4937's worker does.
• A failed snapshot quiesce gets the new SnapshotQuiesceError::Unrecoverable, since neither RollbackSafe, after which the source resumes, nor Rejected, which refuses the request before it quiesces anything, describes a source that can never run again.
• state_unit: roll back a partial start instead of panicking #4872's other later change, which keeps the units running after a start whose outcome is uncertain, and not only after an incomplete rollback, is not part of this pull request.

Validation

At this head, a8a0e3517, with Rust 1.97.1:

• Windows: cargo nextest run --profile agent -p state_unit -p openvmm_core -p openvmm_defs -p openvmm_entry -p petri, 505 passed; cargo clippy --all-targets and cargo doc --no-deps (RUSTDOCFLAGS=-D warnings) for those packages, without warnings; cargo check -p vmm_tests --tests; and cargo xtask fmt --fix, clean. Each commit builds them, and passes the state_unit tests, 24, 28, and 33.
• Linux (NVX's KVM test VM, Azure Linux 3.0): the same commands with underhill_core added, without warnings, 599 passed.
• The issue's reproduction: at 6ddd6ca50, a stop, reset, save, restore, and time advance of a unit whose receiver is dropped panic with UnitRecvError { name: "lost", source: Channel(Closed) }, while its start fails; at this head all six fail with an error naming the unit, locally on Windows and on NVX's KVM and WHP test VMs.
• End to end: NVX's microVM guest-boot, lifecycle, managed-lifecycle, sandbox-lifecycle, restore-downtime, snapshot-core, and snapshot-rollback scenarios passed with 1 and 2 vCPUs, with OpenVMM built from this head's tree on NVX's KVM, MSHV (musl), and WHP test VMs.

The Linux checks and the test VMs ran e9e626604, this head before two of its commit messages were reworded, with the same tree.

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

[github-actions]

3 Petri tests failed (1 unstable)