fix: prevent snapshot COW child disks from being deleted by Disk::Drop#330
Merged
DorianZheng merged 1 commit intoboxlite-ai:mainfrom Mar 4, 2026
Merged
Conversation
create_cow_child_disk() returns Disk with persistent=false. When the Disk goes out of scope, its Drop impl deletes the file via std::fs::remove_file. In do_snapshot_create() and do_snapshot_restore(), the returned Disk was never leaked, causing the newly created COW child QCOW2 files to be immediately deleted. This made the box un-startable after snapshot create or restore, because the required disk.qcow2 and guest-rootfs.qcow2 files no longer existed on disk. Fix: call .leak() on all four create_cow_child_disk() return values in local_snapshot.rs, matching the correct pattern already used in container_rootfs.rs, guest_rootfs.rs, and qcow2.rs::clone_disk_pair(). Added boxlite/tests/snapshot.rs with 5 integration tests covering disk integrity and functional verification after snapshot create and restore.
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.
Impact
All snapshot operations are broken. After
snapshot.create()orsnapshot.restore(), the box becomes un-startable — the operation returns success, but the box's disk files (disk.qcow2,guest-rootfs.qcow2) are silently deleted within the same function call. The nextbox.start()fails because the required disk files no longer exist.LiteBox::snapshots().create(),LiteBox::snapshots().restore().leak()correctly)Root cause
Diskis an RAII type (boxlite/src/disk/image.rs) that deletes its file on drop whenpersistent=false.create_cow_child_disk()always returnsDisk { persistent: false }. Callers must call.leak()to prevent the destructor from deleting the file.In
local_snapshot.rs, all 4 calls tocreate_cow_child_disk()neglect to call.leak():do_snapshot_create()usesif let Err(e) = create_cow_child_disk(...)— theOk(Disk)is never bound, so it drops immediatelydo_snapshot_restore()usescreate_cow_child_disk(...)?;— theDiskis unwrapped by?then dropped at the semicolonEvery other call site in the codebase handles this correctly:
container_rootfs.rs:233—temp_disk.leak()guest_rootfs.rs:150—temp_disk.leak()qcow2.rs:827,833(clone_disk_pair) —create_cow_child_disk(...)?.leak()Fix
Add
.leak()to all 4create_cow_child_disk()return values inlocal_snapshot.rs:do_snapshot_create()container disk (L248)if let Err→matchwithOk(disk) => { disk.leak(); }, error branch keeps rollbackdo_snapshot_create()guest disk (L288)matchpattern, error branch keeps full rollback including container cleanupdo_snapshot_restore()container disk (L375))?;→)?.leak();do_snapshot_restore()guest disk (L393))?;→)?.leak();Tests
Added
boxlite/tests/snapshot.rs— 5 integration tests followingclone_export_import.rsconventions:test_cow_child_disks_exist_after_snapshot_createdisk.qcow2andguest-rootfs.qcow2exist in box dir after createtest_box_restartable_after_snapshot_createecho aliveafter createtest_cow_child_disks_exist_after_snapshot_restoretest_box_startable_after_snapshot_restorecat /root/ver.txtreturns snapshot contenttest_snapshot_list_returns_created_snapshotlist()after createUses
PerTestBoxHome::new()for isolation, publicLiteBox::snapshots()API. VM integration tests requiremake runtime-debug.CI results (same commit
ac6f182, run on fork)cargo fmt --check)cargo clippy -- -D warnings)boxlite-sharedunit)boxlite-sharedunit)Full CI run: https://github.com/acmerfight/boxlite/actions/runs/22585287612
Local verification
cargo fmt --check— passcargo clippy -p boxlite --tests -- -D warnings— 0 warningscargo test -p boxlite --lib -- litebox::local_snapshot— 12/12 unit tests passcargo test -p boxlite --test snapshot— requires VM runtime (make runtime-debug)