Skip to content

Commit 6cdb79c

Browse files
committed
fix(vmm): preserve orphan VM workdir when no .removing marker
Only delete the VM workdir during cleanup if the .removing marker file is present (set by user-initiated remove_vm). Orphaned supervisor processes discovered at startup without the marker now have their workdir preserved, preventing accidental data loss on VMM restart.
1 parent 8630143 commit 6cdb79c

1 file changed

Lines changed: 24 additions & 12 deletions

File tree

vmm/src/app.rs

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -292,17 +292,17 @@ impl App {
292292
// Persist the removing marker so crash recovery can resume
293293
let work_dir = self.work_dir(id);
294294
if let Err(err) = work_dir.set_removing() {
295-
warn!("Failed to write .removing marker for {id}: {err:?}");
295+
warn!("failed to write .removing marker for {id}: {err:?}");
296296
}
297297

298298
// Clean up port forwarding immediately
299299
self.cleanup_port_forward(id).await;
300300

301-
// Spawn background cleanup coroutine
301+
// User-initiated removal always deletes the workdir
302302
let app = self.clone();
303303
let id = id.to_string();
304304
tokio::spawn(async move {
305-
if let Err(err) = app.finish_remove_vm(&id).await {
305+
if let Err(err) = app.finish_remove_vm(&id, true).await {
306306
error!("Background cleanup failed for {id}: {err:?}");
307307
}
308308
});
@@ -311,8 +311,10 @@ impl App {
311311
}
312312

313313
/// Background cleanup: stop supervisor process, wait for it to exit,
314-
/// remove from supervisor, delete workdir, and free CID.
315-
async fn finish_remove_vm(&self, id: &str) -> Result<()> {
314+
/// remove from supervisor, optionally delete workdir, and free CID.
315+
///
316+
/// `delete_workdir`: true for user-initiated removal, false for orphan cleanup.
317+
async fn finish_remove_vm(&self, id: &str, delete_workdir: bool) -> Result<()> {
316318
// Stop the supervisor process (idempotent if already stopped)
317319
if let Err(err) = self.supervisor.stop(id).await {
318320
debug!("supervisor.stop({id}) during removal: {err:?}");
@@ -351,12 +353,20 @@ impl App {
351353
}
352354
}
353355

354-
// Delete the workdir (may already be gone, e.g. manual deletion before reload)
356+
// Only delete the workdir for user-initiated removal or if .removing marker exists.
357+
// Orphaned supervisor processes without the marker keep their data intact.
355358
let vm_path = self.work_dir(id);
356-
if vm_path.path().exists() {
357-
if let Err(err) = fs::remove_dir_all(&vm_path) {
358-
error!("Failed to remove VM directory for {id}: {err:?}");
359+
if delete_workdir || vm_path.is_removing() {
360+
if vm_path.path().exists() {
361+
if let Err(err) = fs::remove_dir_all(&vm_path) {
362+
error!("failed to remove VM directory for {id}: {err:?}");
363+
}
359364
}
365+
} else if vm_path.path().exists() {
366+
info!(
367+
"VM {id} workdir preserved (orphan cleanup): {}",
368+
vm_path.path().display()
369+
);
360370
}
361371

362372
// Free CID and remove from memory (last step)
@@ -371,7 +381,8 @@ impl App {
371381
Ok(())
372382
}
373383

374-
/// Spawn a background task to clean up a VM (stop + remove from supervisor + delete workdir).
384+
/// Spawn a background task to clean up a VM (stop + remove from supervisor).
385+
/// Workdir deletion is based on the `.removing` marker (only present for user-initiated removal).
375386
/// Returns false if a cleanup task is already running for this VM.
376387
fn spawn_finish_remove(&self, id: &str) -> bool {
377388
{
@@ -384,12 +395,13 @@ impl App {
384395
vm.state.removing = true;
385396
}
386397
// If VM is not in memory (e.g. orphaned supervisor process), no entry to guard
387-
// but we still need to clean up the supervisor process and workdir.
398+
// but we still need to clean up the supervisor process.
388399
}
389400
let app = self.clone();
390401
let id = id.to_string();
391402
tokio::spawn(async move {
392-
if let Err(err) = app.finish_remove_vm(&id).await {
403+
// Don't pass delete_workdir=true; rely on .removing marker check inside
404+
if let Err(err) = app.finish_remove_vm(&id, false).await {
393405
error!("Background cleanup failed for {id}: {err:?}");
394406
}
395407
});

0 commit comments

Comments
 (0)