From 4dc9726f7a503548693af20072e8ac2c57d0a008 Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Mon, 31 Aug 2026 20:31:38 +0530 Subject: [PATCH 01/18] Quiesce device workers during suspend and snapshot --- vmm/crates/vmm-core/src/controller.rs | 295 +++++++++++++----- vmm/crates/vmm-core/src/live_snapshot.rs | 48 ++- .../vmm-devices/src/virtio/blk_io_loop.rs | 53 +++- .../vmm-devices/src/virtio/net_io_loop.rs | 120 +++++-- .../vmm-devices/src/virtio/vsock_io_loop.rs | 76 ++++- 5 files changed, 470 insertions(+), 122 deletions(-) diff --git a/vmm/crates/vmm-core/src/controller.rs b/vmm/crates/vmm-core/src/controller.rs index 2a7b5c5..b4e7fc0 100644 --- a/vmm/crates/vmm-core/src/controller.rs +++ b/vmm/crates/vmm-core/src/controller.rs @@ -595,7 +595,31 @@ impl VmmController { false }; #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] - let io_paused_here = paused_here && pause_running_io(vm); + // A Paused VM already has its I/O workers parked by pause(). Only a + // Running VM is quiesced here, and only that transition is undone + // below. This preserves Paused semantics across snapshot success and + // failure paths. + let io_paused_here = if state_before == VmState::Running { + match pause_running_io(vm) { + Ok(paused) => paused, + Err(error) => { + let resume_error = if paused_here { + resume_running_vcpus(vm).err() + } else { + None + }; + remove_owned_scratch_file(&owned_snapshot); + return Err(match resume_error { + Some(resume) => VmmError::Snapshot(format!( + "{error}; failed to resume vCPUs after I/O quiescence failure: {resume}" + )), + None => error, + }); + } + } + } else { + false + }; #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] let mut consumed_dirty = None; @@ -907,22 +931,20 @@ impl VmmController { irq, mmio_base, } = net; - if let Err(e) = kvm_vm.register_irqfd(&irq_evt, irq) { - log::warn!("net irqfd (gsi={irq}): {e}"); - } - if let Err(e) = kvm_vm.register_ioeventfd(mmio_base + 0x50, &io_evt) { - log::warn!("net ioeventfd at 0x{:x}: {e}", mmio_base + 0x50); - } + kvm_vm.register_irqfd(&irq_evt, irq)?; + kvm_vm.register_ioeventfd(mmio_base + 0x50, &io_evt)?; let tap_fd = tap.fd; let kick_fd = { use std::os::fd::AsRawFd; io_evt.as_raw_fd() }; - match vmm_devices::virtio::net_io_loop::spawn_net_io_loop(dev.clone(), tap_fd, kick_fd) - { - Ok(l) => net_io_loops.push(l), - Err(e) => log::warn!("net io loop: {e}"), - } + let io_loop = vmm_devices::virtio::net_io_loop::spawn_net_io_loop( + dev.clone(), + tap_fd, + kick_fd, + ) + .map_err(|error| VmmError::Device(format!("spawn network I/O worker: {error}")))?; + net_io_loops.push(io_loop); net_devices.push(dev); irq_evts.push(irq_evt); irq_evts.push(io_evt); @@ -961,23 +983,19 @@ impl VmmController { // Wire the virtio-vsock exec channel: register its irqfd, bind the // control socket the guest agent dials into, and start the host→guest - // pump. Best-effort — on any failure exec transparently uses serial. + // pump. A configured vsock device is an admitted control channel, so + // worker or socket setup failure must fail creation rather than publish + // a VM whose PTY/SSH/exec behavior differs from its configuration. let (vsock_pump, vsock_exec, vsock_pty) = match vsock { Some(wv) => { - if let Err(e) = kvm_vm.register_irqfd(&wv.irq_evt, wv.irq) { - log::warn!("vsock irqfd (gsi={}): {e}", wv.irq); - } + kvm_vm.register_irqfd(&wv.irq_evt, wv.irq)?; irq_evts.push(wv.irq_evt); // TX QUEUE_NOTIFY → ioeventfd, so the guest's kick runs the TX // path (host socket connect/write) on the pump thread rather than // the seccomped vCPU thread (which would SIGSYS on connect()). // datamatch=1 = QUEUE_TX: only the TX kick routes here; RX/EVENT // (values 0/2) still trap to the vCPU, where they do no host I/O. - if let Err(e) = - kvm_vm.register_ioeventfd_datamatch(wv.mmio_base + 0x50, &wv.io_evt, 1) - { - log::warn!("vsock ioeventfd at 0x{:x}: {e}", wv.mmio_base + 0x50); - } + kvm_vm.register_ioeventfd_datamatch(wv.mmio_base + 0x50, &wv.io_evt, 1)?; use std::os::fd::AsRawFd; let tx_kick_fd = wv.io_evt.as_raw_fd(); let device = wv.device; @@ -985,24 +1003,30 @@ impl VmmController { device.clone(), tx_kick_fd, ) - .ok(); - let pump_wake = pump.as_ref().and_then(|p| p.wake_evt().ok()); - let pty_wake = pump.as_ref().and_then(|p| p.wake_evt().ok()); + .map_err(|error| VmmError::Device(format!("spawn vsock worker: {error}")))?; + let pump_wake = Some( + pump.wake_evt() + .map_err(|error| VmmError::Device(format!("clone vsock wake: {error}")))?, + ); + let pty_wake = Some( + pump.wake_evt() + .map_err(|error| VmmError::Device(format!("clone PTY wake: {error}")))?, + ); irq_evts.push(wv.io_evt); - let exec = match crate::vsock_exec::VsockExecChannel::bind_with_pump_wake( - &wv.control_socket, - pump_wake, - ) { - Ok(c) => Some(c), - Err(e) => { - log::warn!("vsock exec bind {}: {e}", wv.control_socket.display()); - None - } - }; - let pty = pump - .as_ref() - .map(|_| crate::vsock_pty::VsockPtyChannel::new(device, pty_wake)); - (pump, exec, pty) + let exec = Some( + crate::vsock_exec::VsockExecChannel::bind_with_pump_wake( + &wv.control_socket, + pump_wake, + ) + .map_err(|error| { + VmmError::Device(format!( + "bind vsock exec socket {}: {error}", + wv.control_socket.display() + )) + })?, + ); + let pty = Some(crate::vsock_pty::VsockPtyChannel::new(device, pty_wake)); + (Some(pump), exec, pty) } None => (None, None, None), }; @@ -1751,6 +1775,15 @@ impl VmmController { // thousand). snapshot() drives the thread directly; the API must too. #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] pause_running_vcpus(vm)?; + #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] + if let Err(error) = pause_running_io(vm) { + return match resume_running_vcpus(vm) { + Ok(()) => Err(error), + Err(resume) => Err(VmmError::Device(format!( + "{error}; failed to resume vCPUs after I/O quiescence failure: {resume}" + ))), + }; + } vm.state = VmState::Paused; log::info!("VM paused"); Ok(()) @@ -1765,7 +1798,8 @@ impl VmmController { if vm.state == VmState::Running { return Ok(()); } - if vm.state != VmState::Paused { + let state_before = vm.state; + if !matches!(state_before, VmState::Paused | VmState::Suspended) { return Err(VmmError::InvalidConfig(format!( "cannot resume a VM in {:?} state", vm.state @@ -1773,6 +1807,8 @@ impl VmmController { } #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] resume_running_vcpus(vm)?; + #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] + resume_running_io(vm); vm.state = VmState::Running; log::info!("VM resumed"); Ok(()) @@ -2567,43 +2603,62 @@ fn pause_running_vcpus(vm: &VmInstance) -> Result { /// returns with `paused = true`, device state and RAM are stable as long as the /// vCPUs are also paused. #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] -fn set_running_io_paused(running: &RunningVm, paused: bool) { +fn set_running_io_paused(running: &RunningVm, paused: bool) -> Result<()> { if paused { for io_loop in &running.blk_io_loops { - io_loop.pause(); + if let Err(error) = io_loop.pause() { + resume_running_io_workers(running); + return Err(VmmError::Device(format!( + "quiesce block I/O worker: {error}" + ))); + } } for io_loop in &running.net_io_loops { - io_loop.pause(); + if let Err(error) = io_loop.pause() { + resume_running_io_workers(running); + return Err(VmmError::Device(format!( + "quiesce network I/O worker: {error}" + ))); + } } if let Some(pump) = running.vsock_pump.as_ref() { - pump.pause(); + if let Err(error) = pump.pause() { + resume_running_io_workers(running); + return Err(VmmError::Device(format!("quiesce vsock worker: {error}"))); + } } } else { - for io_loop in &running.blk_io_loops { - io_loop.resume(); - } - for io_loop in &running.net_io_loops { - io_loop.resume(); - } - if let Some(pump) = running.vsock_pump.as_ref() { - pump.resume(); - } + resume_running_io_workers(running); + } + Ok(()) +} + +#[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] +fn resume_running_io_workers(running: &RunningVm) { + for io_loop in &running.blk_io_loops { + io_loop.resume(); + } + for io_loop in &running.net_io_loops { + io_loop.resume(); + } + if let Some(pump) = running.vsock_pump.as_ref() { + pump.resume(); } } #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] -fn pause_running_io(vm: &VmInstance) -> bool { +fn pause_running_io(vm: &VmInstance) -> Result { let Some(running) = vm.running.as_ref() else { - return false; + return Ok(false); }; - set_running_io_paused(running, true); - true + set_running_io_paused(running, true)?; + Ok(true) } #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] fn resume_running_io(vm: &VmInstance) { if let Some(running) = vm.running.as_ref() { - set_running_io_paused(running, false); + let _ = set_running_io_paused(running, false); } } @@ -2722,6 +2777,21 @@ fn suspend_vm_in_place(vm: &mut VmInstance) -> Result<()> { } else { false }; + // pause() already parked I/O for a Paused VM. Preserve that state if + // suspend fails; only a Running VM is newly quiesced here. + let io_paused_here = if state_before == VmState::Running { + match pause_running_io(vm) { + Ok(paused) => paused, + Err(error) => { + if paused_here { + resume_running_vcpus(vm)?; + } + return Err(error); + } + } + } else { + false + }; vm.state = VmState::Paused; let result = (|| -> Result<()> { capture_live_state(vm)?; @@ -2815,9 +2885,15 @@ fn suspend_vm_in_place(vm: &mut VmInstance) -> Result<()> { if result.is_err() { vm.state = state_before; - if paused_here && state_before == VmState::Running { - resume_running_vcpus(vm)?; + let resume_result = if paused_here && state_before == VmState::Running { + resume_running_vcpus(vm) + } else { + Ok(()) + }; + if io_paused_here { + resume_running_io(vm); } + resume_result?; } result } @@ -5461,21 +5537,18 @@ fn build_running_vm( irq, mmio_base, } = net; - if let Err(e) = kvm_vm.register_irqfd(&irq_evt, irq) { - log::warn!("net irqfd (gsi={irq}): {e}"); - } - if let Err(e) = kvm_vm.register_ioeventfd(mmio_base + 0x50, &io_evt) { - log::warn!("net ioeventfd at 0x{:x}: {e}", mmio_base + 0x50); - } + kvm_vm.register_irqfd(&irq_evt, irq)?; + kvm_vm.register_ioeventfd(mmio_base + 0x50, &io_evt)?; let tap_fd = tap.fd; let kick_fd = { use std::os::fd::AsRawFd; io_evt.as_raw_fd() }; - match vmm_devices::virtio::net_io_loop::spawn_net_io_loop(dev, tap_fd, kick_fd) { - Ok(l) => net_io_loops.push(l), - Err(e) => log::warn!("net io loop: {e}"), - } + let io_loop = vmm_devices::virtio::net_io_loop::spawn_net_io_loop(dev, tap_fd, kick_fd) + .map_err(|error| { + VmmError::Device(format!("spawn restored network I/O worker: {error}")) + })?; + net_io_loops.push(io_loop); irq_evts.push(irq_evt); irq_evts.push(io_evt); taps.push(tap); @@ -5510,14 +5583,9 @@ fn build_running_vm( // VM re-establishes exec-over-vsock when the guest agent re-dials. let (vsock_pump, vsock_exec, vsock_pty, vsock_reset) = match vsock { Some(wv) => { - if let Err(e) = kvm_vm.register_irqfd(&wv.irq_evt, wv.irq) { - log::warn!("vsock irqfd (gsi={}): {e}", wv.irq); - } + kvm_vm.register_irqfd(&wv.irq_evt, wv.irq)?; irq_evts.push(wv.irq_evt); - if let Err(e) = kvm_vm.register_ioeventfd_datamatch(wv.mmio_base + 0x50, &wv.io_evt, 1) - { - log::warn!("vsock ioeventfd at 0x{:x}: {e}", wv.mmio_base + 0x50); - } + kvm_vm.register_ioeventfd_datamatch(wv.mmio_base + 0x50, &wv.io_evt, 1)?; use std::os::fd::AsRawFd; let tx_kick_fd = wv.io_evt.as_raw_fd(); let device = wv.device; @@ -5532,9 +5600,16 @@ fn build_running_vm( }); let pump = vmm_devices::virtio::vsock_io_loop::spawn_vsock_pump(device.clone(), tx_kick_fd) - .ok(); - let pump_wake = pump.as_ref().and_then(|p| p.wake_evt().ok()); - let pty_wake = pump.as_ref().and_then(|p| p.wake_evt().ok()); + .map_err(|error| { + VmmError::Device(format!("spawn restored vsock worker: {error}")) + })?; + let pump_wake = Some(pump.wake_evt().map_err(|error| { + VmmError::Device(format!("clone restored vsock wake: {error}")) + })?); + let pty_wake = + Some(pump.wake_evt().map_err(|error| { + VmmError::Device(format!("clone restored PTY wake: {error}")) + })?); irq_evts.push(wv.io_evt); let exec = Some( crate::vsock_exec::VsockExecChannel::bind_with_pump_wake( @@ -5548,10 +5623,8 @@ fn build_running_vm( )) })?, ); - let pty = pump - .as_ref() - .map(|_| crate::vsock_pty::VsockPtyChannel::new(device, pty_wake)); - (pump, exec, pty, reset) + let pty = Some(crate::vsock_pty::VsockPtyChannel::new(device, pty_wake)); + (Some(pump), exec, pty, reset) } None => (None, None, None, None), }; @@ -6106,6 +6179,27 @@ mod tests { } } + fn controller_in_state(state: VmState) -> VmmController { + let controller = VmmController::new(); + *controller.lock() = Some(VmInstance { + state, + generation: next_vm_generation(), + created_at: std::time::Instant::now(), + last_snapshot: None, + transient_files: VmTransientFiles::default(), + dirty_logging: false, + config: cfg(), + guest_mem: None, + state_blob: None, + mem_dump: Some(vec![0; 4096]), + #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] + lazy_restore: None, + #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] + running: None, + }); + controller + } + fn net(tap: &str, mac: &str, ip: &str) -> NetConfig { NetConfig { tap: tap.into(), @@ -6200,6 +6294,41 @@ mod tests { assert!(c.status().is_err()); } + #[test] + fn suspended_vm_can_resume_and_reenter_pause_resume_cycle() { + let controller = controller_in_state(VmState::Suspended); + + controller.resume().expect("resume suspended VM"); + assert_eq!( + controller.status().expect("running status").state, + VmState::Running + ); + + controller.pause().expect("pause resumed VM"); + assert_eq!( + controller.status().expect("paused status").state, + VmState::Paused + ); + + controller.resume().expect("resume paused VM"); + assert_eq!( + controller.status().expect("resumed status").state, + VmState::Running + ); + } + + #[test] + fn snapshot_preserves_paused_controller_state() { + let controller = controller_in_state(VmState::Paused); + + controller.snapshot(false).expect("snapshot paused VM"); + + assert_eq!( + controller.status().expect("paused status").state, + VmState::Paused + ); + } + #[cfg(not(feature = "boot"))] #[test] fn status_reports_config_after_create() { diff --git a/vmm/crates/vmm-core/src/live_snapshot.rs b/vmm/crates/vmm-core/src/live_snapshot.rs index 378977d..55e4d52 100644 --- a/vmm/crates/vmm-core/src/live_snapshot.rs +++ b/vmm/crates/vmm-core/src/live_snapshot.rs @@ -107,7 +107,7 @@ impl Drop for VcpuPauseGuard<'_> { /// guest memory (net/vsock pumps). Disengaging, or dropping on an error path, /// releases them. struct IoQuiesceGuard<'a> { - quiesce: &'a dyn Fn(bool), + quiesce: &'a dyn Fn(bool) -> Result<()>, armed: bool, } @@ -152,17 +152,17 @@ impl Drop for DirtyReplayGuard<'_> { } impl<'a> IoQuiesceGuard<'a> { - fn engage(quiesce: &'a dyn Fn(bool)) -> Self { - quiesce(true); - Self { + fn engage(quiesce: &'a dyn Fn(bool) -> Result<()>) -> Result { + quiesce(true)?; + Ok(Self { quiesce, armed: true, - } + }) } fn disengage(mut self) { if self.armed { - (self.quiesce)(false); + let _ = (self.quiesce)(false); self.armed = false; } } @@ -171,7 +171,7 @@ impl<'a> IoQuiesceGuard<'a> { impl Drop for IoQuiesceGuard<'_> { fn drop(&mut self) { if self.armed { - (self.quiesce)(false); + let _ = (self.quiesce)(false); } } } @@ -390,7 +390,7 @@ pub fn live_snapshot( vcpu_threads: &[&VcpuThread], config: &LiveSnapshotConfig, memory_file: &File, - quiesce_io: &dyn Fn(bool), + quiesce_io: &dyn Fn(bool) -> Result<()>, capture_state: F, ) -> Result where @@ -606,7 +606,7 @@ where log::info!("live_snapshot: final stop — pausing all vCPUs, draining I/O"); let final_stop_start = Instant::now(); let final_pause_guard = VcpuPauseGuard::pause_all(vcpu_threads)?; - let io_guard = IoQuiesceGuard::engage(quiesce_io); + let io_guard = IoQuiesceGuard::engage(quiesce_io)?; inject_live_snapshot_failure("final_pause")?; let mut final_dirty = kvm_vm.read_dirty()?; @@ -791,4 +791,34 @@ mod tests { RoundDecision::Continue { .. } )); } + + #[test] + fn io_quiesce_failure_is_propagated_without_arming_release() { + let calls = std::cell::RefCell::new(Vec::new()); + let quiesce = |paused| { + calls.borrow_mut().push(paused); + Err(VmmError::Device("quiescence failed".into())) + }; + + let error = match IoQuiesceGuard::engage(&quiesce) { + Ok(_) => panic!("failed quiescence unexpectedly armed the guard"), + Err(error) => error, + }; + + assert!(error.to_string().contains("quiescence failed")); + assert_eq!(&*calls.borrow(), &[true]); + } + + #[test] + fn io_quiesce_guard_releases_on_drop() { + let calls = std::cell::RefCell::new(Vec::new()); + let quiesce = |paused| { + calls.borrow_mut().push(paused); + Ok(()) + }; + + drop(IoQuiesceGuard::engage(&quiesce).expect("engage I/O quiescence")); + + assert_eq!(&*calls.borrow(), &[true, false]); + } } diff --git a/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs b/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs index 05bd412..832c6ca 100644 --- a/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs +++ b/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs @@ -17,6 +17,7 @@ use vmm_sys_util::eventfd::EventFd; const POLL_TIMEOUT_MS: libc::c_int = 100; const PAUSE_POLL: std::time::Duration = std::time::Duration::from_micros(100); +const QUIESCE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5); /// Handle for one volume's queue worker. Dropping it stops and joins the /// worker before the eventfds and backing storage are released. @@ -41,20 +42,38 @@ impl BlkIoLoop { /// Drain all work published by stopped vCPUs, then park without touching /// guest memory or device state until resumed. - pub fn pause(&self) { + pub fn pause(&self) -> io::Result<()> { if self.thread_gone() { self.fail_if_unexpected_exit(); - return; + return Err(io::Error::new( + io::ErrorKind::BrokenPipe, + "block I/O worker exited before quiescence", + )); } self.pause_req.store(true, Ordering::SeqCst); - let _ = self.wake_evt.write(1); + self.wake_evt.write(1)?; + let deadline = std::time::Instant::now() + QUIESCE_TIMEOUT; while !self.pause_ack.load(Ordering::SeqCst) { if self.thread_gone() { self.fail_if_unexpected_exit(); - return; + self.pause_req.store(false, Ordering::SeqCst); + return Err(io::Error::new( + io::ErrorKind::BrokenPipe, + "block I/O worker exited during quiescence", + )); + } + if std::time::Instant::now() >= deadline { + self.pause_req.store(false, Ordering::SeqCst); + self.device + .fail_worker("block I/O worker quiescence timed out"); + return Err(io::Error::new( + io::ErrorKind::TimedOut, + "block I/O worker quiescence timed out", + )); } std::thread::sleep(PAUSE_POLL); } + Ok(()) } pub fn resume(&self) { @@ -84,6 +103,14 @@ impl Drop for BlkIoLoop { /// Spawn a queue worker. `kick_fd` must be the non-blocking eventfd registered /// for queue 0 at this device's QUEUE_NOTIFY MMIO address. pub fn spawn_blk_io_loop(device: Arc, kick_fd: RawFd) -> io::Result { + // SAFETY: F_GETFD inspects the descriptor without retaining it. The + // controller owns the descriptor for the returned worker's lifetime. + if unsafe { libc::fcntl(kick_fd, libc::F_GETFD) } < 0 { + return Err(io::Error::new( + io::Error::last_os_error().kind(), + "block queue kick descriptor is invalid", + )); + } let stop = Arc::new(AtomicBool::new(false)); let pause_req = Arc::new(AtomicBool::new(false)); let pause_ack = Arc::new(AtomicBool::new(false)); @@ -231,3 +258,21 @@ fn drain_eventfd(fd: RawFd, label: &str) { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn startup_rejects_an_invalid_queue_descriptor() { + let device = Arc::new(VirtioBlkMmio::new_stub(5, 2)); + let error = match spawn_blk_io_loop(device, -1) { + Ok(mut worker) => { + worker.stop(); + panic!("invalid queue descriptor unexpectedly started a block worker") + } + Err(error) => error, + }; + assert!(error.to_string().contains("descriptor is invalid")); + } +} diff --git a/vmm/crates/vmm-devices/src/virtio/net_io_loop.rs b/vmm/crates/vmm-devices/src/virtio/net_io_loop.rs index 53aa9f9..e4fcb97 100644 --- a/vmm/crates/vmm-devices/src/virtio/net_io_loop.rs +++ b/vmm/crates/vmm-devices/src/virtio/net_io_loop.rs @@ -22,6 +22,7 @@ const MAX_FRAME: usize = 1600; /// How long the paused loop sleeps between checks of the pause flag. const PAUSE_POLL: std::time::Duration = std::time::Duration::from_micros(100); +const QUIESCE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5); /// Handle returned by [`spawn_net_io_loop`]. Dropping it stops the thread. pub struct NetIoLoop { @@ -34,6 +35,13 @@ pub struct NetIoLoop { pub device: Arc, } +struct WorkerControl { + stop: Arc, + pause_req: Arc, + pause_ack: Arc, + ready_tx: std::sync::mpsc::SyncSender>, +} + impl NetIoLoop { pub fn stop(&mut self) { self.stop.store(true, Ordering::SeqCst); @@ -50,18 +58,34 @@ impl NetIoLoop { /// without touching guest memory until [`Self::resume`]. Callers must /// pause every vCPU first so the guest cannot publish another descriptor /// after this drain. - pub fn pause(&self) { + pub fn pause(&self) -> io::Result<()> { if self.thread_gone() { - return; + return Err(io::Error::new( + io::ErrorKind::BrokenPipe, + "network I/O worker exited before quiescence", + )); } self.pause_req.store(true, Ordering::SeqCst); - let _ = self.wake_evt.write(1); + self.wake_evt.write(1)?; + let deadline = std::time::Instant::now() + QUIESCE_TIMEOUT; while !self.pause_ack.load(Ordering::SeqCst) { if self.thread_gone() { - return; + self.pause_req.store(false, Ordering::SeqCst); + return Err(io::Error::new( + io::ErrorKind::BrokenPipe, + "network I/O worker exited during quiescence", + )); + } + if std::time::Instant::now() >= deadline { + self.pause_req.store(false, Ordering::SeqCst); + return Err(io::Error::new( + io::ErrorKind::TimedOut, + "network I/O worker quiescence timed out", + )); } std::thread::sleep(PAUSE_POLL); } + Ok(()) } /// Release a pause. Does not wait: the thread re-enters its poll loop on @@ -70,7 +94,6 @@ impl NetIoLoop { self.pause_req.store(false, Ordering::SeqCst); } - /// A finished thread writes no guest memory, so it counts as paused. fn thread_gone(&self) -> bool { self.handle.as_ref().is_none_or(|h| h.is_finished()) } @@ -100,14 +123,18 @@ pub fn spawn_net_io_loop( let pause_ack_t = pause_ack.clone(); let wake_fd = wake_evt.as_raw_fd(); let device_t = device.clone(); + let (ready_tx, ready_rx) = std::sync::mpsc::sync_channel(1); let handle = std::thread::Builder::new() .name("virtio-net-io".into()) .spawn(move || { run( - stop_t, - pause_req_t, - pause_ack_t, + WorkerControl { + stop: stop_t, + pause_req: pause_req_t, + pause_ack: pause_ack_t, + ready_tx, + }, device_t, tap_fd, tx_kick_fd, @@ -115,6 +142,20 @@ pub fn spawn_net_io_loop( ); })?; + match ready_rx.recv() { + Ok(Ok(())) => {} + Ok(Err(error)) => { + let _ = handle.join(); + return Err(error); + } + Err(error) => { + let _ = handle.join(); + return Err(io::Error::other(format!( + "network I/O worker exited during startup: {error}" + ))); + } + } + Ok(NetIoLoop { stop, pause_req, @@ -126,23 +167,28 @@ pub fn spawn_net_io_loop( } fn run( - stop: Arc, - pause_req: Arc, - pause_ack: Arc, + control: WorkerControl, device: Arc, tap_fd: RawFd, tx_kick_fd: RawFd, wake_fd: RawFd, ) { + let WorkerControl { + stop, + pause_req, + pause_ack, + ready_tx, + } = control; // SAFETY: epoll_create1 has no pointer arguments; flags are a valid libc // constant, and errors are handled from the returned fd. let ep = match unsafe { libc::epoll_create1(libc::EPOLL_CLOEXEC) } { fd if fd >= 0 => fd, _ => { - log::error!( - "net_io_loop: epoll_create1 failed: {}", - io::Error::last_os_error() - ); + let error = io::Error::last_os_error(); + let _ = ready_tx.send(Err(io::Error::new( + error.kind(), + format!("create network worker epoll: {error}"), + ))); return; } }; @@ -162,21 +208,30 @@ fn run( } }; if let Err(e) = add(tap_fd, 1) { - log::error!("net_io_loop: epoll add tap: {e}"); + let _ = ready_tx.send(Err(io::Error::new( + e.kind(), + format!("register network tap with epoll: {e}"), + ))); // SAFETY: `ep` is the fd returned by epoll_create1 above and is owned // by this function on this error path. unsafe { libc::close(ep) }; return; } if let Err(e) = add(tx_kick_fd, 2) { - log::error!("net_io_loop: epoll add tx_kick: {e}"); + let _ = ready_tx.send(Err(io::Error::new( + e.kind(), + format!("register network queue kick with epoll: {e}"), + ))); // SAFETY: `ep` is the fd returned by epoll_create1 above and is owned // by this function on this error path. unsafe { libc::close(ep) }; return; } if let Err(e) = add(wake_fd, 3) { - log::error!("net_io_loop: epoll add wake: {e}"); + let _ = ready_tx.send(Err(io::Error::new( + e.kind(), + format!("register network worker wake with epoll: {e}"), + ))); // SAFETY: `ep` is the fd returned by epoll_create1 above and is owned // by this function on this error path. unsafe { libc::close(ep) }; @@ -184,12 +239,20 @@ fn run( } if let Err(e) = vmm_jailer::seccomp::SeccompProfile::device().install() { - log::error!("net_io_loop: seccomp install failed; refusing guest I/O: {e}"); + let _ = ready_tx.send(Err(io::Error::other(format!( + "install network worker sandbox: {e}" + )))); // SAFETY: `ep` is the fd returned by epoll_create1 above and is owned // by this function on this error path. unsafe { libc::close(ep) }; return; } + if ready_tx.send(Ok(())).is_err() { + // SAFETY: `ep` is owned by this worker and no longer needed when its + // creator disappeared before accepting startup readiness. + unsafe { libc::close(ep) }; + return; + } let mut events = [libc::epoll_event { events: 0, u64: 0 }; 4]; let mut buf = [0u8; MAX_FRAME]; @@ -342,6 +405,23 @@ mod tests { use std::sync::Arc; use vm_memory::{Bytes, GuestAddress, GuestMemoryMmap}; + #[test] + fn startup_rejects_an_invalid_tap_before_returning_success() { + let device = Arc::new(VirtioNetMmio::new(7, [0x02, 0, 0, 0, 0, 1])); + let tx_kick = EventFd::new(libc::EFD_NONBLOCK).expect("tx kick"); + let error = match spawn_net_io_loop(device, -1, tx_kick.as_raw_fd()) { + Ok(mut worker) => { + worker.stop(); + panic!("invalid tap unexpectedly started a network worker") + } + Err(error) => error, + }; + assert!( + error.to_string().contains("network tap"), + "unexpected startup error: {error}" + ); + } + /// Stand up an io_loop with a Unix socketpair impersonating a tap, an /// EventFd for TX kicks, and a real virtio-net transport. Verify: /// 1. TX path: guest queues a frame, we kick, the loop drains and @@ -513,7 +593,7 @@ mod tests { .unwrap(); mem.write_obj(1u16, GuestAddress(TX_AVAIL + 6)).unwrap(); mem.write_obj(2u16, GuestAddress(TX_AVAIL + 2)).unwrap(); - io_loop.pause(); + io_loop.pause().expect("pause network worker"); let deadline = std::time::Instant::now() + std::time::Duration::from_secs(2); let n = loop { // SAFETY: `recv` remains a valid writable buffer and `host_fd` is diff --git a/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs b/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs index e8a144e..48cd8b4 100644 --- a/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs +++ b/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs @@ -23,6 +23,7 @@ const POLL_TIMEOUT_MS: libc::c_int = 250; /// How long the paused pump sleeps between checks of the pause flag. const PAUSE_POLL: std::time::Duration = std::time::Duration::from_micros(100); +const QUIESCE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5); /// Handle for the vsock pump thread. Dropping it stops + joins the thread. pub struct VsockPump { @@ -54,18 +55,34 @@ impl VsockPump { /// touching guest memory until [`Self::resume`]. Callers must pause every /// vCPU first so the guest cannot publish another descriptor after this /// drain. - pub fn pause(&self) { + pub fn pause(&self) -> io::Result<()> { if self.thread_gone() { - return; + return Err(io::Error::new( + io::ErrorKind::BrokenPipe, + "vsock worker exited before quiescence", + )); } self.pause_req.store(true, Ordering::SeqCst); - let _ = self.wake_evt.write(1); + self.wake_evt.write(1)?; + let deadline = std::time::Instant::now() + QUIESCE_TIMEOUT; while !self.pause_ack.load(Ordering::SeqCst) { if self.thread_gone() { - return; + self.pause_req.store(false, Ordering::SeqCst); + return Err(io::Error::new( + io::ErrorKind::BrokenPipe, + "vsock worker exited during quiescence", + )); + } + if std::time::Instant::now() >= deadline { + self.pause_req.store(false, Ordering::SeqCst); + return Err(io::Error::new( + io::ErrorKind::TimedOut, + "vsock worker quiescence timed out", + )); } std::thread::sleep(PAUSE_POLL); } + Ok(()) } /// Release a pause. Does not wait: the thread re-enters its poll loop on @@ -74,7 +91,6 @@ impl VsockPump { self.pause_req.store(false, Ordering::SeqCst); } - /// A finished thread writes no guest memory, so it counts as paused. fn thread_gone(&self) -> bool { self.handle.as_ref().is_none_or(|h| h.is_finished()) } @@ -90,6 +106,14 @@ impl Drop for VsockPump { /// QUEUE_NOTIFY register (datamatch=1), so guest kicks wake this thread instead /// of trapping into the vCPU thread. pub fn spawn_vsock_pump(device: Arc, tx_kick_fd: RawFd) -> io::Result { + // SAFETY: F_GETFD inspects the descriptor without retaining it. The caller + // owns the descriptor for the lifetime of the returned worker. + if unsafe { libc::fcntl(tx_kick_fd, libc::F_GETFD) } < 0 { + return Err(io::Error::new( + io::Error::last_os_error().kind(), + "vsock queue kick descriptor is invalid", + )); + } let stop = Arc::new(AtomicBool::new(false)); let pause_req = Arc::new(AtomicBool::new(false)); let pause_ack = Arc::new(AtomicBool::new(false)); @@ -99,6 +123,7 @@ pub fn spawn_vsock_pump(device: Arc, tx_kick_fd: RawFd) -> io:: let device_t = device.clone(); let wake_evt = EventFd::new(libc::EFD_NONBLOCK)?; let wake_fd = wake_evt.as_raw_fd(); + let (ready_tx, ready_rx) = std::sync::mpsc::sync_channel(1); let handle = std::thread::Builder::new() .name("virtio-vsock-pump".into()) @@ -110,9 +135,24 @@ pub fn spawn_vsock_pump(device: Arc, tx_kick_fd: RawFd) -> io:: device_t, tx_kick_fd, wake_fd, + ready_tx, ); })?; + match ready_rx.recv() { + Ok(Ok(())) => {} + Ok(Err(error)) => { + let _ = handle.join(); + return Err(error); + } + Err(error) => { + let _ = handle.join(); + return Err(io::Error::other(format!( + "vsock worker exited during startup: {error}" + ))); + } + } + Ok(VsockPump { stop, pause_req, @@ -130,9 +170,15 @@ fn run( device: Arc, tx_kick_fd: RawFd, wake_fd: RawFd, + ready_tx: std::sync::mpsc::SyncSender>, ) { if let Err(e) = vmm_jailer::seccomp::SeccompProfile::vsock().install() { - log::error!("vsock pump: seccomp install failed; refusing guest I/O: {e}"); + let _ = ready_tx.send(Err(io::Error::other(format!( + "install vsock worker sandbox: {e}" + )))); + return; + } + if ready_tx.send(Ok(())).is_err() { return; } @@ -234,3 +280,21 @@ fn drain_eventfd(fd: RawFd, label: &str) { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn startup_rejects_an_invalid_queue_descriptor() { + let device = Arc::new(VirtioVsockMmio::new(7, 3)); + let error = match spawn_vsock_pump(device, -1) { + Ok(mut worker) => { + worker.stop(); + panic!("invalid queue descriptor unexpectedly started a vsock worker") + } + Err(error) => error, + }; + assert!(error.to_string().contains("descriptor is invalid")); + } +} From 0e21e6a995af052bd2ef045f714692a9b798b0ad Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Mon, 31 Aug 2026 20:39:40 +0530 Subject: [PATCH 02/18] Assert guest identity in suspend qualification --- orch/tests/e2e_suspend_resume.sh | 35 ++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/orch/tests/e2e_suspend_resume.sh b/orch/tests/e2e_suspend_resume.sh index a8f090f..851984e 100755 --- a/orch/tests/e2e_suspend_resume.sh +++ b/orch/tests/e2e_suspend_resume.sh @@ -13,6 +13,17 @@ KEY="suspend-e2e-key" PORT="${SUSPEND_E2E_PORT:-}" MIN_RSS_DROP_KIB="${SUSPEND_MIN_RSS_DROP_KIB:-32768}" MAX_RESUME_EXEC_MS="${SUSPEND_RESUME_EXEC_MAX_MS:-5000}" +EXPECTED_KERNEL_PREFIX="${TARIT_EXPECT_KERNEL_RELEASE_PREFIX:-}" +EXPECTED_OS_ID="${TARIT_EXPECT_OS_ID:-}" + +[[ "$EXPECTED_KERNEL_PREFIX" != *[[:space:]]* ]] || { + echo "FAIL: TARIT_EXPECT_KERNEL_RELEASE_PREFIX must not contain whitespace" >&2 + exit 1 +} +[[ "$EXPECTED_OS_ID" =~ ^[a-z0-9._-]*$ ]] || { + echo "FAIL: TARIT_EXPECT_OS_ID contains unsupported characters" >&2 + exit 1 +} for required in curl python3 setsid ps awk; do command -v "$required" >/dev/null || { @@ -165,6 +176,30 @@ printf '%s' "$VM_JSON" | grep -q '"status":"running"' VMM_PID=$(vmm_pid_for_socket "$DIR/sockets/$VM_ID.sock") kill -0 "$VMM_PID" +if [ -n "$EXPECTED_KERNEL_PREFIX" ]; then + KERNEL_IDENTITY=$(exec_json "$VM_ID" 'uname -r') + printf '%s' "$KERNEL_IDENTITY" | python3 -c ' +import json, sys +expected = sys.argv[1] +result = json.load(sys.stdin) +assert result["exit_code"] == 0, result +actual = result.get("stdout", "").strip() +assert actual.startswith(expected), (expected, actual) +' "$EXPECTED_KERNEL_PREFIX" +fi +if [ -n "$EXPECTED_OS_ID" ]; then + # shellcheck disable=SC2016 # $ID expands inside the guest shell. + OS_IDENTITY=$(exec_json "$VM_ID" '. /etc/os-release && printf "%s\n" "$ID"') + printf '%s' "$OS_IDENTITY" | python3 -c ' +import json, sys +expected = sys.argv[1] +result = json.load(sys.stdin) +assert result["exit_code"] == 0, result +actual = result.get("stdout", "").strip() +assert actual == expected, (expected, actual) +' "$EXPECTED_OS_ID" +fi + PREP=$(exec_json "$VM_ID" "mkdir -p /mnt/tarit-rss && mount -t tmpfs -o size=192m tmpfs /mnt/tarit-rss && dd if=/dev/zero of=/mnt/tarit-rss/fill bs=1M count=160 2>/dev/null && echo suspend-state-ok > /mnt/tarit-rss/state") printf '%s' "$PREP" | grep -q '"exit_code":0' RSS_BEFORE=$(rss_kib "$VMM_PID") From 1675e7673fc200cb760551cdcf575c125e37b01d Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Mon, 31 Aug 2026 20:44:28 +0530 Subject: [PATCH 03/18] Fail closed when device workers stop --- .../vmm-devices/src/virtio/net_io_loop.rs | 47 ++++++++++++++----- .../vmm-devices/src/virtio/net_transport.rs | 17 +++++++ vmm/crates/vmm-devices/src/virtio/vsock.rs | 17 +++++++ .../vmm-devices/src/virtio/vsock_io_loop.rs | 40 ++++++++++++---- 4 files changed, 100 insertions(+), 21 deletions(-) diff --git a/vmm/crates/vmm-devices/src/virtio/net_io_loop.rs b/vmm/crates/vmm-devices/src/virtio/net_io_loop.rs index e4fcb97..35f21ae 100644 --- a/vmm/crates/vmm-devices/src/virtio/net_io_loop.rs +++ b/vmm/crates/vmm-devices/src/virtio/net_io_loop.rs @@ -60,6 +60,7 @@ impl NetIoLoop { /// after this drain. pub fn pause(&self) -> io::Result<()> { if self.thread_gone() { + self.fail_if_unexpected_exit(); return Err(io::Error::new( io::ErrorKind::BrokenPipe, "network I/O worker exited before quiescence", @@ -70,6 +71,7 @@ impl NetIoLoop { let deadline = std::time::Instant::now() + QUIESCE_TIMEOUT; while !self.pause_ack.load(Ordering::SeqCst) { if self.thread_gone() { + self.fail_if_unexpected_exit(); self.pause_req.store(false, Ordering::SeqCst); return Err(io::Error::new( io::ErrorKind::BrokenPipe, @@ -78,6 +80,8 @@ impl NetIoLoop { } if std::time::Instant::now() >= deadline { self.pause_req.store(false, Ordering::SeqCst); + self.device + .fail_worker("network I/O worker quiescence timed out"); return Err(io::Error::new( io::ErrorKind::TimedOut, "network I/O worker quiescence timed out", @@ -97,6 +101,13 @@ impl NetIoLoop { fn thread_gone(&self) -> bool { self.handle.as_ref().is_none_or(|h| h.is_finished()) } + + fn fail_if_unexpected_exit(&self) { + if !self.stop.load(Ordering::SeqCst) { + self.device + .fail_worker("network I/O worker is not running during quiescence"); + } + } } impl Drop for NetIoLoop { @@ -123,23 +134,35 @@ pub fn spawn_net_io_loop( let pause_ack_t = pause_ack.clone(); let wake_fd = wake_evt.as_raw_fd(); let device_t = device.clone(); + let health_device = device.clone(); let (ready_tx, ready_rx) = std::sync::mpsc::sync_channel(1); let handle = std::thread::Builder::new() .name("virtio-net-io".into()) .spawn(move || { - run( - WorkerControl { - stop: stop_t, - pause_req: pause_req_t, - pause_ack: pause_ack_t, - ready_tx, - }, - device_t, - tap_fd, - tx_kick_fd, - wake_fd, - ); + let stop_health = Arc::clone(&stop_t); + let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + run( + WorkerControl { + stop: stop_t, + pause_req: pause_req_t, + pause_ack: pause_ack_t, + ready_tx, + }, + device_t, + tap_fd, + tx_kick_fd, + wake_fd, + ); + })); + if !stop_health.load(Ordering::SeqCst) { + let context = if outcome.is_err() { + "network I/O worker panicked" + } else { + "network I/O worker exited unexpectedly" + }; + health_device.fail_worker(context); + } })?; match ready_rx.recv() { diff --git a/vmm/crates/vmm-devices/src/virtio/net_transport.rs b/vmm/crates/vmm-devices/src/virtio/net_transport.rs index 58c4da2..ab9d32b 100644 --- a/vmm/crates/vmm-devices/src/virtio/net_transport.rs +++ b/vmm/crates/vmm-devices/src/virtio/net_transport.rs @@ -162,6 +162,13 @@ pub struct VirtioNetMmio { } impl VirtioNetMmio { + /// Permanently fail the transport after its isolated queue worker exits or + /// cannot acknowledge quiescence. A later snapshot must not serialize a + /// VM whose network worker can still mutate memory or cannot make progress. + pub fn fail_worker(&self, context: &str) { + self.fail_device(context); + } + fn fail_device(&self, context: &str) { log::error!("virtio-net: {context}"); self.status.fetch_or( @@ -823,6 +830,16 @@ mod tests { Arc::new(GuestMemoryMmap::from_ranges(&[(GuestAddress(0), 4 * 1024 * 1024)]).unwrap()) } + #[test] + fn worker_failure_marks_transport_failed_and_unsnapshotable() { + let dev = VirtioNetMmio::new(6, [0x02, 0, 0, 0, 0, 1]); + + dev.fail_worker("test worker exit"); + + assert_ne!(dev.current_status() & status_bits::FAILED, 0); + assert!(Persist::try_save(&dev).is_err()); + } + #[test] fn poisoned_guest_mem_lock_marks_device_failed_without_panicking() { let dev = Arc::new(VirtioNetMmio::new(6, [0xAA; 6])); diff --git a/vmm/crates/vmm-devices/src/virtio/vsock.rs b/vmm/crates/vmm-devices/src/virtio/vsock.rs index b851277..77f9254 100644 --- a/vmm/crates/vmm-devices/src/virtio/vsock.rs +++ b/vmm/crates/vmm-devices/src/virtio/vsock.rs @@ -312,6 +312,13 @@ pub struct VirtioVsockMmio { } impl VirtioVsockMmio { + /// Permanently fail the transport after its isolated pump exits or cannot + /// acknowledge quiescence. Control-channel failure must be explicit and + /// must prevent a later snapshot from publishing incomplete device state. + pub fn fail_worker(&self, context: &str) { + self.fail_device(context); + } + fn fail_device(&self, context: &str) { log::error!("virtio-vsock: {context}"); self.status.fetch_or( @@ -1437,6 +1444,16 @@ mod tests { Arc::new(GuestMemoryMmap::from_ranges(&[(GuestAddress(0), 4 * 1024 * 1024)]).unwrap()) } + #[test] + fn worker_failure_marks_transport_failed_and_unsnapshotable() { + let dev = VirtioVsockMmio::new(7, GUEST_CID); + + dev.fail_worker("test worker exit"); + + assert_ne!(dev.current_status() & status_bits::FAILED, 0); + assert!(Persist::try_save(&dev).is_err()); + } + #[test] fn poisoned_guest_mem_lock_marks_device_failed_without_panicking() { let dev = Arc::new(VirtioVsockMmio::new(7, GUEST_CID)); diff --git a/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs b/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs index 48cd8b4..83224a9 100644 --- a/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs +++ b/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs @@ -57,6 +57,7 @@ impl VsockPump { /// drain. pub fn pause(&self) -> io::Result<()> { if self.thread_gone() { + self.fail_if_unexpected_exit(); return Err(io::Error::new( io::ErrorKind::BrokenPipe, "vsock worker exited before quiescence", @@ -67,6 +68,7 @@ impl VsockPump { let deadline = std::time::Instant::now() + QUIESCE_TIMEOUT; while !self.pause_ack.load(Ordering::SeqCst) { if self.thread_gone() { + self.fail_if_unexpected_exit(); self.pause_req.store(false, Ordering::SeqCst); return Err(io::Error::new( io::ErrorKind::BrokenPipe, @@ -75,6 +77,7 @@ impl VsockPump { } if std::time::Instant::now() >= deadline { self.pause_req.store(false, Ordering::SeqCst); + self.device.fail_worker("vsock worker quiescence timed out"); return Err(io::Error::new( io::ErrorKind::TimedOut, "vsock worker quiescence timed out", @@ -94,6 +97,13 @@ impl VsockPump { fn thread_gone(&self) -> bool { self.handle.as_ref().is_none_or(|h| h.is_finished()) } + + fn fail_if_unexpected_exit(&self) { + if !self.stop.load(Ordering::SeqCst) { + self.device + .fail_worker("vsock worker is not running during quiescence"); + } + } } impl Drop for VsockPump { @@ -121,6 +131,7 @@ pub fn spawn_vsock_pump(device: Arc, tx_kick_fd: RawFd) -> io:: let pause_req_t = pause_req.clone(); let pause_ack_t = pause_ack.clone(); let device_t = device.clone(); + let health_device = device.clone(); let wake_evt = EventFd::new(libc::EFD_NONBLOCK)?; let wake_fd = wake_evt.as_raw_fd(); let (ready_tx, ready_rx) = std::sync::mpsc::sync_channel(1); @@ -128,15 +139,26 @@ pub fn spawn_vsock_pump(device: Arc, tx_kick_fd: RawFd) -> io:: let handle = std::thread::Builder::new() .name("virtio-vsock-pump".into()) .spawn(move || { - run( - stop_t, - pause_req_t, - pause_ack_t, - device_t, - tx_kick_fd, - wake_fd, - ready_tx, - ); + let stop_health = Arc::clone(&stop_t); + let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + run( + stop_t, + pause_req_t, + pause_ack_t, + device_t, + tx_kick_fd, + wake_fd, + ready_tx, + ); + })); + if !stop_health.load(Ordering::SeqCst) { + let context = if outcome.is_err() { + "vsock worker panicked" + } else { + "vsock worker exited unexpectedly" + }; + health_device.fail_worker(context); + } })?; match ready_rx.recv() { From 931efed143bf6453babe757ce206d32537b66eb8 Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Mon, 31 Aug 2026 22:18:22 +0530 Subject: [PATCH 04/18] Exercise network I/O across suspend --- orch/tests/e2e_suspend_resume.sh | 82 +++++++++++++++++++++++++++++++- 1 file changed, 81 insertions(+), 1 deletion(-) diff --git a/orch/tests/e2e_suspend_resume.sh b/orch/tests/e2e_suspend_resume.sh index 851984e..229d576 100755 --- a/orch/tests/e2e_suspend_resume.sh +++ b/orch/tests/e2e_suspend_resume.sh @@ -15,6 +15,7 @@ MIN_RSS_DROP_KIB="${SUSPEND_MIN_RSS_DROP_KIB:-32768}" MAX_RESUME_EXEC_MS="${SUSPEND_RESUME_EXEC_MAX_MS:-5000}" EXPECTED_KERNEL_PREFIX="${TARIT_EXPECT_KERNEL_RELEASE_PREFIX:-}" EXPECTED_OS_ID="${TARIT_EXPECT_OS_ID:-}" +ENABLE_NET="${TARIT_TEST_ENABLE_NET:-0}" [[ "$EXPECTED_KERNEL_PREFIX" != *[[:space:]]* ]] || { echo "FAIL: TARIT_EXPECT_KERNEL_RELEASE_PREFIX must not contain whitespace" >&2 @@ -24,6 +25,10 @@ EXPECTED_OS_ID="${TARIT_EXPECT_OS_ID:-}" echo "FAIL: TARIT_EXPECT_OS_ID contains unsupported characters" >&2 exit 1 } +[[ "$ENABLE_NET" = 0 || "$ENABLE_NET" = 1 ]] || { + echo "FAIL: TARIT_TEST_ENABLE_NET must be 0 or 1" >&2 + exit 1 +} for required in curl python3 setsid ps awk; do command -v "$required" >/dev/null || { @@ -31,6 +36,16 @@ for required in curl python3 setsid ps awk; do exit 1 } done +if [ "$ENABLE_NET" = 1 ]; then + command -v ip >/dev/null || { + echo "FAIL: required command 'ip' is missing" >&2 + exit 1 + } + if ip -o link show | awk -F': ' '$2 ~ /^insta[0-9]+$/ { found=1 } END { exit !found }'; then + echo "FAIL: pre-existing Tarit TAP would make network lifecycle ambiguous" >&2 + exit 1 + fi +fi if [ -z "$PORT" ]; then PORT=$(python3 - <<'PY' import socket @@ -124,6 +139,18 @@ exec_json() { "$BASE_URL/v1/execute" } +assert_guest_security() { + local vm_id=$1 result + result=$(exec_json "$vm_id" \ + 'test ! -e /dev/kvm && ! grep -Eq "(^|[[:space:]])(vmx|svm)([[:space:]]|$)" /proc/cpuinfo && echo virtualization-hidden') + printf '%s' "$result" | python3 -c ' +import json, sys +result = json.load(sys.stdin) +assert result["exit_code"] == 0, result +assert result.get("stdout", "").strip() == "virtualization-hidden", result +' +} + TARIT_API_KEY="$KEY" \ TARIT_LISTEN="127.0.0.1:$PORT" \ TARIT_RPC_ADDR="$BASE_URL" \ @@ -132,9 +159,10 @@ TARIT_VMM_BIN="$VMM" \ TARIT_KERNEL="$KERNEL" \ TARIT_ROOTFS="$ROOTFS" \ TARIT_ROOTFS_READONLY=0 \ -TARIT_ENABLE_NET=0 \ +TARIT_ENABLE_NET="$ENABLE_NET" \ TARIT_SOCKET_DIR="$DIR/sockets" \ TARIT_DB="$DIR/fleet.db" \ +TARIT_NET_STATE="$DIR/net-state.json" \ TARIT_CONFIG="$DIR/none.toml" \ TARIT_WARM_POOL=0 \ TARIT_MAX_VMS=1 \ @@ -175,6 +203,18 @@ VM_ID=$(printf '%s' "$VM_JSON" | json_field id) printf '%s' "$VM_JSON" | grep -q '"status":"running"' VMM_PID=$(vmm_pid_for_socket "$DIR/sockets/$VM_ID.sock") kill -0 "$VMM_PID" +if [ "$ENABLE_NET" = 1 ]; then + NET_TAP="" + for _ in $(seq 1 40); do + NET_TAP=$(ip -o link show | awk -F': ' '$2 ~ /^insta[0-9]+$/ { print $2; exit }') + [ -n "$NET_TAP" ] && break + sleep 0.1 + done + [ -n "$NET_TAP" ] || { + echo "FAIL: network-enabled VM has no TAP" >&2 + exit 1 + } +fi if [ -n "$EXPECTED_KERNEL_PREFIX" ]; then KERNEL_IDENTITY=$(exec_json "$VM_ID" 'uname -r') @@ -199,6 +239,18 @@ actual = result.get("stdout", "").strip() assert actual == expected, (expected, actual) ' "$EXPECTED_OS_ID" fi +assert_guest_security "$VM_ID" +if [ "$ENABLE_NET" = 1 ]; then + NETWORK_IDENTITY=$(exec_json "$VM_ID" \ + 'test -d /sys/class/net/eth0 && grep -Eq "^eth0[[:space:]]+00000000[[:space:]]" /proc/net/route && cat /sys/class/net/eth0/address && echo network-ready') + printf '%s' "$NETWORK_IDENTITY" | python3 -c ' +import json, sys +result = json.load(sys.stdin) +assert result["exit_code"] == 0, result +output = result.get("stdout", "") +assert "network-ready" in output, result +' +fi PREP=$(exec_json "$VM_ID" "mkdir -p /mnt/tarit-rss && mount -t tmpfs -o size=192m tmpfs /mnt/tarit-rss && dd if=/dev/zero of=/mnt/tarit-rss/fill bs=1M count=160 2>/dev/null && echo suspend-state-ok > /mnt/tarit-rss/state") printf '%s' "$PREP" | grep -q '"exit_code":0' @@ -207,6 +259,9 @@ RSS_BEFORE=$(rss_kib "$VMM_PID") echo "== suspend and verify resource contract ==" SUSPENDED=$(api -H 'Content-Type: application/json' -d '{}' "$BASE_URL/v1/vms/$VM_ID/suspend") printf '%s' "$SUSPENDED" | grep -q '"status":"suspended"' +if [ "$ENABLE_NET" = 1 ]; then + ip link show "$NET_TAP" >/dev/null +fi RSS_AFTER=$RSS_BEFORE for _ in $(seq 1 20); do @@ -237,10 +292,25 @@ echo "== resume, first exec, and verify preserved state ==" START_MS=$(monotonic_ms) RESUMED=$(api -H 'Content-Type: application/json' -d '{}' "$BASE_URL/v1/vms/$VM_ID/resume") printf '%s' "$RESUMED" | grep -q '"status":"running"' +if [ "$ENABLE_NET" = 1 ]; then + ip link show "$NET_TAP" >/dev/null +fi FIRST_EXEC=$(exec_json "$VM_ID" 'cat /mnt/tarit-rss/state') END_MS=$(monotonic_ms) printf '%s' "$FIRST_EXEC" | grep -q 'suspend-state-ok' printf '%s' "$FIRST_EXEC" | grep -q '"exit_code":0' +assert_guest_security "$VM_ID" +if [ "$ENABLE_NET" = 1 ]; then + exec_json "$VM_ID" \ + 'test -d /sys/class/net/eth0 && grep -Eq "^eth0[[:space:]]+00000000[[:space:]]" /proc/net/route && cat /sys/class/net/eth0/address && echo network-ready' | \ + python3 -c ' +import json, sys +result = json.load(sys.stdin) +assert result["exit_code"] == 0, result +output = result.get("stdout", "") +assert "network-ready" in output, result +' +fi RESUME_EXEC_MS=$((END_MS - START_MS)) [ "$RESUME_EXEC_MS" -le "$MAX_RESUME_EXEC_MS" ] || { echo "FAIL: resume-to-first-exec ${RESUME_EXEC_MS}ms exceeded ${MAX_RESUME_EXEC_MS}ms" @@ -257,4 +327,14 @@ done exec_json "$VM_ID" 'cat /mnt/tarit-rss/state' | grep -q 'suspend-state-ok' api -X DELETE "$BASE_URL/v1/vms/$VM_ID" >/dev/null +if [ "$ENABLE_NET" = 1 ]; then + for _ in $(seq 1 40); do + ip link show "$NET_TAP" >/dev/null 2>&1 || break + sleep 0.1 + done + if ip link show "$NET_TAP" >/dev/null 2>&1; then + echo "FAIL: TAP leaked after VM deletion: $NET_TAP" >&2 + exit 1 + fi +fi echo "RESULT: SUSPEND_PASS rss_before_kib=$RSS_BEFORE rss_after_kib=$RSS_AFTER rss_drop_kib=$RSS_DROP resume_first_exec_ms=$RESUME_EXEC_MS" From 57b20155690b9c91202c2156dbf74f722507848d Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Mon, 31 Aug 2026 22:25:39 +0530 Subject: [PATCH 05/18] Test storage quiescence timeout recovery --- .../vmm-integration/blk_io_isolation.rs | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/vmm/crates/vmm-integration/blk_io_isolation.rs b/vmm/crates/vmm-integration/blk_io_isolation.rs index f754fbd..53fdf3c 100644 --- a/vmm/crates/vmm-integration/blk_io_isolation.rs +++ b/vmm/crates/vmm-integration/blk_io_isolation.rs @@ -131,3 +131,66 @@ fn delayed_volume_io_isolated_from_vcpu_and_quiesced_for_snapshot() { "owned snapshot was not cleaned up on stop" ); } + +#[test] +fn storage_quiescence_timeout_fails_snapshot_and_resumes_source() { + let mut data = tempfile::NamedTempFile::new().expect("create data volume"); + data.as_file_mut() + .set_len(4 * 1024 * 1024) + .expect("size data volume"); + + let mut config = agent_vm_config(512); + config.volumes.push(VolumeConfig { + path: data.path().to_string_lossy().into_owned(), + read_only: false, + overlay: None, + inherited_fd: None, + }); + + let controller = Arc::new(VmmController::new()); + controller.create_live(config).expect("boot VM"); + assert_eq!( + guest_stdout(&controller, "printf source-ready"), + "source-ready" + ); + + controller + .set_test_block_service_delay(1, Duration::from_millis(6_500)) + .expect("enable blocking volume delay"); + let writer_controller = Arc::clone(&controller); + let writer = std::thread::spawn(move || { + writer_controller.exec( + "dd if=/dev/zero of=/dev/vdb bs=512 count=1 conv=fsync 2>/dev/null; printf delayed-write", + 20_000, + ) + }); + wait_for_delayed_service(); + + let snapshot_started = Instant::now(); + let error = controller + .snapshot(false) + .expect_err("snapshot unexpectedly ignored block-worker timeout"); + let snapshot_elapsed = snapshot_started.elapsed(); + assert!( + error + .to_string() + .contains("block I/O worker quiescence timed out"), + "unexpected snapshot failure: {error}" + ); + assert!( + (Duration::from_secs(4)..Duration::from_secs(6)).contains(&snapshot_elapsed), + "block-worker timeout was not bounded at five seconds: {snapshot_elapsed:?}" + ); + + assert_eq!( + guest_stdout(&controller, "printf source-resumed"), + "source-resumed", + "snapshot failure left the source vCPU paused" + ); + let writer_result = writer.join().expect("join delayed writer"); + if let Ok((code, stdout, stderr, _)) = writer_result { + assert_eq!(code, 0, "delayed writer failed: {stderr}"); + assert_eq!(stdout, "delayed-write"); + } + controller.stop().expect("stop VM after quiescence timeout"); +} From c683d00397a830e5bb5ef942e9dc9242ab1beab7 Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Mon, 31 Aug 2026 22:39:42 +0530 Subject: [PATCH 06/18] Close rapid I/O resume pause race --- vmm/crates/vmm-core/src/controller.rs | 131 +++++++++++++----- vmm/crates/vmm-core/src/live_snapshot.rs | 35 ++++- .../vmm-devices/src/virtio/blk_io_loop.rs | 51 ++++++- .../vmm-devices/src/virtio/net_io_loop.rs | 42 +++++- .../vmm-devices/src/virtio/vsock_io_loop.rs | 53 ++++++- 5 files changed, 263 insertions(+), 49 deletions(-) diff --git a/vmm/crates/vmm-core/src/controller.rs b/vmm/crates/vmm-core/src/controller.rs index b4e7fc0..db398a5 100644 --- a/vmm/crates/vmm-core/src/controller.rs +++ b/vmm/crates/vmm-core/src/controller.rs @@ -581,7 +581,8 @@ impl VmmController { // and RAM. Pause vCPUs first so the guest cannot enqueue new net/vsock work // after an I/O pump has acknowledged its pause. The pumps are then parked // before capture begins. Resume in the inverse order: vCPUs first, then the - // pumps, so a completion interrupt cannot be delivered to a paused LAPIC. + // pumps before vCPUs so a rapid subsequent pause cannot observe a stale + // worker acknowledgement from this snapshot. #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] let paused_here = if state_before == VmState::Running { match pause_running_vcpus(vm) { @@ -725,17 +726,30 @@ impl VmmController { })(); #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] - let resume_result = if paused_here && state_before == VmState::Running { - resume_running_vcpus(vm) + let io_resume_result = if io_paused_here { + resume_running_io(vm) } else { Ok(()) }; #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] - if io_paused_here { - resume_running_io(vm); - } + let resume_result = match io_resume_result { + Ok(()) if paused_here && state_before == VmState::Running => resume_running_vcpus(vm), + Ok(()) => Ok(()), + Err(error) => Err(error), + }; - vm.state = state_before; + #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] + { + vm.state = if resume_result.is_ok() { + state_before + } else { + VmState::Paused + }; + } + #[cfg(not(all(target_arch = "x86_64", target_os = "linux", feature = "boot")))] + { + vm.state = state_before; + } #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] let snapshot_result = match resume_result { Ok(()) => snapshot_result, @@ -1806,9 +1820,17 @@ impl VmmController { ))); } #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] - resume_running_vcpus(vm)?; + resume_running_io(vm)?; #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] - resume_running_io(vm); + if let Err(error) = resume_running_vcpus(vm) { + let rollback = pause_running_io(vm).err(); + return Err(match rollback { + Some(rollback) => VmmError::Device(format!( + "resume vCPUs: {error}; failed to re-quiesce I/O workers: {rollback}" + )), + None => error, + }); + } vm.state = VmState::Running; log::info!("VM resumed"); Ok(()) @@ -2607,42 +2629,68 @@ fn set_running_io_paused(running: &RunningVm, paused: bool) -> Result<()> { if paused { for io_loop in &running.blk_io_loops { if let Err(error) = io_loop.pause() { - resume_running_io_workers(running); - return Err(VmmError::Device(format!( - "quiesce block I/O worker: {error}" - ))); + let rollback = resume_running_io_workers(running).err(); + return Err(VmmError::Device(match rollback { + Some(rollback) => { + format!("quiesce block I/O worker: {error}; rollback failed: {rollback}") + } + None => format!("quiesce block I/O worker: {error}"), + })); } } for io_loop in &running.net_io_loops { if let Err(error) = io_loop.pause() { - resume_running_io_workers(running); - return Err(VmmError::Device(format!( - "quiesce network I/O worker: {error}" - ))); + let rollback = resume_running_io_workers(running).err(); + return Err(VmmError::Device(match rollback { + Some(rollback) => { + format!("quiesce network I/O worker: {error}; rollback failed: {rollback}") + } + None => format!("quiesce network I/O worker: {error}"), + })); } } if let Some(pump) = running.vsock_pump.as_ref() { if let Err(error) = pump.pause() { - resume_running_io_workers(running); - return Err(VmmError::Device(format!("quiesce vsock worker: {error}"))); + let rollback = resume_running_io_workers(running).err(); + return Err(VmmError::Device(match rollback { + Some(rollback) => { + format!("quiesce vsock worker: {error}; rollback failed: {rollback}") + } + None => format!("quiesce vsock worker: {error}"), + })); } } } else { - resume_running_io_workers(running); + resume_running_io_workers(running)?; } Ok(()) } #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] -fn resume_running_io_workers(running: &RunningVm) { +fn resume_running_io_workers(running: &RunningVm) -> Result<()> { + let mut failures = Vec::new(); for io_loop in &running.blk_io_loops { - io_loop.resume(); + if let Err(error) = io_loop.resume() { + failures.push(format!("block I/O worker: {error}")); + } } for io_loop in &running.net_io_loops { - io_loop.resume(); + if let Err(error) = io_loop.resume() { + failures.push(format!("network I/O worker: {error}")); + } } if let Some(pump) = running.vsock_pump.as_ref() { - pump.resume(); + if let Err(error) = pump.resume() { + failures.push(format!("vsock worker: {error}")); + } + } + if failures.is_empty() { + Ok(()) + } else { + Err(VmmError::Device(format!( + "resume I/O workers: {}", + failures.join("; ") + ))) } } @@ -2656,10 +2704,11 @@ fn pause_running_io(vm: &VmInstance) -> Result { } #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] -fn resume_running_io(vm: &VmInstance) { +fn resume_running_io(vm: &VmInstance) -> Result<()> { if let Some(running) = vm.running.as_ref() { - let _ = set_running_io_paused(running, false); + set_running_io_paused(running, false)?; } + Ok(()) } #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] @@ -2883,19 +2932,31 @@ fn suspend_vm_in_place(vm: &mut VmInstance) -> Result<()> { Ok(()) })(); - if result.is_err() { - vm.state = state_before; - let resume_result = if paused_here && state_before == VmState::Running { - resume_running_vcpus(vm) + if let Err(error) = result { + let io_resume_result = if io_paused_here { + resume_running_io(vm) } else { Ok(()) }; - if io_paused_here { - resume_running_io(vm); - } - resume_result?; + let resume_result = match io_resume_result { + Ok(()) if paused_here && state_before == VmState::Running => resume_running_vcpus(vm), + Ok(()) => Ok(()), + Err(error) => Err(error), + }; + return match resume_result { + Ok(()) => { + vm.state = state_before; + Err(error) + } + Err(resume) => { + vm.state = VmState::Paused; + Err(VmmError::Snapshot(format!( + "{error}; failed to restore running state after suspend failure: {resume}" + ))) + } + }; } - result + Ok(()) } pub(crate) fn private_runtime_dir() -> Result { diff --git a/vmm/crates/vmm-core/src/live_snapshot.rs b/vmm/crates/vmm-core/src/live_snapshot.rs index 55e4d52..1bfbcd7 100644 --- a/vmm/crates/vmm-core/src/live_snapshot.rs +++ b/vmm/crates/vmm-core/src/live_snapshot.rs @@ -160,11 +160,12 @@ impl<'a> IoQuiesceGuard<'a> { }) } - fn disengage(mut self) { + fn disengage(mut self) -> Result<()> { if self.armed { - let _ = (self.quiesce)(false); self.armed = false; + (self.quiesce)(false)?; } + Ok(()) } } @@ -627,12 +628,13 @@ where let state_blob = capture_state()?; inject_live_snapshot_failure("state_capture")?; - // Resume requests are issued to every vCPU before waiting for any one of - // them. The guard then observes every thread leave its park, so downtime - // covers the complete all-vCPU blackout. + // Release I/O workers first and wait for their pause acknowledgements to + // clear. This closes the rapid resume/pause race before any vCPU can + // publish new descriptors. Then observe every vCPU leave its park, so + // downtime covers the complete all-vCPU blackout. + io_guard.disengage()?; final_pause_guard.resume()?; let downtime = final_stop_start.elapsed(); - io_guard.disengage(); // Final residual pages entered the page cache during blackout, but durable // writeback is not part of guest downtime. memory_file @@ -821,4 +823,25 @@ mod tests { assert_eq!(&*calls.borrow(), &[true, false]); } + + #[test] + fn io_quiesce_release_failure_is_reported_once() { + let calls = std::cell::RefCell::new(Vec::new()); + let quiesce = |paused| { + calls.borrow_mut().push(paused); + if paused { + Ok(()) + } else { + Err(VmmError::Device("resume failed".into())) + } + }; + + let error = IoQuiesceGuard::engage(&quiesce) + .expect("engage I/O quiescence") + .disengage() + .expect_err("release failure was ignored"); + + assert!(error.to_string().contains("resume failed")); + assert_eq!(&*calls.borrow(), &[true, false]); + } } diff --git a/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs b/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs index 832c6ca..b8c0d83 100644 --- a/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs +++ b/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs @@ -76,8 +76,39 @@ impl BlkIoLoop { Ok(()) } - pub fn resume(&self) { + /// Release a pause and wait until the worker has left its parked state. + /// This acknowledgement prevents a rapid resume/pause cycle from + /// mistaking the previous pause acknowledgement for the new request. + pub fn resume(&self) -> io::Result<()> { + if self.thread_gone() { + self.fail_if_unexpected_exit(); + return Err(io::Error::new( + io::ErrorKind::BrokenPipe, + "block I/O worker exited before resume", + )); + } self.pause_req.store(false, Ordering::SeqCst); + self.wake_evt.write(1)?; + let deadline = std::time::Instant::now() + QUIESCE_TIMEOUT; + while self.pause_ack.load(Ordering::SeqCst) { + if self.thread_gone() { + self.fail_if_unexpected_exit(); + return Err(io::Error::new( + io::ErrorKind::BrokenPipe, + "block I/O worker exited during resume", + )); + } + if std::time::Instant::now() >= deadline { + self.device + .fail_worker("block I/O worker resume acknowledgement timed out"); + return Err(io::Error::new( + io::ErrorKind::TimedOut, + "block I/O worker resume acknowledgement timed out", + )); + } + std::thread::sleep(PAUSE_POLL); + } + Ok(()) } fn thread_gone(&self) -> bool { @@ -275,4 +306,22 @@ mod tests { }; assert!(error.to_string().contains("descriptor is invalid")); } + + #[test] + fn resume_waits_for_the_pause_acknowledgement_to_clear() { + let device = Arc::new(VirtioBlkMmio::new_stub(5, 2)); + let kick = EventFd::new(libc::EFD_NONBLOCK).expect("queue kick"); + let mut worker = spawn_blk_io_loop(device, kick.as_raw_fd()).expect("start block worker"); + + worker.pause().expect("pause block worker"); + assert!(worker.pause_ack.load(Ordering::SeqCst)); + worker.resume().expect("resume block worker"); + assert!(!worker.pause_ack.load(Ordering::SeqCst)); + + worker.pause().expect("pause block worker again"); + assert!(worker.pause_ack.load(Ordering::SeqCst)); + worker.resume().expect("resume block worker again"); + assert!(!worker.pause_ack.load(Ordering::SeqCst)); + worker.stop(); + } } diff --git a/vmm/crates/vmm-devices/src/virtio/net_io_loop.rs b/vmm/crates/vmm-devices/src/virtio/net_io_loop.rs index 35f21ae..e9430b9 100644 --- a/vmm/crates/vmm-devices/src/virtio/net_io_loop.rs +++ b/vmm/crates/vmm-devices/src/virtio/net_io_loop.rs @@ -92,10 +92,39 @@ impl NetIoLoop { Ok(()) } - /// Release a pause. Does not wait: the thread re-enters its poll loop on - /// its own within [`PAUSE_POLL`]. - pub fn resume(&self) { + /// Release a pause and wait until the worker has left its parked state. + /// This acknowledgement prevents a rapid resume/pause cycle from + /// mistaking the previous pause acknowledgement for the new request. + pub fn resume(&self) -> io::Result<()> { + if self.thread_gone() { + self.fail_if_unexpected_exit(); + return Err(io::Error::new( + io::ErrorKind::BrokenPipe, + "network I/O worker exited before resume", + )); + } self.pause_req.store(false, Ordering::SeqCst); + self.wake_evt.write(1)?; + let deadline = std::time::Instant::now() + QUIESCE_TIMEOUT; + while self.pause_ack.load(Ordering::SeqCst) { + if self.thread_gone() { + self.fail_if_unexpected_exit(); + return Err(io::Error::new( + io::ErrorKind::BrokenPipe, + "network I/O worker exited during resume", + )); + } + if std::time::Instant::now() >= deadline { + self.device + .fail_worker("network I/O worker resume acknowledgement timed out"); + return Err(io::Error::new( + io::ErrorKind::TimedOut, + "network I/O worker resume acknowledgement timed out", + )); + } + std::thread::sleep(PAUSE_POLL); + } + Ok(()) } fn thread_gone(&self) -> bool { @@ -633,7 +662,12 @@ mod tests { }; assert_eq!(&recv[..n], pause_payload); assert_eq!(device.tx_packets.load(Ordering::Relaxed), 2); - io_loop.resume(); + io_loop.resume().expect("resume network worker"); + assert!(!io_loop.pause_ack.load(Ordering::SeqCst)); + io_loop.pause().expect("pause network worker again"); + assert!(io_loop.pause_ack.load(Ordering::SeqCst)); + io_loop.resume().expect("resume network worker again"); + assert!(!io_loop.pause_ack.load(Ordering::SeqCst)); // --- RX: write a frame on the host side; the loop should inject. --- let inbound = b"INBOUND-FRAME"; diff --git a/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs b/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs index 83224a9..fcfa5a9 100644 --- a/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs +++ b/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs @@ -88,10 +88,39 @@ impl VsockPump { Ok(()) } - /// Release a pause. Does not wait: the thread re-enters its poll loop on - /// its own within [`PAUSE_POLL`]. - pub fn resume(&self) { + /// Release a pause and wait until the worker has left its parked state. + /// This acknowledgement prevents a rapid resume/pause cycle from + /// mistaking the previous pause acknowledgement for the new request. + pub fn resume(&self) -> io::Result<()> { + if self.thread_gone() { + self.fail_if_unexpected_exit(); + return Err(io::Error::new( + io::ErrorKind::BrokenPipe, + "vsock worker exited before resume", + )); + } self.pause_req.store(false, Ordering::SeqCst); + self.wake_evt.write(1)?; + let deadline = std::time::Instant::now() + QUIESCE_TIMEOUT; + while self.pause_ack.load(Ordering::SeqCst) { + if self.thread_gone() { + self.fail_if_unexpected_exit(); + return Err(io::Error::new( + io::ErrorKind::BrokenPipe, + "vsock worker exited during resume", + )); + } + if std::time::Instant::now() >= deadline { + self.device + .fail_worker("vsock worker resume acknowledgement timed out"); + return Err(io::Error::new( + io::ErrorKind::TimedOut, + "vsock worker resume acknowledgement timed out", + )); + } + std::thread::sleep(PAUSE_POLL); + } + Ok(()) } fn thread_gone(&self) -> bool { @@ -319,4 +348,22 @@ mod tests { }; assert!(error.to_string().contains("descriptor is invalid")); } + + #[test] + fn resume_waits_for_the_pause_acknowledgement_to_clear() { + let device = Arc::new(VirtioVsockMmio::new(7, 3)); + let kick = EventFd::new(libc::EFD_NONBLOCK).expect("queue kick"); + let mut worker = spawn_vsock_pump(device, kick.as_raw_fd()).expect("start vsock worker"); + + worker.pause().expect("pause vsock worker"); + assert!(worker.pause_ack.load(Ordering::SeqCst)); + worker.resume().expect("resume vsock worker"); + assert!(!worker.pause_ack.load(Ordering::SeqCst)); + + worker.pause().expect("pause vsock worker again"); + assert!(worker.pause_ack.load(Ordering::SeqCst)); + worker.resume().expect("resume vsock worker again"); + assert!(!worker.pause_ack.load(Ordering::SeqCst)); + worker.stop(); + } } From 0dc57cf842ae77ef4141dbd1358411556fa6517b Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Mon, 31 Aug 2026 22:43:35 +0530 Subject: [PATCH 07/18] Exercise rapid suspend resume cycles --- orch/tests/e2e_suspend_resume.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/orch/tests/e2e_suspend_resume.sh b/orch/tests/e2e_suspend_resume.sh index 229d576..a2b1968 100755 --- a/orch/tests/e2e_suspend_resume.sh +++ b/orch/tests/e2e_suspend_resume.sh @@ -326,6 +326,18 @@ for _ in 1 2; do done exec_json "$VM_ID" 'cat /mnt/tarit-rss/state' | grep -q 'suspend-state-ok' +echo "== rapid suspend/resume transitions preserve worker handshakes ==" +for cycle in $(seq 1 20); do + api -H 'Content-Type: application/json' -d '{}' "$BASE_URL/v1/vms/$VM_ID/suspend" | grep -q '"status":"suspended"' + if [ "$ENABLE_NET" = 1 ]; then + ip link show "$NET_TAP" >/dev/null + fi + api -H 'Content-Type: application/json' -d '{}' "$BASE_URL/v1/vms/$VM_ID/resume" | grep -q '"status":"running"' + exec_json "$VM_ID" "printf rapid-cycle-$cycle" | grep -q "rapid-cycle-$cycle" +done +exec_json "$VM_ID" 'cat /mnt/tarit-rss/state' | grep -q 'suspend-state-ok' +assert_guest_security "$VM_ID" + api -X DELETE "$BASE_URL/v1/vms/$VM_ID" >/dev/null if [ "$ENABLE_NET" = 1 ]; then for _ in $(seq 1 40); do From de0af1da437a3cc9fd8b366e2e0524ba16ea4851 Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Mon, 31 Aug 2026 22:55:31 +0530 Subject: [PATCH 08/18] Update suspend and snapshot API documentation --- CHANGELOG.md | 6 ++ PRODUCTION_READINESS.md | 5 +- README.md | 4 +- orch/crates/taritd/src/api.rs | 5 +- orch/docs/API.md | 80 ++++++++++++++----- orch/openapi.yaml | 36 +++++---- .../tarit_sdk/api/default/hibernate_vm.py | 8 +- sdk/python/tarit_sdk/api/default/pause_vm.py | 8 +- .../tarit_sdk/api/default/restore_vm.py | 16 +++- sdk/python/tarit_sdk/api/default/resume_vm.py | 36 +++++++-- .../tarit_sdk/api/default/snapshot_vm.py | 32 ++++---- .../tarit_sdk/api/default/suspend_vm.py | 32 +++++--- sdk/typescript/src/generated/schema.ts | 56 +++++++++---- 13 files changed, 220 insertions(+), 104 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7024a6..3518d6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,12 @@ versions may contain breaking changes. ### Fixed +- Pause, suspend, and snapshot now require block, network, and vsock workers to + acknowledge quiescence before capturing state or releasing guest RAM. Resume + waits for every worker to leave its parked state before restarting vCPUs, so + rapid resume/suspend cycles cannot reuse a stale acknowledgement. Worker + startup, unexpected exit, and five-second quiescence failures are surfaced + instead of publishing a partially functional VM or snapshot. - OCI image conversion now verifies manifest, config, and layer descriptors and streams layers through disk-size-derived expansion, entry-count, file-size, path-length, and layer-count limits before unpack. Rejected images publish no diff --git a/PRODUCTION_READINESS.md b/PRODUCTION_READINESS.md index bd9d106..b97cfdd 100644 --- a/PRODUCTION_READINESS.md +++ b/PRODUCTION_READINESS.md @@ -39,8 +39,9 @@ its scope. and PTY idle time are bounded. Invalid credentials pass through the outer admission limit. - Suspend is distinct from pause: it retains ownership and scheduler quota, - releases resident guest memory, and requires successful rehydration before - resume returns. + parks vCPUs plus block, network, and vsock workers at one boundary, releases + resident guest memory, and requires every worker to leave its parked state + before vCPUs restart and resume returns. - Hibernation releases the VMM and scheduler allocation. HTTP, PTY, SSH, and share ingress activate a hibernated VM through a single-flight restore gate; failed activation leaves a retryable hibernated record instead of a second diff --git a/README.md b/README.md index 978a4d8..dfba88b 100644 --- a/README.md +++ b/README.md @@ -111,7 +111,9 @@ guest RAM; resume brings it back. Restore boots a fresh VMM from a snapshot. Snapshots include the architectural and host-advertised KVM paravirtual MSR state used by the guest. Restore validates that state against the destination before changing a vCPU and rejects incompatible hosts without a partial -restore. +restore. Suspend parks the block, network, and vsock workers after stopping the +vCPUs; resume waits for those workers to leave the parked state before vCPUs +run again. ```sh sudo vmm --socket /tmp/vm.sock snapshot # full snapshot, prints the .snap path diff --git a/orch/crates/taritd/src/api.rs b/orch/crates/taritd/src/api.rs index 2d41b7b..f44b173 100644 --- a/orch/crates/taritd/src/api.rs +++ b/orch/crates/taritd/src/api.rs @@ -1889,9 +1889,8 @@ fn is_network_pool_exhausted(message: &str) -> bool { message.contains("network address pool exhausted") } -/// Restore a snapshot into a running VM. Routes to the node that holds the -/// snapshot file (`host_id`, as returned by the snapshot call) so no cross-node -/// file transfer is needed; `None`/self restores locally. +/// Restore a snapshot from its opaque public handle. The control plane resolves +/// the private host and artifact locator, then routes the restore to that host. async fn restore_vm( State(state): State, Extension(identity): Extension, diff --git a/orch/docs/API.md b/orch/docs/API.md index d0382d5..887cbde 100644 --- a/orch/docs/API.md +++ b/orch/docs/API.md @@ -40,7 +40,7 @@ admin keys can call admin-only routes such as `/v1/cluster`. ```json { "id": "uuid", - "status": "creating|running|paused|suspended|error", + "status": "creating|running|paused|suspended|hibernated|stopped|error", "revision": 3, "startup_path": "cold|warm|snapshot_restore", "memory_mib": 256, @@ -408,26 +408,60 @@ Note: the local SQLite VM row is not deleted. On the owner, a later local `GET` ### `POST /v1/vms/{id}/pause` -Resolve owner and pause the VM. The public handler does not require a JSON body. +Resolve the owner, stop every vCPU, drain and park the block, network, and vsock +workers, then publish the paused state. The public handler does not require a +JSON body. Response `200`: updated `VmRecord` with `status: "paused"`. -Status codes: `200`, `401`, `403`, `404`, `409` (VM is stopped), `500`. +Status codes: `200`, `401`, `403`, `404`, `409` (invalid lifecycle +transition), `500`. + +### `POST /v1/vms/{id}/suspend` + +Resolve the owner and capture a coherent in-process suspend image after every +vCPU and guest-memory-writing device worker acknowledges quiescence. Resident +guest RAM is released, but VM ownership, the VMM process, scheduler capacity, +and tenant quota remain reserved. + +Response `200`: updated `VmRecord` with `status: "suspended"`. + +Status codes: `200`, `401`, `403`, `404`, `409` (invalid lifecycle +transition), `500`. + +### `POST /v1/vms/{id}/hibernate` + +Capture and authenticate a live RAM, device, and private-disk artifact, satisfy +the configured replication policy, stop the resident VMM, and release CPU, +memory, cgroup, network, and scheduler capacity. The logical VM and its tenant +ownership remain durable for later activation. + +Response `200`: updated `VmRecord` with `status: "hibernated"`. + +Status codes: `200`, `401`, `403`, `404`, `409` (VM is not running), `500`, +`503` (durable artifact or peer lifecycle requirements are unavailable). ### `POST /v1/vms/{id}/resume` -Resolve owner and resume a paused VM. The public handler does not require a JSON body. +Resolve the owner and resume a paused or suspended VM. A hibernated VM is +activated through the fenced single-flight restore path, including normal +placement, artifact verification, network repair, and policy restoration. +Device workers must leave their parked state before vCPUs restart, and the +operation returns only after guest readiness succeeds. The public handler does +not require a JSON body. Response `200`: updated `VmRecord` with `status: "running"`. -Status codes: `200`, `401`, `403`, `404`, `409` (VM is stopped), `500`. +Status codes: `200`, `401`, `403`, `404`, `409` (invalid lifecycle state), +`429` (no placement capacity), `500`, `503` (owner or restore prerequisites are +unavailable). ### `POST /v1/vms/{id}/snapshot` -Resolve owner and ask the VMM to write a snapshot. Snapshot files are node-local. -Only full snapshots are accepted. `diff: true` returns `422` before contacting -the VMM because incremental snapshot headers contain parent paths; durable -parent-chain relocation is not implemented yet. +Resolve the owner and publish an authenticated snapshot behind an opaque UUID. +Host paths, physical host identity, and artifact locators remain private. Only +full snapshots are accepted. `diff: true` returns `422` before contacting the +VMM because durable parent-chain relocation is not implemented yet. Request: @@ -441,25 +475,25 @@ Response `200`: ```json { - "path": "/path/on/owner/snapshot", - "host_id": "node-a" + "snapshot_id": "uuid" } ``` -Always preserve `host_id`; pass it to `POST /v1/restore` so the restore routes to the node that has the file. - -Status codes: `200`, `401`, `403`, `404`, `409` (VM is stopped), `500`. +Status codes: `200`, `401`, `403`, `404`, `409` (the lifecycle state does not +support snapshots), `422` (`diff` is true), `500`. ### `POST /v1/restore` -Restore a VM from a snapshot file. If `host_id` is present and is not the receiving node, the request is routed to that host. No snapshot bytes are copied between nodes. +Restore a VM from an opaque tenant-owned snapshot handle. The control plane +resolves its private locator, verifies the authenticated artifact, and routes +the restore to the node that holds it. Clients cannot provide a host, path, or +storage locator. Request: ```json { - "snapshot_path": "/path/on/snapshot-owner/snapshot", - "host_id": "node-a", + "snapshot_id": "uuid", "id": "optional new vm uuid" } ``` @@ -470,12 +504,14 @@ Status codes: | Status | Meaning | | --- | --- | -| `201` | VM restored on the selected node. | +| `201` | VM restored and ready. | | `401` | Missing or wrong `X-API-Key`. | -| `403` | Tenant VM quota reached. | -| `404` | `host_id` not found in the fleet. | -| `429` | Selected node is at local capacity; includes `Retry-After` in seconds. Restore does not exhaustively try other nodes because the snapshot file is node-local. | -| `500` | VMM restore, peer, or fleet failure. | +| `403` | Tenant VM quota is reached. | +| `404` | Snapshot handle not found or belongs to another tenant. | +| `409` | Requested VM id already exists. | +| `429` | The snapshot-owning node has no capacity; includes `Retry-After` in seconds. | +| `500` | Internal restore failure. | +| `503` | The snapshot-owning node is unhealthy, stale, or unavailable. | ### `POST /v1/execute` diff --git a/orch/openapi.yaml b/orch/openapi.yaml index 1a28af6..cb382c2 100644 --- a/orch/openapi.yaml +++ b/orch/openapi.yaml @@ -1135,7 +1135,7 @@ paths: /v1/vms/{id}/pause: post: operationId: pauseVm - summary: Pause sandbox + summary: Pause a VM description: User keys can pause only their tenant's VMs; admin keys can pause any VM. parameters: - name: id @@ -1158,12 +1158,12 @@ paths: "404": description: VM not found "409": - description: VM is stopped + description: Invalid lifecycle transition /v1/vms/{id}/resume: post: operationId: resumeVm - summary: Resume sandbox - description: User keys can resume only their tenant's VMs; admin keys can resume any VM. + summary: Resume a VM + description: Resumes a paused or suspended VM after every device worker leaves its parked state and before vCPUs restart. A hibernated VM activates through the fenced single-flight placement, artifact-verification, network-repair, and readiness path. User keys can resume only their tenant's VMs; admin keys can resume any VM. parameters: - name: id in: path @@ -1185,12 +1185,16 @@ paths: "404": description: VM not found "409": - description: VM is stopped + description: Invalid lifecycle transition + "429": + description: No placement capacity is available for hibernated activation + "503": + description: The owning host or activation prerequisites are unavailable /v1/vms/{id}/suspend: post: operationId: suspendVm - summary: Suspend sandbox - description: Drops resident guest memory while retaining VM ownership, quota, and scheduler reservations. Resume the VM before exec, snapshot, PTY, SSH, or pause operations. + summary: Suspend a VM in place + description: Stops every vCPU, drains and parks block, network, and vsock workers at the same boundary, captures the in-process suspend image, and drops resident guest memory while retaining the VMM, VM ownership, quota, and scheduler reservations. Resume the VM before exec, snapshot, PTY, SSH, or pause operations. parameters: - name: id in: path @@ -1212,11 +1216,11 @@ paths: "404": description: VM not found "409": - description: VM is not running + description: Invalid lifecycle transition /v1/vms/{id}/hibernate: post: operationId: hibernateVm - summary: Hibernate sandbox and release host capacity + summary: Hibernate a VM and release host capacity description: Atomically captures an authenticated live RAM/device/disk snapshot, stops the resident VMM, releases CPU, memory, cgroup, network, and scheduler capacity, and retains a tenant-owned logical VM record for secure resume. parameters: - name: id @@ -1245,8 +1249,8 @@ paths: /v1/vms/{id}/snapshot: post: operationId: snapshotVm - summary: Snapshot sandbox - description: User keys can snapshot only their tenant's VMs; admin keys can snapshot any VM. A running VM uses the bounded live pre-copy path; RAM, device state, and the private disk upper are captured at one atomic final-stop boundary. The response is an opaque handle; paths and physical host identity remain private. + summary: Snapshot a VM + description: User keys can snapshot only their tenant's VMs; admin keys can snapshot any VM. A running VM uses the bounded live pre-copy path; a paused VM captures from its existing stop boundary. RAM, device state, and the private disk upper are captured at one atomic boundary. The response is an opaque handle; paths and physical host identity remain private. parameters: - name: id in: path @@ -1286,7 +1290,7 @@ paths: "422": description: Incremental snapshots are disabled; retry with `diff=false`. "409": - description: VM is stopped + description: VM lifecycle state does not support snapshots /v1/vms/{id}/fork: post: operationId: forkVm @@ -1327,7 +1331,7 @@ paths: /v1/restore: post: operationId: restoreVm - summary: Restore sandbox from a snapshot + summary: Restore a VM from a snapshot description: Restores a VM from an opaque snapshot handle. The control plane resolves its private host and storage locator; tenant VM quotas apply. requestBody: required: true @@ -1348,14 +1352,18 @@ paths: description: Tenant VM quota reached "404": description: Snapshot handle or owning host not found + "409": + description: Requested VM id already exists "429": - description: Selected node is at capacity + description: Snapshot-owning node is at capacity headers: Retry-After: description: Seconds the client should wait before retrying schema: type: integer minimum: 1 + "503": + description: Snapshot-owning node is unhealthy, stale, or unavailable /v1/execute: post: operationId: execute diff --git a/sdk/python/tarit_sdk/api/default/hibernate_vm.py b/sdk/python/tarit_sdk/api/default/hibernate_vm.py index 2529ef7..627fd90 100644 --- a/sdk/python/tarit_sdk/api/default/hibernate_vm.py +++ b/sdk/python/tarit_sdk/api/default/hibernate_vm.py @@ -71,7 +71,7 @@ def sync_detailed( *, client: AuthenticatedClient | Client, ) -> Response[Any | VmRecord]: - """Hibernate sandbox and release host capacity + """Hibernate a VM and release host capacity Atomically captures an authenticated live RAM/device/disk snapshot, stops the resident VMM, releases CPU, memory, cgroup, network, and scheduler capacity, and retains a tenant-owned logical VM record @@ -104,7 +104,7 @@ def sync( *, client: AuthenticatedClient | Client, ) -> Any | VmRecord | None: - """Hibernate sandbox and release host capacity + """Hibernate a VM and release host capacity Atomically captures an authenticated live RAM/device/disk snapshot, stops the resident VMM, releases CPU, memory, cgroup, network, and scheduler capacity, and retains a tenant-owned logical VM record @@ -132,7 +132,7 @@ async def asyncio_detailed( *, client: AuthenticatedClient | Client, ) -> Response[Any | VmRecord]: - """Hibernate sandbox and release host capacity + """Hibernate a VM and release host capacity Atomically captures an authenticated live RAM/device/disk snapshot, stops the resident VMM, releases CPU, memory, cgroup, network, and scheduler capacity, and retains a tenant-owned logical VM record @@ -163,7 +163,7 @@ async def asyncio( *, client: AuthenticatedClient | Client, ) -> Any | VmRecord | None: - """Hibernate sandbox and release host capacity + """Hibernate a VM and release host capacity Atomically captures an authenticated live RAM/device/disk snapshot, stops the resident VMM, releases CPU, memory, cgroup, network, and scheduler capacity, and retains a tenant-owned logical VM record diff --git a/sdk/python/tarit_sdk/api/default/pause_vm.py b/sdk/python/tarit_sdk/api/default/pause_vm.py index 34dc887..93f71fe 100644 --- a/sdk/python/tarit_sdk/api/default/pause_vm.py +++ b/sdk/python/tarit_sdk/api/default/pause_vm.py @@ -67,7 +67,7 @@ def sync_detailed( *, client: AuthenticatedClient | Client, ) -> Response[Any | VmRecord]: - """Pause sandbox + """Pause a VM User keys can pause only their tenant's VMs; admin keys can pause any VM. @@ -98,7 +98,7 @@ def sync( *, client: AuthenticatedClient | Client, ) -> Any | VmRecord | None: - """Pause sandbox + """Pause a VM User keys can pause only their tenant's VMs; admin keys can pause any VM. @@ -124,7 +124,7 @@ async def asyncio_detailed( *, client: AuthenticatedClient | Client, ) -> Response[Any | VmRecord]: - """Pause sandbox + """Pause a VM User keys can pause only their tenant's VMs; admin keys can pause any VM. @@ -153,7 +153,7 @@ async def asyncio( *, client: AuthenticatedClient | Client, ) -> Any | VmRecord | None: - """Pause sandbox + """Pause a VM User keys can pause only their tenant's VMs; admin keys can pause any VM. diff --git a/sdk/python/tarit_sdk/api/default/restore_vm.py b/sdk/python/tarit_sdk/api/default/restore_vm.py index 99f47a0..c545e23 100644 --- a/sdk/python/tarit_sdk/api/default/restore_vm.py +++ b/sdk/python/tarit_sdk/api/default/restore_vm.py @@ -47,10 +47,18 @@ def _parse_response(*, client: AuthenticatedClient | Client, response: httpx.Res response_404 = cast(Any, None) return response_404 + if response.status_code == 409: + response_409 = cast(Any, None) + return response_409 + if response.status_code == 429: response_429 = cast(Any, None) return response_429 + if response.status_code == 503: + response_503 = cast(Any, None) + return response_503 + if client.raise_on_unexpected_status: raise errors.UnexpectedStatus(response.status_code, response.content) else: @@ -71,7 +79,7 @@ def sync_detailed( client: AuthenticatedClient | Client, body: RestoreRequest, ) -> Response[Any | VmRecord]: - """Restore sandbox from a snapshot + """Restore a VM from a snapshot Restores a VM from an opaque snapshot handle. The control plane resolves its private host and storage locator; tenant VM quotas apply. @@ -103,7 +111,7 @@ def sync( client: AuthenticatedClient | Client, body: RestoreRequest, ) -> Any | VmRecord | None: - """Restore sandbox from a snapshot + """Restore a VM from a snapshot Restores a VM from an opaque snapshot handle. The control plane resolves its private host and storage locator; tenant VM quotas apply. @@ -130,7 +138,7 @@ async def asyncio_detailed( client: AuthenticatedClient | Client, body: RestoreRequest, ) -> Response[Any | VmRecord]: - """Restore sandbox from a snapshot + """Restore a VM from a snapshot Restores a VM from an opaque snapshot handle. The control plane resolves its private host and storage locator; tenant VM quotas apply. @@ -160,7 +168,7 @@ async def asyncio( client: AuthenticatedClient | Client, body: RestoreRequest, ) -> Any | VmRecord | None: - """Restore sandbox from a snapshot + """Restore a VM from a snapshot Restores a VM from an opaque snapshot handle. The control plane resolves its private host and storage locator; tenant VM quotas apply. diff --git a/sdk/python/tarit_sdk/api/default/resume_vm.py b/sdk/python/tarit_sdk/api/default/resume_vm.py index e6a0d94..c44854a 100644 --- a/sdk/python/tarit_sdk/api/default/resume_vm.py +++ b/sdk/python/tarit_sdk/api/default/resume_vm.py @@ -47,6 +47,14 @@ def _parse_response(*, client: AuthenticatedClient | Client, response: httpx.Res response_409 = cast(Any, None) return response_409 + if response.status_code == 429: + response_429 = cast(Any, None) + return response_429 + + if response.status_code == 503: + response_503 = cast(Any, None) + return response_503 + if client.raise_on_unexpected_status: raise errors.UnexpectedStatus(response.status_code, response.content) else: @@ -67,9 +75,12 @@ def sync_detailed( *, client: AuthenticatedClient | Client, ) -> Response[Any | VmRecord]: - """Resume sandbox + """Resume a VM - User keys can resume only their tenant's VMs; admin keys can resume any VM. + Resumes a paused or suspended VM after every device worker leaves its parked state and before vCPUs + restart. A hibernated VM activates through the fenced single-flight placement, artifact- + verification, network-repair, and readiness path. User keys can resume only their tenant's VMs; + admin keys can resume any VM. Args: id (UUID): @@ -98,9 +109,12 @@ def sync( *, client: AuthenticatedClient | Client, ) -> Any | VmRecord | None: - """Resume sandbox + """Resume a VM - User keys can resume only their tenant's VMs; admin keys can resume any VM. + Resumes a paused or suspended VM after every device worker leaves its parked state and before vCPUs + restart. A hibernated VM activates through the fenced single-flight placement, artifact- + verification, network-repair, and readiness path. User keys can resume only their tenant's VMs; + admin keys can resume any VM. Args: id (UUID): @@ -124,9 +138,12 @@ async def asyncio_detailed( *, client: AuthenticatedClient | Client, ) -> Response[Any | VmRecord]: - """Resume sandbox + """Resume a VM - User keys can resume only their tenant's VMs; admin keys can resume any VM. + Resumes a paused or suspended VM after every device worker leaves its parked state and before vCPUs + restart. A hibernated VM activates through the fenced single-flight placement, artifact- + verification, network-repair, and readiness path. User keys can resume only their tenant's VMs; + admin keys can resume any VM. Args: id (UUID): @@ -153,9 +170,12 @@ async def asyncio( *, client: AuthenticatedClient | Client, ) -> Any | VmRecord | None: - """Resume sandbox + """Resume a VM - User keys can resume only their tenant's VMs; admin keys can resume any VM. + Resumes a paused or suspended VM after every device worker leaves its parked state and before vCPUs + restart. A hibernated VM activates through the fenced single-flight placement, artifact- + verification, network-repair, and readiness path. User keys can resume only their tenant's VMs; + admin keys can resume any VM. Args: id (UUID): diff --git a/sdk/python/tarit_sdk/api/default/snapshot_vm.py b/sdk/python/tarit_sdk/api/default/snapshot_vm.py index b4f52b6..bb2fa0c 100644 --- a/sdk/python/tarit_sdk/api/default/snapshot_vm.py +++ b/sdk/python/tarit_sdk/api/default/snapshot_vm.py @@ -85,12 +85,12 @@ def sync_detailed( client: AuthenticatedClient | Client, body: SnapshotVmBody, ) -> Response[Any | SnapshotVmResponse200]: - """Snapshot sandbox + """Snapshot a VM User keys can snapshot only their tenant's VMs; admin keys can snapshot any VM. A running VM uses - the bounded live pre-copy path; RAM, device state, and the private disk upper are captured at one - atomic final-stop boundary. The response is an opaque handle; paths and physical host identity - remain private. + the bounded live pre-copy path; a paused VM captures from its existing stop boundary. RAM, device + state, and the private disk upper are captured at one atomic boundary. The response is an opaque + handle; paths and physical host identity remain private. Args: id (UUID): @@ -122,12 +122,12 @@ def sync( client: AuthenticatedClient | Client, body: SnapshotVmBody, ) -> Any | SnapshotVmResponse200 | None: - """Snapshot sandbox + """Snapshot a VM User keys can snapshot only their tenant's VMs; admin keys can snapshot any VM. A running VM uses - the bounded live pre-copy path; RAM, device state, and the private disk upper are captured at one - atomic final-stop boundary. The response is an opaque handle; paths and physical host identity - remain private. + the bounded live pre-copy path; a paused VM captures from its existing stop boundary. RAM, device + state, and the private disk upper are captured at one atomic boundary. The response is an opaque + handle; paths and physical host identity remain private. Args: id (UUID): @@ -154,12 +154,12 @@ async def asyncio_detailed( client: AuthenticatedClient | Client, body: SnapshotVmBody, ) -> Response[Any | SnapshotVmResponse200]: - """Snapshot sandbox + """Snapshot a VM User keys can snapshot only their tenant's VMs; admin keys can snapshot any VM. A running VM uses - the bounded live pre-copy path; RAM, device state, and the private disk upper are captured at one - atomic final-stop boundary. The response is an opaque handle; paths and physical host identity - remain private. + the bounded live pre-copy path; a paused VM captures from its existing stop boundary. RAM, device + state, and the private disk upper are captured at one atomic boundary. The response is an opaque + handle; paths and physical host identity remain private. Args: id (UUID): @@ -189,12 +189,12 @@ async def asyncio( client: AuthenticatedClient | Client, body: SnapshotVmBody, ) -> Any | SnapshotVmResponse200 | None: - """Snapshot sandbox + """Snapshot a VM User keys can snapshot only their tenant's VMs; admin keys can snapshot any VM. A running VM uses - the bounded live pre-copy path; RAM, device state, and the private disk upper are captured at one - atomic final-stop boundary. The response is an opaque handle; paths and physical host identity - remain private. + the bounded live pre-copy path; a paused VM captures from its existing stop boundary. RAM, device + state, and the private disk upper are captured at one atomic boundary. The response is an opaque + handle; paths and physical host identity remain private. Args: id (UUID): diff --git a/sdk/python/tarit_sdk/api/default/suspend_vm.py b/sdk/python/tarit_sdk/api/default/suspend_vm.py index 26ff233..1e6f0c4 100644 --- a/sdk/python/tarit_sdk/api/default/suspend_vm.py +++ b/sdk/python/tarit_sdk/api/default/suspend_vm.py @@ -67,10 +67,12 @@ def sync_detailed( *, client: AuthenticatedClient | Client, ) -> Response[Any | VmRecord]: - """Suspend sandbox + """Suspend a VM in place - Drops resident guest memory while retaining VM ownership, quota, and scheduler reservations. Resume - the VM before exec, snapshot, PTY, SSH, or pause operations. + Stops every vCPU, drains and parks block, network, and vsock workers at the same boundary, captures + the in-process suspend image, and drops resident guest memory while retaining the VMM, VM ownership, + quota, and scheduler reservations. Resume the VM before exec, snapshot, PTY, SSH, or pause + operations. Args: id (UUID): @@ -99,10 +101,12 @@ def sync( *, client: AuthenticatedClient | Client, ) -> Any | VmRecord | None: - """Suspend sandbox + """Suspend a VM in place - Drops resident guest memory while retaining VM ownership, quota, and scheduler reservations. Resume - the VM before exec, snapshot, PTY, SSH, or pause operations. + Stops every vCPU, drains and parks block, network, and vsock workers at the same boundary, captures + the in-process suspend image, and drops resident guest memory while retaining the VMM, VM ownership, + quota, and scheduler reservations. Resume the VM before exec, snapshot, PTY, SSH, or pause + operations. Args: id (UUID): @@ -126,10 +130,12 @@ async def asyncio_detailed( *, client: AuthenticatedClient | Client, ) -> Response[Any | VmRecord]: - """Suspend sandbox + """Suspend a VM in place - Drops resident guest memory while retaining VM ownership, quota, and scheduler reservations. Resume - the VM before exec, snapshot, PTY, SSH, or pause operations. + Stops every vCPU, drains and parks block, network, and vsock workers at the same boundary, captures + the in-process suspend image, and drops resident guest memory while retaining the VMM, VM ownership, + quota, and scheduler reservations. Resume the VM before exec, snapshot, PTY, SSH, or pause + operations. Args: id (UUID): @@ -156,10 +162,12 @@ async def asyncio( *, client: AuthenticatedClient | Client, ) -> Any | VmRecord | None: - """Suspend sandbox + """Suspend a VM in place - Drops resident guest memory while retaining VM ownership, quota, and scheduler reservations. Resume - the VM before exec, snapshot, PTY, SSH, or pause operations. + Stops every vCPU, drains and parks block, network, and vsock workers at the same boundary, captures + the in-process suspend image, and drops resident guest memory while retaining the VMM, VM ownership, + quota, and scheduler reservations. Resume the VM before exec, snapshot, PTY, SSH, or pause + operations. Args: id (UUID): diff --git a/sdk/typescript/src/generated/schema.ts b/sdk/typescript/src/generated/schema.ts index ad0f3ec..e3136c7 100644 --- a/sdk/typescript/src/generated/schema.ts +++ b/sdk/typescript/src/generated/schema.ts @@ -297,7 +297,7 @@ export interface paths { get?: never; put?: never; /** - * Pause sandbox + * Pause a VM * @description User keys can pause only their tenant's VMs; admin keys can pause any VM. */ post: operations["pauseVm"]; @@ -317,8 +317,8 @@ export interface paths { get?: never; put?: never; /** - * Resume sandbox - * @description User keys can resume only their tenant's VMs; admin keys can resume any VM. + * Resume a VM + * @description Resumes a paused or suspended VM after every device worker leaves its parked state and before vCPUs restart. A hibernated VM activates through the fenced single-flight placement, artifact-verification, network-repair, and readiness path. User keys can resume only their tenant's VMs; admin keys can resume any VM. */ post: operations["resumeVm"]; delete?: never; @@ -337,8 +337,8 @@ export interface paths { get?: never; put?: never; /** - * Suspend sandbox - * @description Drops resident guest memory while retaining VM ownership, quota, and scheduler reservations. Resume the VM before exec, snapshot, PTY, SSH, or pause operations. + * Suspend a VM in place + * @description Stops every vCPU, drains and parks block, network, and vsock workers at the same boundary, captures the in-process suspend image, and drops resident guest memory while retaining the VMM, VM ownership, quota, and scheduler reservations. Resume the VM before exec, snapshot, PTY, SSH, or pause operations. */ post: operations["suspendVm"]; delete?: never; @@ -357,7 +357,7 @@ export interface paths { get?: never; put?: never; /** - * Hibernate sandbox and release host capacity + * Hibernate a VM and release host capacity * @description Atomically captures an authenticated live RAM/device/disk snapshot, stops the resident VMM, releases CPU, memory, cgroup, network, and scheduler capacity, and retains a tenant-owned logical VM record for secure resume. */ post: operations["hibernateVm"]; @@ -377,8 +377,8 @@ export interface paths { get?: never; put?: never; /** - * Snapshot sandbox - * @description User keys can snapshot only their tenant's VMs; admin keys can snapshot any VM. A running VM uses the bounded live pre-copy path; RAM, device state, and the private disk upper are captured at one atomic final-stop boundary. The response is an opaque handle; paths and physical host identity remain private. + * Snapshot a VM + * @description User keys can snapshot only their tenant's VMs; admin keys can snapshot any VM. A running VM uses the bounded live pre-copy path; a paused VM captures from its existing stop boundary. RAM, device state, and the private disk upper are captured at one atomic boundary. The response is an opaque handle; paths and physical host identity remain private. */ post: operations["snapshotVm"]; delete?: never; @@ -417,7 +417,7 @@ export interface paths { get?: never; put?: never; /** - * Restore sandbox from a snapshot + * Restore a VM from a snapshot * @description Restores a VM from an opaque snapshot handle. The control plane resolves its private host and storage locator; tenant VM quotas apply. */ post: operations["restoreVm"]; @@ -2134,7 +2134,7 @@ export interface operations { }; content?: never; }; - /** @description VM is stopped */ + /** @description Invalid lifecycle transition */ 409: { headers: { [name: string]: unknown; @@ -2184,13 +2184,27 @@ export interface operations { }; content?: never; }; - /** @description VM is stopped */ + /** @description Invalid lifecycle transition */ 409: { headers: { [name: string]: unknown; }; content?: never; }; + /** @description No placement capacity is available for hibernated activation */ + 429: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + /** @description The owning host or activation prerequisites are unavailable */ + 503: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; }; }; suspendVm: { @@ -2234,7 +2248,7 @@ export interface operations { }; content?: never; }; - /** @description VM is not running */ + /** @description Invalid lifecycle transition */ 409: { headers: { [name: string]: unknown; @@ -2354,7 +2368,7 @@ export interface operations { }; content?: never; }; - /** @description VM is stopped */ + /** @description VM lifecycle state does not support snapshots */ 409: { headers: { [name: string]: unknown; @@ -2481,7 +2495,14 @@ export interface operations { }; content?: never; }; - /** @description Selected node is at capacity */ + /** @description Requested VM id already exists */ + 409: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + /** @description Snapshot-owning node is at capacity */ 429: { headers: { /** @description Seconds the client should wait before retrying */ @@ -2490,6 +2511,13 @@ export interface operations { }; content?: never; }; + /** @description Snapshot-owning node is unhealthy, stale, or unavailable */ + 503: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; }; }; execute: { From 706bb42d259631df1883f28aa2b9980da2e47aa8 Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Mon, 31 Aug 2026 23:15:27 +0530 Subject: [PATCH 09/18] Fail closed on incomplete I/O resume --- CHANGELOG.md | 4 +- orch/crates/taritd/src/ops.rs | 129 ++++++++++++--- orch/docs/API.md | 5 + vmm/crates/vmm-core/src/controller.rs | 93 ++++++++--- vmm/crates/vmm-core/src/error.rs | 19 +++ vmm/crates/vmm-core/src/live_snapshot.rs | 200 ++++++++++++++++++++--- 6 files changed, 376 insertions(+), 74 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3518d6a..925dc57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,7 +67,9 @@ versions may contain breaking changes. waits for every worker to leave its parked state before restarting vCPUs, so rapid resume/suspend cycles cannot reuse a stale acknowledgement. Worker startup, unexpected exit, and five-second quiescence failures are surfaced - instead of publishing a partially functional VM or snapshot. + instead of publishing a partially functional VM or snapshot. If worker + rollback cannot be confirmed, vCPUs remain paused and the orchestrator + durably fences the observed state before returning the failure. - OCI image conversion now verifies manifest, config, and layer descriptors and streams layers through disk-size-derived expansion, entry-count, file-size, path-length, and layer-count limits before unpack. Rejected images publish no diff --git a/orch/crates/taritd/src/ops.rs b/orch/crates/taritd/src/ops.rs index a47b281..5275baf 100644 --- a/orch/crates/taritd/src/ops.rs +++ b/orch/crates/taritd/src/ops.rs @@ -152,7 +152,7 @@ async fn observe_and_compensate_vm_status( compensate_vm_status(state, prior, control_status(observed.state)?).await } -async fn reconcile_snapshot_pause_failure( +async fn reconcile_failed_live_operation( state: &AppState, prior: &VmRecord, primary: OrchError, @@ -167,16 +167,16 @@ async fn reconcile_snapshot_pause_failure( Ok(Ok(status)) => match control_status(status.state) { Ok(status) => status, Err(error) => { - return retain_snapshot_reconciliation( + return retain_live_operation_reconciliation( state, prior, primary, - format!("snapshot pause reconciliation rejected VMM state: {error}"), + format!("live-operation reconciliation rejected VMM state: {error}"), ); } }, Ok(Err(error)) => { - return retain_snapshot_reconciliation( + return retain_live_operation_reconciliation( state, prior, primary, @@ -184,7 +184,7 @@ async fn reconcile_snapshot_pause_failure( ); } Err(error) => { - return retain_snapshot_reconciliation( + return retain_live_operation_reconciliation( state, prior, primary, @@ -211,11 +211,11 @@ async fn reconcile_snapshot_pause_failure( )); } OrchError::Internal(format!( - "{primary}; VM was fenced {} after snapshot compensation", + "{primary}; VM was fenced {} after live-operation compensation", observed.as_str() )) } - Err(compensation) => retain_snapshot_reconciliation( + Err(compensation) => retain_live_operation_reconciliation( state, prior, primary, @@ -227,7 +227,7 @@ async fn reconcile_snapshot_pause_failure( } } -fn retain_snapshot_reconciliation( +fn retain_live_operation_reconciliation( state: &AppState, prior: &VmRecord, primary: OrchError, @@ -2717,10 +2717,10 @@ async fn snapshot_local_locked( let mut bundle = match bundle { Ok(Ok(bundle)) => bundle, Ok(Err(error)) => { - return Err(reconcile_snapshot_pause_failure(state, &vm, error).await); + return Err(reconcile_failed_live_operation(state, &vm, error).await); } Err(error) => { - return Err(reconcile_snapshot_pause_failure( + return Err(reconcile_failed_live_operation( state, &vm, OrchError::Internal(format!("snapshot task failed: {error}")), @@ -3899,19 +3899,19 @@ where TransitionDecision::Apply => {} } let operation_supervisor = Arc::clone(&state.supervisor); - tokio::task::spawn_blocking(move || op(&operation_supervisor, id)) + let operation = tokio::task::spawn_blocking(move || op(&operation_supervisor, id)) .await - .map_err(|e| OrchError::Internal(format!("join: {e}")))? - .map_err(|error| { - tracing::warn!( - vm = %id, - from = current.status.as_str(), - to = new_status.as_str(), - %error, - "VMM lifecycle operation failed" - ); - error - })?; + .map_err(|e| OrchError::Internal(format!("join: {e}")))?; + if let Err(error) = operation { + tracing::warn!( + vm = %id, + from = current.status.as_str(), + to = new_status.as_str(), + %error, + "VMM lifecycle operation failed" + ); + return Err(reconcile_failed_live_operation(state, ¤t, error).await); + } match vm_set_status(state, id, new_status).await { Ok(record) => Ok(record), Err(persist_error) => { @@ -4505,6 +4505,91 @@ mod tests { assert!(!socket.exists()); } + #[cfg(target_os = "linux")] + #[test] + fn failed_pause_is_observed_and_fenced_to_the_actual_vmm_state() { + let (state, _) = test_state_with_durable_writer(); + let id = insert_running_vm(&state); + let initial = vm_get(&state, id).unwrap(); + state.store.lock().unwrap().insert_vm(&initial).unwrap(); + + let socket = PathBuf::from(format!( + "/tmp/taritd-pause-reconcile-{}-{id}.sock", + std::process::id() + )); + let _ = std::fs::remove_file(&socket); + let listener = UnixListener::bind(&socket).unwrap(); + let (requests_tx, requests_rx) = std::sync::mpsc::channel(); + let server = std::thread::spawn(move || loop { + let (mut stream, _) = listener.accept().unwrap(); + let mut length = [0_u8; 4]; + stream.read_exact(&mut length).unwrap(); + let mut body = vec![0; u32::from_be_bytes(length) as usize]; + stream.read_exact(&mut body).unwrap(); + let request: tarit_vmm_client::ApiRequest = serde_json::from_slice(&body).unwrap(); + let response = match &request { + tarit_vmm_client::ApiRequest::Pause => tarit_vmm_client::ApiResponse::Err { + msg: "injected I/O quiescence failure".into(), + }, + tarit_vmm_client::ApiRequest::Status => { + tarit_vmm_client::ApiResponse::Status(tarit_vmm_client::VmStatus { + state: tarit_vmm_client::VmState::Paused, + uptime_ms: 1, + vcpus: 1, + mem_mib: 256, + volumes: 0, + nets: 0, + kernel: "kernel".into(), + vcpu_alive: true, + }) + } + _ => tarit_vmm_client::ApiResponse::Ok, + }; + let encoded = serde_json::to_vec(&response).unwrap(); + stream + .write_all(&(encoded.len() as u32).to_be_bytes()) + .unwrap(); + stream.write_all(&encoded).unwrap(); + stream.flush().unwrap(); + let stopped = matches!(request, tarit_vmm_client::ApiRequest::Stop); + requests_tx.send(request).unwrap(); + if stopped { + break; + } + }); + state + .supervisor + .install_test_control_runtime(id, socket.clone()); + + let error = test_runtime() + .block_on(pause_local(&state, id)) + .expect_err("a failed pause must reconcile an actually paused VMM"); + assert!(error.to_string().contains("fenced paused")); + + let cached = vm_get(&state, id).unwrap(); + let durable = state.store.lock().unwrap().get_vm(id).unwrap(); + assert_eq!(cached.status, VmStatus::Paused); + assert_eq!(durable.status, VmStatus::Paused); + assert_eq!(cached.revision, initial.revision + 2); + assert_eq!(durable.revision, initial.revision + 2); + + state.supervisor.stop_vm(id).unwrap(); + server.join().unwrap(); + let requests = requests_rx.into_iter().collect::>(); + assert!( + matches!( + requests.as_slice(), + [ + tarit_vmm_client::ApiRequest::Pause, + tarit_vmm_client::ApiRequest::Status, + tarit_vmm_client::ApiRequest::Stop + ] + ), + "unexpected VMM request sequence: {requests:?}" + ); + assert!(!socket.exists()); + } + #[cfg(target_os = "linux")] #[test] fn failed_live_snapshot_is_observed_and_fenced_paused() { diff --git a/orch/docs/API.md b/orch/docs/API.md index 887cbde..d93ae3b 100644 --- a/orch/docs/API.md +++ b/orch/docs/API.md @@ -412,6 +412,11 @@ Resolve the owner, stop every vCPU, drain and park the block, network, and vsock workers, then publish the paused state. The public handler does not require a JSON body. +If an I/O transition fails and the VMM cannot confirm a safe rollback, it keeps +the vCPUs paused. The orchestrator observes and durably records that state +before returning the operation failure, allowing a later explicit resume or +delete instead of leaving the control plane marked running. + Response `200`: updated `VmRecord` with `status: "paused"`. Status codes: `200`, `401`, `403`, `404`, `409` (invalid lifecycle diff --git a/vmm/crates/vmm-core/src/controller.rs b/vmm/crates/vmm-core/src/controller.rs index db398a5..79ddff1 100644 --- a/vmm/crates/vmm-core/src/controller.rs +++ b/vmm/crates/vmm-core/src/controller.rs @@ -580,9 +580,9 @@ impl VmmController { // Stop every producer of guest-memory writes while we capture device state // and RAM. Pause vCPUs first so the guest cannot enqueue new net/vsock work // after an I/O pump has acknowledged its pause. The pumps are then parked - // before capture begins. Resume in the inverse order: vCPUs first, then the - // pumps before vCPUs so a rapid subsequent pause cannot observe a stale - // worker acknowledgement from this snapshot. + // before capture begins. Resume the pumps before vCPUs so a rapid + // subsequent pause cannot observe a stale worker acknowledgement from + // this snapshot. #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] let paused_here = if state_before == VmState::Running { match pause_running_vcpus(vm) { @@ -604,11 +604,14 @@ impl VmmController { match pause_running_io(vm) { Ok(paused) => paused, Err(error) => { - let resume_error = if paused_here { + let resume_error = if paused_here && error.vcpus_may_resume_after_io_error() { resume_running_vcpus(vm).err() } else { None }; + if !error.vcpus_may_resume_after_io_error() { + vm.state = VmState::Paused; + } remove_owned_scratch_file(&owned_snapshot); return Err(match resume_error { Some(resume) => VmmError::Snapshot(format!( @@ -1226,6 +1229,14 @@ impl VmmController { capture_live_state_blob(&running, &base_blob) }, ); + let source_vcpus_paused = running + .vcpu_thread + .paused + .load(std::sync::atomic::Ordering::Acquire) + || running + .ap_threads + .iter() + .any(|thread| thread.paused.load(std::sync::atomic::Ordering::Acquire)); // Put the VM back only if the slot still holds the same instance. A // concurrent stop + create would otherwise get this VM's threads and @@ -1235,6 +1246,9 @@ impl VmmController { let mut slot = self.lock(); match slot.as_mut() { Some(vm) if vm.generation == generation && vm.running.is_none() => { + if source_vcpus_paused { + vm.state = VmState::Paused; + } vm.running = reclaimed.take(); } _ => { @@ -1791,6 +1805,10 @@ impl VmmController { pause_running_vcpus(vm)?; #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] if let Err(error) = pause_running_io(vm) { + if !error.vcpus_may_resume_after_io_error() { + vm.state = VmState::Paused; + return Err(error); + } return match resume_running_vcpus(vm) { Ok(()) => Err(error), Err(resume) => Err(VmmError::Device(format!( @@ -2630,34 +2648,43 @@ fn set_running_io_paused(running: &RunningVm, paused: bool) -> Result<()> { for io_loop in &running.blk_io_loops { if let Err(error) = io_loop.pause() { let rollback = resume_running_io_workers(running).err(); - return Err(VmmError::Device(match rollback { - Some(rollback) => { - format!("quiesce block I/O worker: {error}; rollback failed: {rollback}") - } - None => format!("quiesce block I/O worker: {error}"), - })); + return Err(VmmError::IoQuiescence { + message: match rollback.as_ref() { + Some(rollback) => format!( + "quiesce block I/O worker: {error}; rollback failed: {rollback}" + ), + None => format!("quiesce block I/O worker: {error}"), + }, + vcpus_may_resume: rollback.is_none(), + }); } } for io_loop in &running.net_io_loops { if let Err(error) = io_loop.pause() { let rollback = resume_running_io_workers(running).err(); - return Err(VmmError::Device(match rollback { - Some(rollback) => { - format!("quiesce network I/O worker: {error}; rollback failed: {rollback}") - } - None => format!("quiesce network I/O worker: {error}"), - })); + return Err(VmmError::IoQuiescence { + message: match rollback.as_ref() { + Some(rollback) => format!( + "quiesce network I/O worker: {error}; rollback failed: {rollback}" + ), + None => format!("quiesce network I/O worker: {error}"), + }, + vcpus_may_resume: rollback.is_none(), + }); } } if let Some(pump) = running.vsock_pump.as_ref() { if let Err(error) = pump.pause() { let rollback = resume_running_io_workers(running).err(); - return Err(VmmError::Device(match rollback { - Some(rollback) => { - format!("quiesce vsock worker: {error}; rollback failed: {rollback}") - } - None => format!("quiesce vsock worker: {error}"), - })); + return Err(VmmError::IoQuiescence { + message: match rollback.as_ref() { + Some(rollback) => { + format!("quiesce vsock worker: {error}; rollback failed: {rollback}") + } + None => format!("quiesce vsock worker: {error}"), + }, + vcpus_may_resume: rollback.is_none(), + }); } } } else { @@ -2687,10 +2714,10 @@ fn resume_running_io_workers(running: &RunningVm) -> Result<()> { if failures.is_empty() { Ok(()) } else { - Err(VmmError::Device(format!( - "resume I/O workers: {}", - failures.join("; ") - ))) + Err(VmmError::IoQuiescence { + message: format!("resume I/O workers: {}", failures.join("; ")), + vcpus_may_resume: false, + }) } } @@ -2832,9 +2859,12 @@ fn suspend_vm_in_place(vm: &mut VmInstance) -> Result<()> { match pause_running_io(vm) { Ok(paused) => paused, Err(error) => { - if paused_here { + if paused_here && error.vcpus_may_resume_after_io_error() { resume_running_vcpus(vm)?; } + if !error.vcpus_may_resume_after_io_error() { + vm.state = VmState::Paused; + } return Err(error); } } @@ -2986,6 +3016,15 @@ pub(crate) fn private_runtime_dir() -> Result { Ok(dir) } +#[cfg(test)] +fn cleanup_private_runtime_dir() { + // Unit tests create and drop independent VM instances concurrently inside + // one process. They intentionally share the per-process runtime directory, + // so one test must not remove it while another is staging an artifact. + // Individual test artifacts retain their own exact cleanup guards. +} + +#[cfg(not(test))] fn cleanup_private_runtime_dir() { use std::io::ErrorKind; diff --git a/vmm/crates/vmm-core/src/error.rs b/vmm/crates/vmm-core/src/error.rs index 5ef76c5..d67b44f 100644 --- a/vmm/crates/vmm-core/src/error.rs +++ b/vmm/crates/vmm-core/src/error.rs @@ -12,6 +12,13 @@ pub enum VmmError { Loader(String), #[error("device error: {0}")] Device(String), + #[error("device error: {message}")] + IoQuiescence { + message: String, + /// Whether every partially parked worker was confirmed running again. + /// A false value requires callers to keep vCPUs paused. + vcpus_may_resume: bool, + }, #[error("snapshot error: {0}")] Snapshot(String), #[error("io error: {0}")] @@ -20,6 +27,18 @@ pub enum VmmError { pub type Result = std::result::Result; +impl VmmError { + #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "kvm"))] + pub(crate) fn vcpus_may_resume_after_io_error(&self) -> bool { + match self { + Self::IoQuiescence { + vcpus_may_resume, .. + } => *vcpus_may_resume, + _ => true, + } + } +} + impl From for VmmError { fn from(e: tarit_proto::config::ConfigError) -> Self { VmmError::InvalidConfig(e.to_string()) diff --git a/vmm/crates/vmm-core/src/live_snapshot.rs b/vmm/crates/vmm-core/src/live_snapshot.rs index 1bfbcd7..29d059e 100644 --- a/vmm/crates/vmm-core/src/live_snapshot.rs +++ b/vmm/crates/vmm-core/src/live_snapshot.rs @@ -90,6 +90,13 @@ impl<'a> VcpuPauseGuard<'a> { } Ok(()) } + + /// Disarm automatic resume while intentionally leaving every vCPU at the + /// snapshot pause boundary. Used when device workers could not prove that + /// they left quiescence; running the guest in that state would be unsafe. + fn keep_paused(mut self) { + self.armed = false; + } } impl Drop for VcpuPauseGuard<'_> { @@ -177,6 +184,60 @@ impl Drop for IoQuiesceGuard<'_> { } } +trait FinalIoRelease { + fn release(self) -> Result<()>; +} + +impl FinalIoRelease for IoQuiesceGuard<'_> { + fn release(self) -> Result<()> { + self.disengage() + } +} + +trait FinalVcpuRelease { + fn resume(self) -> Result<()>; + fn keep_paused(self); +} + +impl FinalVcpuRelease for VcpuPauseGuard<'_> { + fn resume(self) -> Result<()> { + VcpuPauseGuard::resume(self) + } + + fn keep_paused(self) { + VcpuPauseGuard::keep_paused(self); + } +} + +/// Leave the final-stop boundary in a fail-closed order. Device workers must +/// prove they are running before vCPUs can leave their pause. Capture failures +/// still restore the source when that ordering succeeds. +fn finish_final_stop(capture: Result, io: I, vcpus: V) -> Result +where + I: FinalIoRelease, + V: FinalVcpuRelease, +{ + if let Err(io_error) = io.release() { + vcpus.keep_paused(); + return Err(match capture { + Ok(_) => io_error, + Err(capture_error) => VmmError::Snapshot(format!( + "{capture_error}; failed to resume I/O workers: {io_error}" + )), + }); + } + + let vcpu_resume = vcpus.resume(); + match (capture, vcpu_resume) { + (Ok(value), Ok(())) => Ok(value), + (Err(error), Ok(())) => Err(error), + (Ok(_), Err(error)) => Err(error), + (Err(capture_error), Err(resume_error)) => Err(VmmError::Snapshot(format!( + "{capture_error}; failed to resume vCPUs: {resume_error}" + ))), + } +} + /// Configuration for a live snapshot. #[derive(Debug, Clone)] pub struct LiveSnapshotConfig { @@ -607,33 +668,52 @@ where log::info!("live_snapshot: final stop — pausing all vCPUs, draining I/O"); let final_stop_start = Instant::now(); let final_pause_guard = VcpuPauseGuard::pause_all(vcpu_threads)?; - let io_guard = IoQuiesceGuard::engage(quiesce_io)?; - inject_live_snapshot_failure("final_pause")?; - - let mut final_dirty = kvm_vm.read_dirty()?; - final_dirty.merge(&mem.drain_host_dirty()); - final_dirty.merge(&pending_final_dirty); - let final_dirty_pages = final_dirty.len() as u64; - consumed_dirty.merge(&final_dirty); - let lazy_fence = mem.lazy_snapshot_fence(); - let _final_read_guard = lazy_fence.as_ref().map(|fence| { - fence - .write() - .unwrap_or_else(|poisoned| poisoned.into_inner()) - }); - total_pages_copied += copy_dirty_pages(mem, memory_file, &final_dirty)?; - drop(_final_read_guard); - // Every vCPU is paused, so the registers and device state captured here are - // coherent with the memory image assembled above. - let state_blob = capture_state()?; - inject_live_snapshot_failure("state_capture")?; + let io_guard = match IoQuiesceGuard::engage(quiesce_io) { + Ok(guard) => guard, + Err(error) => { + if error.vcpus_may_resume_after_io_error() { + return match final_pause_guard.resume() { + Ok(()) => Err(error), + Err(resume_error) => Err(VmmError::Snapshot(format!( + "{error}; failed to resume vCPUs after I/O quiescence failure: {resume_error}" + ))), + }; + } else { + final_pause_guard.keep_paused(); + } + return Err(error); + } + }; + let capture_result = (|| -> Result<(Vec, u64)> { + inject_live_snapshot_failure("final_pause")?; + + let mut final_dirty = kvm_vm.read_dirty()?; + final_dirty.merge(&mem.drain_host_dirty()); + final_dirty.merge(&pending_final_dirty); + let final_dirty_pages = final_dirty.len() as u64; + consumed_dirty.merge(&final_dirty); + let lazy_fence = mem.lazy_snapshot_fence(); + let _final_read_guard = lazy_fence.as_ref().map(|fence| { + fence + .write() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + }); + total_pages_copied += copy_dirty_pages(mem, memory_file, &final_dirty)?; + drop(_final_read_guard); + // Every vCPU is paused, so the registers and device state captured here + // are coherent with the memory image assembled above. + let state_blob = capture_state()?; + inject_live_snapshot_failure("state_capture")?; + Ok((state_blob, final_dirty_pages)) + })(); // Release I/O workers first and wait for their pause acknowledgements to // clear. This closes the rapid resume/pause race before any vCPU can - // publish new descriptors. Then observe every vCPU leave its park, so - // downtime covers the complete all-vCPU blackout. - io_guard.disengage()?; - final_pause_guard.resume()?; + // publish new descriptors. An I/O release failure deliberately leaves the + // vCPUs paused. Otherwise observe every vCPU leave its park so downtime + // covers the complete all-vCPU blackout. + let (state_blob, final_dirty_pages) = + finish_final_stop(capture_result, io_guard, final_pause_guard)?; let downtime = final_stop_start.elapsed(); // Final residual pages entered the page cache during blackout, but durable // writeback is not part of guest downtime. @@ -672,6 +752,40 @@ where mod tests { use super::*; + struct FakeIoRelease { + actions: std::rc::Rc>>, + fail: bool, + } + + impl FinalIoRelease for FakeIoRelease { + fn release(self) -> Result<()> { + self.actions.borrow_mut().push("io-release"); + if self.fail { + Err(VmmError::IoQuiescence { + message: "worker did not resume".into(), + vcpus_may_resume: false, + }) + } else { + Ok(()) + } + } + } + + struct FakeVcpuRelease { + actions: std::rc::Rc>>, + } + + impl FinalVcpuRelease for FakeVcpuRelease { + fn resume(self) -> Result<()> { + self.actions.borrow_mut().push("vcpu-resume"); + Ok(()) + } + + fn keep_paused(self) { + self.actions.borrow_mut().push("vcpu-keep-paused"); + } + } + #[test] fn live_snapshot_config_default() { let c = LiveSnapshotConfig::default(); @@ -844,4 +958,42 @@ mod tests { assert!(error.to_string().contains("resume failed")); assert_eq!(&*calls.borrow(), &[true, false]); } + + #[test] + fn final_stop_keeps_vcpus_paused_when_io_release_fails() { + let actions = std::rc::Rc::new(std::cell::RefCell::new(Vec::new())); + let error = finish_final_stop( + Ok(()), + FakeIoRelease { + actions: std::rc::Rc::clone(&actions), + fail: true, + }, + FakeVcpuRelease { + actions: std::rc::Rc::clone(&actions), + }, + ) + .expect_err("I/O release failure unexpectedly resumed the source"); + + assert!(error.to_string().contains("worker did not resume")); + assert_eq!(&*actions.borrow(), &["io-release", "vcpu-keep-paused"]); + } + + #[test] + fn final_stop_restores_source_after_capture_failure_when_io_is_running() { + let actions = std::rc::Rc::new(std::cell::RefCell::new(Vec::new())); + let error = finish_final_stop::<(), _, _>( + Err(VmmError::Snapshot("capture failed".into())), + FakeIoRelease { + actions: std::rc::Rc::clone(&actions), + fail: false, + }, + FakeVcpuRelease { + actions: std::rc::Rc::clone(&actions), + }, + ) + .expect_err("capture failure unexpectedly succeeded"); + + assert!(error.to_string().contains("capture failed")); + assert_eq!(&*actions.borrow(), &["io-release", "vcpu-resume"]); + } } From ef289194fe42a54e1d53f6aec7f8653a8486ce0f Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Mon, 31 Aug 2026 23:22:50 +0530 Subject: [PATCH 10/18] Confirm vCPU rollback before resuming work --- CHANGELOG.md | 7 +-- orch/docs/API.md | 8 ++-- vmm/crates/vmm-core/src/controller.rs | 34 ++++++++++++-- vmm/crates/vmm-core/src/live_snapshot.rs | 58 +++++++++++++++++++++--- 4 files changed, 88 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 925dc57..8a423b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,9 +67,10 @@ versions may contain breaking changes. waits for every worker to leave its parked state before restarting vCPUs, so rapid resume/suspend cycles cannot reuse a stale acknowledgement. Worker startup, unexpected exit, and five-second quiescence failures are surfaced - instead of publishing a partially functional VM or snapshot. If worker - rollback cannot be confirmed, vCPUs remain paused and the orchestrator - durably fences the observed state before returning the failure. + instead of publishing a partially functional VM or snapshot. If a partial + vCPU or device-worker transition cannot be rolled back and acknowledged, + the VM is fenced paused and the orchestrator durably records that state + before returning the failure. - OCI image conversion now verifies manifest, config, and layer descriptors and streams layers through disk-size-derived expansion, entry-count, file-size, path-length, and layer-count limits before unpack. Rejected images publish no diff --git a/orch/docs/API.md b/orch/docs/API.md index d93ae3b..c5f6a52 100644 --- a/orch/docs/API.md +++ b/orch/docs/API.md @@ -412,10 +412,10 @@ Resolve the owner, stop every vCPU, drain and park the block, network, and vsock workers, then publish the paused state. The public handler does not require a JSON body. -If an I/O transition fails and the VMM cannot confirm a safe rollback, it keeps -the vCPUs paused. The orchestrator observes and durably records that state -before returning the operation failure, allowing a later explicit resume or -delete instead of leaving the control plane marked running. +If a vCPU or device-worker transition fails and the VMM cannot confirm a safe +rollback, it fences the VM paused. The orchestrator observes and durably +records that state before returning the operation failure, allowing a later +explicit resume or delete instead of leaving the control plane marked running. Response `200`: updated `VmRecord` with `status: "paused"`. diff --git a/vmm/crates/vmm-core/src/controller.rs b/vmm/crates/vmm-core/src/controller.rs index 79ddff1..0be8151 100644 --- a/vmm/crates/vmm-core/src/controller.rs +++ b/vmm/crates/vmm-core/src/controller.rs @@ -2608,7 +2608,7 @@ fn stop_running_vm(vm: &mut VmInstance) { } #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] -fn pause_running_vcpus(vm: &VmInstance) -> Result { +fn pause_running_vcpus(vm: &mut VmInstance) -> Result { if let Some(r) = vm.running.as_ref() { let vcpu_threads = std::iter::once(&r.vcpu_thread) .chain(r.ap_threads.iter()) @@ -2618,16 +2618,18 @@ fn pause_running_vcpus(vm: &VmInstance) -> Result { } for vcpu_thread in &vcpu_threads { if let Err(error) = vcpu_thread.request_snapshot_pause() { - for armed in &vcpu_threads { - armed.resume(); + let (error, rollback_confirmed) = rollback_failed_vcpu_pause(vcpu_threads, error); + if !rollback_confirmed { + vm.state = VmState::Paused; } return Err(error); } } for vcpu_thread in &vcpu_threads { if let Err(error) = vcpu_thread.wait_snapshot_paused() { - for armed in &vcpu_threads { - armed.resume(); + let (error, rollback_confirmed) = rollback_failed_vcpu_pause(vcpu_threads, error); + if !rollback_confirmed { + vm.state = VmState::Paused; } return Err(error); } @@ -2638,6 +2640,28 @@ fn pause_running_vcpus(vm: &VmInstance) -> Result { } } +#[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] +fn rollback_failed_vcpu_pause( + vcpu_threads: Vec<&VcpuThread>, + primary: VmmError, +) -> (VmmError, bool) { + for vcpu_thread in &vcpu_threads { + vcpu_thread.resume(); + } + let rollback = vcpu_threads + .iter() + .try_for_each(|vcpu_thread| vcpu_thread.wait_snapshot_resumed()); + match rollback { + Ok(()) => (primary, true), + Err(resume_error) => ( + VmmError::Snapshot(format!( + "{primary}; failed to confirm vCPU rollback: {resume_error}" + )), + false, + ), + } +} + /// Park or release every host I/O thread that can mutate guest memory without /// running on a vCPU. The pause methods synchronously acknowledge, so once this /// returns with `paused = true`, device state and RAM are stable as long as the diff --git a/vmm/crates/vmm-core/src/live_snapshot.rs b/vmm/crates/vmm-core/src/live_snapshot.rs index 29d059e..a78d144 100644 --- a/vmm/crates/vmm-core/src/live_snapshot.rs +++ b/vmm/crates/vmm-core/src/live_snapshot.rs @@ -70,10 +70,14 @@ impl<'a> VcpuPauseGuard<'a> { // Arm every vCPU before waiting for any acknowledgement. The guard is // already active, so a partial request failure resumes every thread. for vcpu_thread in &guard.vcpu_threads { - vcpu_thread.request_snapshot_pause()?; + if let Err(error) = vcpu_thread.request_snapshot_pause() { + return Err(finish_failed_vcpu_pause(error, guard)); + } } for vcpu_thread in &guard.vcpu_threads { - vcpu_thread.wait_snapshot_paused()?; + if let Err(error) = vcpu_thread.wait_snapshot_paused() { + return Err(finish_failed_vcpu_pause(error, guard)); + } } Ok(guard) } @@ -83,10 +87,13 @@ impl<'a> VcpuPauseGuard<'a> { for vcpu_thread in &self.vcpu_threads { vcpu_thread.resume(); } + // Every control flag is clear now. Disarm before waiting so a + // failed acknowledgement does not issue a second, unobserved + // resume request from Drop. + self.armed = false; for vcpu_thread in &self.vcpu_threads { vcpu_thread.wait_snapshot_resumed()?; } - self.armed = false; } Ok(()) } @@ -209,6 +216,17 @@ impl FinalVcpuRelease for VcpuPauseGuard<'_> { } } +/// A failed all-vCPU pause must not report a recoverable source until every +/// armed thread has acknowledged the compensating resume. +fn finish_failed_vcpu_pause(primary: VmmError, vcpus: V) -> VmmError { + match vcpus.resume() { + Ok(()) => primary, + Err(resume_error) => VmmError::Snapshot(format!( + "{primary}; failed to confirm vCPU rollback: {resume_error}" + )), + } +} + /// Leave the final-stop boundary in a fail-closed order. Device workers must /// prove they are running before vCPUs can leave their pause. Capture failures /// still restore the source when that ordering succeeds. @@ -662,9 +680,10 @@ where // 3. Inside the pause do only O(residual) work: read both dirty // sources, copy the residual pages, capture state. // - // On any error below, the guards resume all vCPUs and I/O threads as they - // drop. Downtime is measured across the whole pause — including the - // pause/resume handshakes — because that is the blackout the guest sees. + // Capture errors restore I/O and then vCPUs. If either release cannot be + // confirmed, the controller fences the source paused for explicit + // recovery. Downtime covers the complete all-vCPU blackout, including the + // pause/resume handshakes. log::info!("live_snapshot: final stop — pausing all vCPUs, draining I/O"); let final_stop_start = Instant::now(); let final_pause_guard = VcpuPauseGuard::pause_all(vcpu_threads)?; @@ -773,12 +792,17 @@ mod tests { struct FakeVcpuRelease { actions: std::rc::Rc>>, + fail_resume: bool, } impl FinalVcpuRelease for FakeVcpuRelease { fn resume(self) -> Result<()> { self.actions.borrow_mut().push("vcpu-resume"); - Ok(()) + if self.fail_resume { + Err(VmmError::Snapshot("vCPU did not resume".into())) + } else { + Ok(()) + } } fn keep_paused(self) { @@ -970,6 +994,7 @@ mod tests { }, FakeVcpuRelease { actions: std::rc::Rc::clone(&actions), + fail_resume: false, }, ) .expect_err("I/O release failure unexpectedly resumed the source"); @@ -989,6 +1014,7 @@ mod tests { }, FakeVcpuRelease { actions: std::rc::Rc::clone(&actions), + fail_resume: false, }, ) .expect_err("capture failure unexpectedly succeeded"); @@ -996,4 +1022,22 @@ mod tests { assert!(error.to_string().contains("capture failed")); assert_eq!(&*actions.borrow(), &["io-release", "vcpu-resume"]); } + + #[test] + fn failed_vcpu_pause_requires_confirmed_rollback() { + let actions = std::rc::Rc::new(std::cell::RefCell::new(Vec::new())); + let error = finish_failed_vcpu_pause( + VmmError::Snapshot("pause failed".into()), + FakeVcpuRelease { + actions: std::rc::Rc::clone(&actions), + fail_resume: true, + }, + ); + + assert!(error.to_string().contains("pause failed")); + assert!(error + .to_string() + .contains("failed to confirm vCPU rollback")); + assert_eq!(&*actions.borrow(), &["vcpu-resume"]); + } } From 7840aeba886e4a6893dab0fccbcfa61de8b25ea7 Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Mon, 31 Aug 2026 23:32:28 +0530 Subject: [PATCH 11/18] Fix Linux vCPU rollback type resolution --- vmm/crates/vmm-core/src/controller.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vmm/crates/vmm-core/src/controller.rs b/vmm/crates/vmm-core/src/controller.rs index 0be8151..82278af 100644 --- a/vmm/crates/vmm-core/src/controller.rs +++ b/vmm/crates/vmm-core/src/controller.rs @@ -2642,7 +2642,7 @@ fn pause_running_vcpus(vm: &mut VmInstance) -> Result { #[cfg(all(target_arch = "x86_64", target_os = "linux", feature = "boot"))] fn rollback_failed_vcpu_pause( - vcpu_threads: Vec<&VcpuThread>, + vcpu_threads: Vec<&crate::vcpu_thread::VcpuThread>, primary: VmmError, ) -> (VmmError, bool) { for vcpu_thread in &vcpu_threads { From a274132bd0c112a281da3de37921a11462aaa249 Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Mon, 31 Aug 2026 23:43:00 +0530 Subject: [PATCH 12/18] Fix mutable VM access in pause probe --- vmm/crates/vmm-core/src/controller.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vmm/crates/vmm-core/src/controller.rs b/vmm/crates/vmm-core/src/controller.rs index 82278af..6c0e27a 100644 --- a/vmm/crates/vmm-core/src/controller.rs +++ b/vmm/crates/vmm-core/src/controller.rs @@ -410,9 +410,9 @@ impl VmmController { feature = "test-failpoints" ))] pub fn test_vcpu_pause_round_trip(&self) -> Result<()> { - let slot = self.lock(); + let mut slot = self.lock(); let vm = slot - .as_ref() + .as_mut() .ok_or_else(|| VmmError::InvalidConfig("no running VM".into()))?; if pause_running_vcpus(vm)? { resume_running_vcpus(vm)?; From 0148f133359398eeff0a9c5399816c6020829657 Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Tue, 1 Sep 2026 01:41:28 +0530 Subject: [PATCH 13/18] Isolate block delay failpoints per device --- vmm/crates/vmm-core/src/controller.rs | 20 ++++++++ .../vmm-devices/src/virtio/blk_backend.rs | 33 ++++++------ .../vmm-devices/src/virtio/blk_transport.rs | 51 +++++++++++++++++++ .../vmm-integration/blk_io_isolation.rs | 14 +++-- 4 files changed, 98 insertions(+), 20 deletions(-) diff --git a/vmm/crates/vmm-core/src/controller.rs b/vmm/crates/vmm-core/src/controller.rs index 6c0e27a..c8878bf 100644 --- a/vmm/crates/vmm-core/src/controller.rs +++ b/vmm/crates/vmm-core/src/controller.rs @@ -401,6 +401,26 @@ impl VmmController { .map_err(|error| VmmError::Device(format!("set block service delay: {error}"))) } + /// Return the number of requests currently held in one block device's + /// test-only latency injection point. + #[cfg(all( + target_arch = "x86_64", + target_os = "linux", + feature = "boot", + feature = "test-failpoints" + ))] + pub fn test_block_delayed_services(&self, volume_index: usize) -> Result { + let slot = self.lock(); + let running = slot + .as_ref() + .and_then(|vm| vm.running.as_ref()) + .ok_or_else(|| VmmError::InvalidConfig("no running VM".into()))?; + let device = running.blk_devices.get(volume_index).ok_or_else(|| { + VmmError::InvalidConfig(format!("volume index {volume_index} is out of range")) + })?; + Ok(device.test_delayed_services()) + } + /// Pause and immediately resume only the vCPUs. Used to prove that a slow /// storage backend cannot occupy the KVM execution/control thread. #[cfg(all( diff --git a/vmm/crates/vmm-devices/src/virtio/blk_backend.rs b/vmm/crates/vmm-devices/src/virtio/blk_backend.rs index 88af4f3..c0617d2 100644 --- a/vmm/crates/vmm-devices/src/virtio/blk_backend.rs +++ b/vmm/crates/vmm-devices/src/virtio/blk_backend.rs @@ -414,29 +414,20 @@ pub struct BlkBackend { pub sectors: u64, #[cfg(feature = "test-failpoints")] service_delay: std::time::Duration, + #[cfg(feature = "test-failpoints")] + delayed_services: std::sync::Arc, } #[cfg(feature = "test-failpoints")] -static TEST_DELAYED_SERVICES: std::sync::atomic::AtomicUsize = - std::sync::atomic::AtomicUsize::new(0); - -#[cfg(feature = "test-failpoints")] -struct DelayedServiceGuard; +struct DelayedServiceGuard(std::sync::Arc); #[cfg(feature = "test-failpoints")] impl Drop for DelayedServiceGuard { fn drop(&mut self) { - TEST_DELAYED_SERVICES.fetch_sub(1, std::sync::atomic::Ordering::SeqCst); + self.0.fetch_sub(1, std::sync::atomic::Ordering::SeqCst); } } -/// Number of block service calls currently held in the test-only latency -/// injection point. -#[cfg(feature = "test-failpoints")] -pub fn test_delayed_services() -> usize { - TEST_DELAYED_SERVICES.load(std::sync::atomic::Ordering::SeqCst) -} - #[cfg(unix)] fn validate_file_access(file: &File, read_only: bool) -> Result<(), BlkBackendError> { let flags = unsafe { libc::fcntl(file.as_raw_fd(), libc::F_GETFL) }; @@ -520,6 +511,8 @@ impl BlkBackend { sectors, #[cfg(feature = "test-failpoints")] service_delay: std::time::Duration::ZERO, + #[cfg(feature = "test-failpoints")] + delayed_services: std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)), }) } @@ -559,6 +552,8 @@ impl BlkBackend { sectors, #[cfg(feature = "test-failpoints")] service_delay: std::time::Duration::ZERO, + #[cfg(feature = "test-failpoints")] + delayed_services: std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)), }) } @@ -567,6 +562,13 @@ impl BlkBackend { self.service_delay = delay; } + #[cfg(feature = "test-failpoints")] + pub(crate) fn test_delayed_services_counter( + &self, + ) -> std::sync::Arc { + std::sync::Arc::clone(&self.delayed_services) + } + /// Service a single block request. /// /// - `header`: the parsed virtio_blk_req (type, sector) @@ -578,8 +580,9 @@ impl BlkBackend { let _delay_guard = if self.service_delay.is_zero() { None } else { - TEST_DELAYED_SERVICES.fetch_add(1, std::sync::atomic::Ordering::SeqCst); - let guard = DelayedServiceGuard; + self.delayed_services + .fetch_add(1, std::sync::atomic::Ordering::SeqCst); + let guard = DelayedServiceGuard(std::sync::Arc::clone(&self.delayed_services)); std::thread::sleep(self.service_delay); Some(guard) }; diff --git a/vmm/crates/vmm-devices/src/virtio/blk_transport.rs b/vmm/crates/vmm-devices/src/virtio/blk_transport.rs index 25dc81d..8934986 100644 --- a/vmm/crates/vmm-devices/src/virtio/blk_transport.rs +++ b/vmm/crates/vmm-devices/src/virtio/blk_transport.rs @@ -148,6 +148,8 @@ pub struct VirtioBlkMmio { /// Set by the VMM after registering the irqfd with KVM (Linux only). #[cfg(target_os = "linux")] irq_evt: Mutex>, + #[cfg(feature = "test-failpoints")] + delayed_services: std::sync::Arc, /// Diagnostic counter: number of QUEUE_NOTIFY writes received from the /// guest. Used by the OCI-boot-to-login probe to distinguish "guest never /// kicked the queue" (driver didn't activate) from "queue kicked but @@ -175,6 +177,11 @@ impl VirtioBlkMmio { Ok(()) } + #[cfg(feature = "test-failpoints")] + pub fn test_delayed_services(&self) -> usize { + self.delayed_services.load(Ordering::SeqCst) + } + fn fail_device(&self, context: &str) { log::error!("virtio-blk: {context}"); self.status.fetch_or( @@ -281,6 +288,8 @@ impl VirtioBlkMmio { /// Create a new virtio-blk MMIO device with a file-backed backend. pub fn new(irq: u32, backend: BlkBackend) -> Self { + #[cfg(feature = "test-failpoints")] + let delayed_services = backend.test_delayed_services_counter(); Self { irq, device_id: 2, @@ -302,6 +311,8 @@ impl VirtioBlkMmio { interrupt_status: AtomicU32::new(0), #[cfg(target_os = "linux")] irq_evt: Mutex::new(None), + #[cfg(feature = "test-failpoints")] + delayed_services, notify_count: AtomicU64::new(0), status_writes: AtomicU64::new(0), } @@ -330,6 +341,8 @@ impl VirtioBlkMmio { interrupt_status: AtomicU32::new(0), #[cfg(target_os = "linux")] irq_evt: Mutex::new(None), + #[cfg(feature = "test-failpoints")] + delayed_services: std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)), notify_count: AtomicU64::new(0), status_writes: AtomicU64::new(0), } @@ -1255,6 +1268,44 @@ mod tests { std::fs::remove_file(path).unwrap(); } + #[cfg(feature = "test-failpoints")] + #[test] + fn delayed_service_counters_are_isolated_per_device() { + let (backend_a, path_a) = new_test_backend("blk-delay-a"); + let (backend_b, path_b) = new_test_backend("blk-delay-b"); + let mem_a = new_test_mem(); + let mem_b = new_test_mem(); + let dev_a = Arc::new(VirtioBlkMmio::new(5, backend_a)); + let dev_b = VirtioBlkMmio::new(6, backend_b); + configure_test_blk_queue(&dev_a, mem_a.clone()); + configure_test_blk_queue(&dev_b, mem_b.clone()); + setup_blk_out_requests(&mem_a, 1); + setup_blk_out_requests(&mem_b, 1); + dev_a + .set_test_service_delay(std::time::Duration::from_millis(100)) + .unwrap(); + + let delayed = Arc::clone(&dev_a); + let worker = std::thread::spawn(move || delayed.process_queue(0)); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(2); + while dev_a.test_delayed_services() == 0 { + assert!( + std::time::Instant::now() < deadline, + "delayed request did not reach its device-local failpoint" + ); + std::thread::sleep(std::time::Duration::from_millis(1)); + } + assert_eq!(dev_a.test_delayed_services(), 1); + assert_eq!(dev_b.test_delayed_services(), 0); + + worker.join().unwrap().unwrap(); + assert_eq!(dev_a.test_delayed_services(), 0); + assert_eq!(dev_b.test_delayed_services(), 0); + dev_b.process_queue(0).unwrap(); + std::fs::remove_file(path_a).unwrap(); + std::fs::remove_file(path_b).unwrap(); + } + #[test] fn tight_limiter_defers_blk_requests_without_consuming_descriptors() { let (backend, path) = new_test_backend("blk-limited"); diff --git a/vmm/crates/vmm-integration/blk_io_isolation.rs b/vmm/crates/vmm-integration/blk_io_isolation.rs index 53fdf3c..681927a 100644 --- a/vmm/crates/vmm-integration/blk_io_isolation.rs +++ b/vmm/crates/vmm-integration/blk_io_isolation.rs @@ -18,9 +18,13 @@ use vmm_core::controller::VmmController; mod test_support; use test_support::{agent_vm_config, guest_stdout}; -fn wait_for_delayed_service() { +fn wait_for_delayed_service(controller: &VmmController, volume_index: usize) { let deadline = Instant::now() + Duration::from_secs(10); - while vmm_devices::virtio::blk_backend::test_delayed_services() == 0 { + while controller + .test_block_delayed_services(volume_index) + .expect("read delayed block request count") + == 0 + { assert!( Instant::now() < deadline, "delayed block request never started" @@ -71,7 +75,7 @@ fn delayed_volume_io_isolated_from_vcpu_and_quiesced_for_snapshot() { 15_000, ) }); - wait_for_delayed_service(); + wait_for_delayed_service(&controller, 1); let pause_started = Instant::now(); controller @@ -95,7 +99,7 @@ fn delayed_volume_io_isolated_from_vcpu_and_quiesced_for_snapshot() { 15_000, ) }); - wait_for_delayed_service(); + wait_for_delayed_service(&controller, 1); let snapshot_started = Instant::now(); let snapshot = controller .snapshot(false) @@ -164,7 +168,7 @@ fn storage_quiescence_timeout_fails_snapshot_and_resumes_source() { 20_000, ) }); - wait_for_delayed_service(); + wait_for_delayed_service(&controller, 1); let snapshot_started = Instant::now(); let error = controller From a17581de25545564c613320299b23561c6836e84 Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Tue, 1 Sep 2026 01:52:21 +0530 Subject: [PATCH 14/18] Make resume liveness output exact --- vmm/crates/vmm-integration/blk_io_isolation.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vmm/crates/vmm-integration/blk_io_isolation.rs b/vmm/crates/vmm-integration/blk_io_isolation.rs index 681927a..97dccd0 100644 --- a/vmm/crates/vmm-integration/blk_io_isolation.rs +++ b/vmm/crates/vmm-integration/blk_io_isolation.rs @@ -187,8 +187,8 @@ fn storage_quiescence_timeout_fails_snapshot_and_resumes_source() { ); assert_eq!( - guest_stdout(&controller, "printf source-resumed"), - "source-resumed", + guest_stdout(&controller, "echo source-resumed"), + "source-resumed\n", "snapshot failure left the source vCPU paused" ); let writer_result = writer.join().expect("join delayed writer"); From da03e3100d109b0b36a77d65aa6205d2173683e0 Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Tue, 1 Sep 2026 02:03:41 +0530 Subject: [PATCH 15/18] Preserve in-flight storage after snapshot timeout --- vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs | 6 ++++-- vmm/crates/vmm-integration/blk_io_isolation.rs | 11 ++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs b/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs index b8c0d83..1520fff 100644 --- a/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs +++ b/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs @@ -63,9 +63,11 @@ impl BlkIoLoop { )); } if std::time::Instant::now() >= deadline { + // A slow backing operation can legitimately outlive the + // snapshot quiescence budget. Abort this capture boundary, + // but leave the healthy worker and its in-flight descriptor + // intact so the running source can finish the request. self.pause_req.store(false, Ordering::SeqCst); - self.device - .fail_worker("block I/O worker quiescence timed out"); return Err(io::Error::new( io::ErrorKind::TimedOut, "block I/O worker quiescence timed out", diff --git a/vmm/crates/vmm-integration/blk_io_isolation.rs b/vmm/crates/vmm-integration/blk_io_isolation.rs index 97dccd0..e79f40b 100644 --- a/vmm/crates/vmm-integration/blk_io_isolation.rs +++ b/vmm/crates/vmm-integration/blk_io_isolation.rs @@ -191,10 +191,11 @@ fn storage_quiescence_timeout_fails_snapshot_and_resumes_source() { "source-resumed\n", "snapshot failure left the source vCPU paused" ); - let writer_result = writer.join().expect("join delayed writer"); - if let Ok((code, stdout, stderr, _)) = writer_result { - assert_eq!(code, 0, "delayed writer failed: {stderr}"); - assert_eq!(stdout, "delayed-write"); - } + let (code, stdout, stderr, _) = writer + .join() + .expect("join delayed writer") + .expect("delayed writer exec"); + assert_eq!(code, 0, "delayed writer failed: {stderr}"); + assert_eq!(stdout, "delayed-write"); controller.stop().expect("stop VM after quiescence timeout"); } From 74f64245a8389317f5aeaaff124a07b42a13e87d Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Tue, 1 Sep 2026 02:14:06 +0530 Subject: [PATCH 16/18] Retain suspend gate failure diagnostics --- orch/tests/e2e_suspend_resume.sh | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/orch/tests/e2e_suspend_resume.sh b/orch/tests/e2e_suspend_resume.sh index a2b1968..13428f4 100755 --- a/orch/tests/e2e_suspend_resume.sh +++ b/orch/tests/e2e_suspend_resume.sh @@ -65,6 +65,7 @@ BASE_URL="http://127.0.0.1:$PORT" mkdir -p "$DIR/sockets" cleanup() { + local status=$? if [ -n "${TARITD_PGID:-}" ] && kill -0 -- "-$TARITD_PGID" 2>/dev/null; then kill -TERM -- "-$TARITD_PGID" 2>/dev/null || true for _ in $(seq 1 50); do @@ -78,7 +79,16 @@ cleanup() { if [ -n "${TARITD_PID:-}" ]; then wait "$TARITD_PID" 2>/dev/null || true fi - rm -rf -- "$DIR" + if [ "$status" -ne 0 ]; then + echo "FAIL: suspend/resume gate exited $status" >&2 + tail -240 "$DIR/taritd.log" 2>/dev/null || true + fi + if [ "$status" -ne 0 ] && [ "${TARIT_E2E_KEEP_FAILED:-0}" = 1 ]; then + echo "FAIL: retained diagnostic directory: $DIR" >&2 + else + find "$DIR" -depth -delete 2>/dev/null || true + fi + return "$status" } trap cleanup EXIT trap 'exit 130' INT @@ -198,7 +208,16 @@ ACTUAL_PGID=$(ps -o pgid= -p "$TARITD_PID" | tr -d ' ') } echo "== create and populate guest memory ==" -VM_JSON=$(api -H 'Content-Type: application/json' -d '{"vcpus":1,"memory_mib":512}' "$BASE_URL/v1/vms") +CREATE_BODY="$DIR/create-vm.json" +CREATE_CODE=$(curl -sS --max-time 30 -o "$CREATE_BODY" -w '%{http_code}' \ + -H "X-API-Key: $KEY" -H 'Content-Type: application/json' \ + -d '{"vcpus":1,"memory_mib":512}' "$BASE_URL/v1/vms") +if [ "$CREATE_CODE" != 201 ]; then + echo "FAIL: VM create returned HTTP $CREATE_CODE" >&2 + sed -n '1,120p' "$CREATE_BODY" >&2 + exit 1 +fi +VM_JSON=$(<"$CREATE_BODY") VM_ID=$(printf '%s' "$VM_JSON" | json_field id) printf '%s' "$VM_JSON" | grep -q '"status":"running"' VMM_PID=$(vmm_pid_for_socket "$DIR/sockets/$VM_ID.sock") From e0540ba0c2978067bdcef2d7d78d60b09a461e87 Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Tue, 1 Sep 2026 03:04:40 +0530 Subject: [PATCH 17/18] Exercise suspend transitions through the CLI --- orch/tests/e2e_suspend_resume.sh | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/orch/tests/e2e_suspend_resume.sh b/orch/tests/e2e_suspend_resume.sh index 13428f4..81ebead 100755 --- a/orch/tests/e2e_suspend_resume.sh +++ b/orch/tests/e2e_suspend_resume.sh @@ -16,6 +16,7 @@ MAX_RESUME_EXEC_MS="${SUSPEND_RESUME_EXEC_MAX_MS:-5000}" EXPECTED_KERNEL_PREFIX="${TARIT_EXPECT_KERNEL_RELEASE_PREFIX:-}" EXPECTED_OS_ID="${TARIT_EXPECT_OS_ID:-}" ENABLE_NET="${TARIT_TEST_ENABLE_NET:-0}" +TRANSITION_CLIENT="${TARIT_TEST_TRANSITION_CLIENT:-api}" [[ "$EXPECTED_KERNEL_PREFIX" != *[[:space:]]* ]] || { echo "FAIL: TARIT_EXPECT_KERNEL_RELEASE_PREFIX must not contain whitespace" >&2 @@ -29,6 +30,10 @@ ENABLE_NET="${TARIT_TEST_ENABLE_NET:-0}" echo "FAIL: TARIT_TEST_ENABLE_NET must be 0 or 1" >&2 exit 1 } +[[ "$TRANSITION_CLIENT" = api || "$TRANSITION_CLIENT" = cli ]] || { + echo "FAIL: TARIT_TEST_TRANSITION_CLIENT must be api or cli" >&2 + exit 1 +} for required in curl python3 setsid ps awk; do command -v "$required" >/dev/null || { @@ -98,6 +103,17 @@ api() { curl -fsS --max-time 30 -H "X-API-Key: $KEY" "$@" } +vm_transition() { + local action=$1 + if [ "$TRANSITION_CLIENT" = cli ]; then + TARIT_BASE_URL="$BASE_URL" TARIT_API_KEY="$KEY" \ + "$TARITD" --json vm "$action" "$VM_ID" + else + api -H 'Content-Type: application/json' -d '{}' \ + "$BASE_URL/v1/vms/$VM_ID/$action" + fi +} + json_field() { python3 -c 'import json,sys; print(json.load(sys.stdin)[sys.argv[1]])' "$1" } @@ -276,7 +292,7 @@ printf '%s' "$PREP" | grep -q '"exit_code":0' RSS_BEFORE=$(rss_kib "$VMM_PID") echo "== suspend and verify resource contract ==" -SUSPENDED=$(api -H 'Content-Type: application/json' -d '{}' "$BASE_URL/v1/vms/$VM_ID/suspend") +SUSPENDED=$(vm_transition suspend) printf '%s' "$SUSPENDED" | grep -q '"status":"suspended"' if [ "$ENABLE_NET" = 1 ]; then ip link show "$NET_TAP" >/dev/null @@ -309,7 +325,7 @@ CREATE_CODE=$(curl -sS --max-time 10 -o "$DIR/suspended-create.json" -w '%{http_ echo "== resume, first exec, and verify preserved state ==" START_MS=$(monotonic_ms) -RESUMED=$(api -H 'Content-Type: application/json' -d '{}' "$BASE_URL/v1/vms/$VM_ID/resume") +RESUMED=$(vm_transition resume) printf '%s' "$RESUMED" | grep -q '"status":"running"' if [ "$ENABLE_NET" = 1 ]; then ip link show "$NET_TAP" >/dev/null @@ -338,20 +354,20 @@ RESUME_EXEC_MS=$((END_MS - START_MS)) echo "== repeated transitions are idempotent ==" for _ in 1 2; do - api -H 'Content-Type: application/json' -d '{}' "$BASE_URL/v1/vms/$VM_ID/suspend" | grep -q '"status":"suspended"' + vm_transition suspend | grep -q '"status":"suspended"' done for _ in 1 2; do - api -H 'Content-Type: application/json' -d '{}' "$BASE_URL/v1/vms/$VM_ID/resume" | grep -q '"status":"running"' + vm_transition resume | grep -q '"status":"running"' done exec_json "$VM_ID" 'cat /mnt/tarit-rss/state' | grep -q 'suspend-state-ok' echo "== rapid suspend/resume transitions preserve worker handshakes ==" for cycle in $(seq 1 20); do - api -H 'Content-Type: application/json' -d '{}' "$BASE_URL/v1/vms/$VM_ID/suspend" | grep -q '"status":"suspended"' + vm_transition suspend | grep -q '"status":"suspended"' if [ "$ENABLE_NET" = 1 ]; then ip link show "$NET_TAP" >/dev/null fi - api -H 'Content-Type: application/json' -d '{}' "$BASE_URL/v1/vms/$VM_ID/resume" | grep -q '"status":"running"' + vm_transition resume | grep -q '"status":"running"' exec_json "$VM_ID" "printf rapid-cycle-$cycle" | grep -q "rapid-cycle-$cycle" done exec_json "$VM_ID" 'cat /mnt/tarit-rss/state' | grep -q 'suspend-state-ok' @@ -368,4 +384,4 @@ if [ "$ENABLE_NET" = 1 ]; then exit 1 fi fi -echo "RESULT: SUSPEND_PASS rss_before_kib=$RSS_BEFORE rss_after_kib=$RSS_AFTER rss_drop_kib=$RSS_DROP resume_first_exec_ms=$RESUME_EXEC_MS" +echo "RESULT: SUSPEND_PASS transition_client=$TRANSITION_CLIENT rss_before_kib=$RSS_BEFORE rss_after_kib=$RSS_AFTER rss_drop_kib=$RSS_DROP resume_first_exec_ms=$RESUME_EXEC_MS" From 3c9ca2ecaffd5668b5977b6b988bd418c049fdc5 Mon Sep 17 00:00:00 2001 From: Abhishek Anand Date: Tue, 1 Sep 2026 04:14:37 +0530 Subject: [PATCH 18/18] Preserve worker startup diagnostics --- vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs | 9 ++++++--- vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs | 9 ++++++--- vmm/crates/vmm-integration/blk_io_isolation.rs | 2 +- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs b/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs index 1520fff..d0e752a 100644 --- a/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs +++ b/vmm/crates/vmm-devices/src/virtio/blk_io_loop.rs @@ -139,9 +139,10 @@ pub fn spawn_blk_io_loop(device: Arc, kick_fd: RawFd) -> io::Resu // SAFETY: F_GETFD inspects the descriptor without retaining it. The // controller owns the descriptor for the returned worker's lifetime. if unsafe { libc::fcntl(kick_fd, libc::F_GETFD) } < 0 { + let source = io::Error::last_os_error(); return Err(io::Error::new( - io::Error::last_os_error().kind(), - "block queue kick descriptor is invalid", + source.kind(), + format!("block queue kick descriptor is invalid: {source}"), )); } let stop = Arc::new(AtomicBool::new(false)); @@ -306,7 +307,9 @@ mod tests { } Err(error) => error, }; - assert!(error.to_string().contains("descriptor is invalid")); + let message = error.to_string(); + assert!(message.contains("descriptor is invalid")); + assert!(message.contains(&io::Error::from_raw_os_error(libc::EBADF).to_string())); } #[test] diff --git a/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs b/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs index fcfa5a9..2ea8118 100644 --- a/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs +++ b/vmm/crates/vmm-devices/src/virtio/vsock_io_loop.rs @@ -148,9 +148,10 @@ pub fn spawn_vsock_pump(device: Arc, tx_kick_fd: RawFd) -> io:: // SAFETY: F_GETFD inspects the descriptor without retaining it. The caller // owns the descriptor for the lifetime of the returned worker. if unsafe { libc::fcntl(tx_kick_fd, libc::F_GETFD) } < 0 { + let source = io::Error::last_os_error(); return Err(io::Error::new( - io::Error::last_os_error().kind(), - "vsock queue kick descriptor is invalid", + source.kind(), + format!("vsock queue kick descriptor is invalid: {source}"), )); } let stop = Arc::new(AtomicBool::new(false)); @@ -346,7 +347,9 @@ mod tests { } Err(error) => error, }; - assert!(error.to_string().contains("descriptor is invalid")); + let message = error.to_string(); + assert!(message.contains("descriptor is invalid")); + assert!(message.contains(&io::Error::from_raw_os_error(libc::EBADF).to_string())); } #[test] diff --git a/vmm/crates/vmm-integration/blk_io_isolation.rs b/vmm/crates/vmm-integration/blk_io_isolation.rs index e79f40b..21ad7b6 100644 --- a/vmm/crates/vmm-integration/blk_io_isolation.rs +++ b/vmm/crates/vmm-integration/blk_io_isolation.rs @@ -182,7 +182,7 @@ fn storage_quiescence_timeout_fails_snapshot_and_resumes_source() { "unexpected snapshot failure: {error}" ); assert!( - (Duration::from_secs(4)..Duration::from_secs(6)).contains(&snapshot_elapsed), + (Duration::from_millis(4_800)..Duration::from_secs(6)).contains(&snapshot_elapsed), "block-worker timeout was not bounded at five seconds: {snapshot_elapsed:?}" );