fix(OSX timekeeping): self-manage timekeeping on OSX by twitchyliquid64 · Pull Request #986 · gominimal/minimal

twitchyliquid64 · 2026-07-27T21:54:23Z

Fixes: #985

The guest kernel has no TSI or virtio-vsock datagram implementation, so the libkrun-managed timekeeper isnt accessible. We run our own.

Summary by CodeRabbit

New Features
- Added automatic host-to-guest wall-clock synchronization for minimal microVMs.
- Time updates are delivered over a stream-based channel, with improved handling of clock changes (including after time jumps).
- macOS VMM startup now best-effort initializes the timekeeping bridge automatically.
Bug Fixes
- Improved resilience to interrupted/incomplete updates and silent connections; truncated trailing frames now end cleanly without unintended clock adjustments.
- Refined warning behavior to re-enable one-shot warnings after successful time updates.
Tests
- Updated and added async test coverage for stream frame decoding and idle-timeout behavior.

Note

Fix OSX timekeeping by self-managing host-to-guest clock sync over vsock

Adds a new timekeep module in crates/minvmd that runs a background sender thread on macOS, waking every 5s to detect clock jumps and sending heartbeats every 60s as 8-byte little-endian nanoseconds-since-epoch over a libkrun-bridged vsock stream (port 7351).
Refactors the guest-side listener in crates/minimald from a datagram socket on port 123 to a vsock stream server on TIMEKEEP_PORT (7351), reading 8-byte frames and calling clock_settime when drift exceeds threshold; idle connections are dropped after 300s.
The timekeep listener is now always enabled for microVM init (previously only on aarch64), using the fixed guest::TIMEKEEP_PORT.
Behavioral Change: datagram-based time sync on port 123 is removed and replaced with a stream-based protocol; any host or guest component relying on the old mechanism must be updated.

^{Macroscope summarized 048ae24.}

coderabbitai · 2026-07-27T21:54:42Z

📝 Walkthrough

Walkthrough

The change replaces datagram-based guest time synchronization with an 8-byte timestamp stream, adds a macOS host sender over UDS/libkrun, and configures both sides to use port 7351 with retry, clock-jump detection, and stream-focused tests.

Changes

Host time synchronization

Layer / File(s)	Summary
Guest stream listener and frame handling `crates/minimald/src/guest.rs`	minimald accepts vsock streams, decodes little-endian nanosecond frames, updates `CLOCK_REALTIME`, resets warning suppression after success, and handles EOF, truncation, or idle connections.
Host timestamp sender `crates/minvmd/src/lib.rs`, `crates/minvmd/src/timekeep.rs`	minvmd adds the timekeep module, derives `timekeep.sock`, detects wall-clock jumps, and sends periodic timestamp frames with reconnect behavior and wire-format tests.
Guest configuration and bridge startup `crates/minimald/src/main.rs`, `crates/minvmd/src/cmd/vmm_child.rs`	minimald always listens on port 7351; macOS minvmd registers the bridge and starts the sender on a best-effort basis.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

gominimal/minimal#961: Modifies the guest-side timekeeping implementation in the same listener and timestamp-update area.

Sequence Diagram(s)

sequenceDiagram
  participant HostClock
  participant timekeep
  participant libkrun
  participant minimald
  HostClock->>timekeep: detect heartbeat or clock jump
  timekeep->>libkrun: write 8-byte timestamp to bridged socket
  libkrun->>minimald: deliver timestamp over vsock stream
  minimald->>minimald: decode frame and update CLOCK_REALTIME

Poem

A rabbit hops where timestamps stream,
Eight bytes carry the daylight dream.
Through UDS paths the seconds flow,
Port seven-three-five-one rings.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description covers the summary but omits the required Testing and Checklist sections from the template.	Add a ## Testing section with the commands or manual validation performed, and include the ## Checklist items or note why they do not apply.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address `#985` by moving timekeeping to a host-side stream bridge and updating the guest listener accordingly.
Out of Scope Changes check	✅ Passed	The changes shown are all related to the timekeeping fix, including host bridge, guest listener, docs, and tests.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title matches the main change: replacing macOS timekeeping with self-managed guest/host sync.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (1)

