diff --git a/Cargo.lock b/Cargo.lock index 71359874a83..11609458e14 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -112,12 +112,6 @@ dependencies = [ "windows-sys 0.60.2", ] -[[package]] -name = "anyhow" -version = "1.0.100" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a23eb6b1614318a8071c9b2521f36b424b2c83db5eb3a0fead4a6c0809af6e61" - [[package]] name = "arrayvec" version = "0.7.6" @@ -1676,7 +1670,6 @@ version = "0.1.0" dependencies = [ "acpi_tables", "aes-gcm", - "anyhow", "arrayvec", "aws-lc-rs", "base64", diff --git a/src/vmm/Cargo.toml b/src/vmm/Cargo.toml index d2601f4a305..3a364c04dd4 100644 --- a/src/vmm/Cargo.toml +++ b/src/vmm/Cargo.toml @@ -17,7 +17,6 @@ gdb = ["arrayvec", "gdbstub", "gdbstub_arch"] acpi_tables = { path = "../acpi-tables" } aes-gcm = { version = "0.10.1", default-features = false, features = ["aes"] } -anyhow = "1.0.100" arrayvec = { version = "0.7.6", optional = true } aws-lc-rs = { version = "1.14.0", features = ["bindgen"] } base64 = "0.22.1" diff --git a/src/vmm/src/device_manager/mmio.rs b/src/vmm/src/device_manager/mmio.rs index f7514ed1e0c..7dd240b01da 100644 --- a/src/vmm/src/device_manager/mmio.rs +++ b/src/vmm/src/device_manager/mmio.rs @@ -25,7 +25,6 @@ use crate::arch::{RTC_MEM_START, SERIAL_MEM_START}; #[cfg(target_arch = "aarch64")] use crate::devices::legacy::{RTCDevice, SerialDevice}; use crate::devices::pseudo::BootTimer; -use crate::devices::virtio::device::VirtioDevice; use crate::devices::virtio::transport::mmio::MmioTransport; use crate::vstate::bus::{Bus, BusError}; #[cfg(target_arch = "x86_64")] @@ -41,12 +40,6 @@ pub enum MmioError { BusInsert(#[from] BusError), /// Failed to allocate requested resourc: {0} Cmdline(#[from] linux_loader::cmdline::Error), - /// Failed to find the device on the bus. - DeviceNotFound, - /// Invalid device type found on the MMIO bus. - InvalidDeviceType, - /// {0} - InternalDeviceError(String), /// Could not create IRQ for MMIO device: {0} CreateIrq(#[from] std::io::Error), /// Invalid MMIO IRQ configuration. @@ -407,31 +400,6 @@ impl MMIODeviceManager { Ok(()) } - /// Run fn `f()` for the virtio device matching `virtio_type` and `id`. - pub fn with_virtio_device_with_id( - &self, - virtio_type: u32, - id: &str, - f: F, - ) -> Result<(), MmioError> - where - T: VirtioDevice + 'static + Debug, - F: FnOnce(&mut T) -> Result<(), String>, - { - if let Some(device) = self.get_virtio_device(virtio_type, id) { - let virtio_device = device.inner.lock().expect("Poisoned lock").device(); - let mut dev = virtio_device.lock().expect("Poisoned lock"); - f(dev - .as_mut_any() - .downcast_mut::() - .ok_or(MmioError::InvalidDeviceType)?) - .map_err(MmioError::InternalDeviceError)?; - } else { - return Err(MmioError::DeviceNotFound); - } - Ok(()) - } - #[cfg(target_arch = "aarch64")] pub fn virtio_device_info(&self) -> Vec<&MMIODeviceInfo> { let mut device_info = Vec::new(); diff --git a/src/vmm/src/device_manager/mod.rs b/src/vmm/src/device_manager/mod.rs index f35190b5841..b4b64ff6762 100644 --- a/src/vmm/src/device_manager/mod.rs +++ b/src/vmm/src/device_manager/mod.rs @@ -87,12 +87,8 @@ pub enum AttachDeviceError { #[derive(Debug, thiserror::Error, displaydoc::Display)] /// Error while searching for a VirtIO device pub enum FindDeviceError { - /// Device type is invalid - InvalidDeviceType, /// Device not found DeviceNotFound, - /// Internal Device error: {0} - InternalDeviceError(anyhow::Error), } #[derive(Debug)] @@ -372,36 +368,21 @@ impl DeviceManager { } /// Run fn `f()` for the virtio device matching `virtio_type` and `id`. - pub fn try_with_virtio_device_with_id( - &self, - id: &str, - f: F, - ) -> Result + pub fn with_virtio_device(&self, id: &str, f: F) -> Result where T: VirtioDevice + 'static + Debug, - E: std::error::Error + 'static + Send + Sync, - F: FnOnce(&mut T) -> Result, + F: FnOnce(&mut T) -> R, { if let Some(device) = self.get_virtio_device(T::const_device_type(), id) { let mut dev = device.lock().expect("Poisoned lock"); - f(dev + Ok(f(dev .as_mut_any() .downcast_mut::() - .ok_or(FindDeviceError::InvalidDeviceType)?) - .map_err(|e| FindDeviceError::InternalDeviceError(e.into())) + .expect("Invalid device for a given device type"))) } else { Err(FindDeviceError::DeviceNotFound) } } - - /// Run fn `f()` for the virtio device matching `virtio_type` and `id`. - pub fn with_virtio_device_with_id(&self, id: &str, f: F) -> Result - where - T: VirtioDevice + 'static + Debug, - F: FnOnce(&mut T) -> R, - { - self.try_with_virtio_device_with_id(id, |dev: &mut T| Ok::(f(dev))) - } } #[derive(Debug, Default, Clone, Serialize, Deserialize)] @@ -471,7 +452,7 @@ impl<'a> Persist<'a> for DeviceManager { fn restore( constructor_args: Self::ConstructorArgs, state: &Self::State, - ) -> Result { + ) -> std::result::Result { // Setup legacy devices in case of x86 #[cfg(target_arch = "x86_64")] let legacy_devices = Self::create_legacy_devices( diff --git a/src/vmm/src/device_manager/pci_mngr.rs b/src/vmm/src/device_manager/pci_mngr.rs index a9290886997..0b06c31a4d4 100644 --- a/src/vmm/src/device_manager/pci_mngr.rs +++ b/src/vmm/src/device_manager/pci_mngr.rs @@ -10,7 +10,7 @@ use event_manager::{MutEventSubscriber, SubscriberOps}; use log::{debug, error, warn}; use serde::{Deserialize, Serialize}; -use super::persist::{MmdsState, SharedDeviceType}; +use super::persist::MmdsState; use crate::devices::pci::PciSegment; use crate::devices::virtio::balloon::Balloon; use crate::devices::virtio::balloon::persist::{BalloonConstructorArgs, BalloonState}; @@ -419,8 +419,8 @@ impl<'a> Persist<'a> for PciDevices { constructor_args .vm_resources - .update_from_restored_device(SharedDeviceType::Balloon(device.clone())) - .unwrap(); + .balloon + .set_device(device.clone()); pci_devices .restore_pci_device( @@ -444,8 +444,8 @@ impl<'a> Persist<'a> for PciDevices { constructor_args .vm_resources - .update_from_restored_device(SharedDeviceType::VirtioBlock(device.clone())) - .unwrap(); + .block + .add_virtio_device(device.clone()); pci_devices .restore_pci_device( @@ -494,8 +494,8 @@ impl<'a> Persist<'a> for PciDevices { constructor_args .vm_resources - .update_from_restored_device(SharedDeviceType::Network(device.clone())) - .unwrap(); + .net_builder + .add_device(device.clone()); pci_devices .restore_pci_device( @@ -527,8 +527,8 @@ impl<'a> Persist<'a> for PciDevices { constructor_args .vm_resources - .update_from_restored_device(SharedDeviceType::Vsock(device.clone())) - .unwrap(); + .vsock + .set_device(device.clone()); pci_devices .restore_pci_device( @@ -550,8 +550,8 @@ impl<'a> Persist<'a> for PciDevices { constructor_args .vm_resources - .update_from_restored_device(SharedDeviceType::Entropy(device.clone())) - .unwrap(); + .entropy + .set_device(device.clone()); pci_devices .restore_pci_device( diff --git a/src/vmm/src/device_manager/persist.rs b/src/vmm/src/device_manager/persist.rs index f874975981b..14eb629b5b4 100644 --- a/src/vmm/src/device_manager/persist.rs +++ b/src/vmm/src/device_manager/persist.rs @@ -40,7 +40,7 @@ use crate::devices::virtio::vsock::persist::{ }; use crate::devices::virtio::vsock::{Vsock, VsockError, VsockUnixBackend, VsockUnixBackendError}; use crate::mmds::data_store::MmdsVersion; -use crate::resources::{ResourcesError, VmResources}; +use crate::resources::VmResources; use crate::snapshot::Persist; use crate::vmm_config::mmds::MmdsConfigError; use crate::vstate::bus::BusError; @@ -73,8 +73,6 @@ pub enum DevicePersistError { MmdsConfig(#[from] MmdsConfigError), /// Entropy: {0} Entropy(#[from] EntropyError), - /// Resource misconfiguration: {0}. Is the snapshot file corrupted? - ResourcesError(#[from] ResourcesError), /// Could not activate device: {0} DeviceActivation(#[from] ActivateError), } @@ -128,17 +126,6 @@ pub struct DeviceStates { pub entropy_device: Option>, } -/// A type used to extract the concrete `Arc>` for each of the device -/// types when restoring from a snapshot. -#[derive(Debug)] -pub enum SharedDeviceType { - VirtioBlock(Arc>), - Network(Arc>), - Balloon(Arc>), - Vsock(Arc>>), - Entropy(Arc>), -} - pub struct MMIODevManagerConstructorArgs<'a> { pub mem: &'a GuestMemoryMmap, pub vm: &'a Arc, @@ -422,7 +409,8 @@ impl<'a> Persist<'a> for MMIODeviceManager { constructor_args .vm_resources - .update_from_restored_device(SharedDeviceType::Balloon(device.clone()))?; + .balloon + .set_device(device.clone()); restore_helper( device.clone(), @@ -444,7 +432,8 @@ impl<'a> Persist<'a> for MMIODeviceManager { constructor_args .vm_resources - .update_from_restored_device(SharedDeviceType::VirtioBlock(device.clone()))?; + .block + .add_virtio_device(device.clone()); restore_helper( device.clone(), @@ -483,7 +472,8 @@ impl<'a> Persist<'a> for MMIODeviceManager { constructor_args .vm_resources - .update_from_restored_device(SharedDeviceType::Network(device.clone()))?; + .net_builder + .add_device(device.clone()); restore_helper( device.clone(), @@ -512,7 +502,8 @@ impl<'a> Persist<'a> for MMIODeviceManager { constructor_args .vm_resources - .update_from_restored_device(SharedDeviceType::Vsock(device.clone()))?; + .vsock + .set_device(device.clone()); restore_helper( device.clone(), @@ -536,7 +527,8 @@ impl<'a> Persist<'a> for MMIODeviceManager { constructor_args .vm_resources - .update_from_restored_device(SharedDeviceType::Entropy(device.clone()))?; + .entropy + .set_device(device.clone()); restore_helper( device.clone(), diff --git a/src/vmm/src/lib.rs b/src/vmm/src/lib.rs index ace273bb94c..52d850adfdc 100644 --- a/src/vmm/src/lib.rs +++ b/src/vmm/src/lib.rs @@ -136,7 +136,10 @@ use vstate::kvm::Kvm; use vstate::vcpu::{self, StartThreadedError, VcpuSendEventError}; use crate::cpu_config::templates::CpuConfiguration; -use crate::devices::virtio::balloon::{BALLOON_DEV_ID, Balloon, BalloonConfig, BalloonStats}; +use crate::devices::virtio::balloon::{ + BALLOON_DEV_ID, Balloon, BalloonConfig, BalloonError, BalloonStats, +}; +use crate::devices::virtio::block::BlockError; use crate::devices::virtio::block::device::Block; use crate::devices::virtio::net::Net; use crate::logger::{METRICS, MetricsError, error, info, warn}; @@ -248,6 +251,10 @@ pub enum VmmError { VMGenID(#[from] VmGenIdError), /// Failed perform action on device: {0} FindDeviceError(#[from] device_manager::FindDeviceError), + /// Block: {0} + Block(#[from] BlockError), + /// Balloon: {0} + Balloon(#[from] BalloonError), } /// Shorthand type for KVM dirty page bitmap. @@ -519,10 +526,11 @@ impl Vmm { path_on_host: String, ) -> Result<(), VmmError> { self.device_manager - .try_with_virtio_device_with_id(drive_id, |block: &mut Block| { + .with_virtio_device(drive_id, |block: &mut Block| { block.update_disk_image(path_on_host) }) - .map_err(VmmError::FindDeviceError) + .map_err(VmmError::FindDeviceError)??; + Ok(()) } /// Updates the rate limiter parameters for block device with `drive_id` id. @@ -533,17 +541,19 @@ impl Vmm { rl_ops: BucketUpdate, ) -> Result<(), VmmError> { self.device_manager - .try_with_virtio_device_with_id(drive_id, |block: &mut Block| { + .with_virtio_device(drive_id, |block: &mut Block| { block.update_rate_limiter(rl_bytes, rl_ops) }) - .map_err(VmmError::FindDeviceError) + .map_err(VmmError::FindDeviceError)??; + Ok(()) } /// Updates the rate limiter parameters for block device with `drive_id` id. pub fn update_vhost_user_block_config(&mut self, drive_id: &str) -> Result<(), VmmError> { self.device_manager - .try_with_virtio_device_with_id(drive_id, |block: &mut Block| block.update_config()) - .map_err(VmmError::FindDeviceError) + .with_virtio_device(drive_id, |block: &mut Block| block.update_config()) + .map_err(VmmError::FindDeviceError)??; + Ok(()) } /// Updates the rate limiter parameters for net device with `net_id` id. @@ -556,7 +566,7 @@ impl Vmm { tx_ops: BucketUpdate, ) -> Result<(), VmmError> { self.device_manager - .with_virtio_device_with_id(net_id, |net: &mut Net| { + .with_virtio_device(net_id, |net: &mut Net| { net.patch_rate_limiters(rx_bytes, rx_ops, tx_bytes, tx_ops) }) .map_err(VmmError::FindDeviceError) @@ -565,24 +575,27 @@ impl Vmm { /// Returns a reference to the balloon device if present. pub fn balloon_config(&self) -> Result { self.device_manager - .with_virtio_device_with_id(BALLOON_DEV_ID, |dev: &mut Balloon| dev.config()) + .with_virtio_device(BALLOON_DEV_ID, |dev: &mut Balloon| dev.config()) .map_err(VmmError::FindDeviceError) } /// Returns the latest balloon statistics if they are enabled. pub fn latest_balloon_stats(&self) -> Result { - self.device_manager - .try_with_virtio_device_with_id(BALLOON_DEV_ID, |dev: &mut Balloon| dev.latest_stats()) - .map_err(VmmError::FindDeviceError) + let stats = self + .device_manager + .with_virtio_device(BALLOON_DEV_ID, |dev: &mut Balloon| dev.latest_stats()) + .map_err(VmmError::FindDeviceError)??; + Ok(stats) } /// Updates configuration for the balloon device target size. pub fn update_balloon_config(&mut self, amount_mib: u32) -> Result<(), VmmError> { self.device_manager - .try_with_virtio_device_with_id(BALLOON_DEV_ID, |dev: &mut Balloon| { + .with_virtio_device(BALLOON_DEV_ID, |dev: &mut Balloon| { dev.update_size(amount_mib) }) - .map_err(VmmError::FindDeviceError) + .map_err(VmmError::FindDeviceError)??; + Ok(()) } /// Updates configuration for the balloon device as described in `balloon_stats_update`. @@ -591,10 +604,11 @@ impl Vmm { stats_polling_interval_s: u16, ) -> Result<(), VmmError> { self.device_manager - .try_with_virtio_device_with_id(BALLOON_DEV_ID, |dev: &mut Balloon| { + .with_virtio_device(BALLOON_DEV_ID, |dev: &mut Balloon| { dev.update_stats_polling_interval(stats_polling_interval_s) }) - .map_err(VmmError::FindDeviceError) + .map_err(VmmError::FindDeviceError)??; + Ok(()) } /// Signals Vmm to stop and exit. diff --git a/src/vmm/src/resources.rs b/src/vmm/src/resources.rs index 0d2f4bbed22..3d5917a2392 100644 --- a/src/vmm/src/resources.rs +++ b/src/vmm/src/resources.rs @@ -8,7 +8,6 @@ use std::sync::{Arc, Mutex, MutexGuard}; use serde::{Deserialize, Serialize}; use crate::cpu_config::templates::CustomCpuTemplate; -use crate::device_manager::persist::SharedDeviceType; use crate::logger::info; use crate::mmds; use crate::mmds::data_store::{Mmds, MmdsVersion}; @@ -218,40 +217,6 @@ impl VmResources { Ok(mmds.lock().expect("Poisoned lock")) } - /// Updates the resources from a restored device (used for configuring resources when - /// restoring from a snapshot). - pub fn update_from_restored_device( - &mut self, - device: SharedDeviceType, - ) -> Result<(), ResourcesError> { - match device { - SharedDeviceType::VirtioBlock(block) => { - self.block.add_virtio_device(block); - } - - SharedDeviceType::Network(network) => { - self.net_builder.add_device(network); - } - - SharedDeviceType::Balloon(balloon) => { - self.balloon.set_device(balloon); - - if self.machine_config.huge_pages != HugePageConfig::None { - return Err(ResourcesError::BalloonDevice(BalloonConfigError::HugePages)); - } - } - - SharedDeviceType::Vsock(vsock) => { - self.vsock.set_device(vsock); - } - SharedDeviceType::Entropy(entropy) => { - self.entropy.set_device(entropy); - } - } - - Ok(()) - } - /// Add a custom CPU template to the VM resources /// to configure vCPUs. pub fn set_custom_cpu_template(&mut self, cpu_template: CustomCpuTemplate) { @@ -1499,31 +1464,6 @@ mod tests { .unwrap_err(); } - #[test] - fn test_negative_restore_balloon_device_with_huge_pages() { - let mut vm_resources = default_vm_resources(); - vm_resources.balloon = BalloonBuilder::new(); - vm_resources - .update_machine_config(&MachineConfigUpdate { - huge_pages: Some(HugePageConfig::Hugetlbfs2M), - ..Default::default() - }) - .unwrap(); - let err = vm_resources - .update_from_restored_device(SharedDeviceType::Balloon(Arc::new(Mutex::new( - Balloon::new(128, false, 0, true).unwrap(), - )))) - .unwrap_err(); - assert!( - matches!( - err, - ResourcesError::BalloonDevice(BalloonConfigError::HugePages) - ), - "{:?}", - err - ); - } - #[test] fn test_set_entropy_device() { let mut vm_resources = default_vm_resources();