perf(sandbox): make boot readiness an event via guest vsock dial-out - #560
Conversation
|
This run croaked 😵 The workflow encountered an error before any progress could be reported. Please check the link below for details. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Greptile SummaryThe PR replaces cold-boot readiness polling with a guest-initiated hybrid-vsock event.
Confidence Score: 4/5The PR does not appear safe to merge at this HEAD because clock synchronization can still postpone Ready publication after the agent has signaled readiness. Although the prior reply states that AprilNEA fixed the issue in stacked PR #566, this HEAD still awaits the capped clock-sync operation before persisting and publishing Ready, leaving the previously reported delay intact. Files Needing Attention: virt/arcbox-vm/src/sandbox/boot.rs
|
| Filename | Overview |
|---|---|
| virt/arcbox-vm/src/bin/vm-agent.rs | Adds the best-effort guest-to-host readiness notification after both service listeners are bound. |
| virt/arcbox-vm/src/sandbox/boot.rs | Replaces polling with the readiness listener and performs capped, non-fatal cold-boot clock synchronization. |
| virt/arcbox-vm/src/vsock.rs | Adds readiness-socket path derivation, listener lifecycle management, event consumption, and focused tests. |
Sequence Diagram
sequenceDiagram
participant Host as Sandbox host
participant FC as Firecracker
participant Agent as vm-agent
Host->>Host: "Bind {vsock_uds}_51"
Host->>FC: InstanceStart
Agent->>Agent: Bind exec and file listeners
Agent->>FC: Connect CID 2, port 51
FC->>Host: Forward connection to UDS listener
Agent->>Host: Write readiness byte
Host->>Agent: Best-effort clock sync
Host->>Host: Persist and publish Ready
Reviews (2): Last reviewed commit: "perf(sandbox): make boot readiness an ev..." | Re-trigger Greptile
No CI failure to fix — the failed suite is Pullfrog's ownI could not find a CI failure introduced by this PR, because no CI ever ran on this branch. DiagnosisThe check suite named in the trigger ( Confirmed against the API for
Verification instead of a blind fixSince CI has never exercised this branch, I ran the gates it will face once retargeted. All green on
One pre-existing wart (not a blocker, not from this PR)Under the stricter No commit was pushed: the working tree is clean and there is nothing here that a code change would address.
|
567943a to
9455f95
Compare
Replace the cold-boot readiness gate's connect-poll (sync_clock retrying
the FC CONNECT handshake under backoff) with a real event: vm-agent dials
out to host port READY_PORT (51) once its exec and file listeners are up,
and Firecracker forwards that guest-initiated connect to a Unix socket at
{uds_path}_51 the host pre-binds before InstanceStart — accept() is the
readiness signal.
The clock sync stays on the boot path but no longer gates readiness:
best-effort with a 10 s cap, warn-and-continue, mirroring the restore
path. In jailer mode the ready socket is chown'd to the jailed uid/gid so
FC can connect to it. The listener removes a stale socket file on bind
and deletes its per-boot file on drop. An old vm-agent without the
dial-out would sit out the gate timeout, but template freshness keys
rebuild templates from the same build as the host, so the pairing holds.
The restore path is untouched: a resumed guest does not re-dial.
1db7503 to
866ce18
Compare
There was a problem hiding this comment.
Reviewed 3 files across 1 commit (866ce18c), covering the new ReadyListener vsock abstraction, the boot-gate rewrite in sandbox/boot.rs, and the guest agent's signal_ready() dial-out.
✅ No new issues found.
I chased the parts of this that could plausibly be wrong and they all hold up:
- Firecracker hybrid-vsock contract. Guest-initiated connections land on
{uds_path}_{port}with no handshake bytes (unlike the host-initiatedCONNECT/OKdirection), so reading a single byte inwait()is the right shape. Binding beforeInstanceStartis load-bearing — FC repliesVIRTIO_VSOCK_OP_RSTwhen no host listener exists — and the ordering is correct here. - The guest can't wedge on a missing listener. That same immediate RST means
signal_ready()'s blockingconnectfails fast rather than hanging; combined with theeprintln!-and-continue handling, a lost dial-out degrades to a 35s gate timeout instead of a stuck agent.signal_ready()also runs after bothcreate_vsock_listenercalls, which alreadylisten(fd, 128), so there's no accept race on the reverse direction. - Both path derivations agree. Jailed (
boot.rs:827-828, chroot-relative) and direct (boot.rs:862-868) both produce{uds}_51, and 51 doesn't collide with the existing 52/53. BootFailure { process: None, cow_handle: None }is right on the two new early returns — the FC process moves into the instance atboot.rs:713and the cow handle at880-883, both after these returns, matching the neighbouring returns at 908-916 and 1030-1035.Drop-unlink can't clobber a later generation's socket, sincecleanup.rs:47remove_dir_all(&vm_dir)runs before the slot is released atcleanup.rs:66.- The host/guest compat comment checks out. The rootfs cache key embeds a content hash of the injected
vm-agent(rootfs-{layer_key}-{agent_key}.ext4), and everyresolve_templatebranch injects the current binary — so a host expecting the dial-out can't pair with an agent that doesn't send it.
Both new tests are genuinely falsifiable (the suffix test asserts the exact path, and the listener test covers stale-file cleanup, dial, and Drop unlink).
One behavioral note, not a defect: READY no longer proves the host→guest vsock direction works before it's published. The old sync_clock gate exercised a host-initiated connect, so a broken forward path surfaced at the gate; now the first host-initiated call happens after READY. Given the guest dial-out and the host listener share the same device, a one-directional failure seems unlikely enough to not be worth guarding — flagging it only because the gate's coverage narrowed.
I did not re-raise the clock-sync-gating point from the earlier greptile thread; it's resolved and deferred to stacked #566.
ℹ️ Nitpicks
virt/arcbox-vm/src/vsock.rs:65-66— theMSG_CLOCK_SYNCdoc comment still reads "after snapshot restore, and as the cold-boot agent-readiness gate". This PR removes that second role, so the line is now stale. (Outside the diff, hence not inline.)
Claude Opus | 𝕏

Stacked on #559 (← #558 ← master); merge those first, then retarget/rebase. Part of CORE-80, project Sandbox Cold Start (the ptp_kvm clock-deletion half of CORE-80 is deliberately not in this PR).
What
Replaces the readiness gate's detection-by-polling with a real event, using Firecracker hybrid-vsock guest-initiated connections (convention verified against FC docs/vsock.md: a guest connect to port P arrives at the host Unix socket
{uds_path}_P):ReadyListenerbinds{vsock_uds}_51beforeInstanceStart(stale file removed; chown'd to the jailer uid/gid — FC needs write permission to connect; Drop deletes the per-boot socket on every path including boot failures).accept()+ one byte underAGENT_GATE_TIMEOUT; timeout/error takes the samefail_started_bootpath. Clock sync stays on the boot path but strictly best-effort (10 s cap, warn on failure), pending ptp_kvm.Hardware-validated (full stack: #558 + #559 + this)
Cold create→READY p50 1219 → 1118 ms (networked) / 1056 → 1065 ms (no-network), still converged with first-exec (1167/1121 ms) — the event fires the moment the agent is accepting, ~100 ms earlier than a completed clock round trip. Restore path untouched (229 ms p50). Full sandbox smoke green.
Validation
132 arcbox-vm unit tests + 1 integration green (new: ready-socket path pinning, ReadyListener accept/cleanup over a real UDS); clippy clean host + musl bins; fmt clean.