Error type expansion

Signed-off-by: Jonathan Woollett-Light <[email protected]>

Author: Jonathan Woollett-Light

src/api_server/Cargo.toml

@@ -11,6 +11,7 @@ serde = { version = ">=1.0.27", features = ["derive"] }
 serde_derive = ">=1.0.27"
 serde_json = ">=1.0.9"
 derive_more = { version = "0.99.17", default-features = false, features = ["from"] }
+thiserror = "1.0.32"
 
 logger = { path = "../logger" }
 micro_http = { git = "https://github.com/firecracker-microvm/micro-http", rev = "0a58eb1" }

src/api_server/src/lib.rs

@@ -34,22 +34,16 @@ pub type ApiRequest = Box<VmmAction>;
 pub type ApiResponse = Box<std::result::Result<VmmData, VmmActionError>>;
 
 /// Errors thrown when binding the API server to the socket path.
+#[derive(thiserror::Error)]
 pub enum Error {
     /// IO related error.
+    #[error("IO error: {0}")]
     Io(io::Error),
     /// EventFD related error.
+    #[error("EventFd error: {0}")]
     Eventfd(io::Error),
 }
 
-impl fmt::Display for Error {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match *self {
-            Error::Io(ref err) => write!(f, "IO error: {}", err),
-            Error::Eventfd(ref err) => write!(f, "EventFd error: {}", err),
-        }
-    }
-}
-
 impl fmt::Debug for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {

src/arch/Cargo.toml

@@ -14,6 +14,7 @@ versionize = ">=0.1.6"
 versionize_derive = ">=0.1.3"
 vm-fdt = "0.1.0"
 derive_more = { version = "0.99.17", default-features = false, features = ["from"] }
+thiserror = "1.0.32"
 bitflags = ">=1.0.4"
 
 arch_gen = { path = "../arch_gen" }

src/arch/src/x86_64/interrupts.rs

@@ -9,12 +9,14 @@ use kvm_bindings::kvm_lapic_state;
 use kvm_ioctls::VcpuFd;
 use utils::byte_order;
 /// Errors thrown while configuring the LAPIC.
-#[derive(Debug)]
+#[derive(Debug, thiserror::Error, PartialEq)]
 pub enum Error {
-    /// Failure in retrieving the LAPIC configuration.
-    GetLapic(kvm_ioctls::Error),
-    /// Failure in modifying the LAPIC configuration.
-    SetLapic(kvm_ioctls::Error),
+    /// Failure in getting the LAPIC configuration.
+    #[error("Failure in getting the LAPIC configuration: {0}")]
+    Get(kvm_ioctls::Error),
+    /// Failure in setting the LAPIC configuration.
+    #[error("Failure in setting the LAPIC configuration: {0}")]
+    Set(kvm_ioctls::Error),
 }
 type Result<T> = std::result::Result<T, Error>;
 
@@ -45,7 +47,7 @@ fn set_apic_delivery_mode(reg: u32, mode: u32) -> u32 {
 /// # Arguments
 /// * `vcpu` - The VCPU object to configure.
 pub fn set_lint(vcpu: &VcpuFd) -> Result<()> {
-    let mut klapic = vcpu.get_lapic().map_err(Error::GetLapic)?;
+    let mut klapic = vcpu.get_lapic().map_err(Error::Get)?;
 
     let lvt_lint0 = get_klapic_reg(&klapic, APIC_LVT0);
     set_klapic_reg(
@@ -60,7 +62,7 @@ pub fn set_lint(vcpu: &VcpuFd) -> Result<()> {
         set_apic_delivery_mode(lvt_lint1, APIC_MODE_NMI),
     );
 
-    vcpu.set_lapic(&klapic).map_err(Error::SetLapic)
+    vcpu.set_lapic(&klapic).map_err(Error::Set)
 }
 
 #[cfg(test)]

src/arch/src/x86_64/msr.rs

@@ -282,20 +282,44 @@ pub fn create_boot_msr_entries() -> Vec<kvm_msr_entry> {
     ]
 }
 
+/// Error type for [`set_msrs`].
+#[derive(Debug, thiserror::Error)]
+pub enum SetMSRsError {
+    /// Failed to create [`vmm_sys_util::fam::FamStructWrapper`] for MSRs.
+    #[error("Could not create `vmm_sys_util::fam::FamStructWrapper` for MSRs")]
+    Create(utils::fam::Error),
+    /// Settings MSRs resulted in an error.
+    #[error("Setting MSRs resulted in an error: {0}")]
+    Set(#[from] kvm_ioctls::Error),
+    /// Not all given MSRs where set.
+    #[error("Not all given MSRs where set.")]
+    Incomplete,
+}
+
 /// Configure Model Specific Registers (MSRs) required to boot Linux for a given x86_64 vCPU.
 ///
 /// # Arguments
 ///
 /// * `vcpu` - Structure for the VCPU that holds the VCPU's fd.
-pub fn set_msrs(vcpu: &VcpuFd, msr_entries: &[kvm_msr_entry]) -> Result<()> {
-    let msrs = Msrs::from_entries(&msr_entries).map_err(Error::FamError)?;
+///
+/// # Errors
+///
+/// When:
+/// - Failed to create [`vmm_sys_util::fam::FamStructWrapper`] for MSRs.
+/// - [`kvm_ioctls::ioctls::vcpu::VcpuFd::set_msrs`] errors.
+/// - [`kvm_ioctls::ioctls::vcpu::VcpuFd::set_msrs`] fails to write all given MSRs entries.
+pub fn set_msrs(
+    vcpu: &VcpuFd,
+    msr_entries: &[kvm_msr_entry],
+) -> std::result::Result<(), SetMSRsError> {
+    let msrs = Msrs::from_entries(&msr_entries).map_err(SetMSRsError::Create)?;
     vcpu.set_msrs(&msrs)
-        .map_err(Error::SetModelSpecificRegisters)
+        .map_err(SetMSRsError::Set)
         .and_then(|msrs_written| {
-            if msrs_written as u32 != msrs.as_fam_struct_ref().nmsrs {
-                Err(Error::SetModelSpecificRegistersCount)
-            } else {
+            if msrs_written as u32 == msrs.as_fam_struct_ref().nmsrs {
                 Ok(())
+            } else {
+                Err(SetMSRsError::Incomplete)
             }
         })
 }

src/arch/src/x86_64/regs.rs

@@ -5,7 +5,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the THIRD-PARTY file.
 
-use std::mem;
+use std::{fmt, mem};
 
 use kvm_bindings::{kvm_fpu, kvm_regs, kvm_sregs};
 use kvm_ioctls::VcpuFd;
@@ -19,42 +19,76 @@ const PDPTE_START: u64 = 0xa000;
 const PDE_START: u64 = 0xb000;
 
 /// Errors thrown while setting up x86_64 registers.
-#[derive(Debug)]
+#[derive(Debug, thiserror::Error, PartialEq)]
 pub enum Error {
     /// Failed to get SREGs for this CPU.
+    #[error("Failed to get SREGs for this CPU: {0}")]
     GetStatusRegisters(kvm_ioctls::Error),
     /// Failed to set base registers for this CPU.
+    #[error("Failed to set base registers for this CPU: {0}")]
     SetBaseRegisters(kvm_ioctls::Error),
     /// Failed to configure the FPU.
+    #[error("Failed to configure the FPU: {0}")]
     SetFPURegisters(kvm_ioctls::Error),
     /// Failed to set SREGs for this CPU.
+    #[error("Failed to set SREGs for this CPU: {0}")]
     SetStatusRegisters(kvm_ioctls::Error),
     /// Writing the GDT to RAM failed.
+    #[error("Writing the GDT to RAM failed.")]
     WriteGDT,
     /// Writing the IDT to RAM failed.
+    #[error("Writing the IDT to RAM failed")]
     WriteIDT,
     /// Writing PDPTE to RAM failed.
+    #[error("WritePDPTEAddress")]
     WritePDPTEAddress,
     /// Writing PDE to RAM failed.
+    #[error("WritePDEAddress")]
     WritePDEAddress,
     /// Writing PML4 to RAM failed.
+    #[error("WritePML4Address")]
     WritePML4Address,
 }
 type Result<T> = std::result::Result<T, Error>;
 
+/// Error type for [`setup_fpu`].
+#[derive(Debug, derive_more::From, PartialEq)]
+pub struct SetupFpuError(utils::errno::Error);
+impl std::error::Error for SetupFpuError {}
+impl fmt::Display for SetupFpuError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "Failed to setup FPU: {}", self.0)
+    }
+}
+
 /// Configure Floating-Point Unit (FPU) registers for a given CPU.
 ///
 /// # Arguments
 ///
 /// * `vcpu` - Structure for the VCPU that holds the VCPU's fd.
-pub fn setup_fpu(vcpu: &VcpuFd) -> Result<()> {
+///
+/// # Errors
+///
+/// When [`kvm_ioctls::ioctls::vcpu::VcpuFd::set_fpu`] errors.
+pub fn setup_fpu(vcpu: &VcpuFd) -> std::result::Result<(), SetupFpuError> {
     let fpu: kvm_fpu = kvm_fpu {
         fcw: 0x37f,
         mxcsr: 0x1f80,
         ..Default::default()
     };
 
-    vcpu.set_fpu(&fpu).map_err(Error::SetFPURegisters)
+    let res = vcpu.set_fpu(&fpu)?;
+    Ok(res)
+}
+
+/// Error type of [`setup_regs`].
+#[derive(Debug, derive_more::From, PartialEq)]
+pub struct SetupRegistersError(utils::errno::Error);
+impl std::error::Error for SetupRegistersError {}
+impl fmt::Display for SetupRegistersError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "Failed to setup registers:{}", self.0)
+    }
 }
 
 /// Configure base registers for a given CPU.
