Introduce Unrecoverable sandbox state - #1727
Conversation
eb0b4b3 to
da5b23c
Compare
da5b23c to
c180248
Compare
c180248 to
b48dd56
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the Hyperlight host sandbox lifecycle model by introducing an explicit unrecoverable state for MultiUseSandbox instances when certain restore failures (notably VM base mapping updates) leave the sandbox unsafe to continue using. It also updates the public API and tests to query sandbox lifecycle via a new status() method returning SandboxStatus.
Changes:
- Add
SandboxStatus(Ready | Poisoned | Unrecoverable) and exposeMultiUseSandbox::status()(deprecatingpoisoned()). - Make
restore()treat base mapping update failures as terminal by marking the sandboxUnrecoverableand rejecting future operations. - Add hypervisor test fault-injection helpers and expand restore-related tests to cover recoverable vs unrecoverable failure modes.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/hyperlight_host/tests/sandbox_host_tests.rs | Update tests to use status().is_poisoned() instead of poisoned(). |
| src/hyperlight_host/tests/integration_test.rs | Update integration assertions to use SandboxStatus/status(). |
| src/hyperlight_host/src/sandbox/snapshot/file_tests.rs | Update snapshot tests to check poison via status(). |
| src/hyperlight_host/src/sandbox/mod.rs | Re-export SandboxStatus from the sandbox module. |
| src/hyperlight_host/src/sandbox/initialized_multi_use.rs | Introduce SandboxStatus, implement status()/ensure_usable(), update poison/restore behavior, and add new restore failure-mode tests. |
| src/hyperlight_host/src/lib.rs | Re-export SandboxStatus from the crate root. |
| src/hyperlight_host/src/hypervisor/hyperlight_vm/test_support.rs | Add test-only VM fault injection and base mapping state helpers. |
| src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs | Adjust snapshot/scratch mapping update logic and expose test support module. |
| src/hyperlight_host/src/error.rs | Add HyperlightError::UnrecoverableSandbox and classify it as non-poisoning. |
| CHANGELOG.md | Document the new lifecycle API and unrecoverable behavior. |
Suppressed comments (1)
src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs:561
update_scratch_mappinghas the same transactional issue asupdate_snapshot_mapping: it unmaps the old scratch region before mapping the new one, but only updatesself.scratch_memoryafter the new map succeeds. Ifmap_memoryfails,scratch_memorystill points at memory that has been unmapped in the VM, leaving the internal state inconsistent.
if let Some(old_scratch) = self.scratch_memory.as_ref() {
let old_base = hyperlight_common::layout::scratch_base_gpa(old_scratch.mem_size());
let old_rgn = old_scratch.mapping_at(old_base, MemoryRegionType::Scratch);
self.vm.unmap_memory((self.scratch_slot, &old_rgn))?;
}
unsafe { self.vm.map_memory((self.scratch_slot, &rgn))? };
self.scratch_memory = Some(scratch);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
b48dd56 to
47aa821
Compare
There was a problem hiding this comment.
should this be only avaliable when #[test]?
|
LGTM |
Some failures can leave sandboxes in a state that cannot be restored (like poisoned but worse). This PR introduces such state as Unrecoverable. Unrecoverable sandboxes can no longer be used at all, and users need to create new one. I don't expect that this will be common at all, but it was an existing gap in our codebase that should be addressed.
This pr also tries to be a bit more transactional in certain operations, meaning it either fully completes successfully or errors cleanly without leaving sandbox in invalid state.