crates/minvmd/src/timekeep.rs (1)
164-198: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

No connect/write timeout — a wedged bridge can stall this thread indefinitely.

UnixStream::connect and write_all/flush here have no deadline. If the kernel send buffer fills and the guest-side listener isn't reading (e.g. it's itself blocked — see the related comment on guest.rs's read_exact), this dedicated thread can block past TICK, delaying every later heartbeat/jump update. Consider a socket-level write/connect timeout (set_write_timeout on the UnixStream) as defense in depth alongside the guest-side fix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/minvmd/src/timekeep.rs` around lines 164 - 198, Add a socket-level
write timeout to the UnixStream created in the timekeeping loop before it is
stored in conn, using a bounded duration appropriate for the TICK retry cadence;
handle timeout-configuration failure consistently with connection setup. Ensure
write_all and flush in the existing stream send path cannot block indefinitely,
while preserving the current redial and retry behavior on errors.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/minimald/src/guest.rs`:
- Around line 690-705: Update serve_time_updates so each read_exact frame
operation is bounded by a tokio::time::timeout using a deadline of a few host
heartbeat intervals. Treat an elapsed timeout like UnexpectedEof by returning
Ok(()) so run_timekeep_listener can accept reconnects, while preserving existing
handling for successful reads, interruptions, and other I/O errors.

In `@crates/minvmd/src/timekeep.rs`:
- Around line 133-200: Update run’s scheduling state around due, jumped, and the
send result to track a pending jump-triggered update independently of last_sent.
Mark the jump update outstanding before attempting delivery, include that state
in the condition that bypasses the heartbeat, and clear it only after a
successful write/flush; retain it across connection failures so retries occur on
every TICK until delivery succeeds.

---

Nitpick comments:
In `@crates/minvmd/src/timekeep.rs`:
- Around line 164-198: Add a socket-level write timeout to the UnixStream
created in the timekeeping loop before it is stored in conn, using a bounded
duration appropriate for the TICK retry cadence; handle timeout-configuration
failure consistently with connection setup. Ensure write_all and flush in the
existing stream send path cannot block indefinitely, while preserving the
current redial and retry behavior on errors.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f13d57e8-f026-4649-865b-b858d48b4046

📥 Commits

Reviewing files that changed from the base of the PR and between f633a2a and 96fdd5d.

📒 Files selected for processing (5)

crates/minimald/src/guest.rs
crates/minimald/src/main.rs
crates/minvmd/src/cmd/vmm_child.rs
crates/minvmd/src/lib.rs
crates/minvmd/src/timekeep.rs

twitchyliquid64 requested a review from norrietaylor July 27, 2026 21:54

coderabbitai Bot reviewed Jul 27, 2026

View reviewed changes

Comment thread crates/minimald/src/guest.rs Outdated

Comment thread crates/minvmd/src/timekeep.rs

norrietaylor approved these changes Jul 27, 2026

View reviewed changes

fix(OSX timekeeping): self-manage timekeeping on OSX

048ae24

twitchyliquid64 force-pushed the tom/completions branch from 96fdd5d to 048ae24 Compare July 27, 2026 23:11

twitchyliquid64 enabled auto-merge (squash) July 27, 2026 23:12

twitchyliquid64 merged commit 3cbb8c3 into main Jul 27, 2026
29 checks passed

twitchyliquid64 deleted the tom/completions branch July 27, 2026 23:55

Navigation Menu

Uh oh!

fix(OSX timekeeping): self-manage timekeeping on OSX - #986

fix(OSX timekeeping): self-manage timekeeping on OSX#986
twitchyliquid64 merged 1 commit into
mainfrom
tom/completions

twitchyliquid64 commented Jul 27, 2026 •

edited by macroscopeapp Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 27, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

twitchyliquid64 commented Jul 27, 2026 • edited by macroscopeapp Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Fix OSX timekeeping by self-managing host-to-guest clock sync over vsock

Uh oh!

coderabbitai Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

twitchyliquid64 commented Jul 27, 2026 •

edited by macroscopeapp Bot

Loading

coderabbitai Bot commented Jul 27, 2026 •

edited

Loading