add microvm-rootfs and virtio-kernel-raw packages - #229
Merged
Conversation
Gunzip the virtio-linux Image.gz to an uncompressed aarch64 Image so consumers (minvmd) can load it with KRUN_KERNEL_FORMAT_RAW, skipping libkrun's in-VMM gzip decompress (~77 ms, over half of microVM boot-to-READY). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Assemble the minvmd microVM guest rootfs (socat + bash + coreutils closure, bring-up init, /etc/minvmd/manifest boot contract) into a read-only ext4 image via mke2fs. minvmd loads it as a block device (krun_add_disk2 -> /dev/vda). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds two new build packages: microvm-rootfs produces a journalless ext4 rootfs image from a staged runtime tree with a vsock init stub and manifest; virtio-kernel-raw produces a kernel Image by copying or decompressing an upstream vmlinuz. Changesmicrovm-rootfs guest filesystem image
virtio-kernel-raw kernel image
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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
🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@packages/minvmd-rootfs/build.sh`:
- Around line 18-23: The build script's directory-copy loop (for d in usr bin
sbin lib lib64 etc) copies the entire build sandbox into $STAGE, which pulls
build-only packages like e2fsprogs into the guest image; update the staging step
in build.sh to only include runtime closures (not build_deps) or explicitly
remove e2fsprogs outputs before mke2fs runs: either compute and copy the closure
of the runtime packages instead of blindly copying "/", or after the cp loop
delete the e2fsprogs outputs (and any other build_deps) from $STAGE so the final
rootfs.img matches the runtime dependency set declared in build.ncl and
build_deps, ensuring mke2fs uses only runtime artifacts.
- Around line 45-53: The init stub (minvmd-stub-init) currently always starts
the bridge listener even if the READY handshake loop never succeeds; change the
script to detect failure of the handshake (e.g., track a success flag or check
if i reached 50) and, if it never succeeded, print an error message to stderr
and exit with a non-zero status instead of proceeding to exec socat
VSOCK-LISTEN:2222,fork EXEC:cat; only exec the listener when the handshake loop
actually breaks/succeeds.
- Around line 71-76: The prune subshell currently swallows any failure because
the trailing "|| true" applies to the whole "( cd \"$STAGE\" && rm -rf ... &&
find ... )" sequence; change it so only the non-fatal find errors are ignored.
Concretely, keep "cd \"$STAGE\"" and "rm -rf usr/include ..." as-is so failures
propagate, and wrap or append "|| true" only to the find invocation (e.g. ( cd
\"$STAGE\" && rm -rf ... && (find . \( -name '*.a' -o -name '*.la' -o -name
'*.o' \) -delete 2>/dev/null || true) )), ensuring STAGE, the cd command, and
the rm -rf invocation will fail the script while only find's noncritical errors
are tolerated.
In `@packages/virtio-kernel-raw/build.ncl`:
- Around line 6-25: This BuildSpec for name = "virtio-kernel-raw" is missing the
required version plumbing: add a top-level binding for version and set
attrs.upstream_version to that value, and forward the version into the build via
build_args (e.g., include version in build_args) so the spec conforms to the
repo contract; update the BuildSpec to include a version binding, add
attrs.upstream_version referencing version, and add build_args that
pass/version-include the version into the build process so BuildSpec consumers
get the upstream_version and version forwarded.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 878467e7-07a7-4145-a51a-9335a87ef51b
📒 Files selected for processing (4)
packages/minvmd-rootfs/build.nclpackages/minvmd-rootfs/build.shpackages/virtio-kernel-raw/build.nclpackages/virtio-kernel-raw/build.sh
Comment on lines
+45
to
+53
| cat > "$STAGE/sbin/minvmd-stub-init" <<'STUB' | ||
| #!/bin/sh | ||
| i=0 | ||
| while [ "$i" -lt 50 ]; do | ||
| printf 'READY\n' | socat -t2 - VSOCK-CONNECT:2:7350 && break | ||
| i=$((i + 1)) | ||
| sleep 0.1 | ||
| done | ||
| exec socat VSOCK-LISTEN:2222,fork EXEC:cat |
Contributor
There was a problem hiding this comment.
Fail the init stub when the READY handshake never succeeds.
If all 50 attempts miss, PID 1 still starts the bridge listener and the guest never emits the documented READY signal on port 7350. That turns a bring-up failure into a downstream timeout instead of an immediate, diagnosable boot failure.
Suggested fix
i=0
while [ "$i" -lt 50 ]; do
printf 'READY\n' | socat -t2 - VSOCK-CONNECT:2:7350 && break
i=$((i + 1))
sleep 0.1
done
+[ "$i" -lt 50 ] || {
+ echo "ERROR: failed to publish READY on vsock 7350" >&2
+ exit 1
+}
exec socat VSOCK-LISTEN:2222,fork EXEC:cat📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| cat > "$STAGE/sbin/minvmd-stub-init" <<'STUB' | |
| #!/bin/sh | |
| i=0 | |
| while [ "$i" -lt 50 ]; do | |
| printf 'READY\n' | socat -t2 - VSOCK-CONNECT:2:7350 && break | |
| i=$((i + 1)) | |
| sleep 0.1 | |
| done | |
| exec socat VSOCK-LISTEN:2222,fork EXEC:cat | |
| cat > "$STAGE/sbin/minvmd-stub-init" <<'STUB' | |
| #!/bin/sh | |
| i=0 | |
| while [ "$i" -lt 50 ]; do | |
| printf 'READY\n' | socat -t2 - VSOCK-CONNECT:2:7350 && break | |
| i=$((i + 1)) | |
| sleep 0.1 | |
| done | |
| [ "$i" -lt 50 ] || { | |
| echo "ERROR: failed to publish READY on vsock 7350" >&2 | |
| exit 1 | |
| } | |
| exec socat VSOCK-LISTEN:2222,fork EXEC:cat | |
| STUB |
🤖 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 `@packages/minvmd-rootfs/build.sh` around lines 45 - 53, The init stub
(minvmd-stub-init) currently always starts the bridge listener even if the READY
handshake loop never succeeds; change the script to detect failure of the
handshake (e.g., track a success flag or check if i reached 50) and, if it never
succeeded, print an error message to stderr and exit with a non-zero status
instead of proceeding to exec socat VSOCK-LISTEN:2222,fork EXEC:cat; only exec
the listener when the handshake loop actually breaks/succeeds.
…rences Make the packages product-agnostic: microvm-rootfs with /sbin/microvm-init and /etc/microvm/manifest, and scrub internal references from comments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/microvm-rootfs/build.sh (2)
17-22:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBuild-only dependency content is still being shipped in the guest image (Line 17).
The stage copy from
/pullse2fsprogsinto the final rootfs, even though it is described as build-time only. This bloats the image and breaks the build/runtime dependency split.As per coding guidelines, "Declare packages in 'build_deps' if needed during the build but not during package deployment."
🤖 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 `@packages/microvm-rootfs/build.sh` around lines 17 - 22, The current for-loop (for d in usr bin sbin lib lib64 etc; do ... cp -a "/$d" "$STAGE/") copies entire host dirs into $STAGE and pulls build-only packages like e2fsprogs into the image; change the copy to either use rsync/cp with explicit exclude patterns (e.g., exclude paths/filenames owned by e2fsprogs such as /sbin/mke2fs, /sbin/resize2fs, /usr/sbin/*e2fsprogs*) or perform the same cp-then-prune approach by removing those specific binaries from $STAGE right after the loop, and ensure those packages are declared as build_deps instead of runtime deps. Target symbols to edit: the for-loop that runs cp -a and the $STAGE cleanup (or replace with rsync --exclude) so build-only files are not present in the final rootfs.Source: Coding guidelines
70-75:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
|| truemasks prune/setup failures beyondfind(Line 70).Because
|| trueis attached to the full chained expression, failures incd "$STAGE"orrm -rf ...can be swallowed, producing a silently wrong image.Suggested fix
-( cd "$STAGE" && \ - rm -rf usr/include usr/share/man usr/share/doc usr/share/info \ - usr/share/locale usr/share/i18n usr/lib/locale usr/lib/pkgconfig \ - usr/share/aclocal usr/share/gtk-doc usr/share/bash-completion \ - usr/share/gdb && \ - find . \( -name '*.a' -o -name '*.la' -o -name '*.o' \) -delete 2>/dev/null || true ) +( + cd "$STAGE" + rm -rf usr/include usr/share/man usr/share/doc usr/share/info \ + usr/share/locale usr/share/i18n usr/lib/locale usr/lib/pkgconfig \ + usr/share/aclocal usr/share/gtk-doc usr/share/bash-completion \ + usr/share/gdb + find . \( -name '*.a' -o -name '*.la' -o -name '*.o' \) -delete 2>/dev/null || true +)🤖 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 `@packages/microvm-rootfs/build.sh` around lines 70 - 75, The chained command "( cd \"$STAGE\" && rm -rf ... && find . ... -delete 2>/dev/null || true )" currently swallows failures from cd or rm because the trailing "|| true" applies to the whole chain; change it so only the find's non-critical failures are ignored and earlier failures propagate: run cd "$STAGE" and the rm -rf operation (the "cd \"$STAGE\"" and "rm -rf usr/include ... usr/share/gdb" parts) such that they error out on failure (keep the && between them), then run the find command separately and append "|| true" only to the find (or suppress only its stderr) so that only find's errors are tolerated; ensure the commands referencing STAGE, cd "$STAGE", rm -rf, and find . \( -name '*.a' -o -name '*.la' -o -name '*.o' \) -delete are updated accordingly so failures in cd or rm cause the script to exit non-zero.
🤖 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.
Duplicate comments:
In `@packages/microvm-rootfs/build.sh`:
- Around line 17-22: The current for-loop (for d in usr bin sbin lib lib64 etc;
do ... cp -a "/$d" "$STAGE/") copies entire host dirs into $STAGE and pulls
build-only packages like e2fsprogs into the image; change the copy to either use
rsync/cp with explicit exclude patterns (e.g., exclude paths/filenames owned by
e2fsprogs such as /sbin/mke2fs, /sbin/resize2fs, /usr/sbin/*e2fsprogs*) or
perform the same cp-then-prune approach by removing those specific binaries from
$STAGE right after the loop, and ensure those packages are declared as
build_deps instead of runtime deps. Target symbols to edit: the for-loop that
runs cp -a and the $STAGE cleanup (or replace with rsync --exclude) so
build-only files are not present in the final rootfs.
- Around line 70-75: The chained command "( cd \"$STAGE\" && rm -rf ... && find
. ... -delete 2>/dev/null || true )" currently swallows failures from cd or rm
because the trailing "|| true" applies to the whole chain; change it so only the
find's non-critical failures are ignored and earlier failures propagate: run cd
"$STAGE" and the rm -rf operation (the "cd \"$STAGE\"" and "rm -rf usr/include
... usr/share/gdb" parts) such that they error out on failure (keep the &&
between them), then run the find command separately and append "|| true" only to
the find (or suppress only its stderr) so that only find's errors are tolerated;
ensure the commands referencing STAGE, cd "$STAGE", rm -rf, and find . \( -name
'*.a' -o -name '*.la' -o -name '*.o' \) -delete are updated accordingly so
failures in cd or rm cause the script to exit non-zero.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8ba0f581-3ae0-4438-b12f-668b0776fe91
📒 Files selected for processing (3)
packages/microvm-rootfs/build.nclpackages/microvm-rootfs/build.shpackages/virtio-kernel-raw/build.ncl
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/virtio-kernel-raw/build.ncl
The kernel is only gzip-compressed on aarch64 (Image.gz); on x86_64 virtio-linux ships a bzImage, so the unconditional gunzip failed the build there. gzip-test first: decompress when gzip, else pass through. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| # block device. | ||
| build_deps = [ | ||
| { file = "build.sh" } | Local, | ||
| socat, |
Member
There was a problem hiding this comment.
What is socat used for in the build script?
Member
Author
There was a problem hiding this comment.
it is placeholder will replace it later.
| build_deps = [ | ||
| { file = "build.sh" } | Local, | ||
| socat, | ||
| bash, |
Member
There was a problem hiding this comment.
Might make sense to depend on base instead for build_deps
| coreutils, | ||
| e2fsprogs, | ||
| ], | ||
|
|
Member
There was a problem hiding this comment.
I think it would be more pure to depend on the things you need in the rootfs image as runtime_deps, as that’s kinda the vibe of runtime deps. Sounds like you probably want base, socat, core utils as runtime deps and everything else as build deps, but note in practice the union of both will show up in your rootfs.
twitchyliquid64
approved these changes
Jun 7, 2026
twitchyliquid64
left a comment
Member
There was a problem hiding this comment.
I’ll approve it tho so your g2g when you’re happy with it
Address review feedback: - Declare base + socat as runtime_deps and e2fsprogs as a build-only dep (mke2fs packs the image), instead of lumping everything in build_deps. - Remove the build-only e2fsprogs files from the staged tree before packing, so the runtime image carries only the runtime closure (mke2fs runs from the build sandbox PATH, not from $STAGE). Verified absent from rootfs.img. - Fail the bring-up init if the READY handshake never succeeds, rather than starting the listener and turning a boot failure into a downstream timeout. - Scope `|| true` to the find invocation so a cd/rm failure still fails the build.
Add the version binding, attrs.upstream_version, and build_args forwarding the repo's build.ncl contract expects. Depend on base (provides gzip + sh at build) instead of listing gzip/bash/coreutils individually.
Member
Author
|
Addressed the feedback (commits 7133c1e, 3cd24e6): @twitchyliquid64 (dep hygiene) — CodeRabbit:
Both packages |
This was referenced Jun 8, 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
2 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.
Two packages for running a libkrun-based Linux microVM.
virtio-kernel-raw
Gunzips the
virtio-linuxImage.gzto an uncompressed aarch64Image, which libkrun can load withKRUN_KERNEL_FORMAT_RAW— skipping its in-VMM gzip decompress for a faster microVM boot. build_deps: virtio-linux, gzip, bash, coreutils.microvm-rootfs
A read-only ext4 guest rootfs: snapshots the closure of socat + bash + coreutils, adds a small vsock bring-up init (
/sbin/microvm-init: writes aREADYmarker on vsock 7350, then serves an echo on vsock 2222) and an/etc/microvm/manifestcontract, prunes build-only bulk, and packs an ext4 image (mke2fs -O ^has_journal) to load as a virtio-blk block device. build_deps: socat, bash, coreutils, e2fsprogs.Validation
minimal check microvm-rootfs virtio-kernel-raw— all checkers pass (parse, import line, imports, fmt, naming, executable scripts, disallowed-patterns).🤖 Generated with Claude Code
Summary by CodeRabbit