misc: vmm: streamline #[source] and Error

This streamlines the code base to follow best practices for
error handling in Rust: Each error struct implements
std::error::Error (most due via thiserror::Error derive macro)
and sets its source accordingly.

This allows future work that nicely prints the error chains,
for example.

So far, the convention is that each error prints its
sub error as part of its Display::fmt() impl.

Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com

src/main.rs

@@ -83,14 +83,14 @@ enum Error {
     EventMonitorThread(#[source] vmm::Error),
     #[cfg(feature = "guest_debug")]
     #[error("Error parsing --gdb: {0}")]
-    ParsingGdb(option_parser::OptionParserError),
+    ParsingGdb(#[source] option_parser::OptionParserError),
     #[cfg(feature = "guest_debug")]
     #[error("Error parsing --gdb: path required")]
     BareGdb,
     #[error("Error creating log file: {0}")]
-    LogFileCreation(std::io::Error),
+    LogFileCreation(#[source] std::io::Error),
     #[error("Error setting up logger: {0}")]
-    LoggerSetup(log::SetLoggerError),
+    LoggerSetup(#[source] log::SetLoggerError),
     #[error("Failed to gracefully shutdown http api: {0}")]
     HttpApiShutdown(#[source] vmm::Error),
     #[error("Failed to create Landlock object: {0}")]

vmm/src/coredump.rs

@@ -9,6 +9,7 @@ use std::io::Write;
 #[cfg(target_arch = "x86_64")]
 use hypervisor::arch::x86::{DescriptorTable, SegmentRegister};
 use linux_loader::elf;
+use thiserror::Error;
 use vm_memory::ByteValued;
 
 #[derive(Clone)]
@@ -33,12 +34,16 @@ pub struct DumpState {
     pub file: Option<File>,
 }
 
-#[derive(Debug)]
+#[derive(Error, Debug)]
 pub enum GuestDebuggableError {
-    Coredump(anyhow::Error),
-    CoredumpFile(std::io::Error),
-    Pause(vm_migration::MigratableError),
-    Resume(vm_migration::MigratableError),
+    #[error("coredump: {0}")]
+    Coredump(#[source] anyhow::Error),
+    #[error("coredump file: {0}")]
+    CoredumpFile(#[source] std::io::Error),
+    #[error("Failed to pause: {0}")]
+    Pause(#[source] vm_migration::MigratableError),
+    #[error("Failed to resume: {0}")]
+    Resume(#[source] vm_migration::MigratableError),
 }
 
 pub trait GuestDebuggable: vm_migration::Pausable {

vmm/src/gdb.rs

@@ -29,22 +29,32 @@ use gdbstub_arch::aarch64::AArch64 as GdbArch;
 use gdbstub_arch::x86::reg::X86_64CoreRegs as CoreRegs;
 #[cfg(target_arch = "x86_64")]
 use gdbstub_arch::x86::X86_64_SSE as GdbArch;
+use thiserror::Error;
 use vm_memory::{GuestAddress, GuestMemoryAtomic, GuestMemoryError};
 
 use crate::GuestMemoryMmap;
 
 type ArchUsize = u64;
 
-#[derive(Debug)]
+#[derive(Error, Debug)]
 pub enum DebuggableError {
-    SetDebug(hypervisor::HypervisorCpuError),
-    Pause(vm_migration::MigratableError),
-    Resume(vm_migration::MigratableError),
-    ReadRegs(crate::cpu::Error),
-    WriteRegs(crate::cpu::Error),
-    ReadMem(GuestMemoryError),
-    WriteMem(GuestMemoryError),
-    TranslateGva(crate::cpu::Error),
+    #[error("Setting debug failed: {0}")]
+    SetDebug(#[source] hypervisor::HypervisorCpuError),
+    #[error("Pausing failed: {0}")]
+    Pause(#[source] vm_migration::MigratableError),
+    #[error("Resuming failed: {0}")]
+    Resume(#[source] vm_migration::MigratableError),
+    #[error("Reading registers failed: {0}")]
+    ReadRegs(#[source] crate::cpu::Error),
+    #[error("Writing registers failed: {0}")]
+    WriteRegs(#[source] crate::cpu::Error),
+    #[error("Reading memory failed: {0}")]
+    ReadMem(#[source] GuestMemoryError),
+    #[error("Writing memory failed: {0}")]
+    WriteMem(#[source] GuestMemoryError),
+    #[error("Translating GVA failed: {0}")]
+    TranslateGva(#[source] crate::cpu::Error),
+    #[error("The lock is poisened")]
     PoisonedState,
 }
 
@@ -80,13 +90,18 @@ pub trait Debuggable: vm_migration::Pausable {
     fn active_vcpus(&self) -> usize;
 }
 
-#[derive(Debug)]
+#[derive(Error, Debug)]
 pub enum Error {
-    Vm(crate::vm::Error),
+    #[error("VM failed: {0}")]
+    Vm(#[source] crate::vm::Error),
+    #[error("GDB request failed")]
     GdbRequest,
-    GdbResponseNotify(std::io::Error),
-    GdbResponse(mpsc::RecvError),
-    GdbResponseTimeout(mpsc::RecvTimeoutError),
+    #[error("GDB couldn't be notified: {0}")]
+    GdbResponseNotify(#[source] std::io::Error),
+    #[error("GDB response failed: {0}")]
+    GdbResponse(#[source] mpsc::RecvError),
+    #[error("GDB response timeout: {0}")]
+    GdbResponseTimeout(#[source] mpsc::RecvTimeoutError),
 }
 type GdbResult<T> = std::result::Result<T, Error>;
 

vmm/src/vm.rs

@@ -114,7 +114,7 @@ pub enum Error {
 
     #[cfg(target_arch = "aarch64")]
     #[error("Cannot load the UEFI binary in memory: {0:?}")]
-    UefiLoad(arch::aarch64::uefi::Error),
+    UefiLoad(#[source] arch::aarch64::uefi::Error),
 
     #[error("Cannot load the initramfs into memory")]
     InitramfsLoad,
@@ -136,7 +136,7 @@ pub enum Error {
 
     #[cfg(target_arch = "aarch64")]
     #[error("Cannot enable interrupt controller: {0:?}")]
-    EnableInterruptController(interrupt_controller::Error),
+    EnableInterruptController(#[source] interrupt_controller::Error),
 
     #[error("VM state is poisoned")]
     PoisonedState,
@@ -277,7 +277,7 @@ pub enum Error {
 
     #[cfg(feature = "tdx")]
     #[error("Error allocating TDVF memory: {0:?}")]
-    AllocatingTdvfMemory(crate::memory_manager::Error),
+    AllocatingTdvfMemory(#[source] crate::memory_manager::Error),
 
     #[cfg(feature = "tdx")]
     #[error("Error enabling TDX VM: {0}")]
@@ -301,7 +301,7 @@ pub enum Error {
 
     #[cfg(feature = "guest_debug")]
     #[error("Error debugging VM: {0:?}")]
-    Debug(DebuggableError),
+    Debug(#[source] DebuggableError),
 
     #[error("Error spawning kernel loading thread")]
     KernelLoadThreadSpawn(#[source] std::io::Error),