Support custom guest kernels - #1041
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughCustom kernel support now spans public configuration, CLI flags, per-box immutable boot-asset generations, initialization planning, VMM serialization, and Krun configuration. REST runtime creation rejects custom kernels, while local runtime startup reuses or publishes validated kernel generations. ChangesCustom kernel boot support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant BoxOptions
participant BootAssetStore
participant InitPipeline
participant ShimController
participant Krun
CLI->>BoxOptions: Apply KernelFlags
InitPipeline->>BootAssetStore: Prepare or reuse kernel generation
BootAssetStore-->>InitPipeline: PreparedKernel
InitPipeline->>ShimController: Serialize InstanceSpec with kernel
ShimController->>Krun: Start VM with kernel configuration
Krun-->>ShimController: Configure VM context
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
tester seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
📦 BoxLite review — looks good ·
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/boxlite/src/litebox/init/tasks/boot_assets.rs (1)
87-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSuperseded generations and migrated legacy files are never reclaimed.
publish_generationonly cleans up on failure; a successful re-stage leaves the previousgenerations/{id}directory (and, after migration, the legacyboot/kernel+boot/initramfscopies) on disk for the life of the box. Consider pruning generations not referenced bycurrent.jsonafter a successful publish.Also applies to: 203-294
🤖 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 `@src/boxlite/src/litebox/init/tasks/boot_assets.rs` around lines 87 - 101, Update the successful publish flow used by prepare, publish_generation, and the migration path to prune superseded generation directories after current.json is updated, retaining only the generation referenced by current.json. Ensure migrated legacy boot/kernel and boot/initramfs files are also removed once the new generation is successfully published, while preserving cleanup-on-failure behavior.
🤖 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 `@docs/reference/rust/README.md`:
- Around line 593-613: Update the Custom kernel documentation around
KernelOptions::new to state that advanced.kernel with a custom kernel is
supported only by the local runtime and is rejected for REST creation. Keep the
existing Rust configuration example unchanged.
- Around line 628-633: Correct the Default entry for the security field in the
AdvancedBoxOptions reference table to document SecurityOptions::default() as the
fully enabled security profile, including jailer enabled on Linux. Remove the
platform-specific claim that jailer defaults false while preserving the
descriptions of the available isolation options.
In `@src/boxlite/src/runtime/options.rs`:
- Around line 1065-1101: Gate the entire custom_kernel_configuration_roundtrips
test with cfg(any(target_arch = "x86_64", target_arch = "aarch64")) so its
architecture-specific kernel fixture writes and format definitions are
unavailable on unsupported targets. Preserve the existing test behavior on
x86_64 and aarch64.
- Around line 344-363: Update `detect` to read the four-byte ELF prefix with
`read_exact` instead of a single `read`, treating `UnexpectedEof` as a non-ELF
fallback while propagating other read errors through the existing
`BoxliteError::Config` handling. Preserve the current ELF classification when
the full `\x7fELF` signature is read.
- Around line 372-425: Update KernelFormat::detect so signature scanning is
limited to a bounded plausible header window rather than the entire kernel file;
stop reading once that window is exhausted and return Self::Raw when no
signature is found. Preserve the existing earliest-match behavior within the
window and avoid performing the synchronous whole-file read/seek on the async
creation path by moving this detection work off that path or otherwise using the
established blocking-task mechanism.
---
Nitpick comments:
In `@src/boxlite/src/litebox/init/tasks/boot_assets.rs`:
- Around line 87-101: Update the successful publish flow used by prepare,
publish_generation, and the migration path to prune superseded generation
directories after current.json is updated, retaining only the generation
referenced by current.json. Ensure migrated legacy boot/kernel and
boot/initramfs files are also removed once the new generation is successfully
published, while preserving cleanup-on-failure behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ba627719-50e7-4adc-aa4d-cf66d231dee6
📒 Files selected for processing (22)
docs/architecture/README.mddocs/reference/cli/README.mddocs/reference/rust/README.mdsrc/boxlite/src/jailer/mod.rssrc/boxlite/src/lib.rssrc/boxlite/src/litebox/init/mod.rssrc/boxlite/src/litebox/init/tasks/boot_assets.rssrc/boxlite/src/litebox/init/tasks/mod.rssrc/boxlite/src/litebox/init/tasks/vmm_spawn.rssrc/boxlite/src/litebox/init/types.rssrc/boxlite/src/rest/runtime.rssrc/boxlite/src/runtime/advanced_options.rssrc/boxlite/src/runtime/layout.rssrc/boxlite/src/runtime/options.rssrc/boxlite/src/vmm/controller/shim.rssrc/boxlite/src/vmm/krun/engine.rssrc/boxlite/src/vmm/mod.rssrc/cli/README.mdsrc/cli/src/cli.rssrc/cli/src/commands/create.rssrc/cli/src/commands/run.rssrc/deps/libkrun-sys/src/lib.rs
Custom kernels (#1041, #1051) and the capability policy both extend `AdvancedBoxOptions`, the CLI flag set, and option validation, so the two features are combined rather than either replacing the other. Capability name validation moves to `sanitize_common`, which main split out of `sanitize`: a capability list is request data, not a filesystem source, so it must also be checked on the persisted path. The warm-pool capability test now stubs `organizationUsageService`, which the org-quota work (#1028) made a required collaborator of `BoxService::create`.
What
AdvancedBoxOptions--kernel,--kernel-format,--initramfs, and--kernel-argsCLI options forrunandcreateWhy
This lets local BoxLite users boot guest kernels they provide without sending host paths through the shim or changing the default boot path.
Compatibility
Default behavior is unchanged. Custom kernels are opt-in and available only through the local runtime; existing CLI usage remains compatible.
Validation
make fmt:checkBOXLITE_DEPS_STUB=1 make clippyA real custom-kernel VM boot was not run in this environment because the native submodules and
mke2fsare unavailable.Summary by CodeRabbit
boxlite runandboxlite create.health_checksupport.