From f801b0fc72a44524d334a2cc9d9204f649e1255e Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Thu, 10 Mar 2022 14:05:36 +0100 Subject: [PATCH] vmm: device_manager: Factorize virtio device tuple into structure The tuple of information related to each virtio device is too big, and it's better to factorize it through a dedicated structure. Signed-off-by: Sebastien Boeuf --- vmm/src/device_manager.rs | 411 ++++++++++++++------------------------ 1 file changed, 145 insertions(+), 266 deletions(-) diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 905c0c9fd..21c40a886 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -791,6 +791,14 @@ pub enum PciDeviceHandle { VfioUser(Arc>), } +#[derive(Clone)] +struct MetaVirtioDevice { + virtio_device: Arc>, + iommu: bool, + id: String, + pci_segment: u16, +} + pub struct DeviceManager { // Manage address space related to devices address_manager: Arc, @@ -830,12 +838,7 @@ pub struct DeviceManager { memory_manager: Arc>, // The virtio devices on the system - virtio_devices: Vec<( - Arc>, - bool, - String, - u16, - )>, + virtio_devices: Vec, // List of bus devices // Let the DeviceManager keep strong references to the BusDevice devices. @@ -1095,12 +1098,7 @@ impl DeviceManager { console_pty: Option, console_resize_pipe: Option, ) -> DeviceManagerResult<()> { - let mut virtio_devices: Vec<( - Arc>, - bool, - String, - u16, - )> = Vec::new(); + let mut virtio_devices: Vec = Vec::new(); let interrupt_controller = self.add_interrupt_controller()?; @@ -1202,12 +1200,7 @@ impl DeviceManager { #[allow(unused_variables)] fn add_pci_devices( &mut self, - virtio_devices: Vec<( - Arc>, - bool, - String, - u16, - )>, + virtio_devices: Vec, ) -> DeviceManagerResult<()> { let iommu_id = String::from(IOMMU_DEVICE_NAME); @@ -1239,16 +1232,18 @@ impl DeviceManager { let mut iommu_attached_devices = Vec::new(); { - for (device, iommu_attached, id, pci_segment_id) in virtio_devices { - let mapping: &Option> = if iommu_attached { - &iommu_mapping - } else { - &None - }; + for handle in virtio_devices { + let mapping: &Option> = + if handle.iommu { &iommu_mapping } else { &None }; - let dev_id = self.add_virtio_pci_device(device, mapping, id, pci_segment_id)?; + let dev_id = self.add_virtio_pci_device( + handle.virtio_device, + mapping, + handle.id, + handle.pci_segment, + )?; - if iommu_attached { + if handle.iommu { iommu_attached_devices.push(dev_id); } } @@ -1756,12 +1751,7 @@ impl DeviceManager { fn add_virtio_console_device( &mut self, - virtio_devices: &mut Vec<( - Arc>, - bool, - String, - u16, - )>, + virtio_devices: &mut Vec, console_pty: Option, resize_pipe: Option, ) -> DeviceManagerResult>> { @@ -1840,12 +1830,13 @@ impl DeviceManager { ) .map_err(DeviceManagerError::CreateVirtioConsole)?; let virtio_console_device = Arc::new(Mutex::new(virtio_console_device)); - virtio_devices.push(( - Arc::clone(&virtio_console_device) as Arc>, - console_config.iommu, - id.clone(), - 0, - )); + virtio_devices.push(MetaVirtioDevice { + virtio_device: Arc::clone(&virtio_console_device) + as Arc>, + iommu: console_config.iommu, + id: id.clone(), + pci_segment: 0, + }); // Fill the device tree with a new node. In case of restore, we // know there is nothing to do, so we can simply override the @@ -1866,12 +1857,7 @@ impl DeviceManager { fn add_console_device( &mut self, interrupt_manager: &Arc>, - virtio_devices: &mut Vec<( - Arc>, - bool, - String, - u16, - )>, + virtio_devices: &mut Vec, serial_pty: Option, console_pty: Option, console_resize_pipe: Option, @@ -1929,22 +1915,8 @@ impl DeviceManager { Ok(Arc::new(Console { console_resizer })) } - fn make_virtio_devices( - &mut self, - ) -> DeviceManagerResult< - Vec<( - Arc>, - bool, - String, - u16, - )>, - > { - let mut devices: Vec<( - Arc>, - bool, - String, - u16, - )> = Vec::new(); + fn make_virtio_devices(&mut self) -> DeviceManagerResult> { + let mut devices: Vec = Vec::new(); // Create "standard" virtio devices (net/block/rng) devices.append(&mut self.make_virtio_block_devices()?); @@ -1985,12 +1957,7 @@ impl DeviceManager { fn make_virtio_block_device( &mut self, disk_cfg: &mut DiskConfig, - ) -> DeviceManagerResult<( - Arc>, - bool, - String, - u16, - )> { + ) -> DeviceManagerResult { let id = if let Some(id) = &disk_cfg.id { id.clone() } else { @@ -2033,13 +2000,13 @@ impl DeviceManager { .unwrap() .insert(id.clone(), device_node!(id, vhost_user_block_device)); - Ok(( - Arc::clone(&vhost_user_block_device) + Ok(MetaVirtioDevice { + virtio_device: Arc::clone(&vhost_user_block_device) as Arc>, - false, + iommu: false, id, - disk_cfg.pci_segment, - )) + pci_segment: disk_cfg.pci_segment, + }) } else { let mut options = OpenOptions::new(); options.read(true); @@ -2138,20 +2105,16 @@ impl DeviceManager { .unwrap() .insert(id.clone(), device_node!(id, migratable_device)); - Ok((virtio_device, disk_cfg.iommu, id, disk_cfg.pci_segment)) + Ok(MetaVirtioDevice { + virtio_device, + iommu: disk_cfg.iommu, + id, + pci_segment: disk_cfg.pci_segment, + }) } } - fn make_virtio_block_devices( - &mut self, - ) -> DeviceManagerResult< - Vec<( - Arc>, - bool, - String, - u16, - )>, - > { + fn make_virtio_block_devices(&mut self) -> DeviceManagerResult> { let mut devices = Vec::new(); let mut block_devices = self.config.lock().unwrap().disks.clone(); @@ -2168,12 +2131,7 @@ impl DeviceManager { fn make_virtio_net_device( &mut self, net_cfg: &mut NetConfig, - ) -> DeviceManagerResult<( - Arc>, - bool, - String, - u16, - )> { + ) -> DeviceManagerResult { let id = if let Some(id) = &net_cfg.id { id.clone() } else { @@ -2221,12 +2179,13 @@ impl DeviceManager { .unwrap() .insert(id.clone(), device_node!(id, vhost_user_net_device)); - Ok(( - Arc::clone(&vhost_user_net_device) as Arc>, - net_cfg.iommu, + Ok(MetaVirtioDevice { + virtio_device: Arc::clone(&vhost_user_net_device) + as Arc>, + iommu: net_cfg.iommu, id, - net_cfg.pci_segment, - )) + pci_segment: net_cfg.pci_segment, + }) } else { let virtio_net_device = if let Some(ref tap_if_name) = net_cfg.tap { Arc::new(Mutex::new( @@ -2294,26 +2253,18 @@ impl DeviceManager { .unwrap() .insert(id.clone(), device_node!(id, virtio_net_device)); - Ok(( - Arc::clone(&virtio_net_device) as Arc>, - net_cfg.iommu, + Ok(MetaVirtioDevice { + virtio_device: Arc::clone(&virtio_net_device) + as Arc>, + iommu: net_cfg.iommu, id, - net_cfg.pci_segment, - )) + pci_segment: net_cfg.pci_segment, + }) } } /// Add virto-net and vhost-user-net devices - fn make_virtio_net_devices( - &mut self, - ) -> DeviceManagerResult< - Vec<( - Arc>, - bool, - String, - u16, - )>, - > { + fn make_virtio_net_devices(&mut self) -> DeviceManagerResult> { let mut devices = Vec::new(); let mut net_devices = self.config.lock().unwrap().net.clone(); if let Some(net_list_cfg) = &mut net_devices { @@ -2326,16 +2277,7 @@ impl DeviceManager { Ok(devices) } - fn make_virtio_rng_devices( - &mut self, - ) -> DeviceManagerResult< - Vec<( - Arc>, - bool, - String, - u16, - )>, - > { + fn make_virtio_rng_devices(&mut self) -> DeviceManagerResult> { let mut devices = Vec::new(); // Add virtio-rng if required @@ -2356,12 +2298,13 @@ impl DeviceManager { ) .map_err(DeviceManagerError::CreateVirtioRng)?, )); - devices.push(( - Arc::clone(&virtio_rng_device) as Arc>, - rng_config.iommu, - id.clone(), - 0, - )); + devices.push(MetaVirtioDevice { + virtio_device: Arc::clone(&virtio_rng_device) + as Arc>, + iommu: rng_config.iommu, + id: id.clone(), + pci_segment: 0, + }); // Fill the device tree with a new node. In case of restore, we // know there is nothing to do, so we can simply override the @@ -2378,12 +2321,7 @@ impl DeviceManager { fn make_virtio_fs_device( &mut self, fs_cfg: &mut FsConfig, - ) -> DeviceManagerResult<( - Arc>, - bool, - String, - u16, - )> { + ) -> DeviceManagerResult { let id = if let Some(id) = &fs_cfg.id { id.clone() } else { @@ -2517,27 +2455,19 @@ impl DeviceManager { node.migratable = Some(Arc::clone(&virtio_fs_device) as Arc>); self.device_tree.lock().unwrap().insert(id.clone(), node); - Ok(( - Arc::clone(&virtio_fs_device) as Arc>, - false, + Ok(MetaVirtioDevice { + virtio_device: Arc::clone(&virtio_fs_device) + as Arc>, + iommu: false, id, - fs_cfg.pci_segment, - )) + pci_segment: fs_cfg.pci_segment, + }) } else { Err(DeviceManagerError::NoVirtioFsSock) } } - fn make_virtio_fs_devices( - &mut self, - ) -> DeviceManagerResult< - Vec<( - Arc>, - bool, - String, - u16, - )>, - > { + fn make_virtio_fs_devices(&mut self) -> DeviceManagerResult> { let mut devices = Vec::new(); let mut fs_devices = self.config.lock().unwrap().fs.clone(); @@ -2554,12 +2484,7 @@ impl DeviceManager { fn make_virtio_pmem_device( &mut self, pmem_cfg: &mut PmemConfig, - ) -> DeviceManagerResult<( - Arc>, - bool, - String, - u16, - )> { + ) -> DeviceManagerResult { let id = if let Some(id) = &pmem_cfg.id { id.clone() } else { @@ -2723,24 +2648,16 @@ impl DeviceManager { node.migratable = Some(Arc::clone(&virtio_pmem_device) as Arc>); self.device_tree.lock().unwrap().insert(id.clone(), node); - Ok(( - Arc::clone(&virtio_pmem_device) as Arc>, - pmem_cfg.iommu, + Ok(MetaVirtioDevice { + virtio_device: Arc::clone(&virtio_pmem_device) + as Arc>, + iommu: pmem_cfg.iommu, id, - pmem_cfg.pci_segment, - )) + pci_segment: pmem_cfg.pci_segment, + }) } - fn make_virtio_pmem_devices( - &mut self, - ) -> DeviceManagerResult< - Vec<( - Arc>, - bool, - String, - u16, - )>, - > { + fn make_virtio_pmem_devices(&mut self) -> DeviceManagerResult> { let mut devices = Vec::new(); // Add virtio-pmem if required let mut pmem_devices = self.config.lock().unwrap().pmem.clone(); @@ -2757,12 +2674,7 @@ impl DeviceManager { fn make_virtio_vsock_device( &mut self, vsock_cfg: &mut VsockConfig, - ) -> DeviceManagerResult<( - Arc>, - bool, - String, - u16, - )> { + ) -> DeviceManagerResult { let id = if let Some(id) = &vsock_cfg.id { id.clone() } else { @@ -2804,24 +2716,16 @@ impl DeviceManager { .unwrap() .insert(id.clone(), device_node!(id, vsock_device)); - Ok(( - Arc::clone(&vsock_device) as Arc>, - vsock_cfg.iommu, + Ok(MetaVirtioDevice { + virtio_device: Arc::clone(&vsock_device) + as Arc>, + iommu: vsock_cfg.iommu, id, - vsock_cfg.pci_segment, - )) + pci_segment: vsock_cfg.pci_segment, + }) } - fn make_virtio_vsock_devices( - &mut self, - ) -> DeviceManagerResult< - Vec<( - Arc>, - bool, - String, - u16, - )>, - > { + fn make_virtio_vsock_devices(&mut self) -> DeviceManagerResult> { let mut devices = Vec::new(); let mut vsock = self.config.lock().unwrap().vsock.clone(); @@ -2833,16 +2737,7 @@ impl DeviceManager { Ok(devices) } - fn make_virtio_mem_devices( - &mut self, - ) -> DeviceManagerResult< - Vec<( - Arc>, - bool, - String, - u16, - )>, - > { + fn make_virtio_mem_devices(&mut self) -> DeviceManagerResult> { let mut devices = Vec::new(); let mm = self.memory_manager.clone(); @@ -2879,12 +2774,13 @@ impl DeviceManager { self.virtio_mem_devices.push(Arc::clone(&virtio_mem_device)); - devices.push(( - Arc::clone(&virtio_mem_device) as Arc>, - false, - memory_zone_id.clone(), - 0, - )); + devices.push(MetaVirtioDevice { + virtio_device: Arc::clone(&virtio_mem_device) + as Arc>, + iommu: false, + id: memory_zone_id.clone(), + pci_segment: 0, + }); // Fill the device tree with a new node. In case of restore, we // know there is nothing to do, so we can simply override the @@ -2899,16 +2795,7 @@ impl DeviceManager { Ok(devices) } - fn make_virtio_balloon_devices( - &mut self, - ) -> DeviceManagerResult< - Vec<( - Arc>, - bool, - String, - u16, - )>, - > { + fn make_virtio_balloon_devices(&mut self) -> DeviceManagerResult> { let mut devices = Vec::new(); if let Some(balloon_config) = &self.config.lock().unwrap().balloon { @@ -2931,12 +2818,13 @@ impl DeviceManager { self.balloon = Some(virtio_balloon_device.clone()); - devices.push(( - Arc::clone(&virtio_balloon_device) as Arc>, - false, - id.clone(), - 0, - )); + devices.push(MetaVirtioDevice { + virtio_device: Arc::clone(&virtio_balloon_device) + as Arc>, + iommu: false, + id: id.clone(), + pci_segment: 0, + }); self.device_tree .lock() @@ -2947,16 +2835,7 @@ impl DeviceManager { Ok(devices) } - fn make_virtio_watchdog_devices( - &mut self, - ) -> DeviceManagerResult< - Vec<( - Arc>, - bool, - String, - u16, - )>, - > { + fn make_virtio_watchdog_devices(&mut self) -> DeviceManagerResult> { let mut devices = Vec::new(); if !self.config.lock().unwrap().watchdog { @@ -2977,12 +2856,13 @@ impl DeviceManager { ) .map_err(DeviceManagerError::CreateVirtioWatchdog)?, )); - devices.push(( - Arc::clone(&virtio_watchdog_device) as Arc>, - false, - id.clone(), - 0, - )); + devices.push(MetaVirtioDevice { + virtio_device: Arc::clone(&virtio_watchdog_device) + as Arc>, + iommu: false, + id: id.clone(), + pci_segment: 0, + }); self.device_tree .lock() @@ -3569,8 +3449,9 @@ impl DeviceManager { } pub fn update_memory(&self, new_region: &Arc) -> DeviceManagerResult<()> { - for (virtio_device, _, _, _) in self.virtio_devices.iter() { - virtio_device + for handle in self.virtio_devices.iter() { + handle + .virtio_device .lock() .unwrap() .add_memory_region(new_region) @@ -3865,7 +3746,7 @@ impl DeviceManager { virtio_device.lock().unwrap().shutdown(); self.virtio_devices - .retain(|(d, _, _, _)| !Arc::ptr_eq(d, &virtio_device)); + .retain(|handler| !Arc::ptr_eq(&handler.virtio_device, &virtio_device)); } // At this point, the device has been removed from all the list and @@ -3877,64 +3758,62 @@ impl DeviceManager { fn hotplug_virtio_pci_device( &mut self, - device: Arc>, - iommu_attached: bool, - id: String, - pci_segment_id: u16, + handle: MetaVirtioDevice, ) -> DeviceManagerResult { - if iommu_attached { + if handle.iommu { warn!("Placing device behind vIOMMU is not available for hotplugged devices"); } // Add the virtio device to the device manager list. This is important // as the list is used to notify virtio devices about memory updates // for instance. - self.virtio_devices - .push((device.clone(), iommu_attached, id.clone(), pci_segment_id)); + self.virtio_devices.push(handle.clone()); - let bdf = self.add_virtio_pci_device(device, &None, id.clone(), pci_segment_id)?; + let bdf = self.add_virtio_pci_device( + handle.virtio_device, + &None, + handle.id.clone(), + handle.pci_segment, + )?; // Update the PCIU bitmap - self.pci_segments[pci_segment_id as usize].pci_devices_up |= 1 << bdf.device(); + self.pci_segments[handle.pci_segment as usize].pci_devices_up |= 1 << bdf.device(); - Ok(PciDeviceInfo { id, bdf }) + Ok(PciDeviceInfo { id: handle.id, bdf }) } pub fn add_disk(&mut self, disk_cfg: &mut DiskConfig) -> DeviceManagerResult { - let (device, iommu_attached, id, pci_segment_id) = - self.make_virtio_block_device(disk_cfg)?; - self.hotplug_virtio_pci_device(device, iommu_attached, id, pci_segment_id) + let device = self.make_virtio_block_device(disk_cfg)?; + self.hotplug_virtio_pci_device(device) } pub fn add_fs(&mut self, fs_cfg: &mut FsConfig) -> DeviceManagerResult { - let (device, iommu_attached, id, pci_segment_id) = self.make_virtio_fs_device(fs_cfg)?; - self.hotplug_virtio_pci_device(device, iommu_attached, id, pci_segment_id) + let device = self.make_virtio_fs_device(fs_cfg)?; + self.hotplug_virtio_pci_device(device) } pub fn add_pmem(&mut self, pmem_cfg: &mut PmemConfig) -> DeviceManagerResult { - let (device, iommu_attached, id, pci_segment_id) = - self.make_virtio_pmem_device(pmem_cfg)?; - self.hotplug_virtio_pci_device(device, iommu_attached, id, pci_segment_id) + let device = self.make_virtio_pmem_device(pmem_cfg)?; + self.hotplug_virtio_pci_device(device) } pub fn add_net(&mut self, net_cfg: &mut NetConfig) -> DeviceManagerResult { - let (device, iommu_attached, id, pci_segment_id) = self.make_virtio_net_device(net_cfg)?; - self.hotplug_virtio_pci_device(device, iommu_attached, id, pci_segment_id) + let device = self.make_virtio_net_device(net_cfg)?; + self.hotplug_virtio_pci_device(device) } pub fn add_vsock(&mut self, vsock_cfg: &mut VsockConfig) -> DeviceManagerResult { - let (device, iommu_attached, id, pci_segment_id) = - self.make_virtio_vsock_device(vsock_cfg)?; - self.hotplug_virtio_pci_device(device, iommu_attached, id, pci_segment_id) + let device = self.make_virtio_vsock_device(vsock_cfg)?; + self.hotplug_virtio_pci_device(device) } pub fn counters(&self) -> HashMap>> { let mut counters = HashMap::new(); - for (virtio_device, _, id, _) in &self.virtio_devices { - let virtio_device = virtio_device.lock().unwrap(); + for handle in &self.virtio_devices { + let virtio_device = handle.virtio_device.lock().unwrap(); if let Some(device_counters) = virtio_device.counters() { - counters.insert(id.clone(), device_counters.clone()); + counters.insert(handle.id.clone(), device_counters.clone()); } } @@ -4491,8 +4370,8 @@ impl BusDevice for DeviceManager { impl Drop for DeviceManager { fn drop(&mut self) { - for (device, _, _, _) in self.virtio_devices.drain(..) { - device.lock().unwrap().shutdown(); + for handle in self.virtio_devices.drain(..) { + handle.virtio_device.lock().unwrap().shutdown(); } } }