Repository navigation
T28 — Warn within a minute when the VM's switch datapath is lost - #1679
norrietaylor merged 1 commit into
Conversation
The switch datapath can be lost while the gvproxy process keeps running - an unlinked switch socket, a wedged accept loop - which the process-exit detection cannot see. The switch supervisor now probes the switch socket every DEFAULT_DATAPATH_CHECK_INTERVAL (30 s, under NET-023's one-minute bound) and emits a tracing::warn! naming the switch socket when connections stop being accepted, logging recovery at info level. minvmd run passes the interval where the switch is spawned; an intentional teardown sets the stopping flag so no probe warns during teardown.
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
Reviewed: 1 finding, none blocking at Address the comments on the diff. Clobber: none: |
|
Round 1 of 10 on the runtime's review findings: the coder changed nothing. Revise round 1: no new commits. The branch head 74411bf already carries the full task implementation from the previous session (crates/minvmd/src/net.rs: 30 s periodic datapath monitor in the switch supervisor, warn names the switch socket, recovery at info, monitor aborted on teardown; crates/minvmd/src/cmd/run.rs: passes DEFAULT_DATAPATH_CHECK_INTERVAL at HostGvproxy::spawn), and the head passes the test line (1 passed), cargo clippy -p minvmd --all-targets, and cargo fmt --check. This round's single thread was judged invalid, so nothing needed changing: the claimed stall requires tokio to await readiness on EAGAIN, but mio creates the connect socket with SOCK_NONBLOCK and the non-blocking AF_UNIX connect against a saturated backlog fails synchronously. Observed directly with the pinned tokio 1.53.1/mio 1.2.2 (throwaway experiment in /tmp, not in the repo): with 4096 pending connects held open and the accept loop wedged, tokio::net::UnixStream::connect returns Err(EAGAIN…[truncated] Review threads the coder judged:
|
There was a problem hiding this comment.
Reviewed against the T28 plan block on #1437.
Plan compliance. Files touched are exactly the two the block names: crates/minvmd/src/net.rs and crates/minvmd/src/cmd/run.rs. monitor_switch_datapath (net.rs:527-563) probes every 30 s (asserted under a minute), warns once on the loss transition naming the switch socket, logs recovery at info, exits on stopping, and is aborted in stop() and Drop; run.rs:326-330 passes the interval at the spawn site. That is NET-023's "warn within a minute, naming the socket, not only on process exit".
Test line. lost_datapath_warns_within_one_minute (net.rs:1063-1134) keeps a stand-in child alive, drops the listener and unlinks the socket, observes the warn within 5 s at a 50 ms interval, asserts the socket path and the recovery line, and stops cleanly. Task status and PR body record it at exit 0; CI green on all lanes; it also passes locally on macOS through just test-one.
Open thread (net.rs:540, minimal-review MEDIUM, "probe blocks when the accept backlog is saturated"): the coder answered by reply with a measurement on the pinned tokio and mio. The reasoning holds independently: a non-blocking AF_UNIX connect against a full backlog returns EAGAIN synchronously on Linux and ECONNREFUSED on macOS, never EINPROGRESS, so tokio surfaces the error before any readiness await and the probe cannot stall. Resolved as disputed-correct; a timeout would be belt and braces only.
Non-blocking observations.
- Each probe opens and drops a connection on gvproxy's listen socket, so expect one gvproxy-side read-error log line per 30 s.
- On an unexpected gvproxy exit, one extra "datapath lost" warn can follow the "exited unexpectedly" error before the supervisor drops; bounded to one line.
- The test relies on the thread-local subscriber under the current-thread runtime; a switch to multi-thread would fail loudly, not pass falsely.
Approving; merging with the runtime review concluded, the revise round landed, and the last thread resolved.
T28 — Warn within a minute when the VM's switch datapath is lost
slice: S6
kind: task
covers: NET-023
size: S
est: 120 code, 60 tests
executor: agent
difficulty: routine
model: light
diagnostics: the bundle's daemon log tail carries the VM host daemon's datapath warning with the switch socket
observability: one warn log line from the VM host daemon within a minute of the datapath failing, naming the switch socket
layers:
- vmhost:
- crates/minvmd/src/net.rs: the switch runtime adds a periodic datapath liveness check under a minute and warns on loss, not only on the switch process exiting
- crates/minvmd/src/cmd/run.rs: passes the check interval where the switch is spawned
test: cargo nextest run -p minvmd lost_datapath_warns_within_one_minute
produces: none
consumes: none
proof: Break the switch datapath under a running VM and the VM host daemon's log warns within a minute that the datapath is gone.
Proof
Under a running VM, if the switch datapath is lost while gvproxy itself keeps running (switch socket unlinked or no longer accepting connections), the VM host daemon's log now carries a tracing::warn! naming the switch socket within a minute — the supervisor probes the switch socket every 30 s (DEFAULT_DATAPATH_CHECK_INTERVAL, asserted under 60 s) and warns once on the loss transition, logging recovery at info level; process exit still logs its own error via the pre-existing detection. lost_datapath_warns_within_one_minute proves it: it breaks the datapath under a live stand-in switch process (the exit-detection half never fires) and asserts the warning, with the switch socket path, inside a 5 s deadline against a 50 ms interval.
Notes from the agent: Both layer files changed as planned: net.rs adds monitor_switch_datapath (connect-and-drop probe, same primitive as the startup readiness wait), the DEFAULT_DATAPATH_CHECK_INTERVAL const, the GvproxyConfig.with_datapath_check_interval builder, and a datapath_monitor task handle aborted on stop()/Drop; run.rs passes the default interval at the HostGvproxy::spawn site. Design points a reviewer should know: (1) the warn is emitted once per loss (transition) so the log carries one line rather than spamming every interval; recovery is info, and intentional teardown (stopping flag) never warns; (2) the probe detects a dead/unlinked socket and a saturated accept backlog, but a hung accept loop with an empty backlog still completes a connect and is not observable from the host side — documented on the monitor; (3) existing tests pass a 3600 s check interval so the monitor stays quiet about the nonexistent stand-in socket path. This box has no libkrun, so minvmd compiles as the stub (as on stock Linux CI); net.rs is not libkrun-gated and the named test exercises the monitor logic directly, but the live VM lanes (just e2e / just test-vm) were not run here and remain the lanes' evidence.
Tests
cargo nextest run -p minvmd lost_datapath_warns_within_one_minuteVerify
min task run agent-verifypassed ([tasks.verify]).Closes #1622