From fff62d93020f9f06f0f574f419ab69b702eb07f6 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Mon, 19 May 2025 10:12:29 +0200 Subject: [PATCH] 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 On-behalf-of: SAP philipp.schuster@sap.com --- src/main.rs | 6 +++--- vmm/src/coredump.rs | 15 ++++++++++----- vmm/src/gdb.rs | 43 +++++++++++++++++++++++++++++-------------- vmm/src/vm.rs | 8 ++++---- 4 files changed, 46 insertions(+), 26 deletions(-) diff --git a/src/main.rs b/src/main.rs index 7de6b98b6..cfa6a2481 100644 --- a/src/main.rs +++ b/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}")] diff --git a/vmm/src/coredump.rs b/vmm/src/coredump.rs index 195bf8ca4..3ff02e97c 100644 --- a/vmm/src/coredump.rs +++ b/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, } -#[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 { diff --git a/vmm/src/gdb.rs b/vmm/src/gdb.rs index 38305e95a..6f06277b8 100644 --- a/vmm/src/gdb.rs +++ b/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 = std::result::Result; diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 61b45fee2..8db238e30 100644 --- a/vmm/src/vm.rs +++ b/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),