From ad6c0ee52be40413aaa3032bc6758542ae06c2cd Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Thu, 12 Jan 2023 10:07:55 +0100 Subject: [PATCH] virtio-devices: properly join all threads on Drop This change is important to do a proper resource cleanup. We decided to do this repetitive approach as VirtioCommon can't implement Drop without major changes to the corresponding code. Also, devices such as Net can't easily use the epoll_threads-abstraction from VirtioCommon as it has multiple threads with different semantics. Signed-off-by: Philipp Schuster --- virtio-devices/src/balloon.rs | 1 + virtio-devices/src/block.rs | 1 + virtio-devices/src/console.rs | 1 + virtio-devices/src/iommu.rs | 1 + virtio-devices/src/mem.rs | 1 + virtio-devices/src/net.rs | 5 +++++ virtio-devices/src/pmem.rs | 1 + virtio-devices/src/rng.rs | 1 + virtio-devices/src/vhost_user/blk.rs | 6 ++++++ virtio-devices/src/vhost_user/fs.rs | 6 ++++++ virtio-devices/src/vhost_user/net.rs | 13 +++++++++++++ virtio-devices/src/vsock/device.rs | 1 + virtio-devices/src/watchdog.rs | 1 + 13 files changed, 39 insertions(+) diff --git a/virtio-devices/src/balloon.rs b/virtio-devices/src/balloon.rs index 266c30c1e..e8e554b3c 100644 --- a/virtio-devices/src/balloon.rs +++ b/virtio-devices/src/balloon.rs @@ -450,6 +450,7 @@ impl Drop for Balloon { // Ignore the result because there is nothing we can do about it. let _ = kill_evt.write(1); } + self.common.wait_for_epoll_threads(); } } diff --git a/virtio-devices/src/block.rs b/virtio-devices/src/block.rs index bb1912125..7df75b4b9 100644 --- a/virtio-devices/src/block.rs +++ b/virtio-devices/src/block.rs @@ -658,6 +658,7 @@ impl Drop for Block { // Ignore the result because there is nothing we can do about it. let _ = kill_evt.write(1); } + self.common.wait_for_epoll_threads(); } } diff --git a/virtio-devices/src/console.rs b/virtio-devices/src/console.rs index eb64b0ed6..fddbe247c 100644 --- a/virtio-devices/src/console.rs +++ b/virtio-devices/src/console.rs @@ -710,6 +710,7 @@ impl Drop for Console { // Ignore the result because there is nothing we can do about it. let _ = kill_evt.write(1); } + self.common.wait_for_epoll_threads(); } } diff --git a/virtio-devices/src/iommu.rs b/virtio-devices/src/iommu.rs index 6c09105a1..206e4564b 100644 --- a/virtio-devices/src/iommu.rs +++ b/virtio-devices/src/iommu.rs @@ -1000,6 +1000,7 @@ impl Drop for Iommu { // Ignore the result because there is nothing we can do about it. let _ = kill_evt.write(1); } + self.common.wait_for_epoll_threads(); } } diff --git a/virtio-devices/src/mem.rs b/virtio-devices/src/mem.rs index 1a7ea6f1f..a03f452a6 100644 --- a/virtio-devices/src/mem.rs +++ b/virtio-devices/src/mem.rs @@ -902,6 +902,7 @@ impl Drop for Mem { // Ignore the result because there is nothing we can do about it. let _ = kill_evt.write(1); } + self.common.wait_for_epoll_threads(); } } diff --git a/virtio-devices/src/net.rs b/virtio-devices/src/net.rs index 07bf9d6fd..f92c5bb26 100644 --- a/virtio-devices/src/net.rs +++ b/virtio-devices/src/net.rs @@ -670,6 +670,11 @@ impl Drop for Net { } // Needed to ensure all references to tap FDs are dropped (#4868) self.common.wait_for_epoll_threads(); + if let Some(thread) = self.ctrl_queue_epoll_thread.take() { + if let Err(e) = thread.join() { + error!("Error joining thread: {:?}", e); + } + } } } diff --git a/virtio-devices/src/pmem.rs b/virtio-devices/src/pmem.rs index c06b70017..c5f550a87 100644 --- a/virtio-devices/src/pmem.rs +++ b/virtio-devices/src/pmem.rs @@ -364,6 +364,7 @@ impl Drop for Pmem { // Ignore the result because there is nothing we can do about it. let _ = kill_evt.write(1); } + self.common.wait_for_epoll_threads(); } } diff --git a/virtio-devices/src/rng.rs b/virtio-devices/src/rng.rs index d56089167..18add7176 100644 --- a/virtio-devices/src/rng.rs +++ b/virtio-devices/src/rng.rs @@ -228,6 +228,7 @@ impl Drop for Rng { // Ignore the result because there is nothing we can do about it. let _ = kill_evt.write(1); } + self.common.wait_for_epoll_threads(); } } diff --git a/virtio-devices/src/vhost_user/blk.rs b/virtio-devices/src/vhost_user/blk.rs index 47a493cf7..43608f3e1 100644 --- a/virtio-devices/src/vhost_user/blk.rs +++ b/virtio-devices/src/vhost_user/blk.rs @@ -223,6 +223,12 @@ impl Drop for Blk { error!("failed to kill vhost-user-blk: {:?}", e); } } + self.common.wait_for_epoll_threads(); + if let Some(thread) = self.epoll_thread.take() { + if let Err(e) = thread.join() { + error!("Error joining thread: {:?}", e); + } + } } } diff --git a/virtio-devices/src/vhost_user/fs.rs b/virtio-devices/src/vhost_user/fs.rs index f2cd6af52..74a1e37ee 100644 --- a/virtio-devices/src/vhost_user/fs.rs +++ b/virtio-devices/src/vhost_user/fs.rs @@ -462,6 +462,12 @@ impl Drop for Fs { // Ignore the result because there is nothing we can do about it. let _ = kill_evt.write(1); } + self.common.wait_for_epoll_threads(); + if let Some(thread) = self.epoll_thread.take() { + if let Err(e) = thread.join() { + error!("Error joining thread: {:?}", e); + } + } } } diff --git a/virtio-devices/src/vhost_user/net.rs b/virtio-devices/src/vhost_user/net.rs index 80bf1dd46..74d687b93 100644 --- a/virtio-devices/src/vhost_user/net.rs +++ b/virtio-devices/src/vhost_user/net.rs @@ -251,6 +251,19 @@ impl Drop for Net { error!("failed to kill vhost-user-net: {:?}", e); } } + + self.common.wait_for_epoll_threads(); + + if let Some(thread) = self.epoll_thread.take() { + if let Err(e) = thread.join() { + error!("Error joining thread: {:?}", e); + } + } + if let Some(thread) = self.ctrl_queue_epoll_thread.take() { + if let Err(e) = thread.join() { + error!("Error joining thread: {:?}", e); + } + } } } diff --git a/virtio-devices/src/vsock/device.rs b/virtio-devices/src/vsock/device.rs index 057760c78..b0d425ec3 100644 --- a/virtio-devices/src/vsock/device.rs +++ b/virtio-devices/src/vsock/device.rs @@ -397,6 +397,7 @@ where // Ignore the result because there is nothing we can do about it. let _ = kill_evt.write(1); } + self.common.wait_for_epoll_threads(); } } diff --git a/virtio-devices/src/watchdog.rs b/virtio-devices/src/watchdog.rs index aec42edc0..546a6f302 100644 --- a/virtio-devices/src/watchdog.rs +++ b/virtio-devices/src/watchdog.rs @@ -278,6 +278,7 @@ impl Drop for Watchdog { // Ignore the result because there is nothing we can do about it. let _ = kill_evt.write(1); } + self.common.wait_for_epoll_threads(); } }