From 167206dea396ca5aa6147ca60c7dd6d6c1677e21 Mon Sep 17 00:00:00 2001 From: Stefano Garzarella Date: Thu, 29 Sep 2022 14:50:00 +0200 Subject: [PATCH] vsock: try to avoid panic as much as we can Instead of panic, let's propagate errors or print warnings where we can't. Signed-off-by: Stefano Garzarella --- vsock/src/vhu_vsock.rs | 16 ++++++++++------ vsock/src/vhu_vsock_thread.rs | 23 +++++++++++++++++++---- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/vsock/src/vhu_vsock.rs b/vsock/src/vhu_vsock.rs index 44bc8ef..84d1026 100644 --- a/vsock/src/vhu_vsock.rs +++ b/vsock/src/vhu_vsock.rs @@ -78,6 +78,8 @@ pub(crate) enum Error { HandleUnknownEvent, #[error("Failed to accept new local socket connection")] UnixAccept(std::io::Error), + #[error("Failed to bind a unix stream")] + UnixBind(std::io::Error), #[error("Failed to create an epoll fd")] EpollFdCreate(std::io::Error), #[error("Failed to add to epoll")] @@ -118,6 +120,8 @@ pub(crate) enum Error { NoFreeLocalPort, #[error("Backend rx queue is empty")] EmptyBackendRxQ, + #[error("Failed to create an EventFd")] + EventFdCreate(std::io::Error), } impl std::convert::From for std::io::Error { @@ -217,17 +221,17 @@ pub struct VhostUserVsockBackend { impl VhostUserVsockBackend { pub(crate) fn new(vsock_config: VsockConfig) -> Result { - let thread = Mutex::new( - VhostUserVsockThread::new(vsock_config.get_uds_path(), vsock_config.get_guest_cid()) - .unwrap(), - ); + let thread = Mutex::new(VhostUserVsockThread::new( + vsock_config.get_uds_path(), + vsock_config.get_guest_cid(), + )?); let queues_per_thread = vec![QUEUE_MASK]; Ok(Self { guest_cid: vsock_config.get_guest_cid(), threads: vec![thread], queues_per_thread, - exit_event: EventFd::new(EFD_NONBLOCK).expect("Creating exit eventfd"), + exit_event: EventFd::new(EFD_NONBLOCK).map_err(Error::EventFdCreate)?, }) } } @@ -322,7 +326,7 @@ impl VhostUserBackendMut for VhostUserVsockBackend { } fn exit_event(&self, _thread_index: usize) -> Option { - Some(self.exit_event.try_clone().expect("Cloning exit eventfd")) + self.exit_event.try_clone().ok() } } diff --git a/vsock/src/vhu_vsock_thread.rs b/vsock/src/vhu_vsock_thread.rs index 102c62a..c8bd267 100644 --- a/vsock/src/vhu_vsock_thread.rs +++ b/vsock/src/vhu_vsock_thread.rs @@ -41,6 +41,8 @@ pub struct VhostUserVsockThread { pub event_idx: bool, /// Host socket raw file descriptor. host_sock: RawFd, + /// Host socket path + host_sock_path: String, /// Listener listening for new connections on the host. host_listener: UnixListener, /// Used to kill the thread. @@ -62,11 +64,11 @@ pub struct VhostUserVsockThread { impl VhostUserVsockThread { /// Create a new instance of VhostUserVsockThread. pub(crate) fn new(uds_path: String, guest_cid: u64) -> Result { - // TODO: better error handling - if let Ok(()) = std::fs::remove_file(uds_path.clone()) {} + // TODO: better error handling, maybe add a param to force the unlink + let _ = std::fs::remove_file(uds_path.clone()); let host_sock = UnixListener::bind(&uds_path) .and_then(|sock| sock.set_nonblocking(true).map(|_| sock)) - .unwrap(); + .map_err(Error::UnixBind)?; let epoll_fd = epoll::create(true).map_err(Error::EpollFdCreate)?; let epoll_file = unsafe { File::from_raw_fd(epoll_fd) }; @@ -77,6 +79,7 @@ impl VhostUserVsockThread { mem: None, event_idx: false, host_sock: host_sock.as_raw_fd(), + host_sock_path: uds_path.clone(), host_listener: host_sock, kill_evt: EventFd::new(EFD_NONBLOCK).unwrap(), vring_worker: None, @@ -205,7 +208,13 @@ impl VhostUserVsockThread { // Has to be EPOLLIN as it was not connected previously return; } - let mut unix_stream = self.thread_backend.stream_map.remove(&fd).unwrap(); + let mut unix_stream = match self.thread_backend.stream_map.remove(&fd) { + Some(uds) => uds, + None => { + warn!("Error while searching fd in the stream map"); + return; + } + }; // Local peer is sending a "connect PORT\n" command let peer_port = match Self::read_local_stream_port(&mut unix_stream) { @@ -572,3 +581,9 @@ impl VhostUserVsockThread { Ok(false) } } + +impl Drop for VhostUserVsockThread { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.host_sock_path); + } +}