tree-wide: bubble up errors from daemon threads

There was a mix of just unwrapping (panicking) and catching and logging
errors. The unwrapping is not allowing for particulary pretty error
handling, so let's bubble the errors up by not crashing the thread,
but by just returning a Result<()> than is received when joining the
threads.

Not all .unwrap() uses were translated since a followup PR (#389) will
rework that code anyway (and get rid of the .unwrap() in the process).

Signed-off-by: Erik Schilling <erik.schilling@linaro.org>

coverage_config_x86_64.json

@@ -1,5 +1,5 @@
 {
-  "coverage_score": 69.0,
+  "coverage_score": 68.2,
   "exclude_path": "",
   "crate_features": ""
 }

crates/gpio/src/backend.rs

@@ -9,7 +9,7 @@ use log::{error, info, warn};
 use std::num::ParseIntError;
 use std::process::exit;
 use std::sync::{Arc, RwLock};
-use std::thread::spawn;
+use std::thread::{spawn, JoinHandle};
 
 use clap::Parser;
 use thiserror::Error as ThisError;
@@ -35,6 +35,14 @@ pub(crate) enum Error {
     ParseFailure(ParseIntError),
     #[error("Failed to join threads")]
     FailedJoiningThreads,
+    #[error("Could not open gpio device: {0}")]
+    CouldNotOpenDevice(crate::gpio::Error),
+    #[error("Could not create gpio controller: {0}")]
+    CouldNotCreateGpioController(crate::gpio::Error),
+    #[error("Could not create gpio backend: {0}")]
+    CouldNotCreateBackend(crate::vhu_gpio::Error),
+    #[error("Could not create daemon: {0}")]
+    CouldNotCreateDaemon(vhost_user_backend::Error),
 }
 
 #[derive(Parser, Debug)]
@@ -134,7 +142,7 @@ fn start_backend<D: 'static + GpioDevice + Send + Sync>(args: GpioArgs) -> Resul
         let socket = config.socket_path.to_owned() + &i.to_string();
         let device_num = config.devices.inner[i];
 
-        let handle = spawn(move || loop {
+        let handle: JoinHandle<Result<()>> = spawn(move || loop {
             // A separate thread is spawned for each socket and can connect to a separate guest.
             // These are run in an infinite loop to not require the daemon to be restarted once a
             // guest exits.
@@ -143,9 +151,12 @@ fn start_backend<D: 'static + GpioDevice + Send + Sync>(args: GpioArgs) -> Resul
             // threads, and so the code uses unwrap() instead. The panic on a thread won't cause
             // trouble to other threads/guests or the main() function and should be safe for the
             // daemon.
-            let device = D::open(device_num).unwrap();
-            let controller = GpioController::<D>::new(device).unwrap();
-            let backend = Arc::new(RwLock::new(VhostUserGpioBackend::new(controller).unwrap()));
+            let device = D::open(device_num).map_err(Error::CouldNotOpenDevice)?;
+            let controller =
+                GpioController::<D>::new(device).map_err(Error::CouldNotCreateGpioController)?;
+            let backend = Arc::new(RwLock::new(
+                VhostUserGpioBackend::new(controller).map_err(Error::CouldNotCreateBackend)?,
+            ));
             let listener = Listener::new(socket.clone(), true).unwrap();
 
             let mut daemon = VhostUserDaemon::new(
@@ -153,7 +164,7 @@ fn start_backend<D: 'static + GpioDevice + Send + Sync>(args: GpioArgs) -> Resul
                 backend.clone(),
                 GuestMemoryAtomic::new(GuestMemoryMmap::new()),
             )
-            .unwrap();
+            .map_err(Error::CouldNotCreateDaemon)?;
 
             daemon.start(listener).unwrap();
 
@@ -179,7 +190,7 @@ fn start_backend<D: 'static + GpioDevice + Send + Sync>(args: GpioArgs) -> Resul
     }
 
     for handle in handles {
-        handle.join().map_err(|_| Error::FailedJoiningThreads)?;
+        handle.join().map_err(|_| Error::FailedJoiningThreads)??;
     }
 
     Ok(())

crates/i2c/src/main.rs

@@ -12,7 +12,7 @@ use log::{error, info, warn};
 use std::num::ParseIntError;
 use std::process::exit;
 use std::sync::{Arc, RwLock};
-use std::thread::spawn;
+use std::thread::{spawn, JoinHandle};
 
 use clap::Parser;
 use thiserror::Error as ThisError;
@@ -42,6 +42,10 @@ pub(crate) enum Error {
     ParseFailure(ParseIntError),
     #[error("Failed to join threads")]
     FailedJoiningThreads,
+    #[error("Could not create backend: {0}")]
+    CouldNotCreateBackend(vhu_i2c::Error),
+    #[error("Could not create daemon: {0}")]
+    CouldNotCreateDaemon(vhost_user_backend::Error),
 }
 
 #[derive(Parser, Debug)]
@@ -183,7 +187,7 @@ fn start_backend<D: 'static + I2cDevice + Send + Sync>(args: I2cArgs) -> Result<
         let socket = config.socket_path.to_owned() + &i.to_string();
         let i2c_map = i2c_map.clone();
 
-        let handle = spawn(move || loop {
+        let handle: JoinHandle<Result<()>> = spawn(move || loop {
             // A separate thread is spawned for each socket and can connect to a separate guest.
             // These are run in an infinite loop to not require the daemon to be restarted once a
             // guest exits.
@@ -193,7 +197,7 @@ fn start_backend<D: 'static + I2cDevice + Send + Sync>(args: I2cArgs) -> Result<
             // trouble to other threads/guests or the main() function and should be safe for the
             // daemon.
             let backend = Arc::new(RwLock::new(
-                VhostUserI2cBackend::new(i2c_map.clone()).unwrap(),
+                VhostUserI2cBackend::new(i2c_map.clone()).map_err(Error::CouldNotCreateBackend)?,
             ));
             let listener = Listener::new(socket.clone(), true).unwrap();
 
@@ -202,7 +206,7 @@ fn start_backend<D: 'static + I2cDevice + Send + Sync>(args: I2cArgs) -> Result<
                 backend.clone(),
                 GuestMemoryAtomic::new(GuestMemoryMmap::new()),
             )
-            .unwrap();
+            .map_err(Error::CouldNotCreateDaemon)?;
 
             daemon.start(listener).unwrap();
 
@@ -228,7 +232,7 @@ fn start_backend<D: 'static + I2cDevice + Send + Sync>(args: I2cArgs) -> Result<
     }
 
     for handle in handles {
-        handle.join().map_err(|_| Error::FailedJoiningThreads)?;
+        handle.join().map_err(|_| Error::FailedJoiningThreads)??;
     }
 
     Ok(())

crates/rng/src/main.rs

@@ -9,7 +9,7 @@ use log::{error, info, warn};
 use std::fs::File;
 use std::process::exit;
 use std::sync::{Arc, Mutex, RwLock};
-use std::thread;
+use std::thread::{self, JoinHandle};
 
 use clap::Parser;
 use thiserror::Error as ThisError;
@@ -36,6 +36,10 @@ pub(crate) enum Error {
     InvalidSocketCount(u32),
     #[error("Threads can't be joined")]
     FailedJoiningThreads,
+    #[error("Could not create backend: {0}")]
+    CouldNotCreateBackend(std::io::Error),
+    #[error("Could not create daemon: {0}")]
+    CouldNotCreateDaemon(vhost_user_backend::Error),
 }
 
 #[derive(Clone, Parser, Debug, PartialEq)]
@@ -110,13 +114,14 @@ pub(crate) fn start_backend(config: VuRngConfig) -> Result<()> {
         let max_bytes = config.max_bytes;
         let random = Arc::clone(&random_file);
 
-        let handle = thread::spawn(move || loop {
+        let handle: JoinHandle<Result<()>> = thread::spawn(move || loop {
             // If creating the VuRngBackend isn't successull there isn't much else to do than
             // killing the thread, which .unwrap() does.  When that happens an error code is
             // generated and displayed by the runtime mechanic.  Killing a thread doesn't affect
             // the other threads spun-off by the daemon.
             let vu_rng_backend = Arc::new(RwLock::new(
-                VuRngBackend::new(random.clone(), period_ms, max_bytes).unwrap(),
+                VuRngBackend::new(random.clone(), period_ms, max_bytes)
+                    .map_err(Error::CouldNotCreateBackend)?,
             ));
 
             let mut daemon = VhostUserDaemon::new(
@@ -124,7 +129,7 @@ pub(crate) fn start_backend(config: VuRngConfig) -> Result<()> {
                 Arc::clone(&vu_rng_backend),
                 GuestMemoryAtomic::new(GuestMemoryMmap::new()),
             )
-            .unwrap();
+            .map_err(Error::CouldNotCreateDaemon)?;
 
             let listener = Listener::new(socket.clone(), true).unwrap();
             daemon.start(listener).unwrap();
@@ -156,7 +161,7 @@ pub(crate) fn start_backend(config: VuRngConfig) -> Result<()> {
     }
 
     for handle in handles {
-        handle.join().map_err(|_| Error::FailedJoiningThreads)?;
+        handle.join().map_err(|_| Error::FailedJoiningThreads)??;
     }
 
     Ok(())

crates/vsock/src/main.rs

@@ -11,13 +11,14 @@ mod vsock_conn;
 use std::{
     collections::HashMap,
     convert::TryFrom,
+    process::exit,
     sync::{Arc, RwLock},
     thread,
 };
 
 use crate::vhu_vsock::{CidMap, VhostUserVsockBackend, VsockConfig};
 use clap::{Args, Parser};
-use log::{info, warn};
+use log::{error, info, warn};
 use serde::Deserialize;
 use thiserror::Error as ThisError;
 use vhost::{vhost_user, vhost_user::Listener};
@@ -47,6 +48,14 @@ enum VmArgsParseError {
     RequiredKeyNotFound(String),
 }
 
+#[derive(Debug, ThisError)]
+enum BackendError {
+    #[error("Could not create backend: {0}")]
+    CouldNotCreateBackend(vhu_vsock::Error),
+    #[error("Could not create daemon: {0}")]
+    CouldNotCreateDaemon(vhost_user_backend::Error),
+}
+
 #[derive(Args, Clone, Debug, Deserialize)]
 struct VsockParam {
     /// Context identifier of the guest which uniquely identifies the device for its lifetime.
@@ -177,10 +186,15 @@ impl TryFrom<VsockArgs> for Vec<VsockConfig> {
 
 /// This is the public API through which an external program starts the
 /// vhost-user-vsock backend server.
-pub(crate) fn start_backend_server(config: VsockConfig, cid_map: Arc<RwLock<CidMap>>) {
+pub(crate) fn start_backend_server(
+    config: VsockConfig,
+    cid_map: Arc<RwLock<CidMap>>,
+) -> Result<(), BackendError> {
     loop {
-        let backend =
-            Arc::new(VhostUserVsockBackend::new(config.clone(), cid_map.clone()).unwrap());
+        let backend = Arc::new(
+            VhostUserVsockBackend::new(config.clone(), cid_map.clone())
+                .map_err(BackendError::CouldNotCreateBackend)?,
+        );
         cid_map
             .write()
             .unwrap()
@@ -193,7 +207,7 @@ pub(crate) fn start_backend_server(config: VsockConfig, cid_map: Arc<RwLock<CidM
             backend.clone(),
             GuestMemoryAtomic::new(GuestMemoryMmap::new()),
         )
-        .unwrap();
+        .map_err(BackendError::CouldNotCreateDaemon)?;
 
         let mut vring_workers = daemon.get_epoll_handlers();
 
@@ -224,7 +238,7 @@ pub(crate) fn start_backend_server(config: VsockConfig, cid_map: Arc<RwLock<CidM
     }
 }
 
-pub(crate) fn start_backend_servers(configs: &[VsockConfig]) {
+pub(crate) fn start_backend_servers(configs: &[VsockConfig]) -> Result<(), BackendError> {
     let cid_map: Arc<RwLock<CidMap>> = Arc::new(RwLock::new(HashMap::new()));
     let mut handles = Vec::new();
 
@@ -239,8 +253,10 @@ pub(crate) fn start_backend_servers(configs: &[VsockConfig]) {
     }
 
     for handle in handles {
-        handle.join().unwrap();
+        handle.join().unwrap()?;
     }
+
+    Ok(())
 }
 
 fn main() {
@@ -254,7 +270,10 @@ fn main() {
         }
     };
 
-    start_backend_servers(&configs);
+    if let Err(e) = start_backend_servers(&configs) {
+        error!("{e}");
+        exit(1);
+    }
 }
 
 #[cfg(test)]