diff --git a/Cargo.lock b/Cargo.lock index d3ab2bf7dc2..3515fe02ce4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -50,6 +50,7 @@ dependencies = [ "serde", "serde_derive", "serde_json", + "thiserror", "utils", "vmm", ] @@ -67,6 +68,7 @@ dependencies = [ "libc", "linux-loader", "logger", + "thiserror", "utils", "versionize", "versionize_derive", @@ -243,6 +245,7 @@ dependencies = [ "derive_more", "kvm-bindings", "kvm-ioctls", + "thiserror", "utils", ] @@ -1045,6 +1048,7 @@ version = "0.1.0" dependencies = [ "criterion", "libc", + "thiserror", "versionize", "versionize_derive", ] @@ -1077,18 +1081,18 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.30" +version = "1.0.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "854babe52e4df1653706b98fcfc05843010039b406875930a70e4d9644e5c417" +checksum = "f5f6586b7f764adc0231f4c79be7b920e766bb2f3e51b3661cdb263828f19994" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.30" +version = "1.0.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aa32fd3f627f367fe16f893e2597ae3c05020f8bba2666a4e6ea73d377e5714b" +checksum = "12bafc5b54507e0149cdf1b145a5d80ab80a90bcd9275df43d4fff68460f6c21" dependencies = [ "proc-macro2", "quote", @@ -1280,6 +1284,7 @@ dependencies = [ "serde", "serde_json", "snapshot", + "thiserror", "userfaultfd", "utils", "versionize", diff --git a/src/api_server/Cargo.toml b/src/api_server/Cargo.toml index 3bdd069dae5..e344e6cf0a7 100644 --- a/src/api_server/Cargo.toml +++ b/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" } diff --git a/src/api_server/src/lib.rs b/src/api_server/src/lib.rs index 6988005befb..ebc2cbe1942 100644 --- a/src/api_server/src/lib.rs +++ b/src/api_server/src/lib.rs @@ -34,22 +34,16 @@ pub type ApiRequest = Box; pub type ApiResponse = Box>; /// 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 { diff --git a/src/arch/Cargo.toml b/src/arch/Cargo.toml index 5e43772c553..e7bb11ab927 100644 --- a/src/arch/Cargo.toml +++ b/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" } diff --git a/src/arch/src/aarch64/gic/mod.rs b/src/arch/src/aarch64/gic/mod.rs index 5702c08fddd..2d851ffb51c 100644 --- a/src/arch/src/aarch64/gic/mod.rs +++ b/src/arch/src/aarch64/gic/mod.rs @@ -16,15 +16,21 @@ pub use regs::GicState; use super::layout; /// Errors thrown while setting up the GIC. -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum Error { /// Error while calling KVM ioctl for setting up the global interrupt controller. + #[error("Error while calling KVM ioctl for setting up the global interrupt controller: {0}")] CreateGIC(kvm_ioctls::Error), /// Error while setting or getting device attributes for the GIC. + #[error("Error while setting or getting device attributes for the GIC: {0}, {1}, {2}")] DeviceAttribute(kvm_ioctls::Error, bool, u32), /// The number of vCPUs in the GicState doesn't match the number of vCPUs on the system + #[error( + "The number of vCPUs in the GicState doesn't match the number of vCPUs on the system." + )] InconsistentVcpuCount, /// The VgicSysRegsState is invalid + #[error("The VgicSysRegsState is invalid.")] InvalidVgicSysRegState, } type Result = result::Result; diff --git a/src/arch/src/x86_64/interrupts.rs b/src/arch/src/x86_64/interrupts.rs index e84591dcf92..d4653d73064 100644 --- a/src/arch/src/x86_64/interrupts.rs +++ b/src/arch/src/x86_64/interrupts.rs @@ -9,11 +9,13 @@ 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. + /// Failure in getting the LAPIC configuration. + #[error("Failure in getting the LAPIC configuration: {0}")] GetLapic(kvm_ioctls::Error), - /// Failure in modifying the LAPIC configuration. + /// Failure in setting the LAPIC configuration. + #[error("Failure in setting the LAPIC configuration: {0}")] SetLapic(kvm_ioctls::Error), } type Result = std::result::Result; diff --git a/src/arch/src/x86_64/msr.rs b/src/arch/src/x86_64/msr.rs index e57f1e9e0a5..3990800a002 100644 --- a/src/arch/src/x86_64/msr.rs +++ b/src/arch/src/x86_64/msr.rs @@ -282,20 +282,44 @@ pub fn create_boot_msr_entries() -> Vec { ] } +/// 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 were set. + #[error("Not all given MSRs were 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) } }) } diff --git a/src/arch/src/x86_64/regs.rs b/src/arch/src/x86_64/regs.rs index 54c2306032c..ea5f2aca022 100644 --- a/src/arch/src/x86_64/regs.rs +++ b/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 = std::result::Result; +/// 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,22 +117,54 @@ pub fn setup_regs(vcpu: &VcpuFd, boot_ip: u64) -> Result<()> { ..Default::default() }; - vcpu.set_regs(®s).map_err(Error::SetBaseRegisters) + vcpu.set_regs(®s).map_err(SetupRegistersError) +} + +/// Error type for [`setup_sregs`]. +#[derive(Debug, thiserror::Error, PartialEq)] +pub enum SetupSpecialRegistersError { + /// Failed to get special registers + #[error("Failed to get special registers: {0}")] + GetSpecialRegisters(utils::errno::Error), + /// Failed to configure segments and special registers + #[error("Failed to configure segments and special registers: {0}")] + ConfigureSegmentsAndSpecialRegisters(Error), + /// Failed to setup page tables + #[error("Failed to setup page tables: {0}")] + SetupPageTables(Error), + /// Failed to set special registers + #[error("Failed to set special registers: {0}")] + SetSpecialRegisters(utils::errno::Error), } -/// Configures the segment registers and system page tables for a given CPU. +/// Configures the special registers and system page tables for a given CPU. /// /// # Arguments /// /// * `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<(), SetupSpecialRegistersError> { + let mut sregs: kvm_sregs = vcpu + .get_sregs() + .map_err(SetupSpecialRegistersError::GetSpecialRegisters)?; + + configure_segments_and_sregs(mem, &mut sregs) + .map_err(SetupSpecialRegistersError::ConfigureSegmentsAndSpecialRegisters)?; + setup_page_tables(mem, &mut sregs).map_err(SetupSpecialRegistersError::SetupPageTables)?; // TODO(dgreid) - Can this be done once per system instead? + + vcpu.set_sregs(&sregs) + .map_err(SetupSpecialRegistersError::SetSpecialRegisters) } const BOOT_GDT_OFFSET: u64 = 0x500; diff --git a/src/cpuid/Cargo.toml b/src/cpuid/Cargo.toml index 30e8d87d97f..14e244f36a6 100644 --- a/src/cpuid/Cargo.toml +++ b/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" } diff --git a/src/cpuid/src/common.rs b/src/cpuid/src/common.rs index d749e6202f0..e1234eb5f2a 100644 --- a/src/cpuid/src/common.rs +++ b/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, } diff --git a/src/cpuid/src/transformer/common.rs b/src/cpuid/src/transformer/common.rs index ba89fc35f74..fdec5075d78 100644 --- a/src/cpuid/src/transformer/common.rs +++ b/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; } diff --git a/src/cpuid/src/transformer/mod.rs b/src/cpuid/src/transformer/mod.rs index 6dd01d05c8a..d254385c5df 100644 --- a/src/cpuid/src/transformer/mod.rs +++ b/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, } diff --git a/src/seccompiler/src/lib.rs b/src/seccompiler/src/lib.rs index f18412ec059..09b815dd998 100644 --- a/src/seccompiler/src/lib.rs +++ b/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 { diff --git a/src/snapshot/Cargo.toml b/src/snapshot/Cargo.toml index 7dd764f981b..2608fcabe82 100644 --- a/src/snapshot/Cargo.toml +++ b/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" diff --git a/src/snapshot/src/lib.rs b/src/snapshot/src/lib.rs index 1e4c476391d..278b7b8bb1d 100644 --- a/src/snapshot/src/lib.rs +++ b/src/snapshot/src/lib.rs @@ -42,21 +42,28 @@ const BASE_MAGIC_ID: u64 = 0x0710_1984_8664_0000u64; const BASE_MAGIC_ID: u64 = 0x0710_1984_AAAA_0000u64; /// Error definitions for the Snapshot API. -#[derive(Debug, PartialEq)] +#[derive(Debug, thiserror::Error, PartialEq)] pub enum Error { /// CRC64 validation failed. + #[error("CRC64 validation failed: {0}")] Crc64(u64), /// Invalid data version. + #[error("Invalid data version: {0}")] InvalidDataVersion(u16), /// Invalid format version. + #[error("Invalid format version: {0}")] InvalidFormatVersion(u16), /// Magic value does not match arch. + #[error("Magic value does not match arch: {0}")] InvalidMagic(u64), /// Snapshot file is smaller than CRC length. + #[error("Snapshot file is smaller than CRC length.")] InvalidSnapshotSize, /// An IO error occurred. + #[error("An IO error occurred: {0}")] Io(i32), /// A versioned serialization/deserialization error occurred. + #[error("A versioned serialization/deserialization error occurred: {0}")] Versionize(versionize::VersionizeError), } diff --git a/src/vmm/Cargo.toml b/src/vmm/Cargo.toml index 68a64a4d23c..2da11dbf4dc 100644 --- a/src/vmm/Cargo.toml +++ b/src/vmm/Cargo.toml @@ -20,6 +20,7 @@ versionize_derive = ">=0.1.3" vm-superio = ">=0.4.0" vm-allocator = "0.1.0" derive_more = { version = "0.99.17", default-features = false, features = ["from"] } +thiserror = "1.0.32" arch = { path = "../arch" } devices = { path = "../devices" } diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs index bffb43e47cf..757f168375e 100644 --- a/src/vmm/src/builder.rs +++ b/src/vmm/src/builder.rs @@ -95,7 +95,7 @@ pub enum StartMicrovmError { /// Unable to set VmResources. SetVmResources(VmConfigError), } - +impl std::error::Error for StartMicrovmError {} /// It's convenient to automatically convert `linux_loader::cmdline::Error`s /// to `StartMicrovmError`s. impl std::convert::From for StartMicrovmError { @@ -418,7 +418,8 @@ pub fn build_microvm_for_boot( .ok_or_else(|| MissingSeccompFilters("vcpu".to_string()))? .clone(), ) - .map_err(Internal)?; + .map_err(Error::VcpuStart) + .map_err(StartMicrovmError::Internal)?; // Load seccomp filters for the VMM thread. // Execution panics if filters cannot be loaded, use --no-seccomp if skipping filters @@ -430,7 +431,7 @@ pub fn build_microvm_for_boot( .ok_or_else(|| MissingSeccompFilters("vmm".to_string()))?, ) .map_err(Error::SeccompFilters) - .map_err(Internal)?; + .map_err(StartMicrovmError::Internal)?; // The vcpus start off in the `Paused` state, let them run. vmm.resume_vm().map_err(Internal)?; @@ -441,6 +442,65 @@ pub fn build_microvm_for_boot( Ok(vmm) } +/// Error type for [`build_microvm_from_snapshot`]. +#[derive(Debug, thiserror::Error)] +pub enum BuildMicrovmFromSnapshotError { + /// Failed to create micro-VM and vCPUs. + #[error("Failed to create micro-VM and vCPUs: {0}")] + CreateMicrovmAndVcpus(#[from] StartMicrovmError), + /// Only 255 vCPU state are supported, but {0} states where given. + #[error("Only 255 vCPU state are supported, but {0} states where given.")] + TooManyVCPUs(usize), + /// Could not access KVM. + #[error("Could not access KVM: {0}")] + KvmAccess(#[from] utils::errno::Error), + /// The CPUID specification from the snapshot contains features unsupported by and/or fields + /// outside the supported range, of the KVM. + #[error( + "The CPUID specification from the snapshot contains features unsupported by and/or fields \ + outside the supported range, of the KVM" + )] + UnsupportedCPUID, + /// Error configuring the TSC, frequency not present in the given snapshot. + #[error("Error configuring the TSC, frequency not present in the given snapshot.")] + TscFrequencyNotPresent, + /// Could not get TSC to check if TSC scaling was required with the snapshot. + #[cfg(target_arch = "x86_64")] + #[error("Could not get TSC to check if TSC scaling was required with the snapshot: {0}")] + GetTsc(#[from] crate::vstate::vcpu::GetTscError), + /// Could not set TSC scaling within the snapshot. + #[cfg(target_arch = "x86_64")] + #[error("Could not set TSC scaling within the snapshot: {0}")] + SetTsc(#[from] crate::vstate::vcpu::SetTscError), + /// Failed to restore micro-VM state. + #[error("Failed to restore micro-VM state: {0}")] + RestoreState(#[from] crate::vstate::vm::RestoreStateError), + /// Failed to update micr-VM configuration. + #[error("Failed to update micr-VM configuration: {0}")] + VmUpdateConfig(#[from] VmConfigError), + /// Failed to restore MMIO device. + #[error("Failed to restore MMIO device: {0}")] + RestoreMmioDevice(#[from] MicrovmStateError), + /// Failed to emulate MMIO serial. + #[error("Failed to emulate MMIO serial: {0}")] + EmulateSerialInit(#[from] crate::EmulateSerialInitError), + /// Failed start vCPUs as no vCPU seccomp filter found. + #[error("Failed start vCPUs as no vCPU seccomp filter found.")] + MissingVcpuSeccompFilters, + /// Failed start vCPUs. + #[error("Failed start vCPUs: {0}")] + StartVcpus(#[from] crate::StartVcpusError), + /// Failed to restore vCPUs. + #[error("Failed to restore vCPUs: {0}")] + RestoreVcpus(#[from] crate::RestoreVcpusError), + /// Failed to apply VMM secccomp filter as none found. + #[error("Failed to apply VMM secccomp filter as none found.")] + MissingVmmSeccompFilters, + /// Failed to apply VMM secccomp filter. + #[error("Failed to apply VMM secccomp filter: {0}")] + SeccompFiltersInternal(#[from] seccompiler::InstallationError), +} + /// Builds and starts a microVM based on the provided MicrovmState. /// /// An `Arc` reference of the built `Vmm` is also plugged in the `EventManager`, while another @@ -455,11 +515,10 @@ pub fn build_microvm_from_snapshot( track_dirty_pages: bool, seccomp_filters: &BpfThreadMap, vm_resources: &mut VmResources, -) -> std::result::Result>, StartMicrovmError> { - use self::StartMicrovmError::*; - let vcpu_count = u8::try_from(microvm_state.vcpu_states.len()) - .map_err(|_| MicrovmStateError::InvalidInput) - .map_err(RestoreMicrovmState)?; +) -> std::result::Result>, BuildMicrovmFromSnapshotError> { + let vcpu_count = u8::try_from(microvm_state.vcpu_states.len()).map_err(|_| { + BuildMicrovmFromSnapshotError::TooManyVCPUs(microvm_state.vcpu_states.len()) + })?; // Build Vmm. let (mut vmm, vcpus) = create_vmm_and_vcpus( @@ -481,25 +540,12 @@ pub fn build_microvm_from_snapshot( // No TSC freq in snapshot means we have to fail-fast. let state_tsc = microvm_state.vcpu_states[0] .tsc_khz - .ok_or_else(|| { - MicrovmStateError::IncompatibleState( - "Error configuring the TSC, frequency not present in snapshot.".to_string(), - ) - }) - .map_err(RestoreMicrovmState)?; + .ok_or(BuildMicrovmFromSnapshotError::TscFrequencyNotPresent)?; // Scale the TSC frequency for all VCPUs, if needed. - if vcpus[0] - .kvm_vcpu - .is_tsc_scaling_required(state_tsc) - .map_err(|err| MicrovmStateError::IncompatibleState(err.to_string())) - .map_err(RestoreMicrovmState)? - { + if vcpus[0].kvm_vcpu.is_tsc_scaling_required(state_tsc)? { for vcpu in &vcpus { - vcpu.kvm_vcpu - .set_tsc_khz(state_tsc) - .map_err(|err| MicrovmStateError::IncompatibleState(err.to_string())) - .map_err(RestoreMicrovmState)?; + vcpu.kvm_vcpu.set_tsc_khz(state_tsc)?; } } } @@ -508,28 +554,20 @@ pub fn build_microvm_from_snapshot( { let mpidrs = construct_kvm_mpidrs(µvm_state.vcpu_states); // Restore kvm vm state. - vmm.vm - .restore_state(&mpidrs, µvm_state.vm_state) - .map_err(MicrovmStateError::RestoreVmState) - .map_err(RestoreMicrovmState)?; + vmm.vm.restore_state(&mpidrs, µvm_state.vm_state)?; } // Restore kvm vm state. #[cfg(target_arch = "x86_64")] - vmm.vm - .restore_state(µvm_state.vm_state) - .map_err(MicrovmStateError::RestoreVmState) - .map_err(RestoreMicrovmState)?; - - vm_resources - .update_vm_config(&VmUpdateConfig { - vcpu_count: Some(vcpu_count), - mem_size_mib: Some(mem_size_mib(&guest_memory) as usize), - smt: Some(false), - cpu_template: None, - track_dirty_pages: Some(track_dirty_pages), - }) - .map_err(SetVmResources)?; + vmm.vm.restore_state(µvm_state.vm_state)?; + + vm_resources.update_vm_config(&VmUpdateConfig { + vcpu_count: Some(vcpu_count), + mem_size_mib: Some(mem_size_mib(&guest_memory) as usize), + smt: Some(false), + cpu_template: None, + track_dirty_pages: Some(track_dirty_pages), + })?; // Restore devices states. let mmio_ctor_args = MMIODevManagerConstructorArgs { @@ -543,24 +581,20 @@ pub fn build_microvm_from_snapshot( vmm.mmio_device_manager = MMIODeviceManager::restore(mmio_ctor_args, µvm_state.device_states) - .map_err(MicrovmStateError::RestoreDevices) - .map_err(RestoreMicrovmState)?; - vmm.emulate_serial_init() - .map_err(StartMicrovmError::Internal)?; + .map_err(MicrovmStateError::RestoreDevices)?; + vmm.emulate_serial_init()?; // Move vcpus to their own threads and start their state machine in the 'Paused' state. vmm.start_vcpus( vcpus, seccomp_filters .get("vcpu") - .ok_or_else(|| MissingSeccompFilters("vcpu".to_string()))? + .ok_or(BuildMicrovmFromSnapshotError::MissingVcpuSeccompFilters)? .clone(), - ) - .map_err(Internal)?; + )?; // Restore vcpus kvm state. - vmm.restore_vcpu_states(microvm_state.vcpu_states) - .map_err(RestoreMicrovmState)?; + vmm.restore_vcpu_states(microvm_state.vcpu_states)?; let vmm = Arc::new(Mutex::new(vmm)); event_manager.add_subscriber(vmm.clone()); @@ -570,10 +604,8 @@ pub fn build_microvm_from_snapshot( seccompiler::apply_filter( seccomp_filters .get("vmm") - .ok_or_else(|| MissingSeccompFilters("vmm".to_string()))?, - ) - .map_err(Error::SeccompFilters) - .map_err(StartMicrovmError::Internal)?; + .ok_or(BuildMicrovmFromSnapshotError::MissingVmmSeccompFilters)?, + )?; Ok(vmm) } diff --git a/src/vmm/src/lib.rs b/src/vmm/src/lib.rs index d71932525b5..c02439bf8b8 100644 --- a/src/vmm/src/lib.rs +++ b/src/vmm/src/lib.rs @@ -33,12 +33,11 @@ pub mod vmm_config; mod vstate; use std::collections::HashMap; -use std::fmt::{Display, Formatter}; -use std::io; use std::os::unix::io::AsRawFd; use std::sync::mpsc::{RecvTimeoutError, TryRecvError}; use std::sync::{Arc, Barrier, Mutex}; use std::time::Duration; +use std::{fmt, io}; use arch::DeviceType; use devices::legacy::serial::{IER_RDA_BIT, IER_RDA_OFFSET}; @@ -57,6 +56,7 @@ use userfaultfd::Uffd; use utils::epoll::EventSet; use utils::eventfd::EventFd; use vm_memory::{GuestMemory, GuestMemoryMmap, GuestMemoryRegion}; +use vstate::vcpu::{self, KvmVcpuConfigureError, StartThreadedError, VcpuSendEventError}; #[cfg(target_arch = "x86_64")] use crate::device_manager::legacy::PortIODeviceManager; @@ -116,118 +116,99 @@ pub const HTTP_MAX_PAYLOAD_SIZE: usize = 51200; /// Errors associated with the VMM internal logic. These errors cannot be generated by direct user /// input, but can result from bad configuration of the host (for example if Firecracker doesn't /// have permissions to open the KVM fd). -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum Error { /// Legacy devices work with Event file descriptors and the creation can fail because /// of resource exhaustion. #[cfg(target_arch = "x86_64")] + #[error("Error creating legacy device: {0}")] CreateLegacyDevice(device_manager::legacy::Error), /// Device manager error. + #[error("{0}")] DeviceManager(device_manager::mmio::Error), /// Cannot fetch the KVM dirty bitmap. + #[error("Error getting the KVM dirty bitmap. {0}")] DirtyBitmap(kvm_ioctls::Error), /// Cannot read from an Event file descriptor. + #[error("Event fd error: {0}")] EventFd(io::Error), /// I8042 Error. + #[error("I8042 error: {0}")] I8042Error(devices::legacy::I8042DeviceError), /// Cannot access kernel file. + #[error("Cannot access kernel file: {0}")] KernelFile(io::Error), /// Cannot open /dev/kvm. Either the host does not have KVM or Firecracker does not have /// permission to open the file descriptor. + #[error("Failed to validate KVM support: {0}")] KvmContext(vstate::system::Error), - #[cfg(target_arch = "x86_64")] /// Cannot add devices to the Legacy I/O Bus. + #[cfg(target_arch = "x86_64")] + #[error("Cannot add devices to the legacy I/O Bus. {0}")] LegacyIOBus(device_manager::legacy::Error), /// Internal logger error. + #[error("Logger error: {0}")] Logger(LoggerError), /// Internal metrics system error. + #[error("Metrics error: {0}")] Metrics(MetricsError), /// Cannot add a device to the MMIO Bus. + #[error("Cannot add a device to the MMIO Bus. {0}")] RegisterMMIODevice(device_manager::mmio::Error), /// Cannot install seccomp filters. + #[error("Cannot install seccomp filters: {0}")] SeccompFilters(seccompiler::InstallationError), /// Write to the serial console failed. + #[error("Error writing to the serial console: {0}")] Serial(io::Error), /// Cannot create Timer file descriptor. + #[error("Error creating timer fd: {0}")] TimerFd(io::Error), /// Vcpu configuration error. - VcpuConfigure(vstate::vcpu::VcpuError), + #[error("Error configuring the vcpu for boot: {0}")] + VcpuConfigure(KvmVcpuConfigureError), /// Vcpu create error. + #[error("Error creating the vcpu: {0}")] VcpuCreate(vstate::vcpu::Error), /// Cannot send event to vCPU. + #[error("Cannot send event to vCPU. {0}")] VcpuEvent(vstate::vcpu::Error), /// Cannot create a vCPU handle. + #[error("Cannot create a vCPU handle. {0}")] VcpuHandle(vstate::vcpu::Error), - #[cfg(target_arch = "aarch64")] /// Vcpu init error. + #[cfg(target_arch = "aarch64")] + #[error("Error initializing the vcpu: {0}")] VcpuInit(vstate::vcpu::VcpuError), + /// vCPU start error. + #[error("Failed to start vCPUs")] + VcpuStart(StartVcpusError), /// vCPU pause failed. + #[error("Failed to pause the vCPUs.")] VcpuPause, /// vCPU exit failed. + #[error("Failed to exit the vCPUs.")] VcpuExit, /// vCPU resume failed. + #[error("Failed to resume the vCPUs.")] VcpuResume, /// Vcpu send message failed. + #[error("Failed to message the vCPUs.")] VcpuMessage, /// Cannot spawn a new Vcpu thread. + #[error("Cannot spawn Vcpu thread: {0}")] VcpuSpawn(io::Error), /// Vm error. + #[error("Vm error: {0}")] Vm(vstate::vm::Error), /// Error thrown by observer object on Vmm initialization. + #[error("Error thrown by observer object on Vmm initialization: {0}")] VmmObserverInit(utils::errno::Error), /// Error thrown by observer object on Vmm teardown. + #[error("Error thrown by observer object on Vmm teardown: {0}")] VmmObserverTeardown(utils::errno::Error), } -impl Display for Error { - fn fmt(&self, f: &mut Formatter) -> std::fmt::Result { - use self::Error::*; - - match self { - #[cfg(target_arch = "x86_64")] - CreateLegacyDevice(err) => write!(f, "Error creating legacy device: {}", err), - DeviceManager(err) => write!(f, "{}", err), - DirtyBitmap(err) => write!(f, "Error getting the KVM dirty bitmap. {}", err), - EventFd(err) => write!(f, "Event fd error: {}", err), - I8042Error(err) => write!(f, "I8042 error: {}", err), - KernelFile(err) => write!(f, "Cannot access kernel file: {}", err), - KvmContext(err) => write!(f, "Failed to validate KVM support: {}", err), - #[cfg(target_arch = "x86_64")] - LegacyIOBus(err) => write!(f, "Cannot add devices to the legacy I/O Bus. {}", err), - Logger(err) => write!(f, "Logger error: {}", err), - Metrics(err) => write!(f, "Metrics error: {}", err), - RegisterMMIODevice(err) => write!(f, "Cannot add a device to the MMIO Bus. {}", err), - SeccompFilters(err) => write!(f, "Cannot install seccomp filters: {}", err), - Serial(err) => write!(f, "Error writing to the serial console: {}", err), - TimerFd(err) => write!(f, "Error creating timer fd: {}", err), - VcpuConfigure(err) => write!(f, "Error configuring the vcpu for boot: {}", err), - VcpuCreate(err) => write!(f, "Error creating the vcpu: {}", err), - VcpuEvent(err) => write!(f, "Cannot send event to vCPU. {}", err), - VcpuHandle(err) => write!(f, "Cannot create a vCPU handle. {}", err), - #[cfg(target_arch = "aarch64")] - VcpuInit(err) => write!(f, "Error initializing the vcpu: {}", err), - VcpuPause => write!(f, "Failed to pause the vCPUs."), - VcpuExit => write!(f, "Failed to exit the vCPUs."), - VcpuResume => write!(f, "Failed to resume the vCPUs."), - VcpuMessage => write!(f, "Failed to message the vCPUs."), - VcpuSpawn(err) => write!(f, "Cannot spawn Vcpu thread: {}", err), - Vm(err) => write!(f, "Vm error: {}", err), - VmmObserverInit(err) => write!( - f, - "Error thrown by observer object on Vmm initialization: {}", - err - ), - VmmObserverTeardown(err) => { - write!( - f, - "Error thrown by observer object on Vmm teardown: {}", - err - ) - } - } - } -} - /// Trait for objects that need custom initialization and teardown during the Vmm lifetime. pub trait VmmEventsObserver { /// This function will be called during microVm boot. @@ -251,6 +232,47 @@ pub(crate) fn mem_size_mib(guest_memory: &GuestMemoryMmap) -> u64 { guest_memory.iter().map(|region| region.len()).sum::() >> 20 } +/// Error type for [`Vmm::emulate_serial_init`]. +#[derive(Debug, derive_more::From)] +pub struct EmulateSerialInitError(std::io::Error); +impl fmt::Display for EmulateSerialInitError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "Emulate serial init error: {}", self.0) + } +} +impl std::error::Error for EmulateSerialInitError {} + +/// Error type for [`Vmm::start_vcpus`]. +#[derive(Debug, thiserror::Error)] +pub enum StartVcpusError { + /// Vmm observer init error. + #[error("{0}")] + VmmObserverInit(#[from] utils::errno::Error), + /// vCPU handle error. + #[error("{0}")] + VcpuHandle(#[from] StartThreadedError), +} + +/// Error type for [`Vmm::restore_vcpu_states`] +#[derive(Debug, thiserror::Error)] +pub enum RestoreVcpusError { + /// Invalid input. + #[error("Invalid input.")] + InvalidInput, + /// Failed to send event. + #[error("Failed to send event: {0}")] + SendEvent(#[from] VcpuSendEventError), + /// Vcpu is in unexpected state. + #[error("Unexpected vCPU response.")] + UnexpectedVcpuResponse, + /// Failed to restore vCPU state. + #[error("Failed to restore vCPU state: {0}")] + RestoreVcpuState(#[from] vcpu::Error), + /// Not allowed. + #[error("Not allowed: {0}")] + NotAllowed(String), +} + /// Contains the state and associated methods required for the Firecracker VMM. pub struct Vmm { events_observer: Option>, @@ -300,16 +322,22 @@ impl Vmm { } /// Starts the microVM vcpus. + /// + /// # Errors + /// + /// When: + /// - [`vmm::VmmEventsObserver::on_vmm_boot`] errors. + /// - [`vmm::vstate::vcpu::Vcpu::start_threaded`] errors. pub fn start_vcpus( &mut self, mut vcpus: Vec, vcpu_seccomp_filter: Arc, - ) -> Result<()> { + ) -> std::result::Result<(), StartVcpusError> { let vcpu_count = vcpus.len(); let barrier = Arc::new(Barrier::new(vcpu_count + 1)); if let Some(observer) = self.events_observer.as_mut() { - observer.on_vmm_boot().map_err(Error::VmmObserverInit)?; + observer.on_vmm_boot()?; } Vcpu::register_kick_signal_handler(); @@ -322,10 +350,8 @@ impl Vmm { vcpu.kvm_vcpu .set_pio_bus(self.pio_device_manager.io_bus.clone()); - self.vcpus_handles.push( - vcpu.start_threaded(vcpu_seccomp_filter.clone(), barrier.clone()) - .map_err(Error::VcpuHandle)?, - ); + self.vcpus_handles + .push(vcpu.start_threaded(vcpu_seccomp_filter.clone(), barrier.clone())?); } self.instance_info.state = VmState::Paused; // Wait for vCPUs to initialize their TLS before moving forward. @@ -386,7 +412,7 @@ impl Vmm { } /// Sets RDA bit in serial console - pub fn emulate_serial_init(&self) -> Result<()> { + pub fn emulate_serial_init(&self) -> std::result::Result<(), EmulateSerialInitError> { #[cfg(target_arch = "aarch64")] use devices::legacy::SerialDevice; #[cfg(target_arch = "x86_64")] @@ -419,7 +445,7 @@ impl Vmm { serial .serial .write(IER_RDA_OFFSET, IER_RDA_BIT) - .map_err(|_| Error::Serial(std::io::Error::last_os_error()))?; + .map_err(|_| EmulateSerialInitError(std::io::Error::last_os_error()))?; Ok(()) } @@ -465,11 +491,10 @@ impl Vmm { } fn save_vcpu_states(&mut self) -> std::result::Result, MicrovmStateError> { - use self::MicrovmStateError::*; for handle in self.vcpus_handles.iter() { handle .send_event(VcpuEvent::SaveState) - .map_err(SignalVcpu)?; + .map_err(MicrovmStateError::SignalVcpu)?; } let vcpu_responses = self @@ -478,15 +503,15 @@ impl Vmm { // `Iterator::collect` can transform a `Vec` into a `Result`. .map(|handle| handle.response_receiver().recv_timeout(RECV_TIMEOUT_SEC)) .collect::, RecvTimeoutError>>() - .map_err(|_| UnexpectedVcpuResponse)?; + .map_err(|_| MicrovmStateError::UnexpectedVcpuResponse)?; let vcpu_states = vcpu_responses .into_iter() .map(|response| match response { VcpuResponse::SavedState(state) => Ok(*state), - VcpuResponse::Error(err) => Err(SaveVcpuState(err)), + VcpuResponse::Error(err) => Err(MicrovmStateError::SaveVcpuState(err)), VcpuResponse::NotAllowed(reason) => Err(MicrovmStateError::NotAllowed(reason)), - _ => Err(UnexpectedVcpuResponse), + _ => Err(MicrovmStateError::UnexpectedVcpuResponse), }) .collect::, MicrovmStateError>>()?; @@ -497,16 +522,12 @@ impl Vmm { pub fn restore_vcpu_states( &mut self, mut vcpu_states: Vec, - ) -> std::result::Result<(), MicrovmStateError> { - use self::MicrovmStateError::*; - + ) -> std::result::Result<(), RestoreVcpusError> { if vcpu_states.len() != self.vcpus_handles.len() { - return Err(InvalidInput); + return Err(RestoreVcpusError::InvalidInput); } for (handle, state) in self.vcpus_handles.iter().zip(vcpu_states.drain(..)) { - handle - .send_event(VcpuEvent::RestoreState(Box::new(state))) - .map_err(MicrovmStateError::SignalVcpu)?; + handle.send_event(VcpuEvent::RestoreState(Box::new(state)))?; } let vcpu_responses = self @@ -515,17 +536,15 @@ impl Vmm { // `Iterator::collect` can transform a `Vec` into a `Result`. .map(|handle| handle.response_receiver().recv_timeout(RECV_TIMEOUT_SEC)) .collect::, RecvTimeoutError>>() - .map_err(|_| MicrovmStateError::UnexpectedVcpuResponse)?; + .map_err(|_| RestoreVcpusError::UnexpectedVcpuResponse)?; for response in vcpu_responses.into_iter() { match response { - VcpuResponse::RestoredState => (), - VcpuResponse::Error(err) => return Err(MicrovmStateError::RestoreVcpuState(err)), - VcpuResponse::NotAllowed(reason) => { - return Err(MicrovmStateError::NotAllowed(reason)) - } - _ => return Err(MicrovmStateError::UnexpectedVcpuResponse), - } + VcpuResponse::RestoredState => Ok(()), + VcpuResponse::Error(err) => Err(RestoreVcpusError::RestoreVcpuState(err)), + VcpuResponse::NotAllowed(reason) => Err(RestoreVcpusError::NotAllowed(reason)), + _ => Err(RestoreVcpusError::UnexpectedVcpuResponse), + }?; } Ok(()) diff --git a/src/vmm/src/memory_snapshot.rs b/src/vmm/src/memory_snapshot.rs index 9eebe371da0..875aacf8888 100644 --- a/src/vmm/src/memory_snapshot.rs +++ b/src/vmm/src/memory_snapshot.rs @@ -3,7 +3,6 @@ //! Defines functionality for creating guest memory snapshots. -use std::fmt::{Display, Formatter}; use std::fs::File; use std::io::SeekFrom; @@ -64,31 +63,23 @@ where } /// Errors associated with dumping guest memory to file. -#[derive(Debug, derive_more::From)] +#[derive(Debug, thiserror::Error)] pub enum Error { /// Cannot access file. - FileHandle(std::io::Error), + #[error("Cannot access file: {0:?}")] + FileHandle(#[from] std::io::Error), /// Cannot create memory. - CreateMemory(vm_memory::Error), + #[error("Cannot create memory: {0:?}")] + CreateMemory(#[from] vm_memory::Error), /// Cannot create region. - CreateRegion(vm_memory::MmapRegionError), + #[error("Cannot create memory region: {0:?}")] + CreateRegion(#[from] vm_memory::MmapRegionError), /// Cannot fetch system's page size. - PageSize(errno::Error), + #[error("Cannot fetch system's page size: {0:?}")] + PageSize(#[from] errno::Error), /// Cannot dump memory. - WriteMemory(GuestMemoryError), -} - -impl Display for Error { - fn fmt(&self, f: &mut Formatter) -> std::fmt::Result { - use self::Error::*; - match self { - FileHandle(err) => write!(f, "Cannot access file: {:?}", err), - CreateMemory(err) => write!(f, "Cannot create memory: {:?}", err), - CreateRegion(err) => write!(f, "Cannot create memory region: {:?}", err), - PageSize(err) => write!(f, "Cannot fetch system's page size: {:?}", err), - WriteMemory(err) => write!(f, "Cannot dump memory: {:?}", err), - } - } + #[error("Cannot dump memory: {0:?}")] + WriteMemory(#[from] GuestMemoryError), } impl SnapshotMemory for GuestMemoryMmap { diff --git a/src/vmm/src/persist.rs b/src/vmm/src/persist.rs index 309cbce46b3..09aa7768908 100644 --- a/src/vmm/src/persist.rs +++ b/src/vmm/src/persist.rs @@ -27,7 +27,7 @@ use versionize_derive::Versionize; use virtio_gen::virtio_ring::VIRTIO_RING_F_EVENT_IDX; use vm_memory::{GuestMemory, GuestMemoryMmap}; -use crate::builder::{self, StartMicrovmError}; +use crate::builder::{self, BuildMicrovmFromSnapshotError}; use crate::device_manager::persist::{DeviceStates, Error as DevicePersistError}; use crate::memory_snapshot::{GuestMemoryState, SnapshotMemory}; use crate::resources::VmResources; @@ -39,7 +39,7 @@ use crate::vmm_config::machine_config::MAX_SUPPORTED_VCPUS; use crate::vmm_config::snapshot::{ CreateSnapshotParams, LoadSnapshotParams, MemBackendType, SnapshotType, }; -use crate::vstate::vcpu::VcpuState; +use crate::vstate::vcpu::{VcpuSendEventError, VcpuState}; use crate::vstate::vm::VmState; use crate::{mem_size_mib, memory_snapshot, vstate, Error as VmmError, EventManager, Vmm}; @@ -90,48 +90,40 @@ pub struct GuestRegionUffdMapping { } /// Errors related to saving and restoring Microvm state. -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum MicrovmStateError { /// Compatibility checks failed. + #[error("Compatibility checks failed: {0}")] IncompatibleState(String), /// Provided MicroVM state is invalid. + #[error("Provided MicroVM state is invalid.")] InvalidInput, /// Operation not allowed. + #[error("Operation not allowed: {0}")] NotAllowed(String), /// Failed to restore devices. + #[error("Cannot restore devices: {0:?}")] RestoreDevices(DevicePersistError), /// Failed to restore Vcpu state. + #[error("Cannot restore Vcpu state: {0:?}")] RestoreVcpuState(vstate::vcpu::Error), /// Failed to restore VM state. + #[error("Cannot restore Vm state: {0:?}")] RestoreVmState(vstate::vm::Error), /// Failed to save Vcpu state. + #[error("Cannot save Vcpu state: {0:?}")] SaveVcpuState(vstate::vcpu::Error), /// Failed to save VM state. + #[error("Cannot save Vm state: {0:?}")] SaveVmState(vstate::vm::Error), /// Failed to send event. - SignalVcpu(vstate::vcpu::Error), + #[error("Cannot signal Vcpu: {0:?}")] + SignalVcpu(VcpuSendEventError), /// Vcpu is in unexpected state. + #[error("Vcpu is in unexpected state.")] UnexpectedVcpuResponse, } -impl Display for MicrovmStateError { - fn fmt(&self, f: &mut Formatter) -> std::fmt::Result { - use self::MicrovmStateError::*; - match self { - IncompatibleState(msg) => write!(f, "Compatibility checks failed: {}", msg), - InvalidInput => write!(f, "Provided MicroVM state is invalid."), - NotAllowed(msg) => write!(f, "Operation not allowed: {}", msg), - RestoreDevices(err) => write!(f, "Cannot restore devices: {:?}", err), - RestoreVcpuState(err) => write!(f, "Cannot restore Vcpu state: {:?}", err), - RestoreVmState(err) => write!(f, "Cannot restore Vm state: {:?}", err), - SaveVcpuState(err) => write!(f, "Cannot save Vcpu state: {:?}", err), - SaveVmState(err) => write!(f, "Cannot save Vm state: {:?}", err), - SignalVcpu(err) => write!(f, "Cannot signal Vcpu: {:?}", err), - UnexpectedVcpuResponse => write!(f, "Vcpu is in unexpected state."), - } - } -} - /// Errors associated with creating a snapshot. #[derive(Debug)] pub enum CreateSnapshotError { @@ -200,75 +192,6 @@ impl Display for CreateSnapshotError { } } -/// Errors associated with loading a snapshot. -#[derive(Debug)] -pub enum LoadSnapshotError { - /// Failed to build a microVM from snapshot. - BuildMicroVm(StartMicrovmError), - /// Snapshot cpu vendor differs than host cpu vendor. - CpuVendorCheck(String), - /// Failed to create an UFFD Builder. - CreateUffdBuilder(userfaultfd::Error), - /// Failed to deserialize memory. - DeserializeMemory(memory_snapshot::Error), - /// Failed to deserialize microVM state. - DeserializeMicrovmState(snapshot::Error), - /// Snapshot failed sanity checks. - InvalidSnapshot(String), - /// Failed to open memory backing file. - MemoryBackingFile(io::Error), - /// Failed to resume Vm after loading snapshot. - ResumeMicroVm(VmmError), - /// Failed to open the snapshot backing file. - SnapshotBackingFile(&'static str, io::Error), - /// Unable to connect to UDS in order to send information regarding - /// handling guest memory page-fault events. - UdsConnection(io::Error), - /// Failed to register guest memory regions to UFFD. - UffdMemoryRegionsRegister(userfaultfd::Error), - /// Failed to send guest memory layout and path to user fault FD used to handle - /// guest memory page faults. This information is sent to a UDS where a custom - /// page-fault handler process is listening. - UffdSend(kvm_ioctls::Error), -} - -impl Display for LoadSnapshotError { - fn fmt(&self, f: &mut Formatter) -> std::fmt::Result { - use self::LoadSnapshotError::*; - match self { - BuildMicroVm(err) => write!(f, "Cannot build a microVM from snapshot: {}", err), - CreateUffdBuilder(err) => write!(f, "Cannot create UFFD builder: {:?}", err), - CpuVendorCheck(err) => write!(f, "CPU vendor check failed: {}", err), - DeserializeMemory(err) => write!(f, "Cannot deserialize memory: {}", err), - DeserializeMicrovmState(err) => { - write!(f, "Cannot deserialize the microVM state: {:?}", err) - } - InvalidSnapshot(err) => write!(f, "Snapshot sanity check failed: {}", err), - MemoryBackingFile(err) => write!(f, "Cannot open the memory file: {}", err), - ResumeMicroVm(err) => write!( - f, - "Failed to resume microVM after loading snapshot: {}", - err - ), - SnapshotBackingFile(action, err) => write!( - f, - "Cannot perform {} on the snapshot backing file: {}", - action, err - ), - UdsConnection(err) => write!( - f, - "Cannot connect to UDS in order to send information on handling guest memory \ - page-faults due to: {}", - err - ), - UffdMemoryRegionsRegister(err) => { - write!(f, "Cannot register memory regions to UFFD: {:?}.", err) - } - UffdSend(err) => write!(f, "Cannot send FD and memory layout to UFFD: {}", err), - } - } -} - /// Creates a Microvm snapshot. pub fn create_snapshot( vmm: &mut Vmm, @@ -394,85 +317,128 @@ pub fn get_snapshot_data_version( Ok(data_version) } +/// Error type for [`validate_cpu_vendor`]. +#[cfg(target_arch = "x86_64")] +#[derive(Debug, thiserror::Error, PartialEq, Eq)] +pub enum ValidateCpuVendorError { + /// Failed to read host vendor. + #[error("Failed to read host vendor: {0}")] + Host(cpuid::common::Error), + /// Failed to read snapshot vendor. + #[error("Failed to read snapshot vendor: {0}")] + Snapshot(cpuid::common::Error), +} + /// Validates that snapshot CPU vendor matches the host CPU vendor. +/// +/// # Errors +/// +/// When: +/// - Failed to read host vendor. +/// - Failed to read snapshot vendor. #[cfg(target_arch = "x86_64")] pub fn validate_cpu_vendor( microvm_state: &MicrovmState, -) -> std::result::Result<(), LoadSnapshotError> { - let host_vendor_id = get_vendor_id_from_host().map_err(|_| { - LoadSnapshotError::CpuVendorCheck("Failed to read vendor from host.".to_owned()) - })?; +) -> std::result::Result { + let host_vendor_id = get_vendor_id_from_host().map_err(ValidateCpuVendorError::Host)?; let snapshot_vendor_id = get_vendor_id_from_cpuid(µvm_state.vcpu_states[0].cpuid) - .map_err(|_| { + .map_err(|err| { error!("Snapshot CPU vendor is missing."); - LoadSnapshotError::CpuVendorCheck("Failed to read vendor from CPUID.".to_owned()) + ValidateCpuVendorError::Snapshot(err) })?; - if host_vendor_id != snapshot_vendor_id { - let error_string = format!( + if host_vendor_id == snapshot_vendor_id { + info!("Snapshot CPU vendor id: {:?}", &snapshot_vendor_id); + Ok(true) + } else { + error!( "Host CPU vendor id: {:?} differs from the snapshotted one: {:?}", &host_vendor_id, &snapshot_vendor_id ); - error!("{}", error_string); - return Err(LoadSnapshotError::CpuVendorCheck(error_string)); - } else { - info!("Snapshot CPU vendor id: {:?}", &snapshot_vendor_id); + Ok(false) } +} - Ok(()) +/// Error type for [`validate_cpu_manufacturer_id`]. +#[cfg(target_arch = "aarch64")] +#[derive(Debug, thiserror::Error, PartialEq, Eq)] +pub enum ValidateCpuManufacturerIdError { + /// Failed to read host vendor. + #[error("Failed to get manufacturer ID from host: {0}")] + Host(String), + /// Failed to read host vendor. + #[error("Failed to get manufacturer ID from state: {0}")] + Snapshot(String), } /// Validate that Snapshot Manufacturer ID matches /// the one from the Host /// /// The manufacturer ID for the Snapshot is taken from each VCPU state. +/// # Errors +/// +/// When: +/// - Failed to read host vendor. +/// - Failed to read snapshot vendor. #[cfg(target_arch = "aarch64")] pub fn validate_cpu_manufacturer_id( microvm_state: &MicrovmState, -) -> std::result::Result<(), LoadSnapshotError> { +) -> std::result::Result { let host_man_id = get_manufacturer_id_from_host() - .map_err(|err| LoadSnapshotError::CpuVendorCheck(err.to_string()))?; + .map_err(|err| ValidateCpuManufacturerIdError::Host(err.to_string()))?; for state in µvm_state.vcpu_states { let state_man_id = get_manufacturer_id_from_state(state.regs.as_slice()) - .map_err(|err| LoadSnapshotError::CpuVendorCheck(err.to_string()))?; + .map_err(|err| ValidateCpuManufacturerIdError::Snapshot(err.to_string()))?; if host_man_id != state_man_id { - let error_string = format!( + error!( "Host CPU manufacturer ID: {} differs from snapshotted one: {}", &host_man_id, &state_man_id ); - error!("{}", error_string); - return Err(LoadSnapshotError::CpuVendorCheck(error_string)); + return Ok(false); } else { info!("Snapshot CPU manufacturer ID: {:?}", &state_man_id); } } - - Ok(()) + Ok(true) +} +/// Error type for [`snapshot_state_sanity_check`]. +#[derive(Debug, thiserror::Error, PartialEq, Eq)] +pub enum SnapShotStateSanityCheckError { + /// Invalid vCPU count. + #[error("Invalid vCPU count.")] + InvalidVcpuCount, + /// No memory region defined. + #[error("No memory region defined.")] + NoMemory, + /// Failed to validate vCPU vendor. + #[cfg(target_arch = "x86_64")] + #[error("Failed to validate vCPU vendor: {0}")] + ValidateCpuVendor(#[from] ValidateCpuVendorError), + /// Failed to validate vCPU manufacturer id. + #[error("Failed to validate vCPU manufacturer id: {0}")] + #[cfg(target_arch = "aarch64")] + ValidateCpuManufacturerId(#[from] ValidateCpuManufacturerIdError), } /// Performs sanity checks against the state file and returns specific errors. pub fn snapshot_state_sanity_check( microvm_state: &MicrovmState, -) -> std::result::Result<(), LoadSnapshotError> { +) -> std::result::Result<(), SnapShotStateSanityCheckError> { // Check if the snapshot contains at least 1 vCPU state entry. if microvm_state.vcpu_states.is_empty() || microvm_state.vcpu_states.len() > MAX_SUPPORTED_VCPUS.into() { - return Err(LoadSnapshotError::InvalidSnapshot( - "Invalid vCPU count.".to_owned(), - )); + return Err(SnapShotStateSanityCheckError::InvalidVcpuCount); } // Check if the snapshot contains at least 1 mem region. // Upper bound check will be done when creating guest memory by comparing against // KVM max supported value kvm_context.max_memslots(). if microvm_state.memory_state.regions.is_empty() { - return Err(LoadSnapshotError::InvalidSnapshot( - "No memory region defined.".to_owned(), - )); + return Err(SnapShotStateSanityCheckError::NoMemory); } #[cfg(target_arch = "x86_64")] @@ -483,6 +449,34 @@ pub fn snapshot_state_sanity_check( Ok(()) } +/// Error type for [`restore_from_snapshot`]. +#[derive(Debug, thiserror::Error)] +pub enum RestoreFromSnapshotError { + /// Failed to get snapshot state from file. + #[error("Failed to get snapshot state from file: {0}")] + File(#[from] SnapshotStateFromFileError), + /// Invalid snapshot state. + #[error("Invalid snapshot state: {0}")] + Invalid(#[from] SnapShotStateSanityCheckError), + /// Failed to load guest memory + #[error("Failed to load guest memory: {0}")] + GuestMemory(#[from] RestoreFromSnapshotGuestMemoryError), + /// Failed build micro-VM from snapshot. + #[error("Failed build micro-VM from snapshot: {0}")] + Build(#[from] BuildMicrovmFromSnapshotError), +} +/// Sub-Error type for [`restore_from_snapshot`] to contain either [`GuestMemoryFromFileError`] or +/// [`GuestMemoryFromUffdError`] within [`RestoreFromSnapshotError`]. +#[derive(Debug, thiserror::Error)] +pub enum RestoreFromSnapshotGuestMemoryError { + /// Error creating guest memory from file. + #[error("Error creating guest memory from file: {0}")] + File(#[from] GuestMemoryFromFileError), + /// Error creating guest memory from uffd. + #[error("Error creating guest memory from uffd: {0}")] + Uffd(#[from] GuestMemoryFromUffdError), +} + /// Loads a Microvm snapshot producing a 'paused' Microvm. pub fn restore_from_snapshot( instance_info: &InstanceInfo, @@ -491,8 +485,7 @@ pub fn restore_from_snapshot( params: &LoadSnapshotParams, version_map: VersionMap, vm_resources: &mut VmResources, -) -> std::result::Result>, LoadSnapshotError> { - use self::LoadSnapshotError::*; +) -> std::result::Result>, RestoreFromSnapshotError> { let microvm_state = snapshot_state_from_file(¶ms.snapshot_path, version_map)?; // Some sanity checks before building the microvm. @@ -501,9 +494,11 @@ pub fn restore_from_snapshot( let mem_backend_path = ¶ms.mem_backend.backend_path; let mem_state = µvm_state.memory_state; let track_dirty_pages = params.enable_diff_snapshots; + let (guest_memory, uffd) = match params.mem_backend.backend_type { MemBackendType::File => ( - guest_memory_from_file(mem_backend_path, mem_state, track_dirty_pages)?, + guest_memory_from_file(mem_backend_path, mem_state, track_dirty_pages) + .map_err(RestoreFromSnapshotGuestMemoryError::File)?, None, ), MemBackendType::Uffd => guest_memory_from_uffd( @@ -513,7 +508,8 @@ pub fn restore_from_snapshot( // We enable the UFFD_FEATURE_EVENT_REMOVE feature only if a balloon device // is present in the microVM state. microvm_state.device_states.balloon_device.is_some(), - )?, + ) + .map_err(RestoreFromSnapshotGuestMemoryError::Uffd)?, }; builder::build_microvm_from_snapshot( instance_info, @@ -525,31 +521,74 @@ pub fn restore_from_snapshot( seccomp_filters, vm_resources, ) - .map_err(BuildMicroVm) + .map_err(RestoreFromSnapshotError::Build) +} + +/// Error type for [`snapshot_state_from_file`] +#[derive(Debug, thiserror::Error)] +pub enum SnapshotStateFromFileError { + /// Failed to open snapshot file. + #[error("Failed to open snapshot file: {0}")] + Open(std::io::Error), + /// Failed to read snapshot file metadata. + #[error("Failed to read snapshot file metadata: {0}")] + Meta(std::io::Error), + /// Failed to load snapshot state from file. + #[error("Failed to load snapshot state from file: {0}")] + Load(#[from] snapshot::Error), } fn snapshot_state_from_file( snapshot_path: &Path, version_map: VersionMap, -) -> std::result::Result { - use self::LoadSnapshotError::{DeserializeMicrovmState, SnapshotBackingFile}; +) -> std::result::Result { let mut snapshot_reader = - File::open(snapshot_path).map_err(|err| SnapshotBackingFile("open", err))?; - let metadata = std::fs::metadata(snapshot_path) - .map_err(|err| SnapshotBackingFile("metadata retrieval", err))?; + File::open(snapshot_path).map_err(SnapshotStateFromFileError::Open)?; + let metadata = std::fs::metadata(snapshot_path).map_err(SnapshotStateFromFileError::Meta)?; let snapshot_len = metadata.len() as usize; - Snapshot::load(&mut snapshot_reader, snapshot_len, version_map).map_err(DeserializeMicrovmState) + Snapshot::load(&mut snapshot_reader, snapshot_len, version_map) + .map_err(SnapshotStateFromFileError::Load) +} + +/// Error type for [`guest_memory_from_file`]. +#[derive(Debug, thiserror::Error)] +pub enum GuestMemoryFromFileError { + /// Failed to load guest memory. + #[error("Failed to load guest memory: {0}")] + File(#[from] std::io::Error), + /// Failed to restore guest memory. + #[error("Failed to restore guest memory: {0}")] + Restore(#[from] crate::memory_snapshot::Error), } fn guest_memory_from_file( mem_file_path: &Path, mem_state: &GuestMemoryState, track_dirty_pages: bool, -) -> std::result::Result { - use self::LoadSnapshotError::{DeserializeMemory, MemoryBackingFile}; - let mem_file = File::open(mem_file_path).map_err(MemoryBackingFile)?; - GuestMemoryMmap::restore(Some(&mem_file), mem_state, track_dirty_pages) - .map_err(DeserializeMemory) +) -> std::result::Result { + let mem_file = File::open(mem_file_path)?; + let guest_mem = GuestMemoryMmap::restore(Some(&mem_file), mem_state, track_dirty_pages)?; + Ok(guest_mem) +} + +/// Error type for [`guest_memory_from_uffd`] +#[derive(Debug, thiserror::Error)] +pub enum GuestMemoryFromUffdError { + /// Failed to restore guest memory. + #[error("Failed to restore guest memory: {0}")] + Restore(#[from] crate::memory_snapshot::Error), + /// Failed to UFFD object. + #[error("Failed to UFFD object: {0}")] + Create(userfaultfd::Error), + /// Failed to register memory address range with the userfaultfd object. + #[error("Failed to register memory address range with the userfaultfd object: {0}")] + Register(userfaultfd::Error), + /// Failed to connect to UDS Unix stream. + #[error("Failed to connect to UDS Unix stream: {0}")] + Connect(#[from] std::io::Error), + /// Failed to send file descriptor. + #[error("Failed to sends file descriptor: {0}")] + Send(#[from] utils::errno::Error), } fn guest_memory_from_uffd( @@ -557,13 +596,8 @@ fn guest_memory_from_uffd( mem_state: &GuestMemoryState, track_dirty_pages: bool, enable_balloon: bool, -) -> std::result::Result<(GuestMemoryMmap, Option), LoadSnapshotError> { - use self::LoadSnapshotError::{ - CreateUffdBuilder, DeserializeMemory, UdsConnection, UffdMemoryRegionsRegister, UffdSend, - }; - - let guest_memory = - GuestMemoryMmap::restore(None, mem_state, track_dirty_pages).map_err(DeserializeMemory)?; +) -> std::result::Result<(GuestMemoryMmap, Option), GuestMemoryFromUffdError> { + let guest_memory = GuestMemoryMmap::restore(None, mem_state, track_dirty_pages)?; let mut uffd_builder = UffdBuilder::new(); @@ -577,7 +611,7 @@ fn guest_memory_from_uffd( .close_on_exec(true) .non_blocking(true) .create() - .map_err(CreateUffdBuilder)?; + .map_err(GuestMemoryFromUffdError::Create)?; let mut backend_mappings = Vec::with_capacity(guest_memory.num_regions()); for (mem_region, state_region) in guest_memory.iter().zip(mem_state.regions.iter()) { @@ -585,7 +619,7 @@ fn guest_memory_from_uffd( let size = mem_region.size(); uffd.register(host_base_addr as _, size as _) - .map_err(UffdMemoryRegionsRegister)?; + .map_err(GuestMemoryFromUffdError::Register)?; backend_mappings.push(GuestRegionUffdMapping { base_host_virt_addr: host_base_addr as u64, size, @@ -597,43 +631,41 @@ fn guest_memory_from_uffd( // (i.e GuestRegionUffdMapping entries). let backend_mappings = serde_json::to_string(&backend_mappings).unwrap(); - let socket = UnixStream::connect(mem_uds_path).map_err(UdsConnection)?; - socket - .send_with_fd( - backend_mappings.as_bytes(), - // In the happy case we can close the fd since the other process has it open and is - // using it to serve us pages. - // - // The problem is that if other process crashes/exits, firecracker guest memory - // will simply revert to anon-mem behavior which would lead to silent errors and - // undefined behavior. - // - // To tackle this scenario, the page fault handler can notify Firecracker of any - // crashes/exits. There is no need for Firecracker to explicitly send its process ID. - // The external process can obtain Firecracker's PID by calling `getsockopt` with - // `libc::SO_PEERCRED` option like so: - // - // let mut val = libc::ucred { pid: 0, gid: 0, uid: 0 }; - // let mut ucred_size: u32 = mem::size_of::() as u32; - // libc::getsockopt( - // socket.as_raw_fd(), - // libc::SOL_SOCKET, - // libc::SO_PEERCRED, - // &mut val as *mut _ as *mut _, - // &mut ucred_size as *mut libc::socklen_t, - // ); - // - // Per this linux man page: https://man7.org/linux/man-pages/man7/unix.7.html, - // `SO_PEERCRED` returns the credentials (PID, UID and GID) of the peer process - // connected to this socket. The returned credentials are those that were in effect - // at the time of the `connect` call. - // - // Moreover, Firecracker holds a copy of the UFFD fd as well, so that even if the - // page fault handler process does not tear down Firecracker when necessary, the - // uffd will still be alive but with no one to serve faults, leading to guest freeze. - uffd.as_raw_fd(), - ) - .map_err(UffdSend)?; + let socket = UnixStream::connect(mem_uds_path)?; + socket.send_with_fd( + backend_mappings.as_bytes(), + // In the happy case we can close the fd since the other process has it open and is + // using it to serve us pages. + // + // The problem is that if other process crashes/exits, firecracker guest memory + // will simply revert to anon-mem behavior which would lead to silent errors and + // undefined behavior. + // + // To tackle this scenario, the page fault handler can notify Firecracker of any + // crashes/exits. There is no need for Firecracker to explicitly send its process ID. + // The external process can obtain Firecracker's PID by calling `getsockopt` with + // `libc::SO_PEERCRED` option like so: + // + // let mut val = libc::ucred { pid: 0, gid: 0, uid: 0 }; + // let mut ucred_size: u32 = mem::size_of::() as u32; + // libc::getsockopt( + // socket.as_raw_fd(), + // libc::SOL_SOCKET, + // libc::SO_PEERCRED, + // &mut val as *mut _ as *mut _, + // &mut ucred_size as *mut libc::socklen_t, + // ); + // + // Per this linux man page: https://man7.org/linux/man-pages/man7/unix.7.html, + // `SO_PEERCRED` returns the credentials (PID, UID and GID) of the peer process + // connected to this socket. The returned credentials are those that were in effect + // at the time of the `connect` call. + // + // Moreover, Firecracker holds a copy of the UFFD fd as well, so that even if the + // page fault handler process does not tear down Firecracker when necessary, the + // uffd will still be alive but with no one to serve faults, leading to guest freeze. + uffd.as_raw_fd(), + )?; Ok((guest_memory, Some(uffd))) } @@ -865,31 +897,6 @@ mod tests { } } - #[test] - fn test_load_snapshot_error_display() { - use crate::persist::LoadSnapshotError::*; - - let err = BuildMicroVm(StartMicrovmError::InitrdLoad); - let _ = format!("{}{:?}", err, err); - - let err = DeserializeMemory(memory_snapshot::Error::FileHandle( - io::Error::from_raw_os_error(0), - )); - let _ = format!("{}{:?}", err, err); - - let err = DeserializeMicrovmState(snapshot::Error::Io(0)); - let _ = format!("{}{:?}", err, err); - - let err = MemoryBackingFile(io::Error::from_raw_os_error(0)); - let _ = format!("{}{:?}", err, err); - - let err = SnapshotBackingFile("open", io::Error::from_raw_os_error(0)); - let _ = format!("{}{:?}", err, err); - - let err = CpuVendorCheck(String::new()); - let _ = format!("{}{:?}", err, err); - } - #[test] fn test_microvm_state_error_display() { use crate::persist::MicrovmStateError::*; @@ -915,7 +922,7 @@ mod tests { let err = SaveVmState(vstate::vm::Error::NotEnoughMemorySlots); let _ = format!("{}{:?}", err, err); - let err = SignalVcpu(vstate::vcpu::Error::SignalVcpu(errno::Error::new(0))); + let err = SignalVcpu(VcpuSendEventError(errno::Error::new(0))); let _ = format!("{}{:?}", err, err); let err = UnexpectedVcpuResponse; diff --git a/src/vmm/src/resources.rs b/src/vmm/src/resources.rs index 9a5257ac307..80852cbe6a8 100644 --- a/src/vmm/src/resources.rs +++ b/src/vmm/src/resources.rs @@ -242,7 +242,10 @@ impl VmResources { } /// Update the machine configuration of the microVM. - pub fn update_vm_config(&mut self, machine_config: &VmUpdateConfig) -> Result { + pub fn update_vm_config( + &mut self, + machine_config: &VmUpdateConfig, + ) -> std::result::Result<(), VmConfigError> { let vcpu_count = machine_config .vcpu_count .unwrap_or(self.vm_config.vcpu_count); diff --git a/src/vmm/src/rpc_interface.rs b/src/vmm/src/rpc_interface.rs index 2a79237d3f1..06efd415d2c 100644 --- a/src/vmm/src/rpc_interface.rs +++ b/src/vmm/src/rpc_interface.rs @@ -22,7 +22,7 @@ use super::{ resources::VmResources, Vmm, }; use crate::builder::StartMicrovmError; -use crate::persist::{CreateSnapshotError, LoadSnapshotError}; +use crate::persist::{CreateSnapshotError, RestoreFromSnapshotError}; use crate::resources::VmmConfig; use crate::version_map::VERSION_MAP; use crate::vmm_config::balloon::{ @@ -141,8 +141,6 @@ pub enum VmmActionError { InternalVmm(VmmError), /// Loading a microVM snapshot failed. LoadSnapshot(LoadSnapshotError), - /// Loading a microVM snapshot not allowed after configuring boot-specific resources. - LoadSnapshotNotAllowed, /// The action `ConfigureLogger` failed because of bad user input. Logger(LoggerConfigError), /// One of the actions `GetVmConfiguration` or `UpdateVmConfiguration` failed because of bad @@ -186,11 +184,6 @@ impl Display for VmmActionError { DriveConfig(err) => err.to_string(), InternalVmm(err) => format!("Internal Vmm error: {}", err), LoadSnapshot(err) => format!("Load microVM snapshot error: {}", err), - LoadSnapshotNotAllowed => { - "Loading a microVM snapshot not allowed after configuring boot-specific \ - resources." - .to_string() - } Logger(err) => err.to_string(), MachineConfig(err) => err.to_string(), Metrics(err) => err.to_string(), @@ -296,6 +289,20 @@ impl MmdsRequestHandler for PrebootApiController<'_> { } } +/// Error type for [`PrebootApiController::load_snapshot`] +#[derive(Debug, thiserror::Error)] +pub enum LoadSnapshotError { + /// Loading a microVM snapshot not allowed after configuring boot-specific resources. + #[error("Loading a microVM snapshot not allowed after configuring boot-specific resources.")] + LoadSnapshotNotAllowed, + /// Failed to restore from snapshot. + #[error("Failed to restore from snapshot: {0}")] + RestoreFromSnapshot(#[from] RestoreFromSnapshotError), + /// Failed to resume microVM. + #[error("Failed to resume microVM: {0}")] + ResumeMicrovm(#[from] VmmError), +} + impl<'a> PrebootApiController<'a> { /// Constructor for the PrebootApiController. pub fn new( @@ -416,7 +423,9 @@ impl<'a> PrebootApiController<'a> { GetVmmVersion => Ok(VmmData::VmmVersion(self.instance_info.vmm_version.clone())), InsertBlockDevice(config) => self.insert_block_device(config), InsertNetworkDevice(config) => self.insert_net_device(config), - LoadSnapshot(config) => self.load_snapshot(&config), + LoadSnapshot(config) => self + .load_snapshot(&config) + .map_err(VmmActionError::LoadSnapshot), PatchMMDS(value) => self.patch_mmds(value), PutMMDS(value) => self.put_mmds(value), SetBalloonDevice(config) => self.set_balloon_device(config), @@ -521,13 +530,16 @@ impl<'a> PrebootApiController<'a> { // On success, this command will end the pre-boot stage and this controller // will be replaced by a runtime controller. - fn load_snapshot(&mut self, load_params: &LoadSnapshotParams) -> ActionResult { + fn load_snapshot( + &mut self, + load_params: &LoadSnapshotParams, + ) -> std::result::Result { log_dev_preview_warning("Virtual machine snapshots", Option::None); let load_start_us = utils::time::get_time_us(utils::time::ClockType::Monotonic); if self.boot_path { - let err = VmmActionError::LoadSnapshotNotAllowed; + let err = LoadSnapshotError::LoadSnapshotNotAllowed; info!("{}", err); return Err(err); } @@ -536,7 +548,8 @@ impl<'a> PrebootApiController<'a> { self.vm_resources.set_track_dirty_pages(true); } - let result = restore_from_snapshot( + // Restore VM from snapshot + let vmm = restore_from_snapshot( &self.instance_info, &mut self.event_manager, self.seccomp_filters, @@ -544,24 +557,24 @@ impl<'a> PrebootApiController<'a> { VERSION_MAP.clone(), self.vm_resources, ) - .and_then(|vmm| { - let ret = if load_params.resume_vm { - vmm.lock().expect("Poisoned lock").resume_vm() - } else { - Ok(()) - }; - - ret.map(|()| { - self.built_vmm = Some(vmm); - VmmData::Empty - }) - .map_err(LoadSnapshotError::ResumeMicroVm) - }) .map_err(|err| { - // The process is too dirty to recover at this point. + // If restore fails, we consider the process is too dirty to recover. self.fatal_error = Some(FcExitCode::BadConfiguration); - VmmActionError::LoadSnapshot(err) - }); + err + })?; + // Resume VM + if load_params.resume_vm { + vmm.lock() + .expect("Poisoned lock") + .resume_vm() + .map_err(|err| { + // If resume fails, we consider the process is too dirty to recover. + self.fatal_error = Some(FcExitCode::BadConfiguration); + err + })?; + } + // Set the VM + self.built_vmm = Some(vmm); log_dev_preview_warning( "Virtual machine snapshots", @@ -574,7 +587,7 @@ impl<'a> PrebootApiController<'a> { )), ); - result + Ok(VmmData::Empty) } } @@ -828,7 +841,6 @@ mod tests { | (DriveConfig(_), DriveConfig(_)) | (InternalVmm(_), InternalVmm(_)) | (LoadSnapshot(_), LoadSnapshot(_)) - | (LoadSnapshotNotAllowed, LoadSnapshotNotAllowed) | (Logger(_), Logger(_)) | (MachineConfig(_), MachineConfig(_)) | (Metrics(_), Metrics(_)) @@ -1143,7 +1155,7 @@ mod tests { _: &LoadSnapshotParams, _: versionize::VersionMap, _: &mut MockVmRes, - ) -> Result>, LoadSnapshotError> { + ) -> Result>, RestoreFromSnapshotError> { Ok(Arc::new(Mutex::new(MockVmm::default()))) } @@ -2074,7 +2086,9 @@ mod tests { let err = preboot.handle_preboot_request(req); assert_eq!( err, - Err(VmmActionError::LoadSnapshotNotAllowed), + Err(VmmActionError::LoadSnapshot( + LoadSnapshotError::LoadSnapshotNotAllowed + )), "LoadSnapshot should be disallowed after {}", res_name ); diff --git a/src/vmm/src/vmm_config/machine_config.rs b/src/vmm/src/vmm_config/machine_config.rs index 090135be044..4ffb2f0d4c8 100644 --- a/src/vmm/src/vmm_config/machine_config.rs +++ b/src/vmm/src/vmm_config/machine_config.rs @@ -25,6 +25,7 @@ pub enum VmConfigError { /// balloon device was previously installed. InvalidVmState, } +impl std::error::Error for VmConfigError {} impl fmt::Display for VmConfigError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { diff --git a/src/vmm/src/vstate/vcpu/aarch64.rs b/src/vmm/src/vstate/vcpu/aarch64.rs index cb3c24b759d..81bc7d1a2b6 100644 --- a/src/vmm/src/vstate/vcpu/aarch64.rs +++ b/src/vmm/src/vstate/vcpu/aarch64.rs @@ -68,7 +68,7 @@ pub struct KvmVcpu { mpidr: u64, } - +pub type KvmVcpuConfigureError = Error; impl KvmVcpu { /// Constructs a new kvm vcpu with arch specific functionality. /// @@ -102,7 +102,7 @@ impl KvmVcpu { &mut self, guest_mem: &GuestMemoryMmap, kernel_load_addr: GuestAddress, - ) -> Result<()> { + ) -> std::result::Result<(), KvmVcpuConfigureError> { arch::aarch64::regs::setup_boot_regs( &self.fd, self.index, diff --git a/src/vmm/src/vstate/vcpu/mod.rs b/src/vmm/src/vstate/vcpu/mod.rs index 136cd61f1bf..e92ad61b55e 100644 --- a/src/vmm/src/vstate/vcpu/mod.rs +++ b/src/vmm/src/vstate/vcpu/mod.rs @@ -6,13 +6,12 @@ // found in the THIRD-PARTY file. use std::cell::Cell; -use std::fmt::{Display, Formatter}; use std::sync::atomic::{fence, Ordering}; use std::sync::mpsc::{channel, Receiver, Sender, TryRecvError}; #[cfg(test)] use std::sync::Mutex; use std::sync::{Arc, Barrier}; -use std::{io, result, thread}; +use std::{fmt, io, result, thread}; use kvm_bindings::{KVM_SYSTEM_EVENT_RESET, KVM_SYSTEM_EVENT_SHUTDOWN}; use kvm_ioctls::VcpuExit; @@ -42,40 +41,31 @@ pub(crate) use x86_64::{Error as VcpuError, *}; pub(crate) const VCPU_RTSIG_OFFSET: i32 = 0; /// Errors associated with the wrappers over KVM ioctls. -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum Error { /// Error triggered by the KVM subsystem. + #[error("Received error signaling kvm exit: {0}")] FaultyKvmExit(String), /// Failed to signal Vcpu. + #[error("Failed to signal vcpu: {0}")] SignalVcpu(utils::errno::Error), /// Kvm Exit is not handled by our implementation. + #[error("Unexpected kvm exit received: {0}")] UnhandledKvmExit(String), /// Wrapper over error triggered by some vcpu action. + #[error("Failed to run action on vcpu: {0}")] VcpuResponse(VcpuError), /// Cannot spawn a new vCPU thread. + #[error("Cannot spawn a new vCPU thread: {0}")] VcpuSpawn(io::Error), /// Cannot cleanly initialize vcpu TLS. + #[error("Cannot clean init vcpu TLS")] VcpuTlsInit, /// Vcpu not present in TLS. + #[error("Vcpu not present in TLS")] VcpuTlsNotPresent, } -impl Display for Error { - fn fmt(&self, f: &mut Formatter) -> std::fmt::Result { - use self::Error::*; - - match self { - FaultyKvmExit(ref err) => write!(f, "Received error signaling kvm exit: {}", err), - SignalVcpu(err) => write!(f, "Failed to signal vcpu: {}", err), - UnhandledKvmExit(ref err) => write!(f, "Unexpected kvm exit received: {}", err), - VcpuResponse(err) => write!(f, "Failed to run action on vcpu: {}", err), - VcpuSpawn(err) => write!(f, "Cannot spawn a new vCPU thread: {}", err), - VcpuTlsInit => write!(f, "Cannot clean init vcpu TLS"), - VcpuTlsNotPresent => write!(f, "Vcpu not present in TLS"), - } - } -} - pub type Result = result::Result; /// Encapsulates configuration parameters for the guest vCPUS. @@ -92,6 +82,16 @@ pub struct VcpuConfig { // Using this for easier explicit type-casting to help IDEs interpret the code. type VcpuCell = Cell>; +/// Error type for [`Vcpu::start_threaded`]. +#[derive(Debug, derive_more::From)] +pub struct StartThreadedError(std::io::Error); +impl std::error::Error for StartThreadedError {} +impl fmt::Display for StartThreadedError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "Failed to spawn vCPU thread: {}", self.0) + } +} + /// A wrapper around creating and using a vcpu. pub struct Vcpu { // Offers kvm-arch specific functionality. @@ -231,7 +231,7 @@ impl Vcpu { mut self, seccomp_filter: Arc, barrier: Arc, - ) -> Result { + ) -> std::result::Result { let event_sender = self.event_sender.take().expect("vCPU already started"); let response_receiver = self.response_receiver.take().unwrap(); let vcpu_thread = thread::Builder::new() @@ -243,8 +243,7 @@ impl Vcpu { // Synchronization to make sure thread local data is initialized. barrier.wait(); self.run(filter); - }) - .map_err(Error::VcpuSpawn)?; + })?; Ok(VcpuHandle::new( event_sender, @@ -588,6 +587,16 @@ pub struct VcpuHandle { vcpu_thread: Option>, } +/// Error type for [`VcpuHandle::send_event`]. +#[derive(Debug, derive_more::From)] +pub struct VcpuSendEventError(pub utils::errno::Error); +impl std::error::Error for VcpuSendEventError {} +impl fmt::Display for VcpuSendEventError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "Failed to signal vCPU: {}", self.0) + } +} + impl VcpuHandle { pub fn new( event_sender: Sender, @@ -600,8 +609,12 @@ impl VcpuHandle { vcpu_thread: Some(vcpu_thread), } } - - pub fn send_event(&self, event: VcpuEvent) -> Result<()> { + /// Sends event to vCPU. + /// + /// # Errors + /// + /// When [`vmm_sys_util::linux::signal::Killable::kill`] errors. + pub fn send_event(&self, event: VcpuEvent) -> std::result::Result<(), VcpuSendEventError> { // Use expect() to crash if the other thread closed this channel. self.event_sender .send(event) @@ -611,8 +624,7 @@ impl VcpuHandle { .as_ref() // Safe to unwrap since constructor make this 'Some'. .unwrap() - .kill(sigrtmin() + VCPU_RTSIG_OFFSET) - .map_err(Error::SignalVcpu)?; + .kill(sigrtmin() + VCPU_RTSIG_OFFSET)?; Ok(()) } diff --git a/src/vmm/src/vstate/vcpu/x86_64.rs b/src/vmm/src/vstate/vcpu/x86_64.rs index b4f90937f16..d05c2c74e6e 100644 --- a/src/vmm/src/vstate/vcpu/x86_64.rs +++ b/src/vmm/src/vstate/vcpu/x86_64.rs @@ -6,8 +6,11 @@ // found in the THIRD-PARTY file. use std::fmt::{Display, Formatter}; -use std::result; +use std::{fmt, result}; +use arch::x86_64::interrupts; +use arch::x86_64::msr::SetMSRsError; +use arch::x86_64::regs::{SetupFpuError, SetupRegistersError, SetupSpecialRegistersError}; use cpuid::{c3, filter_cpuid, t2, t2s, VmSpec}; use kvm_bindings::{ kvm_debugregs, kvm_lapic_state, kvm_mp_state, kvm_regs, kvm_sregs, kvm_vcpu_events, kvm_xcrs, @@ -155,6 +158,50 @@ impl Display for Error { type Result = result::Result; +/// Error type for [`KvmVcpu::get_tsc_khz`] and [`KvmVcpu::is_tsc_scaling_required`]. +#[derive(Debug, derive_more::From, PartialEq)] +pub struct GetTscError(utils::errno::Error); +impl fmt::Display for GetTscError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self.0) + } +} +impl std::error::Error for GetTscError {} +/// Error type for [`KvmVcpu::set_tsc_khz`]. +#[derive(Debug, derive_more::From, PartialEq)] +pub struct SetTscError(kvm_ioctls::Error); +impl fmt::Display for SetTscError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self.0) + } +} +impl std::error::Error for SetTscError {} + +/// Error type for [`KvmVcpu::configure`]. +#[derive(Debug, thiserror::Error)] +pub enum KvmVcpuConfigureError { + #[error("Failed to create `VmSpec`: {0}")] + VmSpec(cpuid::Error), + #[error("Failed to filter CPUID: {0}")] + FilterCpuid(cpuid::Error), + #[error("Failed to set CPUID entries: {0}")] + SetCpuidEntries(cpuid::Error), + #[error("Failed to set CPUID: {0}")] + SetCpuid(#[from] utils::errno::Error), + #[error("Failed to push MSR entry to FamStructWrapper.")] + PushMsrEntries(utils::fam::Error), + #[error("Failed to set MSRs: {0}")] + SetMsrs(#[from] SetMSRsError), + #[error("Failed to setup registers: {0}")] + SetupRegisters(#[from] SetupRegistersError), + #[error("Failed to setup FPU: {0}")] + SetupFpu(#[from] SetupFpuError), + #[error("Failed to setup special registers: {0}")] + SetupSpecialRegisters(#[from] SetupSpecialRegistersError), + #[error("Faiuled to configure LAPICs: {0}")] + SetLint(#[from] interrupts::Error), +} + /// A wrapper around creating and using a kvm x86_64 vcpu. pub struct KvmVcpu { pub index: u8, @@ -199,52 +246,52 @@ impl KvmVcpu { kernel_start_addr: GuestAddress, vcpu_config: &VcpuConfig, mut cpuid: CpuId, - ) -> Result<()> { + ) -> std::result::Result<(), KvmVcpuConfigureError> { let cpuid_vm_spec = VmSpec::new(self.index, vcpu_config.vcpu_count, vcpu_config.smt) - .map_err(Error::CpuId)?; - - filter_cpuid(&mut cpuid, &cpuid_vm_spec).map_err(|err| { - METRICS.vcpu.filter_cpuid.inc(); - error!( - "Failure in configuring CPUID for vcpu {}: {:?}", - self.index, err - ); - Error::CpuId(err) - })?; + .map_err(KvmVcpuConfigureError::VmSpec)?; + + filter_cpuid(&mut cpuid, &cpuid_vm_spec) + .map_err(|err| { + METRICS.vcpu.filter_cpuid.inc(); + error!( + "Failure in configuring CPUID for vcpu {}: {:?}", + self.index, err + ); + err + }) + .map_err(KvmVcpuConfigureError::FilterCpuid)?; match vcpu_config.cpu_template { - CpuFeaturesTemplate::T2 => { - t2::set_cpuid_entries(&mut cpuid, &cpuid_vm_spec).map_err(Error::CpuId)? - } - CpuFeaturesTemplate::T2S => { - t2s::set_cpuid_entries(&mut cpuid, &cpuid_vm_spec).map_err(Error::CpuId)? - } - CpuFeaturesTemplate::C3 => { - c3::set_cpuid_entries(&mut cpuid, &cpuid_vm_spec).map_err(Error::CpuId)? - } + CpuFeaturesTemplate::T2 => t2::set_cpuid_entries(&mut cpuid, &cpuid_vm_spec) + .map_err(KvmVcpuConfigureError::SetCpuidEntries)?, + CpuFeaturesTemplate::T2S => t2s::set_cpuid_entries(&mut cpuid, &cpuid_vm_spec) + .map_err(KvmVcpuConfigureError::SetCpuidEntries)?, + CpuFeaturesTemplate::C3 => c3::set_cpuid_entries(&mut cpuid, &cpuid_vm_spec) + .map_err(KvmVcpuConfigureError::SetCpuidEntries)?, CpuFeaturesTemplate::None => {} } - self.fd.set_cpuid2(&cpuid).map_err(Error::VcpuSetCpuid)?; + self.fd + .set_cpuid2(&cpuid) + .map_err(KvmVcpuConfigureError::SetCpuid)?; + // Set MSRs let mut msr_boot_entries = arch::x86_64::msr::create_boot_msr_entries(); if vcpu_config.cpu_template == CpuFeaturesTemplate::T2S { for msr in t2s::msr_entries_to_save() { self.msr_list .push(*msr) - .map_err(|_| Error::VcpuTemplateError)?; + .map_err(KvmVcpuConfigureError::PushMsrEntries)?; } t2s::update_msr_entries(&mut msr_boot_entries); } - arch::x86_64::msr::set_msrs(&self.fd, &msr_boot_entries) - .map_err(Error::MSRSConfiguration)?; - arch::x86_64::regs::setup_regs(&self.fd, kernel_start_addr.raw_value() as u64) - .map_err(Error::REGSConfiguration)?; - arch::x86_64::regs::setup_fpu(&self.fd).map_err(Error::FPUConfiguration)?; - arch::x86_64::regs::setup_sregs(guest_mem, &self.fd).map_err(Error::SREGSConfiguration)?; - arch::x86_64::interrupts::set_lint(&self.fd).map_err(Error::LocalIntConfiguration)?; + arch::x86_64::msr::set_msrs(&self.fd, &msr_boot_entries)?; + arch::x86_64::regs::setup_regs(&self.fd, kernel_start_addr.raw_value() as u64)?; + arch::x86_64::regs::setup_fpu(&self.fd)?; + arch::x86_64::regs::setup_sregs(guest_mem, &self.fd)?; + arch::x86_64::interrupts::set_lint(&self.fd)?; Ok(()) } @@ -254,8 +301,13 @@ impl KvmVcpu { } /// Get the current TSC frequency for this vCPU. - pub fn get_tsc_khz(&self) -> Result { - self.fd.get_tsc_khz().map_err(Error::VcpuGetTSC) + /// + /// # Errors + /// + /// When [`kvm_ioctls::VcpuFd::get_tsc_khz`] errrors. + pub fn get_tsc_khz(&self) -> std::result::Result { + let res = self.fd.get_tsc_khz()?; + Ok(res) } /// Save the KVM internal state. @@ -333,8 +385,15 @@ impl KvmVcpu { }) } - // Checks whether the TSC needs scaling when restoring a snapshot. - pub fn is_tsc_scaling_required(&self, state_tsc_freq: u32) -> Result { + /// Checks whether the TSC needs scaling when restoring a snapshot. + /// + /// # Errors + /// + /// When + pub fn is_tsc_scaling_required( + &self, + state_tsc_freq: u32, + ) -> std::result::Result { // Compare the current TSC freq to the one found // in the state. If they are different, we need to // scale the TSC to the freq found in the state. @@ -346,8 +405,9 @@ impl KvmVcpu { } // Scale the TSC frequency of this vCPU to the one provided as a parameter. - pub fn set_tsc_khz(&self, tsc_freq: u32) -> Result<()> { - self.fd.set_tsc_khz(tsc_freq).map_err(Error::VcpuSetTSC) + pub fn set_tsc_khz(&self, tsc_freq: u32) -> std::result::Result<(), SetTscError> { + let res = self.fd.set_tsc_khz(tsc_freq)?; + Ok(res) } /// Use provided state to populate KVM internal state. diff --git a/src/vmm/src/vstate/vm.rs b/src/vmm/src/vstate/vm.rs index b102169eeef..769fe02236e 100644 --- a/src/vmm/src/vstate/vm.rs +++ b/src/vmm/src/vstate/vm.rs @@ -5,8 +5,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the THIRD-PARTY file. -use std::fmt::{Display, Formatter}; -use std::result; +use std::fmt::Formatter; +use std::{fmt, result}; #[cfg(target_arch = "aarch64")] use arch::aarch64::gic::GICDevice; @@ -65,7 +65,36 @@ pub enum Error { RestoreGic(arch::aarch64::gic::Error), } -impl Display for Error { +/// Error type for [`Vm::restore_state`] +#[cfg(target_arch = "x86_64")] +#[derive(Debug, thiserror::Error, PartialEq)] +pub enum RestoreStateError { + #[error("{0}")] + SetPit2(kvm_ioctls::Error), + #[error("{0}")] + SetClock(kvm_ioctls::Error), + #[error("{0}")] + SetIrqChipPicMaster(kvm_ioctls::Error), + #[error("{0}")] + SetIrqChipPicSlave(kvm_ioctls::Error), + #[error("{0}")] + SetIrqChipIoAPIC(kvm_ioctls::Error), +} + +/// Error type for [`Vm::restore_state`] +#[cfg(target_arch = "aarch64")] +#[derive(Debug, derive_more::From)] +pub struct RestoreStateError(arch::aarch64::gic::Error); +#[cfg(target_arch = "aarch64")] +impl fmt::Display for RestoreStateError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self.0) + } +} +#[cfg(target_arch = "aarch64")] +impl std::error::Error for RestoreStateError {} + +impl fmt::Display for Error { fn fmt(&self, f: &mut Formatter) -> std::fmt::Result { use self::Error::*; @@ -259,21 +288,32 @@ impl Vm { } #[cfg(target_arch = "x86_64")] - /// Restores the Kvm Vm state. - pub fn restore_state(&self, state: &VmState) -> Result<()> { + /// Restores the KVM VM state. + /// + /// # Errors + /// + /// When: + /// - [`kvm_ioctls::VmFd::set_pit`] errors. + /// - [`kvm_ioctls::VmFd::set_clock`] errors. + /// - [`kvm_ioctls::VmFd::set_irqchip`] errors. + /// - [`kvm_ioctls::VmFd::set_irqchip`] errors. + /// - [`kvm_ioctls::VmFd::set_irqchip`] errors. + pub fn restore_state(&self, state: &VmState) -> std::result::Result<(), RestoreStateError> { self.fd .set_pit2(&state.pitstate) - .map_err(Error::VmSetPit2)?; - self.fd.set_clock(&state.clock).map_err(Error::VmSetClock)?; + .map_err(RestoreStateError::SetPit2)?; + self.fd + .set_clock(&state.clock) + .map_err(RestoreStateError::SetClock)?; self.fd .set_irqchip(&state.pic_master) - .map_err(Error::VmSetIrqChip)?; + .map_err(RestoreStateError::SetIrqChipPicMaster)?; self.fd .set_irqchip(&state.pic_slave) - .map_err(Error::VmSetIrqChip)?; + .map_err(RestoreStateError::SetIrqChipPicSlave)?; self.fd .set_irqchip(&state.ioapic) - .map_err(Error::VmSetIrqChip)?; + .map_err(RestoreStateError::SetIrqChipIoAPIC)?; Ok(()) } @@ -287,11 +327,20 @@ impl Vm { }) } + /// Restore the KVM VM state + /// + /// # Errors + /// + /// When [`GICDevice::restore_device`] errors. #[cfg(target_arch = "aarch64")] - pub fn restore_state(&self, mpidrs: &[u64], state: &VmState) -> Result<()> { + pub fn restore_state( + &self, + mpidrs: &[u64], + state: &VmState, + ) -> std::result::Result<(), RestoreStateError> { self.get_irqchip() .restore_device(mpidrs, &state.gic) - .map_err(Error::RestoreGic) + .map_err(RestoreStateError) } pub(crate) fn set_kvm_memory_regions( diff --git a/src/vmm/tests/integration_tests.rs b/src/vmm/tests/integration_tests.rs index 8c8153341ad..08ffe40192b 100644 --- a/src/vmm/tests/integration_tests.rs +++ b/src/vmm/tests/integration_tests.rs @@ -7,7 +7,7 @@ use std::{io, thread}; use snapshot::Snapshot; use utils::tempfile::TempFile; use vmm::builder::{build_microvm_for_boot, build_microvm_from_snapshot, setup_serial_device}; -use vmm::persist::{self, snapshot_state_sanity_check, LoadSnapshotError, MicrovmState}; +use vmm::persist::{self, snapshot_state_sanity_check, MicrovmState}; use vmm::resources::VmResources; use vmm::seccomp_filters::{get_filters, SeccompConfig}; use vmm::utilities::mock_devices::MockSerialInput; @@ -261,6 +261,7 @@ fn test_create_and_load_snapshot() { #[test] fn test_snapshot_load_sanity_checks() { + use vmm::persist::SnapShotStateSanityCheckError; use vmm::vmm_config::machine_config::MAX_SUPPORTED_VCPUS; let mut microvm_state = get_microvm_state_from_snapshot(); @@ -271,13 +272,10 @@ fn test_snapshot_load_sanity_checks() { microvm_state.memory_state.regions.clear(); // Validate sanity checks fail because there is no mem region in state. - let err = snapshot_state_sanity_check(µvm_state).unwrap_err(); - match err { - LoadSnapshotError::InvalidSnapshot(err_msg) => { - assert_eq!(err_msg, "No memory region defined.") - } - _ => unreachable!(), - } + assert_eq!( + snapshot_state_sanity_check(µvm_state), + Err(SnapShotStateSanityCheckError::NoMemory) + ); // Create MAX_SUPPORTED_VCPUS vCPUs starting from 1 vCPU. for _ in 0..(MAX_SUPPORTED_VCPUS as f64).log2() as usize { @@ -292,21 +290,19 @@ fn test_snapshot_load_sanity_checks() { .push(microvm_state.vcpu_states[0].clone()); // Validate sanity checks fail because there are too many vCPUs. - let err = snapshot_state_sanity_check(µvm_state).unwrap_err(); - match err { - LoadSnapshotError::InvalidSnapshot(err_msg) => assert_eq!(err_msg, "Invalid vCPU count."), - _ => unreachable!(), - } + assert_eq!( + snapshot_state_sanity_check(µvm_state), + Err(SnapShotStateSanityCheckError::InvalidVcpuCount) + ); // Remove all vCPUs states from microvm state. microvm_state.vcpu_states.clear(); // Validate sanity checks fail because there is no vCPU in state. - let err = snapshot_state_sanity_check(µvm_state).unwrap_err(); - match err { - LoadSnapshotError::InvalidSnapshot(err_msg) => assert_eq!(err_msg, "Invalid vCPU count."), - _ => unreachable!(), - } + assert_eq!( + snapshot_state_sanity_check(µvm_state), + Err(SnapShotStateSanityCheckError::InvalidVcpuCount) + ); } fn get_microvm_state_from_snapshot() -> MicrovmState { @@ -344,7 +340,7 @@ fn test_snapshot_cpu_vendor_mismatch() { // Check if the snapshot created above passes validation since // the snapshot was created locally. - assert!(validate_cpu_vendor(µvm_state).is_ok()); + assert_eq!(validate_cpu_vendor(µvm_state), Ok(true)); // Modify the vendor id in CPUID. for entry in microvm_state.vcpu_states[0].cpuid.as_mut_slice().iter_mut() { @@ -358,8 +354,9 @@ fn test_snapshot_cpu_vendor_mismatch() { } } - // This must fail as the cpu vendor has been mangled. - assert!(validate_cpu_vendor(µvm_state).is_err()); + // It succeeds in checking if the CPU vendor is valid, in this process it discovers the CPU + // vendor not valid. + assert_eq!(validate_cpu_vendor(µvm_state), Ok(false)); // Negative test: remove the vendor id from cpuid. for entry in microvm_state.vcpu_states[0].cpuid.as_mut_slice().iter_mut() { @@ -368,8 +365,14 @@ fn test_snapshot_cpu_vendor_mismatch() { } } - // This must fail as the cpu vendor has been mangled. - assert!(validate_cpu_vendor(µvm_state).is_err()); + // It succeeds in checking if the CPU vendor is valid, in this process it discovers the CPU + // vendor not valid. + assert_eq!( + validate_cpu_vendor(µvm_state), + Err(vmm::persist::ValidateCpuVendorError::Snapshot( + cpuid::common::Error::NotSupported + )) + ); } #[cfg(target_arch = "x86_64")] @@ -409,13 +412,13 @@ fn test_snapshot_cpu_vendor() { #[test] fn test_snapshot_cpu_vendor_missing() { use arch::regs::MIDR_EL1; - use vmm::persist::validate_cpu_manufacturer_id; + use vmm::persist::{validate_cpu_manufacturer_id, ValidateCpuManufacturerIdError}; let mut microvm_state = get_microvm_state_from_snapshot(); // Check if the snapshot created above passes validation since // the snapshot was created locally. - assert!(validate_cpu_manufacturer_id(µvm_state).is_ok()); + assert_eq!(validate_cpu_manufacturer_id(µvm_state), Ok(true)); // Remove the MIDR_EL1 value from the VCPU states, by setting it to 0 for state in microvm_state.vcpu_states.as_mut_slice().iter_mut() { @@ -425,7 +428,10 @@ fn test_snapshot_cpu_vendor_missing() { } } } - assert!(validate_cpu_manufacturer_id(µvm_state).is_err()); + assert!(matches!( + validate_cpu_manufacturer_id(µvm_state), + Err(ValidateCpuManufacturerIdError::Snapshot(_)) + )); } #[cfg(target_arch = "aarch64")] @@ -438,7 +444,7 @@ fn test_snapshot_cpu_vendor_mismatch() { // Check if the snapshot created above passes validation since // the snapshot was created locally. - assert!(validate_cpu_manufacturer_id(µvm_state).is_ok()); + assert_eq!(validate_cpu_manufacturer_id(µvm_state), Ok(true)); // Change the MIDR_EL1 value from the VCPU states, to contain an // invalid manufacturer ID @@ -449,5 +455,5 @@ fn test_snapshot_cpu_vendor_mismatch() { } } } - assert!(validate_cpu_manufacturer_id(µvm_state).is_err()); + assert_eq!(validate_cpu_manufacturer_id(µvm_state), Ok(false)); } diff --git a/tests/integration_tests/build/test_binary_size.py b/tests/integration_tests/build/test_binary_size.py index 221ab730769..c166c2dca1a 100644 --- a/tests/integration_tests/build/test_binary_size.py +++ b/tests/integration_tests/build/test_binary_size.py @@ -13,9 +13,9 @@ SIZES_DICT = { "x86_64": { - "FC_BINARY_SIZE_TARGET": 2367352, + "FC_BINARY_SIZE_TARGET": 2520632, "JAILER_BINARY_SIZE_TARGET": 797632, - "FC_BINARY_SIZE_LIMIT": 2485719, + "FC_BINARY_SIZE_LIMIT": 2646663, "JAILER_BINARY_SIZE_LIMIT": 869608, }, "aarch64": { diff --git a/tests/integration_tests/build/test_coverage.py b/tests/integration_tests/build/test_coverage.py index d798d236888..eaee6b48a14 100644 --- a/tests/integration_tests/build/test_coverage.py +++ b/tests/integration_tests/build/test_coverage.py @@ -29,9 +29,9 @@ # Checkout the cpuid crate. In the future other # differences may appear. if utils.is_io_uring_supported(): - COVERAGE_DICT = {"Intel": 84.77, "AMD": 84.11, "ARM": 83.78} + COVERAGE_DICT = {"Intel": 83.96, "AMD": 83.31, "ARM": 83.42} else: - COVERAGE_DICT = {"Intel": 81.89, "AMD": 81.25, "ARM": 80.86} + COVERAGE_DICT = {"Intel": 81.12, "AMD": 80.47, "ARM": 80.51} PROC_MODEL = proc.proc_type() diff --git a/tests/integration_tests/functional/test_api.py b/tests/integration_tests/functional/test_api.py index a67940994d6..8edfd6f3abb 100644 --- a/tests/integration_tests/functional/test_api.py +++ b/tests/integration_tests/functional/test_api.py @@ -491,8 +491,8 @@ def test_api_machine_config(test_microvm_with_api): if utils.get_cpu_vendor() == utils.CpuVendor.AMD: # We shouldn't be able to apply Intel templates on AMD hosts fail_msg = ( - "Internal error while starting microVM: Error configuring" - " the vcpu for boot: Cpuid error: InvalidVendor" + "Internal error while starting microVM: Error configuring the vcpu for boot: Failed to " + "set CPUID entries: The operation is not permitted for the current vendor." ) assert test_microvm.api_session.is_status_bad_request(response.status_code) assert fail_msg in response.text diff --git a/tests/integration_tests/functional/test_snapshot_basic.py b/tests/integration_tests/functional/test_snapshot_basic.py index 54c6b584e7f..f14e182dbdf 100644 --- a/tests/integration_tests/functional/test_snapshot_basic.py +++ b/tests/integration_tests/functional/test_snapshot_basic.py @@ -367,7 +367,11 @@ def test_load_snapshot_failure_handling(test_microvm_with_api): "Response status code %d, content: %s.", response.status_code, response.text ) assert vm.api_session.is_status_bad_request(response.status_code) - assert "Cannot deserialize the microVM state" in response.text + assert ( + "Load microVM snapshot error: Failed to restore from snapshot: Failed to get snapshot " + "state from file: Failed to load snapshot state from file: Snapshot file is smaller " + "than CRC length." + ) in response.text # Check if FC process is closed wait_process_termination(vm.jailer_clone_pid) @@ -512,7 +516,11 @@ def test_negative_snapshot_permissions(bin_cloner_path): ) except AssertionError as error: # Check if proper error is returned. - assert "Cannot open the memory file: Permission denied" in str(error) + assert ( + "Load microVM snapshot error: Failed to restore from snapshot: Failed to load guest " + "memory: Error creating guest memory from file: Failed to load guest memory: " + "Permission denied (os error 13)" + ) in str(error) else: assert False, "Negative test failed" @@ -526,9 +534,9 @@ def test_negative_snapshot_permissions(bin_cloner_path): except AssertionError as error: # Check if proper error is returned. assert ( - "Cannot perform open on the snapshot backing file:" - " Permission denied" in str(error) - ) + "Load microVM snapshot error: Failed to restore from snapshot: Failed to get snapshot " + "state from file: Failed to open snapshot file: Permission denied (os error 13)" + ) in str(error) else: assert False, "Negative test failed" diff --git a/tests/integration_tests/functional/test_uffd.py b/tests/integration_tests/functional/test_uffd.py index cf33ec50dcd..5f28395846a 100644 --- a/tests/integration_tests/functional/test_uffd.py +++ b/tests/integration_tests/functional/test_uffd.py @@ -115,10 +115,10 @@ def test_bad_socket_path(bin_cloner_path, test_microvm_with_api): assert vm.api_session.is_status_bad_request(response.status_code) assert ( - "Load microVM snapshot error: Cannot connect to UDS in order to " - "send information on handling guest memory page-faults due to: " - "No such file or directory (os error 2)" in response.text - ) + "Load microVM snapshot error: Failed to restore from snapshot: Failed to load guest " + "memory: Error creating guest memory from uffd: Failed to connect to UDS Unix stream: No " + "such file or directory (os error 2)" + ) in response.text def test_unbinded_socket(bin_cloner_path, test_microvm_with_api): @@ -148,10 +148,10 @@ def test_unbinded_socket(bin_cloner_path, test_microvm_with_api): assert vm.api_session.is_status_bad_request(response.status_code) assert ( - "Load microVM snapshot error: Cannot connect to UDS in order to" - " send information on handling guest memory page-faults due to: " - "Connection refused (os error 111)" in response.text - ) + "Load microVM snapshot error: Failed to restore from snapshot: Failed to load guest " + "memory: Error creating guest memory from uffd: Failed to connect to UDS Unix stream: " + "Connection refused (os error 111)" + ) in response.text def test_valid_handler(bin_cloner_path, test_microvm_with_api, uffd_handler_paths):