@@ -63,7 +97,11 @@ pub fn setup_fpu(vcpu: &VcpuFd) -> Result<()> {
 ///
 /// * `vcpu` - Structure for the VCPU that holds the VCPU's fd.
 /// * `boot_ip` - Starting instruction pointer.
-pub fn setup_regs(vcpu: &VcpuFd, boot_ip: u64) -> Result<()> {
+///
+/// # Errors
+///
+/// When [`kvm_ioctls::ioctls::vcpu::VcpuFd::set_regs`] errors.
+pub fn setup_regs(vcpu: &VcpuFd, boot_ip: u64) -> std::result::Result<(), SetupRegistersError> {
     let regs: kvm_regs = kvm_regs {
         rflags: 0x0000_0000_0000_0002u64,
         rip: boot_ip,
@@ -79,7 +117,24 @@ pub fn setup_regs(vcpu: &VcpuFd, boot_ip: u64) -> Result<()> {
         ..Default::default()
     };
 
-    vcpu.set_regs(&regs).map_err(Error::SetBaseRegisters)
+    vcpu.set_regs(&regs).map_err(SetupRegistersError)
+}
+
+/// Error type for [`setup_sregs`].
+#[derive(Debug, thiserror::Error, PartialEq)]
+pub enum SetupSegmentRegistersError {
+    /// Failed to get segment registers
+    #[error("Failed to get segment registers: {0}")]
+    GetSegmentRegisters(utils::errno::Error),
+    /// Failed to configure segments and segment registers
+    #[error("Failed to configure segments and segment registers: {0}")]
+    ConfigureSegmentsAndSegmentRegisters(Error),
+    /// Failed to setup page tables
+    #[error("Failed to setup page tables: {0}")]
+    SetupPageTables(Error),
+    /// Failed to set segment registers
+    #[error("Failed to set segment registers: {0}")]
+    SetSegmentRegisters(utils::errno::Error),
 }
 
 /// Configures the segment registers and system page tables for a given CPU.
@@ -88,13 +143,28 @@ pub fn setup_regs(vcpu: &VcpuFd, boot_ip: u64) -> Result<()> {
 ///
 /// * `mem` - The memory that will be passed to the guest.
 /// * `vcpu` - Structure for the VCPU that holds the VCPU's fd.
-pub fn setup_sregs(mem: &GuestMemoryMmap, vcpu: &VcpuFd) -> Result<()> {
-    let mut sregs: kvm_sregs = vcpu.get_sregs().map_err(Error::GetStatusRegisters)?;
-
-    configure_segments_and_sregs(mem, &mut sregs)?;
-    setup_page_tables(mem, &mut sregs)?; // TODO(dgreid) - Can this be done once per system instead?
-
-    vcpu.set_sregs(&sregs).map_err(Error::SetStatusRegisters)
+///
+/// # Errors
+///
+/// When:
+/// - [`kvm_ioctls::ioctls::vcpu::VcpuFd::get_sregs`] errors.
+/// - [`configure_segments_and_sregs`] errors.
+/// - [`setup_page_tables`] errors
+/// - [`kvm_ioctls::ioctls::vcpu::VcpuFd::set_sregs`] errors.
+pub fn setup_sregs(
+    mem: &GuestMemoryMmap,
+    vcpu: &VcpuFd,
+) -> std::result::Result<(), SetupSegmentRegistersError> {
+    let mut sregs: kvm_sregs = vcpu
+        .get_sregs()
+        .map_err(SetupSegmentRegistersError::GetSegmentRegisters)?;
+
+    configure_segments_and_sregs(mem, &mut sregs)
+        .map_err(SetupSegmentRegistersError::ConfigureSegmentsAndSegmentRegisters)?;
+    setup_page_tables(mem, &mut sregs).map_err(SetupSegmentRegistersError::SetupPageTables)?; // TODO(dgreid) - Can this be done once per system instead?
+
+    vcpu.set_sregs(&sregs)
+        .map_err(SetupSegmentRegistersError::SetSegmentRegisters)
 }
 
 const BOOT_GDT_OFFSET: u64 = 0x500;

src/cpuid/Cargo.toml

@@ -9,6 +9,7 @@ license = "Apache-2.0"
 kvm-bindings = { version = ">=0.5.0", features = ["fam-wrappers"] }
 kvm-ioctls = ">=0.9.0"
 derive_more = { version = "0.99.17", default-features = false, features = ["from"] }
+thiserror = "1.0.32"
 
 utils = { path = "../utils"}
 arch = { path = "../arch" }

src/cpuid/src/common.rs

@@ -18,11 +18,13 @@ pub const VENDOR_ID_INTEL: &[u8; 12] = b"GenuineIntel";
 pub const VENDOR_ID_AMD: &[u8; 12] = b"AuthenticAMD";
 
 /// cpuid related error.
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, thiserror::Error, PartialEq, Eq)]
 pub enum Error {
     /// The function was called with invalid parameters.
+    #[error("The function was called with invalid parameters.")]
     InvalidParameters(String),
     /// Function not supported on the current architecture.
+    #[error("Function not supported on the current architecture.")]
     NotSupported,
 }
 

src/cpuid/src/transformer/common.rs

@@ -114,16 +114,18 @@ pub fn use_host_cpuid_function(
             break;
         }
 
-        cpuid.push(kvm_cpuid_entry2 {
-            function,
-            index: count,
-            flags: 0,
-            eax: entry.eax,
-            ebx: entry.ebx,
-            ecx: entry.ecx,
-            edx: entry.edx,
-            padding: [0, 0, 0],
-        })?;
+        cpuid
+            .push(kvm_cpuid_entry2 {
+                function,
+                index: count,
+                flags: 0,
+                eax: entry.eax,
+                ebx: entry.ebx,
+                ecx: entry.ecx,
+                edx: entry.edx,
+                padding: [0, 0, 0],
+            })
+            .map_err(Error::FamError)?;
 
         count += 1;
     }

src/cpuid/src/transformer/mod.rs

@@ -53,15 +53,19 @@ impl VmSpec {
 }
 
 /// Errors associated with processing the CPUID leaves.
-#[derive(Debug, Clone, derive_more::From)]
+#[derive(Debug, Clone, thiserror::Error)]
 pub enum Error {
     /// A FamStructWrapper operation has failed
+    #[error("A FamStructWrapper operation has failed.")]
     FamError(utils::fam::Error),
     /// A call to an internal helper method failed
-    InternalError(super::common::Error),
+    #[error("A call to an internal helper method failed: {0}")]
+    InternalError(#[from] super::common::Error),
     /// The operation is not permitted for the current vendor
+    #[error("The operation is not permitted for the current vendor.")]
     InvalidVendor,
     /// The maximum number of addressable logical CPUs cannot be stored in an `u8`.
+    #[error("The maximum number of addressable logical CPUs cannot be stored in an `u8`.")]
     VcpuCountOverflow,
 }
 

src/seccompiler/src/lib.rs

@@ -56,6 +56,7 @@ pub enum InstallationError {
     /// Error returned by `prctl`.
     Prctl(i32),
 }
+impl std::error::Error for InstallationError {}
 
 impl Display for InstallationError {
     fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {

src/snapshot/Cargo.toml

@@ -10,6 +10,7 @@ license = "Apache-2.0"
 libc = "0.2"
 versionize = ">=0.1.6"
 versionize_derive = ">=0.1.3"
+thiserror = "1.0.32"
 
 [dev-dependencies]
 criterion = "0.3.0"