feat(minvmd,minimald): per-VM writable /dev/vdb ext4 volume - #672
Merged
Conversation
…bles Host side of the per-VM writable ext4 volume (spec #583 Unit 1). Adds the krun_add_disk3 FFI + SyncMode tri-state (KRUN_SYNC_{NONE,RELAXED,FULL}, a u32 not a bool) and Context::add_disk_with_sync. volume.rs provisions a sparse raw image (ftruncate, allocate-on-write) at the resolved path — MINVMD_DATA_VOLUME_PATH override, else <state>/data-vol.raw honouring XDG_STATE_HOME — and is idempotent. vmm_child attaches /dev/vdb on every boot with the MINVMD_DISK_SYNC / MINVMD_DISK_DIRECT_IO tunables (default relaxed / false). Refs: #583 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The guest formats /dev/vdb on first boot (ext4 superblock-gated mkfs, sized 1 MiB below the device to survive libkrun's backing-file trailer shave) and mounts it read-write at /var/lib/minimal; cache + state then relocate onto it, resolving the EXDEV hardlink constraint and persisting session state across clean restarts. Mount presence uses try_exists so a stat error is not silently treated as "no volume". Refs: #583 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
microvm-rootfs now ships the /var/lib/minimal mountpoint and e2fsprogs + util-linux (gominimal/pkgs#365/#366) so the guest can mkfs.ext4 and mount /dev/vdb (and fstrim-reclaim it). Re-pin locked_commit to 12f324d8. Refs: #583 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
krun_add_disk3 (this PR's /dev/vdb attach) needs libkrun >= 1.19.0. The macOS "Verify libkrun" steps (boot-e2e / autospawn-e2e / build-macos) and the Linux/KVM job now assert the libkrun in use exports krun_add_disk3, failing with an actionable "upgrade the runner" message instead of a cryptic dyld symbol-not-found at VM boot. Refs: #583 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a per-VM writable data volume attached as ChangesWritable data volume feature
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/minvmd/src/volume.rs (1)
102-160: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
create_sparse_rawerrors instead of recovering on a concurrent creator.
create_new(true)means a second caller racing to create the same path (e.g. two boots sharing an explicitMINVMD_DATA_VOLUME_PATH, or the per-host default path noted above under concurrent VMs) gets a hardVolumeError::Createrather than being treated as "already provisioned." Sinceensure_sparse_rawis documented as idempotent, consider re-checkingmetadata()on anAlreadyExistserror fromcreate_newand returningOk(())in that case.♻️ Suggested handling for the AlreadyExists race
let file = OpenOptions::new() .read(true) .write(true) .create_new(true) .open(path) - .map_err(|source| VolumeError::Create { - path: path.to_path_buf(), - source, - })?; + .map_err(|source| VolumeError::Create { + path: path.to_path_buf(), + source, + })?; + // Note: if `create_new` raced with another provisioner and lost, the + // caller's `ensure_sparse_raw` should treat `AlreadyExists` as success + // rather than surfacing an error here.🤖 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/volume.rs` around lines 102 - 160, ensure_sparse_raw and create_sparse_raw should handle the concurrent creation race more gracefully: when OpenOptions::create_new(true) in create_sparse_raw fails with AlreadyExists, treat it as an idempotent success instead of surfacing VolumeError::Create. Update ensure_sparse_raw to re-check metadata after that error path and return Ok(()) if the sparse volume already exists, keeping the behavior consistent with the idempotent contract.crates/minvmd/src/vm.rs (1)
294-317: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSilent fallback on invalid
MINVMD_DISK_SYNC/MINVMD_DISK_DIRECT_IOvalues.An unparseable/garbage
MINVMD_DISK_SYNCvalue (e.g. a typo like"relax") silently falls back toRelaxedwith no diagnostic, making a misconfiguration hard to notice.volume.rs's analogous size-mismatch path logs atracing::warn!instead of silently accepting the default.♻️ Suggested diff to log on fallback
let sync_mode = std::env::var(DISK_SYNC_ENV) .ok() - .and_then(|v| match v.trim().to_ascii_lowercase().as_str() { - "none" | "false" | "0" => Some(SyncMode::None), - "relaxed" | "true" | "1" => Some(SyncMode::Relaxed), - "full" | "2" => Some(SyncMode::Full), - _ => None, - }) - .unwrap_or(SyncMode::Relaxed); + .and_then(|v| match v.trim().to_ascii_lowercase().as_str() { + "none" | "false" | "0" => Some(SyncMode::None), + "relaxed" | "true" | "1" => Some(SyncMode::Relaxed), + "full" | "2" => Some(SyncMode::Full), + other => { + tracing::warn!(value = other, "unrecognized MINVMD_DISK_SYNC value; defaulting to relaxed"); + None + } + }) + .unwrap_or(SyncMode::Relaxed);🤖 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/vm.rs` around lines 294 - 317, The issue is that resolve_disk_flags silently falls back to defaults when DISK_SYNC_ENV or DISK_DIRECT_IO_ENV contain invalid values. Update the parsing in resolve_disk_flags to emit a tracing::warn! when an unrecognized value is encountered, similar to the size-mismatch handling in volume.rs, while still returning the existing default SyncMode::Relaxed or false. Use the DISK_SYNC_ENV and DISK_DIRECT_IO_ENV symbols to keep the warning context clear.
🤖 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 305-312: Reject undersized volumes in run_mkfs_ext4 before calling
mkfs.ext4: after computing fs_blocks from device_size_bytes, check whether the
result is zero (or otherwise below the minimum ext4 usable size after
MKFS_MARGIN_BYTES) and return an std::io::Error with ErrorKind::InvalidInput
instead of invoking the command. Use run_mkfs_ext4 and the fs_blocks calculation
as the key locations to update, keeping the existing mkfs.ext4 command path only
for valid block counts.
---
Nitpick comments:
In `@crates/minvmd/src/vm.rs`:
- Around line 294-317: The issue is that resolve_disk_flags silently falls back
to defaults when DISK_SYNC_ENV or DISK_DIRECT_IO_ENV contain invalid values.
Update the parsing in resolve_disk_flags to emit a tracing::warn! when an
unrecognized value is encountered, similar to the size-mismatch handling in
volume.rs, while still returning the existing default SyncMode::Relaxed or
false. Use the DISK_SYNC_ENV and DISK_DIRECT_IO_ENV symbols to keep the warning
context clear.
In `@crates/minvmd/src/volume.rs`:
- Around line 102-160: ensure_sparse_raw and create_sparse_raw should handle the
concurrent creation race more gracefully: when OpenOptions::create_new(true) in
create_sparse_raw fails with AlreadyExists, treat it as an idempotent success
instead of surfacing VolumeError::Create. Update ensure_sparse_raw to re-check
metadata after that error path and return Ok(()) if the sparse volume already
exists, keeping the behavior consistent with the idempotent contract.
🪄 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: ab3f84e2-eb9e-4092-83c5-4895f9f9d8c3
📒 Files selected for processing (13)
.github/workflows/ci-linux-kvm.yml.github/workflows/ci-macos.yml.minimal/minimal.tomlcrates/minimald/src/guest.rscrates/minimald/src/main.rscrates/minvmd/src/cmd/mod.rscrates/minvmd/src/cmd/vmm_child.rscrates/minvmd/src/krun/ctx.rscrates/minvmd/src/krun/mod.rscrates/minvmd/src/krun/raw.rscrates/minvmd/src/lib.rscrates/minvmd/src/vm.rscrates/minvmd/src/volume.rs
…storm On first boot the guest mkfs.ext4's the (default 32 GiB) /dev/vdb. With ext4's default lazy inode-table init, the ext4lazyinit kernel thread zeroes ~512 MiB of inode tables in the background right after mount — sustained I/O that lands exactly when the guest is bringing up its vsock bridge + SSH server on 2 vCPUs. That starved the first host->guest connect: a gvproxy-forwarder control request timed out (5s) and `minimal ls` failed with `ssh connect: Disconnected` (autospawn-e2e). boot-e2e was unaffected because its check used a 2 GiB volume. Format with `-E lazy_itable_init=0,lazy_journal_init=0` (do the zeroing now, at mkfs, not in the background) and `-i 65536` (~524K inodes / ~128 MiB table instead of ~2M / ~512 MiB) so the synchronous init is small and one-time; later boots detect the superblock and skip mkfs entirely. Refs: #583 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
run_mkfs_ext4 derived the ext4 block count as `device_size_bytes.saturating_sub(MKFS_MARGIN_BYTES) / EXT4_BLOCK_BYTES`, which is 0 for any device <= the 1 MiB margin — handing mkfs.ext4 a nonsensical size argument. Reject a device below MKFS_MIN_DEVICE_BYTES (16 MiB, comfortably above ext4's journal minimum) with InvalidInput before invoking mkfs. Only guards a misconfigured MINVMD_VOLUME_BYTES or malformed device; the real volume is GiB-scale. Refs: #583 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lish Server::run awaited start_host_proxies before entering the accept loop, and the DM1 host-expose there (expose_proxy_on_host -> gvproxy /services/forwarder/expose) blocks up to a 5s control-request timeout when the host gvproxy is slow. In that window the SSH listener is bound but not accepting, so the first host->guest connect lands in the gap and fails with `ssh connect: Disconnected` — seen in autospawn-e2e, where `minimal ls` connects right after READY. Attaching the per-VM data volume made the host gvproxy slow enough to hit this on macOS/HVF, exposing the latent bug (main passes only because gvproxy responds fast). The expose is already best-effort (warns + continues on failure), so detach it: bind the proxy listeners synchronously as before, but spawn the host-expose so it never gates the SSH accept loop. Refs: #583 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This reverts commit 671bec4. Detaching the host-side expose removed an accidental serialization that main relies on. With the expose spawned, the first host->guest SSH connect overlaps the guest->host expose control request on libkrun's single vsock device and wedges it (#588), so cold `minimal ls` fails with `ssh connect: Disconnected` in autospawn-e2e. Awaiting the expose (as on main) serializes vsock usage so the accept loop's first connection stays clean. The 5s cold-boot delay when the host gvproxy is absent is pre-existing on main and out of scope here. Refs: #588
norrietaylor
added a commit
that referenced
this pull request
Jul 8, 2026
…ning Downgrade `lifecycle-vsock-persistent` from settled to needs-spike: a held-open guest->host lifecycle socket collides with the host->guest SSH bridge under libkrun's concurrent-vsock wedge (#588) — the same failure that red-lit autospawn-e2e on #672. Record the spike under knowledge gaps. Fold #672's empirical mkfs hardening into the guest-boot section: the undersize guard, the trailing margin for libkrun's backing-file trailer shave, and eager inode/journal init that avoids a lazy-init storm. Refs: #583, #588, #672
norrietaylor
marked this pull request as ready for review
July 8, 2026 20:11
resolve_disk_flags silently swallowed unrecognized MINVMD_DISK_SYNC and MINVMD_DISK_DIRECT_IO values, falling back to the defaults with no signal. Emit a tracing::warn! naming the env var and value when a set value is unrecognized, keeping the existing relaxed/false defaults.
ensure_sparse_raw probes metadata then create_new; a provisioner racing between the two surfaced AlreadyExists as VolumeError::Create, breaking the documented idempotent contract. Treat an AlreadyExists creation race as success (the volume now exists) and keep the image as-is.
…nit1-v2 # Conflicts: # .github/workflows/ci-macos.yml
ci-macos boots minimald as pid-1 in the boot/e2e VM, yet its paths filter omitted crates/minimald — so guest-daemon-only changes skipped every macOS VM check, including autospawn-e2e. Add crates/minimald/** to the push + pull_request filters so guest-daemon changes get the coverage that exercises them.
The Auto-spawn E2E boots the guest, which publishes its host-side proxy via the host gvproxy over the vsock shuttle. With no gvproxy the publish blocks until its 5s control-request timeout, holding Server::run's SSH accept loop closed; the cold `minimal ls` connect-retry deadline then expires and the list fails (`ssh connect: Disconnected`). This is a pre-existing ~40ms race that /dev/vdb's ~60ms first-boot mkfs tips red. Fetch gvproxy and export MINVMD_GVPROXY_BIN so the publish completes immediately, matching a real host.
Replace the second `is_minimal_microvm()` branch (per Tom's review) with a hidden `mk_mount_state_volume: Option<String>` field on ListenArgs, set once in the microVM config block. The mount is then driven by `if let Some(dev) = listen_args.mk_mount_state_volume`. State/cache relocation stays gated on the mount succeeding — pointing state at an unmounted /var/lib/minimal would land it on the read-only rootfs (that becomes eager once Unit 2 makes mount failure loud).
Per Tom's review: make explicit that the non-default mkfs.ext4 options are not independent tuning but fall out of two deliberate choices — eager inode/journal init (whose entailment is the reduced inode count) and surviving libkrun's backing-file trailer shave (whose entailment is the explicit block count + pinned block size). ext4's defaults are otherwise left alone.
Per Tom's review, source the state-dir base from the shared `paths::minimal_state_dir()` resolver instead of the minvmd-local `StateDir::default_path`. Keep the explicit `XDG_STATE_HOME` override first: `dirs` (and thus `minimal_state_dir`) ignores it on macOS, and the e2e tests isolate state via `XDG_STATE_HOME` — without it the volume would escape the tests' temp dir onto the real state dir. Promote `paths` from a dev- to a regular dependency.
Resolves Tom's review point: the image is sparse and offline resize is hard, so size it big up front. Measured first-boot cost is flat in the volume size — mkfs.ext4 writes only ~5-8 MiB of metadata regardless (sparse backing + reduced inode ratio), so format+mount stays ~40 ms at 32/64/128/256 GiB on an M-series host. No boot-time penalty, so the default goes to 256 GiB; MINVMD_VOLUME_BYTES still overrides. Refs: #583
Measurement (32/64/128/256 GiB) shows mke2fs already defaults to a ~65536 bytes/inode ratio at these volume sizes — `-i 65536` produces an identical inode count (32 GiB -> 524288, 256 GiB -> 4194304, both 65536 B/inode). And the eager inode-table init is cheap because the volume is sparse (zero-writes land in holes; ~sub-ms at every size), not because of the inode count. So the override earns nothing: drop it plus MKFS_BYTES_PER_INODE and correct the doc rationale. Per Tom's review. Refs: #583
The wrapped closure exceeded rustfmt width (fmt CI). Also drop the stale "reduced inode ratio" credit from the DEFAULT_VOLUME_BYTES note now that the -i override is gone — the flat first-boot cost is purely the sparse backing.
… accept start_host_proxies awaits expose_proxy_on_host on Server::run's boot path, before the SSH accept loop serves. The `/services/forwarder/expose` control request runs to the full 5s GVPROXY_CONTROL_TIMEOUT even with a host gvproxy present (the forwarder control path isn't reachable over the shuttle in every deployment), holding the accept loop closed. The cold `minimal ls` connect-retry deadline (~5s) then expires as the loop would resume, so the first list fails with `ssh connect: Disconnected` — the volume's ~60ms first-boot mkfs tips a pre-existing race red on the e2e autospawn gate. Cap the publish at HOST_EXPOSE_PUBLISH_TIMEOUT (1s): a best-effort publish must never stall the SSH bridge. Best-effort semantics unchanged. Refs: #588
Provisioning a host gvproxy did not fix the autospawn stall — the boot log shows the `/services/forwarder/expose` control request still times out at 5s even with gvproxy present. The bounded expose (HOST_EXPOSE_PUBLISH_TIMEOUT) fixes it in the daemon instead, so the fetch step + MINVMD_GVPROXY_BIN export are dead weight; remove them.
msample
reviewed
Jul 9, 2026
msample
approved these changes
Jul 9, 2026
With boot-e2e and autospawn-e2e merged into one `e2e` job (upstream), the session harness's leftover `__krun-vmm` (its Drop kills minvmd but not the detached VMM grandchild) lingers into Auto-spawn. As a second concurrent VM it wedges the host->guest bridge over libkrun's vsock (#588): with the expose now bounded, the accept loop is up early yet the boot log shows no connection ever accepted. Reap the VMM so Auto-spawn runs single-VM, as it did when it was a standalone job. The proper fix is the session harness reaping its own grandchild (process group); tracked separately.
twitchyliquid64
approved these changes
Jul 9, 2026
norrietaylor
added a commit
that referenced
this pull request
Jul 9, 2026
Integrates #672 (per-VM /dev/vdb volume) with the composite extraction: - The two copies of main's new libkrun >= 1.19.0 (krun_add_disk3) symbol check in ci-macos.yml fold into the setup-libkrun-macos composite's verify step, so the release mac build gets it too. - The KVM lane's new krun_add_disk3 assert now reads LIBKRUN_PREFIX (published by the setup-libkrun-linux composite) — the KRUN_PREFIX env it referenced was retired with the composite extraction, so the auto-merged step would have probed an empty path. - main's crates/minimald/** path additions and the Session-E2E VM reap step (#588 fix for the merged e2e job) are kept as-is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
norrietaylor
added a commit
that referenced
this pull request
Jul 9, 2026
The __krun-vmm-only reap from #672 is insufficient: main run 29032763009 — on the very commit that added it — still failed the cold `minimal ls` with `ssh connect: Disconnected` against a fully healthy guest (READY emitted, minimald listening on vsock:2222, no connection ever accepted): the #588 bridge wedge. The remaining leftover is the host gvproxy switch, which minvmd owns and Guest::drop never kills; the failing runs' proxy-publish WARN corroborates a lingering gvproxy. Reap any stray minvmd first (so nothing respawns), then the VMM and gvproxy. The proper fix — the session harness reaping its own process group — stays tracked under #588; this keeps the merged e2e job's steps isolated in the meantime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
norrietaylor
added a commit
that referenced
this pull request
Jul 9, 2026
…ipt (#685) * ci: extract shared setup composites for rust, libkrun, and guest artifacts Four new composite actions dissolve copy-pasted setup blocks: - setup-rust: free-disk + protoc + rust-cache preamble, previously inlined in ci.yml clippy AND duplicated inside core-tests - setup-libkrun-macos: the brew slp/krun tap install, previously copy-pasted across ci-macos.yml and release.yml (the release job's separate dylib-existence check is dropped like ci-macos's was in #678: a failed brew install already fails the step, and a missing libkrun still fails loudly at link/rewrite time) - setup-libkrun-linux: fetch-libkrun.sh + the LIBKRUN_PREFIX / LD_LIBRARY_PATH exports, previously duplicated between ci-linux-kvm.yml and release.yml's amd64 build - guest-artifacts: the kernel + rootfs cache pulls, previously repeated in ci-macos.yml, ci-linux-kvm.yml, and release.yml The two fetch composites retry 3x to absorb transient cache/network hiccups; commands, arguments, and env are otherwise unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(release): extract minvmd linkage rewrite into a script The ~60-line inline install_name_tool rewrite + otool verification in build-release-macos-arm64 (grown across #664/#668/#670) moves to scripts/rewrite-macos-linkage.sh, command-for-command. A script lets CI exercise the production @rpath rewrite on a throwaway copy of the debug binary later, instead of the rewrite only ever running at release time. Signing stays the caller's job (Developer ID, last mutation before upload, per #680). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(setup-minimal): drop dead cache-minimal install conditional The Install Minimal step guarded on steps.cache-minimal.outputs.cache-hit, but no step with id cache-minimal exists (leftover from a removed cache step), so the condition was always true. Remove it; the step runs unconditionally as it already did in practice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(scripts): harden rewrite-macos-linkage.sh Three latent bugs surfaced by review once the previously-inline code became a parameterized, reusable script: - otool -L's header line is the binary's own path; a binary living under a path containing "libkrun" (e.g. a throwaway CI copy) would false-match and silently skip the real rewrite. Skip the header (NR>1). - install_name_tool -rpath errors when the bare @loader_path entry is already gone, so a second run on an already-rewritten binary hard-failed. Retarget only when the dev rpath is present. - set -o pipefail aborted the `current=` capture before the annotated "no libkrun load command" diagnostic could fire when otool itself fails. Guard the capture with || true; the -z check handles both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: address review findings in shared composites and their callers - Trigger the VM lanes when their composite actions change: ci-macos.yml and ci-linux-kvm.yml path filters now include the composites their setup runs through, so a composite-only edit can no longer merge unexercised and break the lanes post-merge. - Restore the libkrun.dylib existence check in setup-libkrun-macos: brew exits 0 for an installed-but-unlinked keg, so "a failed install fails the step" was unsound on a persistent shared runner; without the check a release build dies later with a raw `ld: library 'krun' not found` instead of a provisioning pointer. - Fail fast on deterministic errors: the mip CLI is validated/built once, outside the retry loops (guest-artifacts, setup-libkrun-linux), so a compile error or bad mip path fails in one attempt instead of being re-run into the job timeout. Only the network-bound materialize retries. - One retry implementation: scripts/ci/retry.sh replaces the three hand-copied divergent loops, and the previously-unretried gvproxy downloads (ci-linux-kvm + release) now use it too. - release.yml: drop the inline LIBKRUN_PREFIX re-hardcode (the composite publishes it); document why the job-wide LD_LIBRARY_PATH is safe (the prefix holds only libkrun/libkrunfw by construction); fix the linkage-step comment that implied a CI consumer exists. Not fixed: ci-netns.yml still inlines its rust preamble — its free-disk config differs (no remove_tool_cache) and the file is slated for deletion when the networking proofs are mothballed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: drop the fetch retries — lane history shows zero fetch failures Actions history for both VM lanes (last ~200 runs each, plus the first attempts of every manually re-run run) records not a single failure in the retried steps: the kernel/rootfs cache pulls, the libkrun prefix fetch, and the gvproxy download have never failed. The retries were insurance without receipts; remove them and the now-unused scripts/ci/retry.sh, returning the composites to plain extractions of the original steps (the mip prebuild and input pre-validation existed only to keep deterministic work out of the retry loops, so they go too). The transient failures the history DOES show are apt-get installs — the only main-branch KVM lane failure in the window and both of its manual reruns died in "Install build dependencies". Retry belongs there if anywhere, left for a separate change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(macos): reap the whole Session-E2E process tree before auto-spawn The __krun-vmm-only reap from #672 is insufficient: main run 29032763009 — on the very commit that added it — still failed the cold `minimal ls` with `ssh connect: Disconnected` against a fully healthy guest (READY emitted, minimald listening on vsock:2222, no connection ever accepted): the #588 bridge wedge. The remaining leftover is the host gvproxy switch, which minvmd owns and Guest::drop never kills; the failing runs' proxy-publish WARN corroborates a lingering gvproxy. Reap any stray minvmd first (so nothing respawns), then the VMM and gvproxy. The proper fix — the session harness reaping its own process group — stays tracked under #588; this keeps the merged e2e job's steps isolated in the meantime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
19 tasks
norrietaylor
added a commit
that referenced
this pull request
Jul 10, 2026
* ci: extract shared setup composites for rust, libkrun, and guest artifacts Four new composite actions dissolve copy-pasted setup blocks: - setup-rust: free-disk + protoc + rust-cache preamble, previously inlined in ci.yml clippy AND duplicated inside core-tests - setup-libkrun-macos: the brew slp/krun tap install, previously copy-pasted across ci-macos.yml and release.yml (the release job's separate dylib-existence check is dropped like ci-macos's was in #678: a failed brew install already fails the step, and a missing libkrun still fails loudly at link/rewrite time) - setup-libkrun-linux: fetch-libkrun.sh + the LIBKRUN_PREFIX / LD_LIBRARY_PATH exports, previously duplicated between ci-linux-kvm.yml and release.yml's amd64 build - guest-artifacts: the kernel + rootfs cache pulls, previously repeated in ci-macos.yml, ci-linux-kvm.yml, and release.yml The two fetch composites retry 3x to absorb transient cache/network hiccups; commands, arguments, and env are otherwise unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(release): extract minvmd linkage rewrite into a script The ~60-line inline install_name_tool rewrite + otool verification in build-release-macos-arm64 (grown across #664/#668/#670) moves to scripts/rewrite-macos-linkage.sh, command-for-command. A script lets CI exercise the production @rpath rewrite on a throwaway copy of the debug binary later, instead of the rewrite only ever running at release time. Signing stays the caller's job (Developer ID, last mutation before upload, per #680). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(setup-minimal): drop dead cache-minimal install conditional The Install Minimal step guarded on steps.cache-minimal.outputs.cache-hit, but no step with id cache-minimal exists (leftover from a removed cache step), so the condition was always true. Remove it; the step runs unconditionally as it already did in practice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(scripts): harden rewrite-macos-linkage.sh Three latent bugs surfaced by review once the previously-inline code became a parameterized, reusable script: - otool -L's header line is the binary's own path; a binary living under a path containing "libkrun" (e.g. a throwaway CI copy) would false-match and silently skip the real rewrite. Skip the header (NR>1). - install_name_tool -rpath errors when the bare @loader_path entry is already gone, so a second run on an already-rewritten binary hard-failed. Retarget only when the dev rpath is present. - set -o pipefail aborted the `current=` capture before the annotated "no libkrun load command" diagnostic could fire when otool itself fails. Guard the capture with || true; the -z check handles both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: address review findings in shared composites and their callers - Trigger the VM lanes when their composite actions change: ci-macos.yml and ci-linux-kvm.yml path filters now include the composites their setup runs through, so a composite-only edit can no longer merge unexercised and break the lanes post-merge. - Restore the libkrun.dylib existence check in setup-libkrun-macos: brew exits 0 for an installed-but-unlinked keg, so "a failed install fails the step" was unsound on a persistent shared runner; without the check a release build dies later with a raw `ld: library 'krun' not found` instead of a provisioning pointer. - Fail fast on deterministic errors: the mip CLI is validated/built once, outside the retry loops (guest-artifacts, setup-libkrun-linux), so a compile error or bad mip path fails in one attempt instead of being re-run into the job timeout. Only the network-bound materialize retries. - One retry implementation: scripts/ci/retry.sh replaces the three hand-copied divergent loops, and the previously-unretried gvproxy downloads (ci-linux-kvm + release) now use it too. - release.yml: drop the inline LIBKRUN_PREFIX re-hardcode (the composite publishes it); document why the job-wide LD_LIBRARY_PATH is safe (the prefix holds only libkrun/libkrunfw by construction); fix the linkage-step comment that implied a CI consumer exists. Not fixed: ci-netns.yml still inlines its rust preamble — its free-disk config differs (no remove_tool_cache) and the file is slated for deletion when the networking proofs are mothballed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: drop the fetch retries — lane history shows zero fetch failures Actions history for both VM lanes (last ~200 runs each, plus the first attempts of every manually re-run run) records not a single failure in the retried steps: the kernel/rootfs cache pulls, the libkrun prefix fetch, and the gvproxy download have never failed. The retries were insurance without receipts; remove them and the now-unused scripts/ci/retry.sh, returning the composites to plain extractions of the original steps (the mip prebuild and input pre-validation existed only to keep deterministic work out of the retry loops, so they go too). The transient failures the history DOES show are apt-get installs — the only main-branch KVM lane failure in the window and both of its manual reruns died in "Install build dependencies". Retry belongs there if anywhere, left for a separate change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(macos): reap the whole Session-E2E process tree before auto-spawn The __krun-vmm-only reap from #672 is insufficient: main run 29032763009 — on the very commit that added it — still failed the cold `minimal ls` with `ssh connect: Disconnected` against a fully healthy guest (READY emitted, minimald listening on vsock:2222, no connection ever accepted): the #588 bridge wedge. The remaining leftover is the host gvproxy switch, which minvmd owns and Guest::drop never kills; the failing runs' proxy-publish WARN corroborates a lingering gvproxy. Reap any stray minvmd first (so nothing respawns), then the VMM and gvproxy. The proper fix — the session harness reaping its own process group — stays tracked under #588; this keeps the merged e2e job's steps isolated in the meantime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(macos): consolidate libkrun on our own pinned source build CI previously tested against the slp/krun Homebrew bottle (a full-feature build, version drifting with the tap) while the release shipped our trimmed source build (blk,net; no gpu, no init-blob) — which nothing executed before it reached users. Consolidate both on one build: - vendor/libkrun/libkrun.lock pins the containers/libkrun version AND its resolved commit (replaces the LIBKRUN_REF env pin; the build fetches the commit, so a moved tag cannot change what we build). - scripts/build-libkrun-macos.sh builds the trimmed dylib at the pin, asserts self-containment and the krun_add_disk3 (>= 1.19.0) API floor, sets the install name to @rpath/libkrun.1.dylib, ad-hoc signs, and stages libkrun.1.dylib + a libkrun.dylib linker symlink into a prefix. - setup-libkrun-macos builds on miss into a commit-keyed prefix ($HOME/.cache/minimal-ci/libkrun/<commit>) — actions/cache on GitHub-hosted runners, the persistent directory itself on the mini — verifies the staged dylib, and publishes LIBKRUN_PREFIX. minvmd's build.rs prefers LIBKRUN_PREFIX over /opt/homebrew and bakes it as an rpath, so a leftover brew libkrun on the runner is ignored; brew leaves the CI path entirely (and with it the installed-but-unlinked keg failure mode). - release build-libkrun-macos-arm64 ships from the same composite: the shipped dylib is by construction the one every macOS CI lane linked and booted. Developer ID signing flow unchanged (#680). With the @rpath install name, minvmd records @rpath/libkrun.1.dylib at link time — verified locally: the script builds a self-contained dylib (Hypervisor.framework/libiconv/libSystem only), minvmd links with the @rpath load command + @loader_path and prefix rpaths, and the binary loads. rewrite-macos-linkage.sh's -change becomes a natural no-op; only its @loader_path -> @loader_path/../lib retarget still mutates release binaries. Refs: #687 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(macos): address CodeRabbit review on the libkrun build - Key the on-disk libkrun prefix by BOTH build inputs (pinned commit + build-script hash), not commit alone: the persistent mini could otherwise keep serving a dylib built by an older build-libkrun-macos.sh after a script change. This mirrors the invalidation the actions/cache key already gave GitHub-hosted runners; stale sibling prefixes on the mini are inert. - cargo build --locked: build exactly upstream's committed Cargo.lock (verified present at the pin) so a silent dependency re-resolve cannot undermine the reproducible-build guarantee. Verified locally: the --locked build completes at the pin and stages into the new hash-suffixed prefix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(macos): build minimal in its own cargo invocation in the e2e job The e2e's combined `cargo build -p minvmd --bin minvmd -p minimal --bin minimal` unified minvmd's default `libkrun` feature into the CLI's default-features=false opt-out, so `minimal` linked libkrun — the exact footgun release.yml already documents and avoids with separate invocations. The brew era masked it: brew's libkrun carries an absolute /opt/homebrew install name, so the mislinked CLI still loaded. The own-build dylib's @rpath install name exposed it — the autospawn e2e died with dyld "Library not loaded: @rpath/libkrun.1.dylib / no LC_RPATH's found" from target/debug/ minimal, which bakes no rpaths. Split the build (mirroring release.yml) and add the release job's "minimal links only system libraries" assert to the e2e, so a unification regression fails at build time with a pointed message instead of a dyld error mid-test. Net effect of this PR's own-build switch: CI now catches a mislinked CLI that brew silently tolerated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 10, 2026
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
3 participants
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.
Implements first stage a per-VM writable ext4 volume that carries the package cache, rootfs-staging trees, and session state on one filesystem off RAM onto durable storage.
The host creates a sparse raw file (ensure_sparse_raw); the guest formats it on first boot via mkfs.ext4 keyed on ext4 superblock detection (byte offset 1080 magic 0x53EF) — idempotent, platform-portable (macOS has no mke2fs). Subsequent boots detect the superblock and skip mkfs.
Next steps include
Notes: minimum libkrun is now 1.19.0
Summary by CodeRabbit
Summary
New Features
Bug Fixes
Documentation
Tests / CI
libkrunartifacts and the expected disk export.