Repository navigation
Detach QEMU from agv instead of passing -daemonize - #5
Merged
Merged
Conversation
QEMU's own daemonize forks without exec'ing. On macOS the Objective-C runtime refuses to initialize a class for the first time in a forked child and aborts the process — and that is reachable from `savevm`, because HVF's GIC state save calls into Hypervisor.framework, which is Objective-C underneath. So `agv suspend` on Apple Silicon killed QEMU with SIGABRT, and agv saw only "QMP socket closed unexpectedly". Upstream has this confirmed and unfixed since August 2024 (https://gitlab.com/qemu-project/qemu/-/work_items/2515), where it also hits emulated targets on macOS, so waiting for a fix is not a plan. `spawn_detached` takes over what the flag did, in the shape agv already uses for the AVF runner and the forward supervisors: own process group, stdio to a log file, PID written from Rust. Three consequences: - `-pidfile` goes too. agv writes `pid` itself, so it exists before `start` returns rather than whenever QEMU gets to it. - The "QEMU is up" signal that `-daemonize`'s parent exit provided has to be rebuilt: `wait_for_qmp_socket` polls for the socket and fails fast if the process dies first, mirroring `wait_for_avf_socket`. - QEMU's stdout/stderr go to `<instance>/qemu.log` instead of a pipe that was only read if startup failed. Apple's abort message names the problem outright; agv used to throw it away. `agv suspend` now reports a QEMU that died with the tail of that log attached. The child is reaped rather than forgotten, which the other detached spawns get away with but this one cannot. Under `-daemonize` the process agv spawned exited at once and the real QEMU was orphaned to init, so a PID check was always honest. Spawned directly, QEMU is agv's own child, and an unwaited child becomes a zombie whose PID keeps answering `kill(pid, 0)` — `is_process_alive` would call a dead VM running for as long as the agv process lived. `qemu_start_and_force_stop` and `qemu_start_and_graceful_stop` caught exactly that. A tokio task holds the handle and waits; if agv exits first, init reaps as before. Verified on macOS aarch64: `suspend_and_resume_preserves_state` and `auto_suspend_idle_vm_suspends` both pass, having failed before this; the full slow suite is 14/14 where it was 12/2, and every other test file passes with --include-ignored. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Six findings, all real; the first was a regression this branch
introduced.
Startup success was inferred from `qmp.sock` existing. QEMU creates
that socket very early — before it opens -drive files, binds the
hostfwd port, realizes devices, or applies -loadvm — and does not
unlink it when it then exits with an error. Measured here with a
missing disk image: socket at 6.1ms, exit status 1 at 6.1ms, socket
still on disk. So `agv start` returned Ok for a QEMU that was already
gone, the VM was marked running, and the user met an unrelated SSH
timeout minutes later while qemu.log held the real cause. Worst on
`agv resume`, where a failed -loadvm would take a VM from suspended to
broken with the snapshot still intact. Readiness is now a completed
`QmpClient::connect` — greeting plus qmp_capabilities — which proves
QEMU is serving and past the point those failures occur, and liveness
is checked before the socket rather than after.
The rest:
- A stale qmp.sock is swept before spawn. Nothing else removes one:
`reconcile_status` cleans up only when the recorded status is
`running`, so a `broken` VM whose QEMU later dies keeps the file, and
the next start would have been satisfied by the previous boot's.
- The post-EOF liveness check now allows 2s for the exit to land. An
aborting QEMU has its fds closed by the kernel — which is the EOF we
just saw — while kill(pid, 0) keeps succeeding until it is reaped, so
an immediate check concluded "still alive" exactly when the qemu.log
excerpt was worth printing.
- A failed PID-file write kills the child instead of leaking a QEMU
that holds the qcow2 lock and hostfwd port with nothing able to
address it.
- force_kill is limited to the timeout path. On the exited path the
reaper has already collected the process, so the PID may belong to
something else by then.
- The log excerpt is one flat message rather than an anyhow context
layer. `main.rs` renders with `{err:#}`, which joins the chain with
": ", so a multi-line excerpt came out ahead of the reason it
belonged to.
Fixing the readiness check surfaced a latent bug it had been hiding.
QEMU's sun_path length check is `>` where every client's is `>=`, so at
exactly sizeof(sun_path) it binds a socket nothing can connect to —
confirmed on macOS at 104 bytes, with QEMU creating it and both Rust
and Python refusing it as "path too long". Such a VM started and was
then uncontrollable, failing later and confusingly in stop or suspend.
`build_qemu_args` now rejects it up front via `SocketAddr::from_pathname`,
which applies exactly the check the client will.
That in turn broke `auto_suspend_active_session_keeps_vm_running`,
correctly: macOS TMPDIR is 48 characters, leaving 24 for a VM name, and
that one is 25. `test_data_dir` anchors at /tmp, raising the budget to
68.
Verified on macOS aarch64: create_test 14/14 with --include-ignored,
every other test file green the same way, `just verify` and
`cargo +1.88 check --all-targets --locked` clean. A corrupt disk image
now fails `agv start` in 0.11s quoting QEMU's own message.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Fixes
agv suspendkilling QEMU on Apple Silicon, and makes a QEMU that dies say why.The bug
QEMU's
-daemonizeforks without exec'ing. On macOS the Objective-C runtime refuses to initialize a class for the first time in a forked child and aborts the process — andsavevmreaches exactly that, because HVF's GIC state save calls intoHypervisor.framework, which is Objective-C underneath. Soagv suspendon a QEMU-backend VM died with SIGABRT, and agv reported onlyQMP socket closed unexpectedly.Confirmed from the crash report (
termination: OBJC, SIGABRT):Isolated to the flag: same VM, same 37-argument command line,
-daemonizepresent → QEMU dies; absent →savevmreturns and QEMU survives.Upstream has this confirmed and unfixed since August 2024 — qemu-project/qemu#2515 — where it also hits emulated targets on macOS, so it is not suspend-specific and waiting is not a plan. Linux is unaffected: HVF is macOS-only, and
platform_args()gates-accel hvfbehindcfg(target_os = "macos").The change
spawn_detachedtakes over what the flag did, in the shape agv already uses for the AVF runner and forward supervisors: own process group, stdio to a log file, PID written from Rust.-pidfilegoes too — agv writespiditself, so it exists beforestartreturns.-daemonize's parent exit gave us is rebuilt aswait_for_qmp_ready.<instance>/qemu.loginstead of a pipe read only on startup failure. Apple's abort message names the problem outright; agv used to discard it.Two bugs found while building it
Zombies. Copying the AVF pattern exactly — including
mem::forget(child)— brokeqemu_start_and_force_stop. Under-daemonizethe spawned process exited at once and real QEMU was orphaned to init, which reaped it, so a PID check was always honest. Spawned directly, QEMU is agv's own child, and an unwaited child becomes a zombie whose PID keeps answeringkill(pid, 0). A tokio task now holds the handle and waits.Readiness by file existence was too weak (caught in review). QEMU creates
qmp.sockbefore it opens-drivefiles or applies-loadvm, and does not unlink it on error exit — measured: socket at 6.1ms, exit status 1 at 6.1ms, socket still on disk. Readiness is now a completedQmpClient::connect(greeting +qmp_capabilities). That in turn surfaced a latent bug: QEMU'ssun_pathcheck is>where every client's is>=, so at exactlysizeof(sun_path)it binds a socket nothing can connect to. Now rejected up front.Verification (macOS aarch64)
suspend_and_resume_preserves_stateandauto_suspend_idle_vm_suspendspass, having failed before thiscreate_test --include-ignored14/14 (was 12/2)--include-ignoredjust verifyandcargo +1.88 check --all-targets --lockedcleanagv startin 0.11s quoting QEMU's own message🤖 Generated with Claude Code