From a50b3784fe9bbd4eeb231e4524b93c00672e5179 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Wed, 23 Nov 2022 15:01:16 +0100 Subject: [PATCH] virtio-devices: Create a proper result type for VirtioPciDevice Creating a dedicated Result type for VirtioPciDevice, associated with the new VirtioPciDeviceError enum. This allows for a clearer handling of the errors generated through VirtioPciDevice::new(). Signed-off-by: Sebastien Boeuf --- virtio-devices/src/transport/mod.rs | 2 +- virtio-devices/src/transport/pci_device.rs | 87 ++++++++++++++-------- vmm/src/device_manager.rs | 2 +- 3 files changed, 56 insertions(+), 35 deletions(-) diff --git a/virtio-devices/src/transport/mod.rs b/virtio-devices/src/transport/mod.rs index 038d4cb98..3a43b73cf 100644 --- a/virtio-devices/src/transport/mod.rs +++ b/virtio-devices/src/transport/mod.rs @@ -6,7 +6,7 @@ use vmm_sys_util::eventfd::EventFd; mod pci_common_config; mod pci_device; pub use pci_common_config::{VirtioPciCommonConfig, VIRTIO_PCI_COMMON_CONFIG_ID}; -pub use pci_device::{VirtioPciDevice, VirtioPciDeviceActivator}; +pub use pci_device::{VirtioPciDevice, VirtioPciDeviceActivator, VirtioPciDeviceError}; pub trait VirtioTransport { fn ioeventfds(&self, base_addr: u64) -> Vec<(&EventFd, u64)>; diff --git a/virtio-devices/src/transport/pci_device.rs b/virtio-devices/src/transport/pci_device.rs index e92876b9d..2b13204e3 100644 --- a/virtio-devices/src/transport/pci_device.rs +++ b/virtio-devices/src/transport/pci_device.rs @@ -13,6 +13,7 @@ use crate::{ DEVICE_ACKNOWLEDGE, DEVICE_DRIVER, DEVICE_DRIVER_OK, DEVICE_FAILED, DEVICE_FEATURES_OK, DEVICE_INIT, }; +use anyhow::anyhow; use libc::EFD_NONBLOCK; use pci::{ BarReprogrammingParams, MsixCap, MsixConfig, PciBarConfiguration, PciBarRegionType, @@ -23,9 +24,9 @@ use std::any::Any; use std::cmp; use std::io::Write; use std::ops::Deref; -use std::result; use std::sync::atomic::{AtomicBool, AtomicU16, AtomicUsize, Ordering}; use std::sync::{Arc, Barrier, Mutex}; +use thiserror::Error; use versionize::{VersionMap, Versionize, VersionizeResult}; use versionize_derive::Versionize; use virtio_queue::{Queue, QueueT}; @@ -40,7 +41,7 @@ use vm_migration::{ Migratable, MigratableError, Pausable, Snapshot, Snapshottable, Transportable, VersionMapped, }; use vm_virtio::AccessPlatform; -use vmm_sys_util::{errno::Result, eventfd::EventFd}; +use vmm_sys_util::eventfd::EventFd; use super::pci_common_config::VirtioPciCommonConfigState; @@ -311,6 +312,13 @@ impl VirtioPciDeviceActivator { } } +#[derive(Error, Debug)] +pub enum VirtioPciDeviceError { + #[error("Failed creating VirtioPciDevice: {0}")] + CreateVirtioPciDevice(#[source] anyhow::Error), +} +pub type Result = std::result::Result; + pub struct VirtioPciDevice { id: String, @@ -389,7 +397,12 @@ impl VirtioPciDevice { let mut locked_device = device.lock().unwrap(); let mut queue_evts = Vec::new(); for _ in locked_device.queue_max_sizes().iter() { - queue_evts.push(EventFd::new(EFD_NONBLOCK)?) + queue_evts.push(EventFd::new(EFD_NONBLOCK).map_err(|e| { + VirtioPciDeviceError::CreateVirtioPciDevice(anyhow!( + "Failed creating eventfd: {}", + e + )) + })?) } let num_queues = locked_device.queue_max_sizes().len(); @@ -405,18 +418,25 @@ impl VirtioPciDevice { let pci_device_id = VIRTIO_PCI_DEVICE_ID_BASE + locked_device.device_type() as u16; - let interrupt_source_group = interrupt_manager.create_group(MsiIrqGroupConfig { - base: 0, - count: msix_num as InterruptIndex, - })?; + let interrupt_source_group = interrupt_manager + .create_group(MsiIrqGroupConfig { + base: 0, + count: msix_num as InterruptIndex, + }) + .map_err(|e| { + VirtioPciDeviceError::CreateVirtioPciDevice(anyhow!( + "Failed creating MSI interrupt group: {}", + e + )) + })?; let msix_state = vm_migration::versioned_state_from_id(snapshot.as_ref(), pci::MSIX_CONFIG_ID).map_err( |e| { - std::io::Error::new( - std::io::ErrorKind::Other, - format!("Failed to get MsixConfigState from Snapshot: {}", e), - ) + VirtioPciDeviceError::CreateVirtioPciDevice(anyhow!( + "Failed to get MsixConfigState from Snapshot: {}", + e + )) }, )?; @@ -454,10 +474,10 @@ impl VirtioPciDevice { let pci_configuration_state = vm_migration::versioned_state_from_id(snapshot.as_ref(), pci::PCI_CONFIGURATION_ID) .map_err(|e| { - std::io::Error::new( - std::io::ErrorKind::Other, - format!("Failed to get PciConfigurationState from Snapshot: {}", e), - ) + VirtioPciDeviceError::CreateVirtioPciDevice(anyhow!( + "Failed to get PciConfigurationState from Snapshot: {}", + e + )) })?; let configuration = PciConfiguration::new( @@ -477,13 +497,10 @@ impl VirtioPciDevice { let common_config_state = vm_migration::versioned_state_from_id(snapshot.as_ref(), VIRTIO_PCI_COMMON_CONFIG_ID) .map_err(|e| { - std::io::Error::new( - std::io::ErrorKind::Other, - format!( - "Failed to get VirtioPciCommonConfigState from Snapshot: {}", - e - ), - ) + VirtioPciDeviceError::CreateVirtioPciDevice(anyhow!( + "Failed to get VirtioPciCommonConfigState from Snapshot: {}", + e + )) })?; let common_config = if let Some(common_config_state) = common_config_state { @@ -508,10 +525,10 @@ impl VirtioPciDevice { .map(|s| s.to_versioned_state(&id)) .transpose() .map_err(|e| { - std::io::Error::new( - std::io::ErrorKind::Other, - format!("Failed to get VirtioPciDeviceState from Snapshot: {}", e), - ) + VirtioPciDeviceError::CreateVirtioPciDevice(anyhow!( + "Failed to get VirtioPciDeviceState from Snapshot: {}", + e + )) })?; let (device_activated, interrupt_status) = if let Some(state) = state { @@ -592,10 +609,10 @@ impl VirtioPciDevice { && virtio_pci_device.is_driver_ready() { virtio_pci_device.activate().map_err(|e| { - std::io::Error::new( - std::io::ErrorKind::Other, - format!("Failed activating the device: {}", e), - ) + VirtioPciDeviceError::CreateVirtioPciDevice(anyhow!( + "Failed activating the device: {}", + e + )) })?; } @@ -1079,7 +1096,11 @@ impl PciDevice for VirtioPciDevice { Ok(()) } - fn move_bar(&mut self, old_base: u64, new_base: u64) -> result::Result<(), std::io::Error> { + fn move_bar( + &mut self, + old_base: u64, + new_base: u64, + ) -> std::result::Result<(), std::io::Error> { // We only update our idea of the bar in order to support free_bars() above. // The majority of the reallocation is done inside DeviceManager. for bar in self.bar_regions.iter_mut() { @@ -1236,11 +1257,11 @@ impl BusDevice for VirtioPciDevice { } impl Pausable for VirtioPciDevice { - fn pause(&mut self) -> result::Result<(), MigratableError> { + fn pause(&mut self) -> std::result::Result<(), MigratableError> { Ok(()) } - fn resume(&mut self) -> result::Result<(), MigratableError> { + fn resume(&mut self) -> std::result::Result<(), MigratableError> { Ok(()) } } diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index dcedabbea..dde88b6de 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -226,7 +226,7 @@ pub enum DeviceManagerError { UnRegisterIoevent(anyhow::Error), /// Cannot create virtio device - VirtioDevice(vmm_sys_util::errno::Error), + VirtioDevice(virtio_devices::transport::VirtioPciDeviceError), /// Cannot add PCI device AddPciDevice(pci::PciRootError),