diff --git a/virtio-devices/src/balloon.rs b/virtio-devices/src/balloon.rs index 37ebb056f..74b83dff2 100644 --- a/virtio-devices/src/balloon.rs +++ b/virtio-devices/src/balloon.rs @@ -458,7 +458,6 @@ pub struct Balloon { config: VirtioBalloonConfig, seccomp_action: SeccompAction, exit_evt: EventFd, - interrupt_cb: Option>, } impl Balloon { @@ -523,20 +522,15 @@ impl Balloon { config, seccomp_action, exit_evt, - interrupt_cb: None, }) } pub fn resize(&mut self, size: u64) -> Result<(), Error> { self.config.num_pages = (size >> VIRTIO_BALLOON_PFN_SHIFT) as u32; - if let Some(interrupt_cb) = &self.interrupt_cb { - interrupt_cb - .trigger(VirtioInterruptType::Config) - .map_err(Error::FailedSignal) - } else { - Ok(()) - } + self.common + .trigger_interrupt(VirtioInterruptType::Config) + .map_err(Error::FailedSignal) } // Get the actual size of the virtio-balloon. @@ -647,8 +641,6 @@ impl VirtioDevice for Balloon { None }; - self.interrupt_cb = Some(interrupt_cb.clone()); - let mut handler = BalloonEpollHandler { mem, queues: virtqueues, @@ -688,10 +680,9 @@ impl VirtioDevice for Balloon { self.common.access_platform() } - fn reset(&mut self) -> Option> { - let result = self.common.reset(); + fn reset(&mut self) { + self.common.reset(); event!("virtio-device", "reset", "id", &self.id); - result } } diff --git a/virtio-devices/src/block.rs b/virtio-devices/src/block.rs index 5765d5a39..d37db6cd8 100644 --- a/virtio-devices/src/block.rs +++ b/virtio-devices/src/block.rs @@ -1025,13 +1025,9 @@ impl Block { self.common.resume().map_err(Error::ResumeVcpus)?; - if let Some(interrupt_cb) = self.common.interrupt_cb.as_ref() { - interrupt_cb - .trigger(VirtioInterruptType::Config) - .map_err(Error::ConfigChange) - } else { - Ok(()) - } + self.common + .trigger_interrupt(VirtioInterruptType::Config) + .map_err(Error::ConfigChange) } #[cfg(fuzzing)] @@ -1178,11 +1174,10 @@ impl VirtioDevice for Block { Ok(()) } - fn reset(&mut self) -> Option> { - let result = self.common.reset(); + fn reset(&mut self) { + self.common.reset(); self.set_writeback_mode(true); event!("virtio-device", "reset", "id", &self.id); - result } fn counters(&self) -> Option>> { diff --git a/virtio-devices/src/console.rs b/virtio-devices/src/console.rs index 0a4cf65cf..180be8dfa 100644 --- a/virtio-devices/src/console.rs +++ b/virtio-devices/src/console.rs @@ -773,10 +773,9 @@ impl VirtioDevice for Console { Ok(()) } - fn reset(&mut self) -> Option> { - let result = self.common.reset(); + fn reset(&mut self) { + self.common.reset(); event!("virtio-device", "reset", "id", &self.id); - result } fn set_access_platform(&mut self, access_platform: Arc) { diff --git a/virtio-devices/src/device.rs b/virtio-devices/src/device.rs index 84f064259..2349db6b5 100644 --- a/virtio-devices/src/device.rs +++ b/virtio-devices/src/device.rs @@ -123,11 +123,8 @@ pub trait VirtioDevice: Send { /// Activates this device for real usage. fn activate(&mut self, context: ActivationContext) -> ActivateResult; - /// Optionally deactivates this device and returns ownership of the guest memory map, interrupt - /// event, and queue events. - fn reset(&mut self) -> Option> { - None - } + /// Optionally deactivates this device. + fn reset(&mut self) {} /// Returns the list of shared memory regions required by the device. fn get_shm_regions(&self) -> Option { @@ -290,7 +287,7 @@ impl VirtioCommon { Ok(()) } - pub fn reset(&mut self) -> Option> { + pub fn reset(&mut self) { self.queue_evts.clear(); // Resume the virtio thread if it was paused. Reset must always @@ -315,8 +312,16 @@ impl VirtioCommon { } } - // Return the interrupt - Some(self.interrupt_cb.take().unwrap()) + // Drop the interrupt callback clone + self.interrupt_cb = None; + } + + pub fn trigger_interrupt(&self, int_type: VirtioInterruptType) -> std::io::Result<()> { + if let Some(interrupt_cb) = &self.interrupt_cb { + interrupt_cb.trigger(int_type) + } else { + Ok(()) + } } // Wait for the worker thread to finish and return @@ -406,12 +411,9 @@ impl Pausable for VirtioCommon { } // Also trigger interrupts into the guest to wake up the driver to avoid a "livelock" - if let Some(interrupt_cb) = &self.interrupt_cb { - for i in 0..self.queue_evts.len() { - interrupt_cb - .trigger(crate::VirtioInterruptType::Queue(i as u16)) - .ok(); - } + for i in 0..self.queue_evts.len() { + self.trigger_interrupt(crate::VirtioInterruptType::Queue(i as u16)) + .ok(); } Ok(()) diff --git a/virtio-devices/src/iommu.rs b/virtio-devices/src/iommu.rs index 513d510b5..bc03e6775 100644 --- a/virtio-devices/src/iommu.rs +++ b/virtio-devices/src/iommu.rs @@ -1120,10 +1120,9 @@ impl VirtioDevice for Iommu { Ok(()) } - fn reset(&mut self) -> Option> { - let result = self.common.reset(); + fn reset(&mut self) { + self.common.reset(); event!("virtio-device", "reset", "id", &self.id); - result } } diff --git a/virtio-devices/src/mem.rs b/virtio-devices/src/mem.rs index 727fd72c0..afc168a6e 100644 --- a/virtio-devices/src/mem.rs +++ b/virtio-devices/src/mem.rs @@ -737,7 +737,6 @@ pub struct Mem { dma_mapping_handlers: Arc>>>, blocks_state: Arc>, exit_evt: EventFd, - interrupt_cb: Option>, } impl Mem { @@ -830,7 +829,6 @@ impl Mem { dma_mapping_handlers: Arc::new(Mutex::new(BTreeMap::new())), blocks_state, exit_evt, - interrupt_cb: None, }) } @@ -844,15 +842,11 @@ impl Mem { Error::ResizeError(anyhow!("Failed to update virtio configuration: {e:?}")) })?; - if let Some(interrupt_cb) = self.interrupt_cb.as_ref() { - interrupt_cb - .trigger(VirtioInterruptType::Config) - .map_err(|e| { - Error::ResizeError(anyhow!("Failed to signal the guest about resize: {e:?}")) - }) - } else { - Ok(()) - } + self.common + .trigger_interrupt(VirtioInterruptType::Config) + .map_err(|e| { + Error::ResizeError(anyhow!("Failed to signal the guest about resize: {e:?}")) + }) } pub fn add_dma_mapping_handler( @@ -966,8 +960,6 @@ impl VirtioDevice for Mem { let (_, queue, queue_evt) = queues.remove(0); - self.interrupt_cb = Some(interrupt_cb.clone()); - let mut handler = MemEpollHandler { mem, region: self.region.clone(), @@ -1016,10 +1008,9 @@ impl VirtioDevice for Mem { Ok(()) } - fn reset(&mut self) -> Option> { - let result = self.common.reset(); + fn reset(&mut self) { + self.common.reset(); event!("virtio-device", "reset", "id", &self.id); - result } } diff --git a/virtio-devices/src/net.rs b/virtio-devices/src/net.rs index d7e1d1f36..13c64b270 100644 --- a/virtio-devices/src/net.rs +++ b/virtio-devices/src/net.rs @@ -868,10 +868,9 @@ impl VirtioDevice for Net { Ok(()) } - fn reset(&mut self) -> Option> { - let result = self.common.reset(); + fn reset(&mut self) { + self.common.reset(); event!("virtio-device", "reset", "id", &self.id); - result } fn counters(&self) -> Option>> { diff --git a/virtio-devices/src/pmem.rs b/virtio-devices/src/pmem.rs index 80d41ad5d..e0ec26531 100644 --- a/virtio-devices/src/pmem.rs +++ b/virtio-devices/src/pmem.rs @@ -438,10 +438,9 @@ impl VirtioDevice for Pmem { Err(ActivateError::BadActivate) } - fn reset(&mut self) -> Option> { - let result = self.common.reset(); + fn reset(&mut self) { + self.common.reset(); event!("virtio-device", "reset", "id", &self.id); - result } fn userspace_mappings(&self) -> Vec { diff --git a/virtio-devices/src/rng.rs b/virtio-devices/src/rng.rs index 9d86cf7d8..b639cc192 100644 --- a/virtio-devices/src/rng.rs +++ b/virtio-devices/src/rng.rs @@ -311,10 +311,9 @@ impl VirtioDevice for Rng { Err(ActivateError::BadActivate) } - fn reset(&mut self) -> Option> { - let result = self.common.reset(); + fn reset(&mut self) { + self.common.reset(); event!("virtio-device", "reset", "id", &self.id); - result } fn set_access_platform(&mut self, access_platform: Arc) { diff --git a/virtio-devices/src/transport/pci_device.rs b/virtio-devices/src/transport/pci_device.rs index fe3cf7105..41743e419 100644 --- a/virtio-devices/src/transport/pci_device.rs +++ b/virtio-devices/src/transport/pci_device.rs @@ -300,7 +300,7 @@ pub struct VirtioPciDeviceState { } pub struct VirtioPciDeviceActivator { - interrupt: Option>, + interrupt: Arc, memory: Option>, device: Arc>, device_activated: Arc, @@ -315,7 +315,7 @@ impl VirtioPciDeviceActivator { let mut locked_device = self.device.lock().unwrap(); locked_device.activate(crate::device::ActivationContext { mem: self.memory.take().unwrap(), - interrupt_cb: self.interrupt.take().unwrap(), + interrupt_cb: self.interrupt, queues: self.queues.take().unwrap(), device_status: self.status, })?; @@ -822,7 +822,7 @@ impl VirtioPciDevice { } VirtioPciDeviceActivator { - interrupt: self.virtio_interrupt.take(), + interrupt: self.virtio_interrupt.as_ref().unwrap().clone(), memory: Some(self.memory.clone()), device: self.device.clone(), queues: Some(queues), @@ -1250,10 +1250,7 @@ impl PciDevice for VirtioPciDevice { if self.is_driver_init() { if self.device_activated.swap(false, Ordering::SeqCst) { let mut device = self.device.lock().unwrap(); - if let Some(virtio_interrupt) = device.reset() { - // Upon reset the device returns its interrupt EventFD - self.virtio_interrupt = Some(virtio_interrupt); - } + device.reset(); } // Reset queue readiness and the common configuration diff --git a/virtio-devices/src/vdpa.rs b/virtio-devices/src/vdpa.rs index e1cf48d9a..44b5cfde8 100644 --- a/virtio-devices/src/vdpa.rs +++ b/virtio-devices/src/vdpa.rs @@ -443,14 +443,13 @@ impl VirtioDevice for Vdpa { self.activate_vdpa(&mem.memory(), virtio_interrupt.as_ref(), &queues) .map_err(ActivateError::ActivateVdpa)?; - // Store the virtio interrupt handler as we need to return it on reset self.common.interrupt_cb = Some(virtio_interrupt); event!("vdpa", "activated", "id", &self.id); Ok(()) } - fn reset(&mut self) -> Option> { + fn reset(&mut self) { // Backend reset failures are logged but don't skip local cleanup: // reset must converge to fresh state regardless of backend state. if let Err(e) = self.reset_vdpa() { @@ -459,8 +458,8 @@ impl VirtioDevice for Vdpa { event!("vdpa", "reset", "id", &self.id); - // Return the virtio interrupt handler - self.common.interrupt_cb.take() + // Drop the interrupt callback clone + self.common.interrupt_cb = None; } fn set_access_platform(&mut self, access_platform: Arc) { diff --git a/virtio-devices/src/vhost_user/blk.rs b/virtio-devices/src/vhost_user/blk.rs index 7e434dd9f..5b64bbb5a 100644 --- a/virtio-devices/src/vhost_user/blk.rs +++ b/virtio-devices/src/vhost_user/blk.rs @@ -28,7 +28,7 @@ use super::{DEFAULT_VIRTIO_FEATURES, Error, Result}; use crate::seccomp_filters::Thread; use crate::thread_helper::spawn_virtio_thread; use crate::vhost_user::{VhostUserCommon, VhostUserState}; -use crate::{GuestMemoryMmap, GuestRegionMmap, VIRTIO_F_ACCESS_PLATFORM, VirtioInterrupt}; +use crate::{GuestMemoryMmap, GuestRegionMmap, VIRTIO_F_ACCESS_PLATFORM}; const DEFAULT_QUEUE_NUMBER: usize = 1; @@ -307,8 +307,8 @@ impl VirtioDevice for Blk { Ok(()) } - fn reset(&mut self) -> Option> { - self.vu_common.reset(&self.id) + fn reset(&mut self) { + self.vu_common.reset(&self.id); } fn shutdown(&mut self) { diff --git a/virtio-devices/src/vhost_user/fs.rs b/virtio-devices/src/vhost_user/fs.rs index 60a60a86c..37c7c420e 100644 --- a/virtio-devices/src/vhost_user/fs.rs +++ b/virtio-devices/src/vhost_user/fs.rs @@ -25,7 +25,7 @@ use crate::thread_helper::spawn_virtio_thread; use crate::vhost_user::{VhostUserCommon, VhostUserState}; use crate::{ ActivateResult, GuestMemoryMmap, GuestRegionMmap, MmapRegion, VIRTIO_F_ACCESS_PLATFORM, - VirtioCommon, VirtioDevice, VirtioDeviceType, VirtioInterrupt, VirtioSharedMemoryList, + VirtioCommon, VirtioDevice, VirtioDeviceType, VirtioSharedMemoryList, }; const NUM_QUEUE_OFFSET: usize = 1; @@ -285,8 +285,8 @@ impl VirtioDevice for Fs { Ok(()) } - fn reset(&mut self) -> Option> { - self.vu_common.reset(&self.id) + fn reset(&mut self) { + self.vu_common.reset(&self.id); } fn shutdown(&mut self) { diff --git a/virtio-devices/src/vhost_user/generic_vhost_user.rs b/virtio-devices/src/vhost_user/generic_vhost_user.rs index 0d2aaf682..3ac01406b 100644 --- a/virtio-devices/src/vhost_user/generic_vhost_user.rs +++ b/virtio-devices/src/vhost_user/generic_vhost_user.rs @@ -26,7 +26,7 @@ use crate::thread_helper::spawn_virtio_thread; use crate::vhost_user::{VhostUserCommon, VhostUserState}; use crate::{ ActivateResult, GuestMemoryMmap, GuestRegionMmap, MmapRegion, VIRTIO_F_ACCESS_PLATFORM, - VirtioCommon, VirtioDevice, VirtioInterrupt, VirtioSharedMemoryList, + VirtioCommon, VirtioDevice, VirtioSharedMemoryList, }; pub type State = VhostUserState<()>; @@ -308,8 +308,8 @@ impl VirtioDevice for GenericVhostUser { Ok(()) } - fn reset(&mut self) -> Option> { - self.vu_common.reset(&self.id) + fn reset(&mut self) { + self.vu_common.reset(&self.id); } fn shutdown(&mut self) { diff --git a/virtio-devices/src/vhost_user/mod.rs b/virtio-devices/src/vhost_user/mod.rs index 07c8d4555..9dd3d7f79 100644 --- a/virtio-devices/src/vhost_user/mod.rs +++ b/virtio-devices/src/vhost_user/mod.rs @@ -347,7 +347,6 @@ pub struct VhostUserCommon { pub vu_num_queues: usize, pub migration_started: bool, pub server: bool, - pub interrupt_cb: Option>, pub vring_bases: Option>, pub epoll_thread: Option>, } @@ -395,8 +394,6 @@ impl VhostUserCommon { ) .map_err(ActivateError::VhostUserSetup)?; - self.interrupt_cb = Some(interrupt_cb.clone()); - Ok(VhostUserEpollHandler { vu: vu.clone(), mem, @@ -428,7 +425,7 @@ impl VhostUserCommon { Ok(()) } - pub fn reset(&mut self, id: &str) -> Option> { + pub fn reset(&mut self, id: &str) { // Resume the virtio thread if it was paused. Reset must always // converge to fresh state, so backend resume / reset failures are // logged but don't skip the rest of the teardown. @@ -454,8 +451,8 @@ impl VhostUserCommon { event!("virtio-device", "reset", "id", id); - // Return the interrupt - Some(self.virtio_common.interrupt_cb.take().unwrap()) + // Drop the interrupt callback clone + self.virtio_common.interrupt_cb = None; } pub fn shutdown(&mut self) { @@ -525,12 +522,10 @@ impl VhostUserCommon { MigratableError::Resume(anyhow!("Error resuming vhost-user backend: {e:?}")) })?; } - if let Some(interrupt_cb) = &self.interrupt_cb { - for i in 0..self.vu_num_queues { - interrupt_cb - .trigger(crate::VirtioInterruptType::Queue(i as u16)) - .ok(); - } + for i in 0..self.vu_num_queues { + self.virtio_common + .trigger_interrupt(crate::VirtioInterruptType::Queue(i as u16)) + .ok(); } Ok(()) } diff --git a/virtio-devices/src/vhost_user/net.rs b/virtio-devices/src/vhost_user/net.rs index 8563a2124..d77c80a51 100644 --- a/virtio-devices/src/vhost_user/net.rs +++ b/virtio-devices/src/vhost_user/net.rs @@ -29,7 +29,7 @@ use crate::vhost_user::vu_common_ctrl::{VhostUserConfig, VhostUserHandle}; use crate::vhost_user::{DEFAULT_VIRTIO_FEATURES, Error, Result, VhostUserCommon, VhostUserState}; use crate::{ ActivateResult, GuestMemoryMmap, GuestRegionMmap, NetCtrlEpollHandler, - VIRTIO_F_ACCESS_PLATFORM, VirtioCommon, VirtioDevice, VirtioDeviceType, VirtioInterrupt, + VIRTIO_F_ACCESS_PLATFORM, VirtioCommon, VirtioDevice, VirtioDeviceType, }; const DEFAULT_QUEUE_NUMBER: usize = 2; @@ -364,8 +364,8 @@ impl VirtioDevice for Net { Ok(()) } - fn reset(&mut self) -> Option> { - self.vu_common.reset(&self.id) + fn reset(&mut self) { + self.vu_common.reset(&self.id); } fn shutdown(&mut self) { diff --git a/virtio-devices/src/vsock/device.rs b/virtio-devices/src/vsock/device.rs index c20288c2d..1d5985037 100644 --- a/virtio-devices/src/vsock/device.rs +++ b/virtio-devices/src/vsock/device.rs @@ -488,10 +488,9 @@ where Ok(()) } - fn reset(&mut self) -> Option> { - let result = self.common.reset(); + fn reset(&mut self) { + self.common.reset(); event!("virtio-device", "reset", "id", &self.id); - result } fn shutdown(&mut self) { diff --git a/virtio-devices/src/watchdog.rs b/virtio-devices/src/watchdog.rs index 742a2e024..e362532e6 100644 --- a/virtio-devices/src/watchdog.rs +++ b/virtio-devices/src/watchdog.rs @@ -379,10 +379,9 @@ impl VirtioDevice for Watchdog { Ok(()) } - fn reset(&mut self) -> Option> { - let result = self.common.reset(); + fn reset(&mut self) { + self.common.reset(); event!("virtio-device", "reset", "id", &self.id); - result } }