Repository navigation
feat(minvmd,minimald): boot minimald as pid-1 via initramfs - #373
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR moves booting to an initramfs-based three-artifact flow, adds minimald PID-1 guest init and READY signaling, replaces raw krun format constants with enums, refactors VmConfig and VMM wiring for ChangesInitramfs-driven Boot Architecture
Sequence Diagram(s)sequenceDiagram
participant CI
participant MinvmdHost
participant MinimaldGuest
CI->>MinvmdHost: provide kernel rootfs initramfs artifacts
MinvmdHost->>MinimaldGuest: boot VM with initramfs as pid1
MinimaldGuest->>MinimaldGuest: mount filesystems and enter rootfs
MinimaldGuest->>MinvmdHost: send READY marker over vsock
MinvmdHost->>MinvmdHost: continue boot e2e flow after READY
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
This was referenced Jun 9, 2026
Member
Author
|
@CodeRabbit review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
crates/minimald/src/main.rs (1)
241-276: 💤 Low valueConsider logging READY marker failure in the fallback path for consistency.
Line 250 silently discards the READY marker error (
let _ = ...), while Lines 269-271 log a warning when the same operation fails. For consistent observability, consider logging in both paths:- let _ = guest::emit_ready_marker().await; + if let Err(e) = guest::emit_ready_marker().await { + tracing::warn!(error = %e, "initramfs fallback: READY marker failed"); + }🤖 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/minimald/src/main.rs` around lines 241 - 276, In run_initramfs replace the silent discard of guest::emit_ready_marker() in the enter_rootfs fallback with the same warning logging used later: call guest::emit_ready_marker().await and if it returns Err(e) use tracing::warn!(error = %e, "initramfs: READY marker failed (initramfs-only fallback)") so the READY marker failure is logged consistently; locate the let _ = guest::emit_ready_marker().await line inside the if let Err(e) = guest::enter_rootfs(...) { ... } block and change it to an if let Err(e) = guest::emit_ready_marker().await { tracing::warn!(...) }..github/workflows/ci-macos.yml (1)
61-74: 💤 Low valueCache key might not invalidate on Rust toolchain updates.
The cache key
${{ runner.os }}-stage2-artifacts-${{ hashFiles('**/Cargo.lock') }}includes the OS and dependency lock but not the Rust toolchain version. Ifstableupdates (e.g., 1.75 → 1.76), the cache will serve artifacts built with the older compiler, which cargo rebuilds incrementally but may slow down CI.For tighter cache semantics, consider including the Rust version:
key: ${{ runner.os }}-stage2-artifacts-${{ steps.toolchain.outputs.cachekey }}-${{ hashFiles('**/Cargo.lock') }}(where
steps.toolchainreferences thedtolnay/rust-toolchain@stablestep withid: toolchain).This is optional—incremental builds handle version mismatches correctly, but it improves cache hit quality.
🤖 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 @.github/workflows/ci-macos.yml around lines 61 - 74, The cache key for actions/cache currently uses runner.os and Cargo.lock hash but omits the Rust toolchain, so updates to the toolchain can cause stale artifact cache hits; update the cache key used by the actions/cache step (key property) to include the toolchain cache identifier produced by the rust toolchain setup step (reference the setup step id: toolchain and its output, e.g., steps.toolchain.outputs.cachekey) so the key becomes runner.os + toolchain cache key + Cargo.lock hash to ensure cache invalidation on toolchain changes.scripts/build-initramfs.sh (1)
31-31: ⚡ Quick winConsider sorting
findoutput for reproducible builds.The
find . | cpiopipeline creates archives whose content order depends on filesystem iteration order, leading to non-deterministic output. For reproducible builds and easier supply-chain verification, sort the file list before passing tocpio:-( cd "$STAGE" && find . | cpio -o -H newc ) > "$DEST" +( cd "$STAGE" && find . | sort | cpio -o -H newc ) > "$DEST"🤖 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 `@scripts/build-initramfs.sh` at line 31, The find→cpio pipeline using STAGE and DEST is non-deterministic because filesystem iteration order varies; fix it by sorting the file list before piping to cpio (use a locale‑stable sort, e.g. set LC_ALL=C and pipe find's output through sort) so the archive content order is reproducible, i.e. ensure the command that currently reads the find . | cpio -o -H newc invocation is changed to pass sorted input to cpio.
🤖 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/minvmd/src/bin/krun_smoke_child.rs`:
- Around line 69-73: The smoke test currently picks KernelFormat::ImageGz for
aarch64 in krun_smoke_child.rs but production image.rs::kernel_format() returns
KernelFormat::Raw for aarch64 (and libkrun documents ImageGz as x86_64-only);
update the selection logic in krun_smoke_child.rs so that for target_arch =
"aarch64" you use KernelFormat::Raw (uncompressed Image) and reserve
KernelFormat::ImageGz only for x86_64 paths, ensuring the smoke test mirrors
production boot config and avoids using ImageGz on aarch64.
In `@crates/minvmd/src/image.rs`:
- Around line 62-76: Add unit tests for resolve_initramfs_path to mirror the
coverage of resolve_kernel_path()/resolve_rootfs_path(): create three tests
named resolve_initramfs_path_errors_on_missing_env,
resolve_initramfs_path_errors_on_empty_env, and
resolve_initramfs_path_returns_path_when_set (place them after the existing
resolve_rootfs_path_errors_on_missing_env test); each should manipulate the
MINVMD_INITRAMFS env var (remove it, set to empty, set to a valid path), call
resolve_initramfs_path(), assert it returns VmError::MissingEnv for
missing/empty cases (matching var: "MINVMD_INITRAMFS") and returns the expected
PathBuf for the valid path case, and ensure the env var is cleaned up after each
test.
---
Nitpick comments:
In @.github/workflows/ci-macos.yml:
- Around line 61-74: The cache key for actions/cache currently uses runner.os
and Cargo.lock hash but omits the Rust toolchain, so updates to the toolchain
can cause stale artifact cache hits; update the cache key used by the
actions/cache step (key property) to include the toolchain cache identifier
produced by the rust toolchain setup step (reference the setup step id:
toolchain and its output, e.g., steps.toolchain.outputs.cachekey) so the key
becomes runner.os + toolchain cache key + Cargo.lock hash to ensure cache
invalidation on toolchain changes.
In `@crates/minimald/src/main.rs`:
- Around line 241-276: In run_initramfs replace the silent discard of
guest::emit_ready_marker() in the enter_rootfs fallback with the same warning
logging used later: call guest::emit_ready_marker().await and if it returns
Err(e) use tracing::warn!(error = %e, "initramfs: READY marker failed
(initramfs-only fallback)") so the READY marker failure is logged consistently;
locate the let _ = guest::emit_ready_marker().await line inside the if let
Err(e) = guest::enter_rootfs(...) { ... } block and change it to an if let
Err(e) = guest::emit_ready_marker().await { tracing::warn!(...) }.
In `@scripts/build-initramfs.sh`:
- Line 31: The find→cpio pipeline using STAGE and DEST is non-deterministic
because filesystem iteration order varies; fix it by sorting the file list
before piping to cpio (use a locale‑stable sort, e.g. set LC_ALL=C and pipe
find's output through sort) so the archive content order is reproducible, i.e.
ensure the command that currently reads the find . | cpio -o -H newc invocation
is changed to pass sorted input to cpio.
🪄 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: cb13f9f7-63ff-40f3-bdbb-3d1c8b122bf8
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.github/workflows/ci-macos.yml.gitignoreCargo.tomlcrates/minimald/Cargo.tomlcrates/minimald/src/guest.rscrates/minimald/src/lib.rscrates/minimald/src/main.rscrates/minvmd/README.mdcrates/minvmd/src/bin/krun_smoke_child.rscrates/minvmd/src/cmd/vmm_child.rscrates/minvmd/src/image.rscrates/minvmd/src/krun/ctx.rscrates/minvmd/src/krun/mod.rscrates/minvmd/src/krun/raw.rscrates/minvmd/src/vm.rscrates/minvmd/tests/boot_e2e.rsscripts/build-initramfs.sh
Comment on lines
+62
to
+76
| /// Resolve the initramfs path from `MINVMD_INITRAMFS`. | ||
| /// | ||
| /// This is the cpio initramfs whose `/init` is minimald (booted as pid-1). | ||
| /// Returns `VmError::MissingEnv` when the variable is unset or empty. | ||
| pub fn resolve_initramfs_path() -> Result<PathBuf, VmError> { | ||
| let val = std::env::var("MINVMD_INITRAMFS").map_err(|_| VmError::MissingEnv { | ||
| var: "MINVMD_INITRAMFS", | ||
| })?; | ||
| if val.is_empty() { | ||
| return Err(VmError::MissingEnv { | ||
| var: "MINVMD_INITRAMFS", | ||
| }); | ||
| } | ||
| Ok(PathBuf::from(val)) | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add test coverage for the new function.
The function follows the same pattern as resolve_kernel_path() and resolve_rootfs_path(), but unlike those functions, it has no tests. Add test cases covering:
- Missing
MINVMD_INITRAMFSenv var (should returnVmError::MissingEnv) - Empty
MINVMD_INITRAMFSenv var (should returnVmError::MissingEnv) - Valid path (should return the
PathBuf)
🧪 Proposed test coverage
Add these tests after the existing resolve_rootfs_path_errors_on_missing_env test:
#[test]
fn resolve_initramfs_path_errors_on_missing_env() {
unsafe { std::env::remove_var("MINVMD_INITRAMFS") };
let err = resolve_initramfs_path().unwrap_err();
assert!(
matches!(
err,
VmError::MissingEnv {
var: "MINVMD_INITRAMFS"
}
),
"expected MissingEnv, got {err:?}"
);
}
#[test]
fn resolve_initramfs_path_returns_path_when_set() {
unsafe { std::env::set_var("MINVMD_INITRAMFS", "/boot/initramfs.cpio") };
let path = resolve_initramfs_path().unwrap();
assert_eq!(path, std::path::PathBuf::from("/boot/initramfs.cpio"));
unsafe { std::env::remove_var("MINVMD_INITRAMFS") };
}
#[test]
fn resolve_initramfs_path_errors_on_empty_env() {
unsafe { std::env::set_var("MINVMD_INITRAMFS", "") };
let err = resolve_initramfs_path().unwrap_err();
assert!(
8000
matches!(
err,
VmError::MissingEnv {
var: "MINVMD_INITRAMFS"
}
),
"expected MissingEnv, got {err:?}"
);
unsafe { std::env::remove_var("MINVMD_INITRAMFS") };
}🤖 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/image.rs` around lines 62 - 76, Add unit tests for
resolve_initramfs_path to mirror the coverage of
resolve_kernel_path()/resolve_rootfs_path(): create three tests named
resolve_initramfs_path_errors_on_missing_env,
resolve_initramfs_path_errors_on_empty_env, and
resolve_initramfs_path_returns_path_when_set (place them after the existing
resolve_rootfs_path_errors_on_missing_env test); each should manipulate the
MINVMD_INITRAMFS env var (remove it, set to empty, set to a valid path), call
resolve_initramfs_path(), assert it returns VmError::MissingEnv for
missing/empty cases (matching var: "MINVMD_INITRAMFS") and returns the expected
PathBuf for the valid path case, and ensure the env var is cleaned up after each
test.
twitchyliquid64
approved these changes
Jun 10, 2026
evanspearman
requested changes
Jun 10, 2026
Ship minimald as the kernel's initramfs `/init`: `build-initramfs.sh` cross-compiles the static aarch64 binary and packs it as a newc cpio. The kernel boots the initramfs and runs `/init` (minimald) as pid-1; minimald mounts `/dev` (devtmpfs), mounts the generic upstream rootfs (`/dev/vda`) + pseudo-fs, chroots in, and emits the boot READY marker. This replaces the block-root boot path (kernel `init=/sbin/<exec>` on an ext4 root): minimald is no longer baked into the rootfs, so the upstream `microvm-rootfs` package stays generic. Session state is on a tmpfs (`/run/minimal`); no data disk. - minvmd: `VmConfig` boots an initramfs (required) + the rootfs as a `/dev/vda` block device; drops `exec_target` and the block-root cmdline. `MINVMD_INITRAMFS` is the new required boot input. - minimald: `/init` detection (argv[0] basename) + `run_initramfs` (mount + READY + serve on the guest UDS); `guest` module mount helpers. - Type the libkrun kernel/disk formats as `#[repr(u32)]` enums (`KernelFormat`/`DiskFormat`) per review on #367. - CI: cross-build the initramfs on the Linux artifacts job; `boot_e2e` boots it with `MINVMD_INITRAMFS` and asserts READY. The vsock session round-trip (relay + `run_on_uds` serving) lands in a follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
norrietaylor
force-pushed
the
minvmd-stage2-initramfs-boot
branch
from
June 10, 2026 19:11
c6b5b85 to
e697a95
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci-macos.yml (1)
10-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
crates/minimald/**to path filters to avoid CI blind spots.This workflow now validates the initramfs boot path, but PRs that only change
crates/minimald/**will not trigger it underpush/pull_requestpath scoping. That can let boot regressions merge without running the macOS gate.Suggested patch
on: push: branches: ["main"] paths: - "crates/minvmd/**" + - "crates/minimald/**" - "crates/minimal2/**" - "Cargo.toml" - "Cargo.lock" - ".github/workflows/ci-macos.yml" - "scripts/**" pull_request: branches: ["main"] paths: - "crates/minvmd/**" + - "crates/minimald/**" - "crates/minimal2/**" - "Cargo.toml" - "Cargo.lock" - ".github/workflows/ci-macos.yml" - "scripts/**"🤖 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 @.github/workflows/ci-macos.yml around lines 10 - 25, The workflow's push and pull_request path filters under the ci-macos workflow omit the "crates/minimald/**" directory, so changes there won't trigger the macOS CI; update both path lists (the 'paths:' entries shown in the diff for push and pull_request) to include "crates/minimald/**" alongside the existing entries so PRs and pushes touching that crate will run this workflow.
🤖 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.
Outside diff comments:
In @.github/workflows/ci-macos.yml:
- Around line 10-25: The workflow's push and pull_request path filters under the
ci-macos workflow omit the "crates/minimald/**" directory, so changes there
won't trigger the macOS CI; update both path lists (the 'paths:' entries shown
in the diff for push and pull_request) to include "crates/minimald/**" alongside
the existing entries so PRs and pushes touching that crate will run this
workflow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4ac7147f-2ae4-4b05-9901-aa63593beab0
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.github/workflows/ci-macos.yml.gitignoreCargo.tomlcrates/minimald/Cargo.tomlcrates/minimald/src/guest.rscrates/minimald/src/lib.rscrates/minimald/src/main.rscrates/minvmd/README.mdcrates/minvmd/src/bin/krun_smoke_child.rscrates/minvmd/src/cmd/vmm_child.rscrates/minvmd/src/image.rscrates/minvmd/src/krun/ctx.rscrates/minvmd/src/krun/mod.rscrates/minvmd/src/krun/raw.rscrates/minvmd/src/vm.rscrates/minvmd/tests/boot_e2e.rsscripts/build-initramfs.sh
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (14)
- crates/minvmd/src/bin/krun_smoke_child.rs
- crates/minvmd/tests/boot_e2e.rs
- crates/minvmd/src/krun/mod.rs
- crates/minimald/Cargo.toml
- Cargo.toml
- crates/minimald/src/lib.rs
- crates/minvmd/src/cmd/vmm_child.rs
- scripts/build-initramfs.sh
- crates/minvmd/src/image.rs
- crates/minvmd/src/krun/ctx.rs
- crates/minvmd/src/vm.rs
- crates/minvmd/src/krun/raw.rs
- crates/minimald/src/main.rs
- crates/minimald/src/guest.rs
evanspearman
approved these changes
Jun 10, 2026
This was referenced Jun 26, 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
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.
Ships minimald as the kernel's initramfs
/init, replacing the block-root stub boot. Stage-2 PR 1 of 2.What
scripts/build-initramfs.shcross-compiles minimald (static aarch64-musl) and packs it as a newc cpio/init./dev(devtmpfs) + the generic upstream rootfs (/dev/vda) + pseudo-fs,chroots in, and emits the boot READY marker.VmConfigboots an initramfs (required) + the rootfs as a/dev/vdablock device. Dropsexec_targetand the block-rootinit=cmdline.#[repr(u32)]enums (KernelFormat/DiskFormat) per Evan's review on feat(minvmd): boot a microVM from kernel + rootfs built as minimal packages #367.Why initramfs over block-root
The block-root path (
init=/sbin/minimaldon an ext4 root) required baking minimald into the rootfs. Shipping it in the initramfs keeps the upstreammicrovm-rootfspackage generic — minimald is delivered separately. Block-root was a stepping stone (decided in the #361 / #368 restack).Proof
boot_e2eboots the initramfs withMINVMD_INITRAMFSand asserts the READY round-trip. CI cross-builds the initramfs on the Linux artifacts job. Validated locally:cross build+cross clippyclean for aarch64-musl; minvmd build + clippy + 52 unit tests green.Stack
minvmd-stage2-vsock-session— serve sessions over the bridge (relay +run_on_uds+ session e2e).Replaces #361 + #368 (to be closed).
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Refactor
Chores / CI
Tests