feat: add experimental nested virtualization - #1054
Merged
Merged
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds release-candidate nested virtualization with experimental opt-in, runtime and import-policy enforcement, VMM/libkrun activation, ChangesNested virtualization support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Runtime
participant ShimController
participant Krun
participant Guest
CLI->>Runtime: Enable nested_virtualization with experimental opt-in
Runtime->>ShimController: Pass nested virtualization in InstanceSpec
ShimController->>Krun: Probe and enable nested virtualization
Krun-->>Runtime: Return success, Unsupported, or Engine result
ShimController->>Guest: Send ContainerInitRequest with /dev/kvm
Guest->>Guest: Validate device and build OCI LinuxDevice
Guest-->>Runtime: Return categorized init result
Possibly related issues
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. |
1 similar 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. |
DorianZheng
force-pushed
the
agent/nested-virtualization-rc
branch
5 times, most recently
from
July 27, 2026 05:52
43cde51 to
78e0f73
Compare
DorianZheng
marked this pull request as ready for review
July 27, 2026 16:33
📦 BoxLite review — looks good ·
|
DorianZheng
added
the
e2e-local
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/guest/src/container/spec.rs (1)
36-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
is_empty()alongsidelen()for Clippy.
ContainerDevicesexposes a publiclen()without a publicis_empty(), soclippy::len_without_is_emptywill flag this impl. Addpub fn is_empty(&self) -> bool { self.0.is_empty() }or explicitly allow the lint if it should intentionally be omitted.♻️ Proposed fix
pub fn len(&self) -> usize { self.0.len() } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() + }🤖 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/guest/src/container/spec.rs` around lines 36 - 54, Add a public is_empty method to ContainerDevices alongside len, delegating to the wrapped Vec via self.0.is_empty(), so the type satisfies Clippy’s len_without_is_empty lint.
🤖 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 @.github/workflows/e2e-local.yml:
- Line 348: Update the E2E workflow around BOXLITE_TEST_NESTED_VIRTUALIZATION so
nested virtualization tests run only on runners whose reused or newly created
instance has verified nested-virtualization support. Add a dedicated
hardware-smoke gate or validate the reused runner and recreate it when
unsupported before enabling the nested-test environment variable.
In `@src/boxlite/src/runtime/options.rs`:
- Around line 997-1010: Update the JSON fixtures in
nested_virtualization_option_roundtrips so both BoxOptions deserialization
inputs include valid values for the required rootfs, env, volumes, network, and
ports fields, while preserving the true nested_virtualization assertion and the
legacy default-to-false assertion.
In `@src/deps/libkrun-sys/vendor/libkrunfw`:
- Line 1: Update the vendored libkrunfw submodule reference at libkrunfw to a
commit that exists on its configured remote, preserving the intended v5.4.0/KVM
host-support and FFI compatibility changes so reviewers can inspect the resolved
source.
---
Nitpick comments:
In `@src/guest/src/container/spec.rs`:
- Around line 36-54: Add a public is_empty method to ContainerDevices alongside
len, delegating to the wrapped Vec via self.0.is_empty(), so the type satisfies
Clippy’s len_without_is_empty lint.
🪄 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: 5389da0e-435b-4660-bd9b-315f2b27c11c
📒 Files selected for processing (50)
.github/workflows/e2e-local.ymldocs/architecture/README.mddocs/experimental/nested-virtualization.mdopenapi/box.openapi.yamlopenapi/reference-server/server.pyopenapi/reference-server/tests/test_handle_cache.pysdks/node/src/options.rssdks/python/src/runtime.rssdks/python/tests/test_untrusted_import.pysrc/boxlite/src/experimental.rssrc/boxlite/src/experimental/nested_virtualization.rssrc/boxlite/src/jailer/builder.rssrc/boxlite/src/jailer/mod.rssrc/boxlite/src/jailer/sandbox/mod.rssrc/boxlite/src/litebox/crash_report.rssrc/boxlite/src/litebox/init/mod.rssrc/boxlite/src/litebox/init/tasks/guest_connect.rssrc/boxlite/src/litebox/init/tasks/guest_init.rssrc/boxlite/src/litebox/init/tasks/vmm_spawn.rssrc/boxlite/src/portal/interfaces/container.rssrc/boxlite/src/portal/interfaces/guest.rssrc/boxlite/src/portal/interfaces/mod.rssrc/boxlite/src/rest/runtime.rssrc/boxlite/src/runtime/core.rssrc/boxlite/src/runtime/import.rssrc/boxlite/src/runtime/options.rssrc/boxlite/src/runtime/rt_impl.rssrc/boxlite/src/vmm/controller/shim.rssrc/boxlite/src/vmm/controller/spawn.rssrc/boxlite/src/vmm/exit_info.rssrc/boxlite/src/vmm/krun/context.rssrc/boxlite/src/vmm/krun/engine.rssrc/boxlite/src/vmm/mod.rssrc/cli/src/cli.rssrc/cli/src/commands/create.rssrc/cli/src/commands/run.rssrc/cli/src/commands/serve/handlers/advanced.rssrc/cli/src/commands/serve/mod.rssrc/cli/tests/nested_virtualization_rc.rssrc/deps/libkrun-sys/build.rssrc/deps/libkrun-sys/src/lib.rssrc/deps/libkrun-sys/vendor/libkrunfwsrc/guest/src/container/lifecycle.rssrc/guest/src/container/mod.rssrc/guest/src/container/spec.rssrc/guest/src/container/start.rssrc/guest/src/service/container.rssrc/guest/src/service/guest.rssrc/shared/proto/boxlite/v1/service.protosrc/shim/src/main.rs
Fold the shim exit-file error category into a single ExitInfo::Error field, replacing the parallel write and read structs and their helper functions. The shim now writes the type the host reads, and the exit file is parsed once through CrashReport instead of twice. Resolve container devices directly into an oci_spec LinuxDevice, dropping the intermediate mapping type, the arbitrary device cap, and the duplicate-destination scan. Device failures now report their own context instead of borrowing the container-creation message. Relocate the RC nested-virtualization option under `advanced`, beside `kernel`, so both release-candidate knobs sit behind the same facade. Keep a single path-access builder in the jailer and append caller grants in Jailer::context, thread nested virtualization into ShimSpawner explicitly rather than cloning BoxOptions to override one field, and reduce the untrusted-import rejections to one shared rule. An unrecognized error category now decodes as a generic engine failure rather than discarding the whole record: a box keeps its exit file across upgrades, so the BoxLite reading one is not always the BoxLite that wrote it. Claude-Session: https://claude.ai/code/session_01YL9HKQHBrZR9u1qY7FDyKd
DorianZheng
force-pushed
the
agent/nested-virtualization-rc
branch
from
July 27, 2026 17:49
3417407 to
7972146
Compare
![boxlite-agent[bot]](https://avatars.githubusercontent.com/in/4244458?s=60&v=4){kind=small}
Comment on lines
+57
to
+76
| fn resolve_device(device: ProtoContainerDevice) -> BoxliteResult<LinuxDevice> { | ||
| let source = validate_absolute_path("source", &device.source)?; | ||
| let destination = validate_absolute_path("destination", &device.destination)?; | ||
| if destination == Path::new("/dev") || !destination.starts_with("/dev") { | ||
| return Err(unsupported_device( | ||
| &destination, | ||
| "must be below /dev".to_string(), | ||
| )); | ||
| } | ||
| if let Some(file_mode) = device.file_mode { | ||
| if file_mode & !0o777 != 0 { | ||
| return Err(unsupported_device( | ||
| &destination, | ||
| format!("has invalid file mode {file_mode:#o}"), | ||
| )); | ||
| } | ||
| } | ||
|
|
||
| let metadata = std::fs::symlink_metadata(&source) | ||
| .map_err(|error| unsupported_device(&source, format!("is unavailable: {error}")))?; |
There was a problem hiding this comment.
🧹 device source path unrestricted to /dev
resolve_device confines destination under /dev but not source, so any absolute normalized path could be requested as a device source if a future caller exposes ContainerDevice beyond the current hardcoded /dev/kvm use in GuestInitTask::kvm_device.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/boxlite/src/runtime/options.rs (1)
1113-1127: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSame missing-required-fields bug as before — now under
advanced, still unfixed.
serde_json::from_strfor bothstoredandlegacyomitsrootfs,env,volumes,network,ports, which have no#[serde(default)](pertest_box_options_serde_defaults's own comment: "Must include all required fields that don't have serde defaults"). Both.unwrap()calls will panic on missing-field deserialization errors before the assertions run.🐛 Proposed fix
fn nested_virtualization_option_roundtrips() { - let stored: BoxOptions = - serde_json::from_str(r#"{"advanced":{"nested_virtualization":true}}"#).unwrap(); + let stored: BoxOptions = serde_json::from_str( + r#"{ + "rootfs": {"Image": "alpine:latest"}, + "env": [], + "volumes": [], + "network": {"Enabled": {"allow_net": []}}, + "ports": [], + "advanced": {"nested_virtualization": true} + }"#, + ) + .unwrap(); assert!(stored.advanced.nested_virtualization); assert_eq!( serde_json::to_value(stored).unwrap()["advanced"]["nested_virtualization"], serde_json::Value::Bool(true) ); - let legacy: BoxOptions = serde_json::from_str("{}").unwrap(); + let legacy: BoxOptions = serde_json::from_str( + r#"{ + "rootfs": {"Image": "alpine:latest"}, + "env": [], + "volumes": [], + "network": {"Enabled": {"allow_net": []}}, + "ports": [] + }"#, + ) + .unwrap(); assert!(!legacy.advanced.nested_virtualization); }🤖 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/runtime/options.rs` around lines 1113 - 1127, Update the nested_virtualization_option_roundtrips test fixtures to include every required BoxOptions field lacking #[serde(default)], including rootfs, env, volumes, network, and ports, in both serialized JSON inputs. Preserve the existing assertions for nested_virtualization being enabled when present and defaulting to false when absent, consistent with test_box_options_serde_defaults.
🧹 Nitpick comments (2)
sdks/python/tests/test_untrusted_import.py (1)
44-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the public test function.
test_untrusted_import_rejects_nested_virtualizationis a public module-level function without a docstring. Add one describing that untrusted archives requesting nested virtualization must be rejected before disk installation.Suggested change
`@pytest.mark.asyncio` async def test_untrusted_import_rejects_nested_virtualization(tmp_path) -> None: + """Reject untrusted nested-virtualization archives before disk installation."""🤖 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 `@sdks/python/tests/test_untrusted_import.py` around lines 44 - 45, Add a docstring to the public test function test_untrusted_import_rejects_nested_virtualization describing that untrusted archives requesting nested virtualization are rejected before disk installation; leave the test behavior unchanged.Source: Coding guidelines
src/boxlite/src/runtime/rt_impl.rs (1)
488-514: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDoc comment is now inaccurate: nested virtualization is also compared.
The comment states only the capability policy is compared, but the function now also enforces the one-way
nested_virtualizationrule below it.✏️ Proposed fix
/// Reject reuse when the request disagrees with the box's stored options. /// - /// Only the capability policy is compared: silently adopting a box whose - /// privileges differ from the request is the case that matters for safety. + /// Compares the capability policy (must match exactly) and nested + /// virtualization (one-way: a nested-capable box satisfies any request, + /// not vice versa): silently adopting a box whose privileges differ from + /// the request is the case that matters for safety. fn check_options_compatibility(🤖 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/runtime/rt_impl.rs` around lines 488 - 514, Update the doc comment for check_options_compatibility to state that it compares capability policy and enforces the one-way nested_virtualization compatibility rule, while preserving the existing safety rationale and implementation 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.
Duplicate comments:
In `@src/boxlite/src/runtime/options.rs`:
- Around line 1113-1127: Update the nested_virtualization_option_roundtrips test
fixtures to include every required BoxOptions field lacking #[serde(default)],
including rootfs, env, volumes, network, and ports, in both serialized JSON
inputs. Preserve the existing assertions for nested_virtualization being enabled
when present and defaulting to false when absent, consistent with
test_box_options_serde_defaults.
---
Nitpick comments:
In `@sdks/python/tests/test_untrusted_import.py`:
- Around line 44-45: Add a docstring to the public test function
test_untrusted_import_rejects_nested_virtualization describing that untrusted
archives requesting nested virtualization are rejected before disk installation;
leave the test behavior unchanged.
In `@src/boxlite/src/runtime/rt_impl.rs`:
- Around line 488-514: Update the doc comment for check_options_compatibility to
state that it compares capability policy and enforces the one-way
nested_virtualization compatibility rule, while preserving the existing safety
rationale and implementation behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ba2bb140-a72e-4f83-8410-f53959f83cd7
📒 Files selected for processing (46)
.github/workflows/e2e-local.ymldocs/architecture/README.mddocs/experimental/nested-virtualization.mdopenapi/box.openapi.yamlopenapi/reference-server/server.pyopenapi/reference-server/tests/test_handle_cache.pysdks/python/src/runtime.rssdks/python/tests/test_untrusted_import.pysrc/boxlite/src/experimental.rssrc/boxlite/src/experimental/nested_virtualization.rssrc/boxlite/src/jailer/builder.rssrc/boxlite/src/jailer/mod.rssrc/boxlite/src/jailer/sandbox/mod.rssrc/boxlite/src/litebox/crash_report.rssrc/boxlite/src/litebox/init/mod.rssrc/boxlite/src/litebox/init/tasks/guest_connect.rssrc/boxlite/src/litebox/init/tasks/guest_init.rssrc/boxlite/src/litebox/init/tasks/vmm_spawn.rssrc/boxlite/src/portal/interfaces/container.rssrc/boxlite/src/rest/runtime.rssrc/boxlite/src/runtime/advanced_options.rssrc/boxlite/src/runtime/import.rssrc/boxlite/src/runtime/options.rssrc/boxlite/src/runtime/rt_impl.rssrc/boxlite/src/vmm/controller/shim.rssrc/boxlite/src/vmm/controller/spawn.rssrc/boxlite/src/vmm/exit_info.rssrc/boxlite/src/vmm/krun/context.rssrc/boxlite/src/vmm/krun/engine.rssrc/boxlite/src/vmm/mod.rssrc/cli/src/cli.rssrc/cli/src/commands/create.rssrc/cli/src/commands/run.rssrc/cli/src/commands/serve/handlers/advanced.rssrc/cli/src/commands/serve/mod.rssrc/cli/tests/nested_virtualization_rc.rssrc/deps/libkrun-sys/build.rssrc/deps/libkrun-sys/src/lib.rssrc/deps/libkrun-sys/vendor/libkrunfwsrc/guest/src/container/lifecycle.rssrc/guest/src/container/mod.rssrc/guest/src/container/spec.rssrc/guest/src/container/start.rssrc/guest/src/service/container.rssrc/shared/proto/boxlite/v1/service.protosrc/shim/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (38)
- src/deps/libkrun-sys/vendor/libkrunfw
- src/boxlite/src/jailer/sandbox/mod.rs
- src/guest/src/container/mod.rs
- src/boxlite/src/experimental/nested_virtualization.rs
- .github/workflows/e2e-local.yml
- openapi/box.openapi.yaml
- openapi/reference-server/tests/test_handle_cache.py
- src/boxlite/src/litebox/init/mod.rs
- docs/architecture/README.md
- src/cli/src/commands/run.rs
- src/boxlite/src/vmm/mod.rs
- src/cli/tests/nested_virtualization_rc.rs
- src/boxlite/src/jailer/mod.rs
- openapi/reference-server/server.py
- src/deps/libkrun-sys/src/lib.rs
- src/cli/src/commands/create.rs
- src/guest/src/container/start.rs
- src/cli/src/commands/serve/handlers/advanced.rs
- src/cli/src/commands/serve/mod.rs
- src/boxlite/src/vmm/krun/engine.rs
- src/boxlite/src/litebox/init/tasks/guest_connect.rs
- src/boxlite/src/litebox/init/tasks/guest_init.rs
- src/boxlite/src/jailer/builder.rs
- src/shim/src/main.rs
- src/boxlite/src/experimental.rs
- src/boxlite/src/runtime/import.rs
- src/boxlite/src/litebox/init/tasks/vmm_spawn.rs
- src/boxlite/src/vmm/krun/context.rs
- src/boxlite/src/vmm/exit_info.rs
- src/guest/src/container/lifecycle.rs
- src/boxlite/src/rest/runtime.rs
- src/guest/src/service/container.rs
- sdks/python/src/runtime.rs
- src/boxlite/src/vmm/controller/shim.rs
- src/guest/src/container/spec.rs
- src/deps/libkrun-sys/build.rs
- src/boxlite/src/vmm/controller/spawn.rs
- src/cli/src/cli.rs
added 2 commits
July 28, 2026 11:29
Device requests confined the destination but not the source, so a caller could name any normalized absolute path as a device to read major/minor from. Only /dev/kvm reaches this today, but the guest validates inbound RPC rather than trusting the host, and one end of a two-ended check was missing. Both ends now go through the same validator. Claude-Session: https://claude.ai/code/session_01YL9HKQHBrZR9u1qY7FDyKd
The E2E job set BOXLITE_TEST_NESTED_VIRTUALIZATION unconditionally, but the runner is a reused tagged instance whose hardware this workflow does not choose. A runner can expose /dev/kvm while its KVM module reports no nesting, which turns an infrastructure gap into a confusing ioctl failure inside the test. Probe the loaded module first and enable the test only when it reports nesting. When it does not, emit a warning annotation: a silent skip is indistinguishable from a pass. Claude-Session: https://claude.ai/code/session_01YL9HKQHBrZR9u1qY7FDyKd
9 tasks
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.
Summary
ExperimentalFeature::NestedVirtualizationand a hidden--nested-virtualizationCLI opt-in/dev/kvmthrough a generic Docker-styledevicestransport into the OCI workloadUsage
Verification
make fmt:checkmake clippymake test:unit:rust(899 BoxLite + 47 shared tests)make test:integration:cli FILTER=nested_virtualization SETUP_DONE=1(5 passed)BOXLITE_TEST_NESTED_VIRTUALIZATION=1 make test:integration:cli FILTER=test_nested_virtualization_can_create_kvm_vmBOXLITE_INSIDE_BOXLITE_POSTREBASE_OKandaarch64Summary by CodeRabbit
New Features
/dev/kvmdevice handling.Bug Fixes
Documentation
Chores