From ebb13544bccc407a6b389f9d5ef84654fd0f2102 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Mon, 16 Sep 2019 15:37:07 -0700 Subject: [PATCH] Allow for proper error propagation Signed-off-by: Sebastien Boeuf --- src/lib.rs | 281 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 185 insertions(+), 96 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ecac22c..ada0129 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,6 +4,7 @@ // Copyright 2019 Alibaba Cloud Computing. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +use std::error; use std::fs::File; use std::io; use std::num::Wrapping; @@ -27,6 +28,8 @@ use vmm_sys_util::eventfd::EventFd; #[derive(Debug)] /// Errors related to vhost-user daemon. pub enum Error { + /// Failed to create a new vhost-user handler. + NewVhostUserHandler(VhostUserHandlerError), /// Failed creating vhost-user slave handler. CreateSlaveReqHandler(VhostUserError), /// Failed starting daemon thread. @@ -35,20 +38,18 @@ pub enum Error { WaitDaemon(std::boxed::Box), /// Failed handling a vhost-user request. HandleRequest(VhostUserError), - /// Could not find the mapping from memory regions. - MissingMemoryMapping, - /// Failed to create epoll file descriptor. - EpollCreateFd(io::Error), - /// Failed to add a file descriptor to the epoll handler. - EpollCtl(io::Error), - /// Failed while waiting for events. - EpollWait(io::Error), - /// Failed to signal used queue. - SignalUsedQueue(io::Error), + /// Failed to handle the event. + HandleEvent(io::Error), + /// Failed to process queue. + ProcessQueue(VringEpollHandlerError), + /// Failed to register listener. + RegisterListener(io::Error), + /// Failed to unregister listener. + UnregisterListener(io::Error), } /// Result of vhost-user daemon operations. -pub type Result = std::result::Result; +pub type Result = result::Result; /// This trait must be implemented by the caller in order to provide backend /// specific implementation. @@ -66,7 +67,11 @@ pub trait VhostUserBackend: Send + Sync + 'static { /// listeners onto specific file descriptors. The library can handle /// virtqueues on its own, but does not know what to do with events /// happening on custom listeners. - fn handle_event(&mut self, device_event: u16, evset: epoll::Events) -> Result; + fn handle_event( + &mut self, + device_event: u16, + evset: epoll::Events, + ) -> result::Result; /// This function is responsible for the actual processing that needs to /// happen when one of the virtqueues is available. @@ -75,13 +80,13 @@ pub trait VhostUserBackend: Send + Sync + 'static { q_idx: u16, avail_desc: &DescriptorChain, mem: &GuestMemoryMmap, - ) -> Result; + ) -> result::Result; /// Get virtio device configuration. fn get_config(&self, offset: u32, size: u32) -> Vec; /// Set virtio device configuration. - fn set_config(&mut self, offset: u32, buf: &[u8]); + fn set_config(&mut self, offset: u32, buf: &[u8]) -> result::Result<(), io::Error>; } /// This structure is the public API the backend is allowed to interact with @@ -102,7 +107,9 @@ impl VhostUserDaemon { /// custom events from the backend, but they get to be registered later /// during the sequence. pub fn new(name: String, sock_path: String, backend: S) -> Result { - let handler = Arc::new(Mutex::new(VhostUserHandler::new(backend))); + let handler = Arc::new(Mutex::new( + VhostUserHandler::new(backend).map_err(Error::NewVhostUserHandler)?, + )); let vring_handler = handler.lock().unwrap().get_vring_handler(); Ok(VhostUserDaemon { @@ -151,31 +158,23 @@ impl VhostUserDaemon { /// This lets entire control to the caller about what needs to be done for /// this special event, without forcing it to run its own dedicated epoll /// loop for it. - pub fn register_listener( - &self, - fd: RawFd, - ev_type: epoll::Events, - data: u64, - ) -> result::Result<(), io::Error> { + pub fn register_listener(&self, fd: RawFd, ev_type: epoll::Events, data: u64) -> Result<()> { self.vring_handler .read() .unwrap() .register_listener(fd, ev_type, data) + .map_err(Error::RegisterListener) } /// Unregister a custom event. If the custom event is triggered after this /// function has been called, nothing will happen as it will be removed /// from the list of file descriptors the epoll loop is listening to. - pub fn unregister_listener( - &self, - fd: RawFd, - ev_type: epoll::Events, - data: u64, - ) -> result::Result<(), io::Error> { + pub fn unregister_listener(&self, fd: RawFd, ev_type: epoll::Events, data: u64) -> Result<()> { self.vring_handler .read() .unwrap() .unregister_listener(fd, ev_type, data) + .map_err(Error::RegisterListener) } /// Trigger the processing of a virtqueue. This function is meant to be @@ -189,7 +188,11 @@ impl VhostUserDaemon { /// of `process_queue`. With this twisted trick, all common parts related /// to the virtqueues can remain part of the library. pub fn process_queue(&self, q_idx: u16) -> Result<()> { - self.vring_handler.write().unwrap().process_queue(q_idx) + self.vring_handler + .write() + .unwrap() + .process_queue(q_idx) + .map_err(Error::ProcessQueue) } } @@ -224,6 +227,53 @@ impl Vring { } } +#[derive(Debug)] +/// Errors related to vring epoll handler. +pub enum VringEpollHandlerError { + /// Failed to process the queue from the backend. + ProcessQueueBackendProcessing(io::Error), + /// Failed to signal used queue. + SignalUsedQueue(io::Error), + /// Failed to read the event from kick EventFd. + HandleEventReadKick(io::Error), + /// Failed to handle the event from the backend. + HandleEventBackendHandling(io::Error), + /// Failed to register vring listener. + RegisterVringListener(io::Error), + /// Failed to unregister vring listener. + UnregisterVringListener(io::Error), +} + +impl std::fmt::Display for VringEpollHandlerError { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + match self { + VringEpollHandlerError::ProcessQueueBackendProcessing(e) => { + write!(f, "failed processing queue from backend: {}", e) + } + VringEpollHandlerError::SignalUsedQueue(e) => { + write!(f, "failed signalling used queue: {}", e) + } + VringEpollHandlerError::HandleEventReadKick(e) => { + write!(f, "failed reading from kick eventfd: {}", e) + } + VringEpollHandlerError::HandleEventBackendHandling(e) => { + write!(f, "failed handling event from backend: {}", e) + } + VringEpollHandlerError::RegisterVringListener(e) => { + write!(f, "failed registering vring listener: {}", e) + } + VringEpollHandlerError::UnregisterVringListener(e) => { + write!(f, "failed unregistering vring listener: {}", e) + } + } + } +} + +impl error::Error for VringEpollHandlerError {} + +/// Result of vring epoll handler operations. +type VringEpollHandlerResult = std::result::Result; + struct VringEpollHandler { backend: Arc>, vrings: Vec>>, @@ -236,7 +286,7 @@ impl VringEpollHandler { self.mem = mem; } - fn process_queue(&mut self, q_idx: u16) -> Result<()> { + fn process_queue(&mut self, q_idx: u16) -> VringEpollHandlerResult<()> { let vring = &mut self.vrings[q_idx as usize].write().unwrap(); let mut used_desc_heads = vec![(0, 0); vring.queue.size as usize]; let mut used_count = 0; @@ -247,7 +297,7 @@ impl VringEpollHandler { .write() .unwrap() .process_queue(q_idx, &avail_desc, &mem) - .unwrap(); + .map_err(VringEpollHandlerError::ProcessQueueBackendProcessing)?; used_desc_heads[used_count] = (avail_desc.index, used_len); used_count += 1; @@ -260,44 +310,52 @@ impl VringEpollHandler { if used_count > 0 { if let Some(call) = &vring.call { - return call.write(1).map_err(Error::SignalUsedQueue); + call.write(1) + .map_err(VringEpollHandlerError::SignalUsedQueue)?; } } Ok(()) } - fn handle_event(&mut self, device_event: u16, evset: epoll::Events) -> Result { + fn handle_event( + &mut self, + device_event: u16, + evset: epoll::Events, + ) -> VringEpollHandlerResult { let num_queues = self.vrings.len(); match device_event as usize { x if x < num_queues => { if let Some(kick) = &self.vrings[device_event as usize].read().unwrap().kick { - kick.read().unwrap(); + kick.read() + .map_err(VringEpollHandlerError::HandleEventReadKick)?; } - self.process_queue(device_event).unwrap(); - + self.process_queue(device_event)?; Ok(false) } _ => self .backend .write() .unwrap() - .handle_event(device_event, evset), + .handle_event(device_event, evset) + .map_err(VringEpollHandlerError::HandleEventBackendHandling), } } - fn register_vring_listener(&self, q_idx: usize) -> result::Result<(), io::Error> { + fn register_vring_listener(&self, q_idx: usize) -> VringEpollHandlerResult<()> { if let Some(fd) = &self.vrings[q_idx].read().unwrap().kick { self.register_listener(fd.as_raw_fd(), epoll::Events::EPOLLIN, q_idx as u64) + .map_err(VringEpollHandlerError::RegisterVringListener) } else { Ok(()) } } - fn unregister_vring_listener(&self, q_idx: usize) -> result::Result<(), io::Error> { + fn unregister_vring_listener(&self, q_idx: usize) -> VringEpollHandlerResult<()> { if let Some(fd) = &self.vrings[q_idx].read().unwrap().kick { self.unregister_listener(fd.as_raw_fd(), epoll::Events::EPOLLIN, q_idx as u64) + .map_err(VringEpollHandlerError::UnregisterVringListener) } else { Ok(()) } @@ -332,12 +390,24 @@ impl VringEpollHandler { } } +#[derive(Debug)] +/// Errors related to vring worker. +enum VringWorkerError { + /// Failed while waiting for events. + EpollWait(io::Error), + /// Failed to handle event. + HandleEvent(VringEpollHandlerError), +} + +/// Result of vring worker operations. +type VringWorkerResult = std::result::Result; + struct VringWorker { handler: Arc>>, } impl VringWorker { - fn run(&self, epoll_fd: RawFd) -> Result<()> { + fn run(&self, epoll_fd: RawFd) -> VringWorkerResult<()> { const EPOLL_EVENTS_LEN: usize = 100; let mut events = vec![epoll::Event::new(epoll::Events::empty(), 0); EPOLL_EVENTS_LEN]; @@ -355,7 +425,7 @@ impl VringWorker { // appropriate to retry, by calling into epoll_wait(). continue; } - return Err(Error::EpollWait(e)); + return Err(VringWorkerError::EpollWait(e)); } }; @@ -371,7 +441,13 @@ impl VringWorker { let ev_type = event.data as u16; - if self.handler.write().unwrap().handle_event(ev_type, evset)? { + if self + .handler + .write() + .unwrap() + .handle_event(ev_type, evset) + .map_err(VringWorkerError::HandleEvent)? + { break 'epoll; } } @@ -381,6 +457,34 @@ impl VringWorker { } } +#[derive(Debug)] +/// Errors related to vhost-user handler. +pub enum VhostUserHandlerError { + /// Failed to create epoll file descriptor. + EpollCreateFd(io::Error), + /// Failed to spawn vring worker. + SpawnVringWorker(io::Error), + /// Could not find the mapping from memory regions. + MissingMemoryMapping, +} + +impl std::fmt::Display for VhostUserHandlerError { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + match self { + VhostUserHandlerError::EpollCreateFd(e) => write!(f, "failed creating epoll fd: {}", e), + VhostUserHandlerError::SpawnVringWorker(e) => { + write!(f, "failed spawning the vring worker: {}", e) + } + VhostUserHandlerError::MissingMemoryMapping => write!(f, "Missing memory mapping"), + } + } +} + +impl error::Error for VhostUserHandlerError {} + +/// Result of vhost-user handler operations. +type VhostUserHandlerResult = std::result::Result; + struct VhostUserHandler { backend: Arc>, vring_handler: Arc>>, @@ -395,14 +499,14 @@ struct VhostUserHandler { } impl VhostUserHandler { - fn new(backend: S) -> Self { + fn new(backend: S) -> VhostUserHandlerResult { let num_queues = backend.num_queues(); let max_queue_size = backend.max_queue_size(); let arc_backend = Arc::new(RwLock::new(backend)); let vrings = vec![Arc::new(RwLock::new(Vring::new(max_queue_size as u16))); num_queues]; // Create the epoll file descriptor - let epoll_fd = epoll::create(true).unwrap(); + let epoll_fd = epoll::create(true).map_err(VhostUserHandlerError::EpollCreateFd)?; let vring_handler = Arc::new(RwLock::new(VringEpollHandler { backend: arc_backend.clone(), @@ -415,11 +519,11 @@ impl VhostUserHandler { }; thread::Builder::new() - .name("vring_epoll_handler".to_string()) + .name("vring_worker".to_string()) .spawn(move || worker.run(epoll_fd)) - .unwrap(); + .map_err(VhostUserHandlerError::SpawnVringWorker)?; - VhostUserHandler { + Ok(VhostUserHandler { backend: arc_backend, vring_handler, owned: false, @@ -430,14 +534,14 @@ impl VhostUserHandler { max_queue_size, memory: None, vrings, - } + }) } fn get_vring_handler(&self) -> Arc>> { self.vring_handler.clone() } - fn vmm_va_to_gpa(&self, vmm_va: u64) -> Result { + fn vmm_va_to_gpa(&self, vmm_va: u64) -> VhostUserHandlerResult { if let Some(memory) = &self.memory { for mapping in memory.mappings.iter() { if vmm_va >= mapping.vmm_addr && vmm_va < mapping.vmm_addr + mapping.size { @@ -446,7 +550,7 @@ impl VhostUserHandler { } } - Err(Error::MissingMemoryMapping) + Err(VhostUserHandlerError::MissingMemoryMapping) } } @@ -532,7 +636,9 @@ impl VhostUserSlaveReqHandler for VhostUserHandler { }); } - let mem = GuestMemoryMmap::from_ranges_with_files(regions).unwrap(); + let mem = GuestMemoryMmap::from_ranges_with_files(regions).map_err(|e| { + VhostUserError::ReqHandlerError(io::Error::new(io::ErrorKind::Other, e)) + })?; self.vring_handler.write().unwrap().update_memory(Some(mem)); self.memory = Some(Memory { mappings }); @@ -565,9 +671,15 @@ impl VhostUserSlaveReqHandler for VhostUserHandler { } if self.memory.is_some() { - let desc_table = self.vmm_va_to_gpa(descriptor).unwrap(); - let avail_ring = self.vmm_va_to_gpa(available).unwrap(); - let used_ring = self.vmm_va_to_gpa(used).unwrap(); + let desc_table = self.vmm_va_to_gpa(descriptor).map_err(|e| { + VhostUserError::ReqHandlerError(io::Error::new(io::ErrorKind::Other, e)) + })?; + let avail_ring = self.vmm_va_to_gpa(available).map_err(|e| { + VhostUserError::ReqHandlerError(io::Error::new(io::ErrorKind::Other, e)) + })?; + let used_ring = self.vmm_va_to_gpa(used).map_err(|e| { + VhostUserError::ReqHandlerError(io::Error::new(io::ErrorKind::Other, e)) + })?; self.vrings[index as usize] .write() .unwrap() @@ -609,7 +721,9 @@ impl VhostUserSlaveReqHandler for VhostUserHandler { .read() .unwrap() .unregister_vring_listener(index as usize) - .unwrap(); + .map_err(|e| { + VhostUserError::ReqHandlerError(io::Error::new(io::ErrorKind::Other, e)) + })?; let next_avail = self.vrings[index as usize] .read() @@ -626,22 +740,12 @@ impl VhostUserSlaveReqHandler for VhostUserHandler { return Err(VhostUserError::InvalidParam); } - if self.vrings[index as usize].read().unwrap().kick.is_some() { + if let Some(kick) = self.vrings[index as usize].write().unwrap().kick.take() { // Close file descriptor set by previous operations. - let _ = unsafe { - libc::close( - self.vrings[index as usize] - .write() - .unwrap() - .kick - .take() - .unwrap() - .as_raw_fd(), - ) - }; + let _ = unsafe { libc::close(kick.as_raw_fd()) }; } self.vrings[index as usize].write().unwrap().kick = - Some(unsafe { EventFd::from_raw_fd(fd.unwrap()) }); + fd.map(|x| unsafe { EventFd::from_raw_fd(x) }); // Quotation from vhost-user spec: // Client must start ring upon receiving a kick (that is, detecting @@ -655,7 +759,9 @@ impl VhostUserSlaveReqHandler for VhostUserHandler { .read() .unwrap() .register_vring_listener(index as usize) - .unwrap(); + .map_err(|e| { + VhostUserError::ReqHandlerError(io::Error::new(io::ErrorKind::Other, e)) + })?; Ok(()) } @@ -665,22 +771,12 @@ impl VhostUserSlaveReqHandler for VhostUserHandler { return Err(VhostUserError::InvalidParam); } - if self.vrings[index as usize].write().unwrap().call.is_some() { + if let Some(call) = self.vrings[index as usize].write().unwrap().call.take() { // Close file descriptor set by previous operations. - let _ = unsafe { - libc::close( - self.vrings[index as usize] - .write() - .unwrap() - .call - .take() - .unwrap() - .as_raw_fd(), - ) - }; + let _ = unsafe { libc::close(call.as_raw_fd()) }; } self.vrings[index as usize].write().unwrap().call = - Some(unsafe { EventFd::from_raw_fd(fd.unwrap()) }); + fd.map(|x| unsafe { EventFd::from_raw_fd(x) }); Ok(()) } @@ -690,22 +786,12 @@ impl VhostUserSlaveReqHandler for VhostUserHandler { return Err(VhostUserError::InvalidParam); } - if self.vrings[index as usize].read().unwrap().err.is_some() { + if let Some(err) = self.vrings[index as usize].write().unwrap().err.take() { // Close file descriptor set by previous operations. - let _ = unsafe { - libc::close( - self.vrings[index as usize] - .write() - .unwrap() - .err - .take() - .unwrap() - .as_raw_fd(), - ) - }; + let _ = unsafe { libc::close(err.as_raw_fd()) }; } self.vrings[index as usize].write().unwrap().err = - Some(unsafe { EventFd::from_raw_fd(fd.unwrap()) }); + fd.map(|x| unsafe { EventFd::from_raw_fd(x) }); Ok(()) } @@ -764,7 +850,10 @@ impl VhostUserSlaveReqHandler for VhostUserHandler { return Err(VhostUserError::InvalidParam); } - self.backend.write().unwrap().set_config(offset, buf); - Ok(()) + self.backend + .write() + .unwrap() + .set_config(offset, buf) + .map_err(VhostUserError::ReqHandlerError) } }