From 1adfb7e9f83cdc77fdb092f480b0e753f9b10e8f 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 (cherry picked from commit ad6c0ee52be40413aaa3032bc6758542ae06c2cd) --- 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 eb0823f62..f9d773a77 100644 --- a/virtio-devices/src/balloon.rs +++ b/virtio-devices/src/balloon.rs @@ -443,6 +443,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 466395524..4387fb57f 100644 --- a/virtio-devices/src/block.rs +++ b/virtio-devices/src/block.rs @@ -581,6 +581,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 378f3bbc8..bb7c03a98 100644 --- a/virtio-devices/src/console.rs +++ b/virtio-devices/src/console.rs @@ -705,6 +705,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 3dbbc1dbb..adf745374 100644 --- a/virtio-devices/src/iommu.rs +++ b/virtio-devices/src/iommu.rs @@ -985,6 +985,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 b8df227a0..c4c62a187 100644 --- a/virtio-devices/src/mem.rs +++ b/virtio-devices/src/mem.rs @@ -898,6 +898,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 7ce6e0d85..5b5516045 100644 --- a/virtio-devices/src/net.rs +++ b/virtio-devices/src/net.rs @@ -633,6 +633,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 2ab3e022d..7c76f7ab6 100644 --- a/virtio-devices/src/pmem.rs +++ b/virtio-devices/src/pmem.rs @@ -358,6 +358,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 2992969f2..e47623c71 100644 --- a/virtio-devices/src/rng.rs +++ b/virtio-devices/src/rng.rs @@ -227,6 +227,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 e67568b80..8d659e41d 100644 --- a/virtio-devices/src/vhost_user/blk.rs +++ b/virtio-devices/src/vhost_user/blk.rs @@ -215,6 +215,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 b2a07d04a..69d736cf3 100644 --- a/virtio-devices/src/vhost_user/fs.rs +++ b/virtio-devices/src/vhost_user/fs.rs @@ -451,6 +451,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 177adbd59..66a0a6cd2 100644 --- a/virtio-devices/src/vhost_user/net.rs +++ b/virtio-devices/src/vhost_user/net.rs @@ -231,6 +231,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 666c0e2ed..ff834f8d2 100644 --- a/virtio-devices/src/vsock/device.rs +++ b/virtio-devices/src/vsock/device.rs @@ -396,6 +396,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 105a8f788..9d0e362bb 100644 --- a/virtio-devices/src/watchdog.rs +++ b/virtio-devices/src/watchdog.rs @@ -276,6 +276,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(); } }