Repository navigation
openvmm: preserve the snapshot VM clock cut - #128
Merged
Merged
Conversation
tangruize
force-pushed
the
fix/snapshot-clock-cut
branch
from
October 10, 2026 12:04
6ee50ba to
b99ca56
Compare
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
2 open findings
What changed in this PR
This PR fixes guest clock drift across snapshot save/restore by recording the time interval between when VM time (RTC/VmTime) is cut during quiesce and when the public time-ABI capture anchor is recorded, then applying that interval during restore.
Changes:
- Record and serialize a wall-clock sample when
VmTimeKeeperstops, and expose it via the vmtime saved-state. - Add
vm_time_cut_to_anchor_nstoSnapshotTimeContractand compute/validate combined VM-time downtime (cut_to_anchor + anchor_downtime). - Restore path now advances VM time/RTC by the combined downtime; docs/tests updated accordingly.
| File | Description |
|---|---|
| vm/vmcore/src/vmtime.rs | Captures stop wall-clock timestamp alongside stopped VM time and persists it in saved-state; updates keeper stop plumbing and tests. |
| openvmm/openvmm_helpers/src/snapshot/time.rs | Validates the new contract field and adds tests for combined VM-time downtime behavior. |
| openvmm/openvmm_entry/src/microvm/restore.rs | Updates test fixtures to include the new optional time contract field. |
| openvmm/openvmm_defs/src/time_abi.rs | Adds optional vm_time_cut_to_anchor_ns and a helper to compute/validate combined VM-time downtime. |
| openvmm/openvmm_core/src/worker/dispatch/time_abi.rs | Computes vm_time_cut_to_anchor_ns from vmtime saved-state, uses combined downtime for VM time advancement, and logs legacy behavior. |
| openvmm/openvmm_core/src/worker/dispatch/microvm.rs | Records the cut-to-anchor interval after saving state units (x86_64 only). |
| Guide/src/user_guide/openvmm/snapshots.md | Documents the new cut-to-anchor interval and legacy snapshot behavior. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ppenna
force-pushed
the
fix/snapshot-clock-cut
branch
from
October 11, 2026 17:47
b435a48 to
c170be6
Compare
ppenna
added a commit
to microsoft/nvx
that referenced
this pull request
Oct 11, 2026
c170be6fa (nanvix/openvmm#128, rebased onto main at 2130f24b5) keeps VM time and the RTC in step with the guest TSC across a microVM snapshot. Capture quiesces the state units, which stops VM time and the RTC, before it takes the time ABI capture anchor, while the guest TSC keeps running. Restore advanced VM time and the RTC by the anchor-to-restore downtime D alone, so they resumed behind the TSC by the interval from that cut to the anchor. VM time's stop now samples host UTC, which the vmtime state unit saves, and capture records the interval Q from that sample to the anchor's host UTC in the time contract's new optional field 12, vm_time_cut_to_anchor_ns. Restore advances VM time and the RTC by D + Q, while the TSC, the LAPIC timers, and the restore packet keep D, and it rejects a recorded Q or a D + Q above 30 days with E_DOWNTIME_EXCESSIVE. Recording Q is best effort: capture logs a warning and omits it when the saved stop sample is missing or Q would be negative or above 30 days, and a snapshot without it, including every earlier one, restores with Q = 0 and a warning. Manifests stay at version 6. The time ABI specification follows: its notation defines Q, and Downtime semantics, capture step 4, the SnapshotTimeContract table, restore steps 2, 5, and 15, E_DOWNTIME_EXCESSIVE, and the Guide appendix describe it. Snapshot and restore's restore sequence advances VM time, the RTC, and the PIT by the downtime plus Q. The upstream roadmap records the new pin, extends the branch DAG to position 310, and indexes positions 309 and 310. The rebased commits are patch-equivalent to the reviewed head b435a486b: git range-diff matches both, and the stable patch IDs of the complete diffs against their bases are equal. On the rebased head, clippy, rustdoc, the unit tests, and xtask fmt pass on Windows and Linux for openvmm_core, openvmm_defs, openvmm_entry, openvmm_helpers, and vmcore. nvx.py verify passes with this pin, and the default test-microvm suite and the public managed-execution scenarios pass on bare-metal KVM, MSHV, and WHP hosts and on a local WHP host, as does KVM's debug-kernel suite. Every restore that logged its clocks recorded Q, from 0.16 to 25 ms, and advanced VM time by exactly D + Q. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ppenna
force-pushed
the
fix/snapshot-clock-cut
branch
from
October 11, 2026 18:36
c170be6 to
9907316
Compare
ppenna
added a commit
to microsoft/nvx
that referenced
this pull request
Oct 11, 2026
99073163c (nanvix/openvmm#128, rebased onto main at 2130f24b5) keeps VM time and the RTC in step with the guest TSC across a microVM snapshot. Capture quiesces the state units, which stops VM time and the RTC, before it takes the time ABI capture anchor, while the guest TSC keeps running. Restore advanced VM time and the RTC by the anchor-to-restore downtime D alone, so they resumed behind the TSC by the interval from that cut to the anchor. VM time's keeper now reads the host monotonic clock that the capture anchor samples on both sides of its stop and saves the midpoint with VM time's state, and capture records the interval Q from that cut to the anchor's host monotonic time in the time contract's new optional field 12, vm_time_cut_to_anchor_ns, so that a host wall-clock step cannot enter it. Restore advances VM time and the RTC by D + Q, while the TSC, the LAPIC timers, and the restore packet keep D, and it rejects a recorded Q or a D + Q above 30 days with E_DOWNTIME_EXCESSIVE. Recording Q is best effort: capture logs a warning and omits it when the saved stop sample is missing or Q would be negative or above 30 days, and a snapshot without it, including every earlier one, restores with Q = 0 and a warning. Manifests stay at version 6. The time ABI specification follows: its notation defines Q, and Downtime semantics, capture step 4, the SnapshotTimeContract table, restore steps 2, 5, and 15, E_DOWNTIME_EXCESSIVE, and the Guide appendix describe it. Snapshot and restore's restore sequence advances VM time, the RTC, and the PIT by the downtime plus Q. The upstream roadmap records the new pin, extends the branch DAG to position 310, and indexes positions 309 and 310. The pinned commits are the reviewed head b435a486b's two commits, rebased onto main without conflicts, which git range-diff and the stable patch IDs of their complete diffs showed to be patch-equivalent, and then amended to measure the cut on the host monotonic clock instead of host UTC, as review of this pin found that a host wall-clock step between the cut and the anchor would otherwise enter Q. The amendment adds a test that such a step leaves Q unchanged. On 99073163c, clippy, rustdoc, the unit tests, and xtask fmt pass on Windows and Linux for openvmm_core, openvmm_defs, openvmm_entry, openvmm_helpers, and vmcore, and nvx.py verify passes with this pin. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ppenna
force-pushed
the
fix/snapshot-clock-cut
branch
from
October 11, 2026 19:01
9907316 to
6d6e9c1
Compare
ppenna
added a commit
to microsoft/nvx
that referenced
this pull request
Oct 11, 2026
6d6e9c1e8 (nanvix/openvmm#128, rebased onto main at 2130f24b5) keeps VM time and the RTC in step with the guest TSC across a microVM snapshot. Capture quiesces the state units, which stops VM time and the RTC, before it takes the time ABI capture anchor, while the guest TSC keeps running. Restore advanced VM time and the RTC by the anchor-to-restore downtime D alone, so they resumed behind the TSC by the interval from that cut to the anchor. VM time's keeper now reads the host monotonic clock that the capture anchor samples at the instant VM time stops and saves the reading with VM time's state, and capture records the interval Q from that cut to the anchor's host monotonic time in the time contract's new optional field 12, vm_time_cut_to_anchor_ns, so that a host wall-clock step cannot enter it. Restore advances VM time and the RTC by D + Q, while the TSC, the LAPIC timers, and the restore packet keep D, and it rejects a recorded Q or a D + Q above 30 days with E_DOWNTIME_EXCESSIVE. Recording Q is best effort: capture logs a warning and omits it when the saved stop sample is missing or Q would be negative or above 30 days, and a snapshot without it, including every earlier one, restores with Q = 0 and a warning. Manifests stay at version 6. The time ABI specification follows: its notation defines Q, and Downtime semantics, capture step 4, the SnapshotTimeContract table, restore steps 2, 5, and 15, E_DOWNTIME_EXCESSIVE, and the Guide appendix describe it. Snapshot and restore's restore sequence advances VM time, the RTC, and the PIT by the downtime plus Q. The upstream roadmap records the new pin, extends the branch DAG to position 310, and indexes positions 309 and 310. The pinned commits are the reviewed head b435a486b's two commits, rebased onto main without conflicts, which git range-diff and the stable patch IDs of their complete diffs showed to be patch-equivalent, and then amended after review of this pin: the cut is measured on the host monotonic clock instead of host UTC, which let a host wall-clock step between the cut and the anchor enter Q, and the primary VM time keeper reads that clock where it computes the stop time, so that no scheduling delay or host suspend around the asynchronous stop can misplace the cut. A test shows that a host wall-clock step leaves Q unchanged. On 6d6e9c1e8, clippy, rustdoc, the unit tests, and xtask fmt pass on Windows and Linux for openvmm_core, openvmm_defs, openvmm_entry, openvmm_helpers, and vmcore, and nvx.py verify passes with this pin. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
During microVM snapshot capture, VmTimeKeeper stops virtual devices and the guest RTC before the public time-ABI capture anchor is recorded. On restore, VM time and the RTC were only advanced by the anchor-to-restore downtime, omitting the elapsed stop-to-anchor interval. This causes the guest system clock to resume behind the hardware TSC by that duration. Fix this by: 1. Reading a host clock beside the instant at which VmTimeKeeper's primary keeper computes the stop time and saving the reading in vmtime. The keeper's owner supplies the clock: the VM worker gives time ABI microVMs the host monotonic clock that the capture anchor samples, so a host wall-clock step cannot enter the interval, and the reading describes the same cut as the stop time. 2. Deriving the cut-to-anchor interval from the anchor's host monotonic time and storing it as an optional field (vm_time_cut_to_anchor_ns) in SnapshotTimeContract. 3. Advancing VM time and the RTC by (anchor_downtime + cut_to_anchor_ns) on restore, bounded by MAX_DOWNTIME_NS. Existing snapshots without this field decode as absent and retain legacy anchor-only advancement. Co-authored-by: Pedro Henrique Penna <ppenna@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The VM-time cut is optional snapshot metadata. Failing to derive it from freshly saved vmtime state should not abort an otherwise usable snapshot, since restore can retain the legacy capture-anchor downtime behavior. Share the vmtime state-unit name between registration and lookup, validate the cut before publishing it, and warn while omitting the field when saved state is missing, malformed, or inconsistent with the capture anchor. A test shows that a host wall-clock step between the cut and the anchor leaves the recorded cut unchanged. Co-authored-by: Pedro Henrique Penna <ppenna@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ppenna
force-pushed
the
fix/snapshot-clock-cut
branch
from
October 12, 2026 19:46
6d6e9c1 to
d43dd87
Compare
ppenna
added a commit
to microsoft/nvx
that referenced
this pull request
Oct 12, 2026
d43dd87d5 (nanvix/openvmm#128, rebased onto main at 70dde4c65) keeps VM time and the RTC in step with the guest TSC across a microVM snapshot. Capture quiesces the state units, which stops VM time and the RTC, before it takes the time ABI capture anchor, while the guest TSC keeps running. Restore advanced VM time and the RTC by the anchor-to-restore downtime D alone, so they resumed behind the TSC by the interval from that cut to the anchor. VM time's keeper now reads the host monotonic clock that the capture anchor samples at the instant VM time stops and saves the reading with VM time's state, and capture records the interval Q from that cut to the anchor's host monotonic time in the time contract's new optional field 12, vm_time_cut_to_anchor_ns, so that a host wall-clock step cannot enter it. Restore advances VM time and the RTC by D + Q, while the TSC, the LAPIC timers, and the restore packet keep D, and it rejects a recorded Q or a D + Q above 30 days with E_DOWNTIME_EXCESSIVE. Recording Q is best effort: capture logs a warning and omits it when the saved stop sample is missing or Q would be negative or above 30 days, and a snapshot without it, including every earlier one, restores with Q = 0 and a warning. Manifests stay at version 6. The time ABI specification follows: its notation defines Q, and Downtime semantics, capture step 4, the SnapshotTimeContract table, restore steps 2, 5, and 15, E_DOWNTIME_EXCESSIVE, and the Guide appendix describe it. Snapshot and restore's restore sequence advances VM time, the RTC, and the PIT by the downtime plus Q. The upstream roadmap records the new pin, retains the partition halt-state fix at position 309, extends the branch DAG to position 311, and indexes the clock-cut commits at positions 310 and 311. The original reviewed head b435a486b was rebased and amended after review of this pin: Q uses the host monotonic clock rather than UTC, and the primary keeper reads it where it computes the stop time rather than around the asynchronous stop. This preserves the fixes for host wall-clock steps and scheduling delays, including the regression test. Rebase the validated source head 6d6e9c1e8 onto the latest OpenVMM main 70dde4c65 without conflicts. Both commits match in git range-diff and the complete diffs against their bases share stable patch ID 8b563430cc7079186a2019aac1c321ccc5ff8ea2. The new pin retains nanvix/openvmm#129's partition halt-state fix, now part of both default branches, and fast-forwards the current NVX pin by exactly two commits. Rebase this single NVX commit onto dev at d206d61, preserving the upstream changes and resolving the gitlink and roadmap conflicts with the combined revision. On the rebased source, cargo check, clippy, rustdoc, all 500 unit tests, and xtask fmt pass for the five clock-cut packages and vmm_core. NVX verification and the affected CLI, snapshot, and timing tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Rebased this branch onto the latest Both commits are unchanged patches relative to the previously validated head On the new source head, Windows package-scoped check, clippy, rustdoc, nextest (500/500 tests), and formatting checks passed for the five clock-cut packages plus |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fix a clock drift issue where the guest system clock (
VmTime/RTC) falls behind the hardware TSC after a snapshot save and restore.Problem & Root Cause
During microVM snapshot capture, quiescing and state serialization take place in stages:
StateUnits::quiesce_for_save()stops virtual devices andVmTimeKeeper(stoppingVmTimeand the virtual RTC).Because
VmTimestops earlier than the public time-ABI anchor, there is a positive time interval between the VM-time cut and the anchor capture. On restore,VmTimeand the RTC were only advanced by the anchor-to-restore downtime, omitting this cut-to-anchor interval. Consequently, across a snapshot and restore cycle, the guest system clock resumes behind the hardware TSC by that elapsed interval.Moving the public capture anchor earlier is not an option: its host sample is paired with the captured TSC and remains the correct source point for TSC and LAPIC timer restoration.
Solution
Record the interval between the VM-time cut and the public capture anchor, and apply it to VM-time and RTC advancement upon restore:
VmTimeKeeperstops, saved as capture metadata in thevmtimestate unit.vm_time_cut_to_anchor_ns), storing it as an optional field inSnapshotTimeContract.VmTimeand the RTC byanchor_downtime + vm_time_cut_to_anchor_ns.MAX_DOWNTIME_NSpolicy against the combined duration.Compatibility
vm_time_cut_to_anchor_nsis optional. Older snapshots without this field decode with it as absent, preserving the previous anchor-only advancement behavior while logging a warning.