vmm: Clean Error handling up

We used to have errors definitions spread across vmm, vm, api,
and http.

We now have a cleaner separation: All API routines only return an
ApiResult. All VM operations, including the VMM wrappers, return a
VmResult. This makes it easier to carry errors up to the HTTP caller.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>

vmm/src/api/http_endpoint.rs

@@ -4,9 +4,10 @@
 //
 
 use crate::api::http::EndpointHandler;
-use crate::api::VmConfig;
+use crate::api::{
-use crate::api::{vm_boot, vm_create, vm_info, vm_reboot, vm_shutdown, ApiRequest, VmAction};
+    vm_boot, vm_create, vm_info, vm_reboot, vm_shutdown, ApiError, ApiRequest, ApiResult, VmAction,
-use crate::{Error, Result};
+    VmConfig,
+};
 use micro_http::{Body, Method, Request, Response, StatusCode, Version};
 use serde_json::Error as SerdeError;
 use std::sync::mpsc::Sender;
@@ -20,22 +21,22 @@ pub enum HttpError {
     SerdeJsonDeserialize(SerdeError),
 
     /// Could not create a VM
-    VmCreate(Error),
+    VmCreate(ApiError),
 
     /// Could not boot a VM
-    VmBoot(Error),
+    VmBoot(ApiError),
 
     /// Could not get the VM information
-    VmInfo(Error),
+    VmInfo(ApiError),
 
     /// Could not shut a VM down
-    VmShutdown(Error),
+    VmShutdown(ApiError),
 
     /// Could not reboot a VM
-    VmReboot(Error),
+    VmReboot(ApiError),
 
     /// Could not act on a VM
-    VmAction(Error),
+    VmAction(ApiError),
 }
 
 fn error_response(error: HttpError, status: StatusCode) -> Response {
@@ -90,7 +91,7 @@ pub struct VmActionHandler {
     action_fn: VmActionFn,
 }
 
-type VmActionFn = Box<dyn Fn(EventFd, Sender<ApiRequest>) -> Result<()> + Send + Sync>;
+type VmActionFn = Box<dyn Fn(EventFd, Sender<ApiRequest>) -> ApiResult<()> + Send + Sync>;
 
 impl VmActionHandler {
     pub fn new(action: VmAction) -> Self {
@@ -114,9 +115,9 @@ impl EndpointHandler for VmActionHandler {
         match req.method() {
             Method::Put => {
                 match (self.action_fn)(api_notifier, api_sender).map_err(|e| match e {
-                    Error::ApiVmBoot(_) => HttpError::VmBoot(e),
+                    ApiError::VmBoot(_) => HttpError::VmBoot(e),
-                    Error::ApiVmShutdown(_) => HttpError::VmShutdown(e),
+                    ApiError::VmShutdown(_) => HttpError::VmShutdown(e),
-                    Error::ApiVmReboot(_) => HttpError::VmReboot(e),
+                    ApiError::VmReboot(_) => HttpError::VmReboot(e),
                     _ => HttpError::VmAction(e),
                 }) {
                     Ok(_) => Response::new(Version::Http11, StatusCode::OK),

vmm/src/api/mod.rs

@@ -38,16 +38,25 @@ pub mod http_endpoint;
 
 use crate::config::VmConfig;
 use crate::vm::{Error as VmError, VmState};
-use crate::{Error, Result};
+use std::io;
-use std::sync::mpsc::{channel, Sender};
+use std::sync::mpsc::{channel, RecvError, SendError, Sender};
 use std::sync::Arc;
 use vmm_sys_util::eventfd::EventFd;
 
 /// API errors are sent back from the VMM API server through the ApiResponse.
 #[derive(Debug)]
 pub enum ApiError {
-    /// The VM could not be created.
+    /// Cannot write to EventFd.
-    VmCreate(VmError),
+    EventFdWrite(io::Error),
+
+    /// API request send error
+    RequestSend(SendError<ApiRequest>),
+
+    /// Wrong reponse payload type
+    ResponsePayloadType,
+
+    /// API response receive error
+    ResponseRecv(RecvError),
 
     /// The VM could not boot.
     VmBoot(VmError),
@@ -55,8 +64,11 @@ pub enum ApiError {
     /// The VM is already created.
     VmAlreadyCreated,
 
+    /// The VM could not be created.
+    VmCreate(VmError),
+
     /// The VM info is not available.
-    VmInfo,
+    VmInfo(VmError),
 
     /// The VM config is missing.
     VmMissingConfig,
@@ -71,8 +83,9 @@ pub enum ApiError {
     VmShutdown(VmError),
 
     /// The VM could not reboot.
-    VmReboot,
+    VmReboot(VmError),
 }
+pub type ApiResult<T> = std::result::Result<T, ApiError>;
 
 #[derive(Clone, Deserialize, Serialize)]
 pub struct VmInfo {
@@ -122,19 +135,16 @@ pub fn vm_create(
     api_evt: EventFd,
     api_sender: Sender<ApiRequest>,
     config: Arc<VmConfig>,
-) -> Result<()> {
+) -> ApiResult<()> {
     let (response_sender, response_receiver) = channel();
 
     // Send the VM creation request.
     api_sender
         .send(ApiRequest::VmCreate(config, response_sender))
-        .map_err(Error::ApiRequestSend)?;
+        .map_err(ApiError::RequestSend)?;
-    api_evt.write(1).map_err(Error::EventFdWrite)?;
+    api_evt.write(1).map_err(ApiError::EventFdWrite)?;
 
-    response_receiver
+    response_receiver.recv().map_err(ApiError::ResponseRecv)??;
-        .recv()
-        .map_err(Error::ApiResponseRecv)?
-        .map_err(Error::ApiVmCreate)?;
 
     Ok(())
 }
@@ -153,7 +163,7 @@ pub enum VmAction {
     Reboot,
 }
 
-fn vm_action(api_evt: EventFd, api_sender: Sender<ApiRequest>, action: VmAction) -> Result<()> {
+fn vm_action(api_evt: EventFd, api_sender: Sender<ApiRequest>, action: VmAction) -> ApiResult<()> {
     let (response_sender, response_receiver) = channel();
 
     let request = match action {
@@ -163,63 +173,51 @@ fn vm_action(api_evt: EventFd, api_sender: Sender<ApiRequest>, action: VmAction)
     };
 
     // Send the VM request.
-    api_sender.send(request).map_err(Error::ApiRequestSend)?;
+    api_sender.send(request).map_err(ApiError::RequestSend)?;
-    api_evt.write(1).map_err(Error::EventFdWrite)?;
+    api_evt.write(1).map_err(ApiError::EventFdWrite)?;
 
     match action {
         VmAction::Boot => {
-            response_receiver
+            response_receiver.recv().map_err(ApiError::ResponseRecv)??;
-                .recv()
-                .map_err(Error::ApiResponseRecv)?
-                .map_err(Error::ApiVmBoot)?;
         }
 
         VmAction::Shutdown => {
-            response_receiver
+            response_receiver.recv().map_err(ApiError::ResponseRecv)??;
-                .recv()
-                .map_err(Error::ApiResponseRecv)?
-                .map_err(Error::ApiVmShutdown)?;
         }
 
         VmAction::Reboot => {
-            response_receiver
+            response_receiver.recv().map_err(ApiError::ResponseRecv)??;
-                .recv()
-                .map_err(Error::ApiResponseRecv)?
-                .map_err(Error::ApiVmReboot)?;
         }
     }
 
     Ok(())
 }
 
-pub fn vm_boot(api_evt: EventFd, api_sender: Sender<ApiRequest>) -> Result<()> {
+pub fn vm_boot(api_evt: EventFd, api_sender: Sender<ApiRequest>) -> ApiResult<()> {
     vm_action(api_evt, api_sender, VmAction::Boot)
 }
 
-pub fn vm_shutdown(api_evt: EventFd, api_sender: Sender<ApiRequest>) -> Result<()> {
+pub fn vm_shutdown(api_evt: EventFd, api_sender: Sender<ApiRequest>) -> ApiResult<()> {
     vm_action(api_evt, api_sender, VmAction::Shutdown)
 }
 
-pub fn vm_reboot(api_evt: EventFd, api_sender: Sender<ApiRequest>) -> Result<()> {
+pub fn vm_reboot(api_evt: EventFd, api_sender: Sender<ApiRequest>) -> ApiResult<()> {
     vm_action(api_evt, api_sender, VmAction::Reboot)
 }
 
-pub fn vm_info(api_evt: EventFd, api_sender: Sender<ApiRequest>) -> Result<VmInfo> {
+pub fn vm_info(api_evt: EventFd, api_sender: Sender<ApiRequest>) -> ApiResult<VmInfo> {
     let (response_sender, response_receiver) = channel();
 
     // Send the VM request.
     api_sender
         .send(ApiRequest::VmInfo(response_sender))
-        .map_err(Error::ApiRequestSend)?;
+        .map_err(ApiError::RequestSend)?;
-    api_evt.write(1).map_err(Error::EventFdWrite)?;
+    api_evt.write(1).map_err(ApiError::EventFdWrite)?;
 
-    let vm_info = response_receiver
+    let vm_info = response_receiver.recv().map_err(ApiError::ResponseRecv)??;
-        .recv()
-        .map_err(Error::ApiResponseRecv)?
-        .map_err(|_| Error::ApiVmInfo)?;
 
     match vm_info {
         ApiResponsePayload::VmInfo(info) => Ok(info),
-        _ => Err(Error::ApiVmInfo),
+        _ => Err(ApiError::ResponsePayloadType),
     }
 }

vmm/src/lib.rs

@@ -37,30 +37,9 @@ pub enum Error {
     /// API request receive error
     ApiRequestRecv(RecvError),
 
-    /// API response receive error
-    ApiResponseRecv(RecvError),
-
-    /// API request send error
-    ApiRequestSend(SendError<ApiRequest>),
-
     /// API response send error
     ApiResponseSend(SendError<ApiResponse>),
 
-    /// Cannot create a VM from the API
-    ApiVmCreate(ApiError),
-
-    /// Cannot boot a VM from the API
-    ApiVmBoot(ApiError),
-
-    /// Cannot get the VM info
-    ApiVmInfo,
-
-    /// Cannot shut a VM down from the API
-    ApiVmShutdown(ApiError),
-
-    /// Cannot reboot a VM from the API
-    ApiVmReboot(ApiError),
-
     /// Cannot bind to the UNIX domain socket path
     Bind(io::Error),
 
@@ -73,9 +52,6 @@ pub enum Error {
     /// Cannot read from EventFd.
     EventFdRead(io::Error),
 
-    /// Cannot write to EventFd.
-    EventFdWrite(io::Error),
-
     /// Cannot create epoll context.
     Epoll(io::Error),
 
@@ -85,17 +61,8 @@ pub enum Error {
     /// Cannot handle the VM STDIN stream
     Stdin(VmError),
 
-    /// Cannot create a VM
+    /// Cannot reboot the VM
-    VmCreate(VmError),
+    VmReboot(VmError),
-
-    /// Cannot boot a VM
-    VmBoot(VmError),
-
-    /// Cannot fetch the VM information
-    VmInfo,
-
-    /// The Vm is not created
-    VmNotCreated,
 
     /// Cannot shut a VM down
     VmShutdown(VmError),
@@ -196,7 +163,7 @@ pub fn start_vmm_thread(
                         // based on the same VM config, boot it and restart
                         // the control loop.
 
-                        vmm.vm_reboot()?;
+                        vmm.vm_reboot().map_err(Error::VmReboot)?;
 
                         // Continue and restart the VMM control loop
                         continue 'outer;
@@ -264,12 +231,12 @@ impl Vmm {
         })
     }
 
-    fn vm_reboot(&mut self) -> Result<()> {
+    fn vm_reboot(&mut self) -> result::Result<(), VmError> {
         // Without ACPI, a reset is equivalent to a shutdown
         #[cfg(not(feature = "acpi"))]
         {
             if let Some(ref mut vm) = self.vm {
-                vm.shutdown().map_err(Error::VmShutdown)?;
+                vm.shutdown()?;
                 return Ok(());
             }
         }
@@ -277,29 +244,29 @@ impl Vmm {
         // First we stop the current VM and create a new one.
         if let Some(ref mut vm) = self.vm {
             let config = vm.get_config();
-            vm.shutdown().map_err(Error::VmShutdown)?;
+            vm.shutdown()?;
 
-            let exit_evt = self.exit_evt.try_clone().map_err(Error::EventFdClone)?;
+            let exit_evt = self.exit_evt.try_clone().map_err(VmError::EventFdClone)?;
-            let reset_evt = self.reset_evt.try_clone().map_err(Error::EventFdClone)?;
+            let reset_evt = self.reset_evt.try_clone().map_err(VmError::EventFdClone)?;
 
-            self.vm = Some(Vm::new(config, exit_evt, reset_evt).map_err(Error::VmCreate)?);
+            self.vm = Some(Vm::new(config, exit_evt, reset_evt)?);
         }
 
         // Then we start the new VM.
         if let Some(ref mut vm) = self.vm {
-            vm.boot().map_err(Error::VmBoot)?;
+            vm.boot()?;
         } else {
-            return Err(Error::VmNotCreated);
+            return Err(VmError::VmNotCreated);
         }
 
         Ok(())
     }
 
-    fn vm_info(&self) -> Result<VmInfo> {
+    fn vm_info(&self) -> result::Result<VmInfo, VmError> {
         match &self.vm_config {
             Some(config) => {
                 let state = match &self.vm {
-                    Some(vm) => vm.get_state().unwrap(),
+                    Some(vm) => vm.get_state()?,
                     None => VmState::Created,
                 };
 
@@ -308,7 +275,7 @@ impl Vmm {
                     state,
                 })
             }
-            None => Err(Error::VmNotCreated),
+            None => Err(VmError::VmNotCreated),
         }
     }
 
@@ -448,8 +415,7 @@ impl Vmm {
                                 ApiRequest::VmReboot(sender) => {
                                     let response = match self.vm_reboot() {
                                         Ok(_) => Ok(ApiResponsePayload::Empty),
-                                        Err(Error::VmNotCreated) => Err(ApiError::VmNotBooted),
+                                        Err(e) => Err(ApiError::VmReboot(e)),
-                                        Err(_) => Err(ApiError::VmReboot),
                                     };
 
                                     sender.send(response).map_err(Error::ApiResponseSend)?;
@@ -457,7 +423,7 @@ impl Vmm {
                                 ApiRequest::VmInfo(sender) => {
                                     let response = match self.vm_info() {
                                         Ok(info) => Ok(ApiResponsePayload::VmInfo(info)),
-                                        Err(_) => Err(ApiError::VmInfo),
+                                        Err(e) => Err(ApiError::VmInfo(e)),
                                     };
 
                                     sender.send(response).map_err(Error::ApiResponseSend)?;

vmm/src/vm.rs

@@ -209,6 +209,12 @@ pub enum Error {
 
     /// Failed to create a new KVM instance
     KvmNew(io::Error),
+
+    /// VM is not created
+    VmNotCreated,
+
+    /// Cannot clone EventFd.
+    EventFdClone(io::Error),
 }
 pub type Result<T> = result::Result<T, Error>;