fix(vmm): fix patch of pci devices

The vmm was only checking the mmio device manager for finding the device
to update. Use the generic device manager instead.

Also update unit tests that expect a specific string.

Signed-off-by: Riccardo Mancini <[email protected]>
Signed-off-by: Babis Chalios <[email protected]>

Author: Manciukic

src/vmm/src/device_manager/mod.rs

@@ -81,6 +81,17 @@ pub enum AttachDeviceError {
     PciTransport(#[from] PciManagerError),
 }
 
+#[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(String),
+}
+
 #[derive(Debug)]
 /// A manager of all peripheral devices of Firecracker
 pub struct DeviceManager {
@@ -342,6 +353,30 @@ impl DeviceManager {
             )
         }
     }
+
+    /// Run fn `f()` for the virtio device matching `virtio_type` and `id`.
+    pub fn with_virtio_device_with_id<T, F>(
+        &self,
+        virtio_type: u32,
+        id: &str,
+        f: F,
+    ) -> Result<(), FindDeviceError>
+    where
+        T: VirtioDevice + 'static + Debug,
+        F: FnOnce(&mut T) -> Result<(), String>,
+    {
+        if let Some(device) = self.get_virtio_device(virtio_type, id) {
+            let mut dev = device.lock().expect("Poisoned lock");
+            f(dev
+                .as_mut_any()
+                .downcast_mut::<T>()
+                .ok_or(FindDeviceError::InvalidDeviceType)?)
+            .map_err(FindDeviceError::InternalDeviceError)?;
+        } else {
+            return Err(FindDeviceError::DeviceNotFound);
+        }
+        Ok(())
+    }
 }
 
 #[derive(Debug, Default, Clone, Serialize, Deserialize)]

src/vmm/src/lib.rs

@@ -252,6 +252,8 @@ pub enum VmmError {
     VmmObserverTeardown(vmm_sys_util::errno::Error),
     /// VMGenID error: {0}
     VMGenID(#[from] VmGenIdError),
+    /// Failed perform action on device: {0}
+    FindDeviceError(#[from] device_manager::FindDeviceError),
 }
 
 /// Shorthand type for KVM dirty page bitmap.
@@ -535,13 +537,12 @@ impl Vmm {
         path_on_host: String,
     ) -> Result<(), VmmError> {
         self.device_manager
-            .mmio_devices
             .with_virtio_device_with_id(TYPE_BLOCK, drive_id, |block: &mut Block| {
                 block
                     .update_disk_image(path_on_host)
                     .map_err(|err| err.to_string())
             })
-            .map_err(VmmError::MmioDeviceManager)
+            .map_err(VmmError::FindDeviceError)
     }
 
     /// Updates the rate limiter parameters for block device with `drive_id` id.
@@ -552,23 +553,21 @@ impl Vmm {
         rl_ops: BucketUpdate,
     ) -> Result<(), VmmError> {
         self.device_manager
-            .mmio_devices
             .with_virtio_device_with_id(TYPE_BLOCK, drive_id, |block: &mut Block| {
                 block
                     .update_rate_limiter(rl_bytes, rl_ops)
                     .map_err(|err| err.to_string())
             })
-            .map_err(VmmError::MmioDeviceManager)
+            .map_err(VmmError::FindDeviceError)
     }
 
     /// 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
-            .mmio_devices
             .with_virtio_device_with_id(TYPE_BLOCK, drive_id, |block: &mut Block| {
                 block.update_config().map_err(|err| err.to_string())
             })
-            .map_err(VmmError::MmioDeviceManager)
+            .map_err(VmmError::FindDeviceError)
     }
 
     /// Updates the rate limiter parameters for net device with `net_id` id.
@@ -581,12 +580,11 @@ impl Vmm {
         tx_ops: BucketUpdate,
     ) -> Result<(), VmmError> {
         self.device_manager
-            .mmio_devices
             .with_virtio_device_with_id(TYPE_NET, net_id, |net: &mut Net| {
                 net.patch_rate_limiters(rx_bytes, rx_ops, tx_bytes, tx_ops);
                 Ok(())
             })
-            .map_err(VmmError::MmioDeviceManager)
+            .map_err(VmmError::FindDeviceError)
     }
 
     /// Returns a reference to the balloon device if present.

tests/integration_tests/functional/test_api.py

@@ -772,7 +772,7 @@ def test_send_ctrl_alt_del(uvm_plain_any):
 def _drive_patch(test_microvm, io_engine):
     """Exercise drive patch test scenarios."""
     # Patches without mandatory fields for virtio block are not allowed.
-    expected_msg = "Unable to patch the block device: MMIO Device manager error: Running method expected different backend. Please verify the request arguments"
+    expected_msg = "Running method expected different backend."
     with pytest.raises(RuntimeError, match=expected_msg):
         test_microvm.api.drive.patch(drive_id="scratch")
 
@@ -814,7 +814,7 @@ def _drive_patch(test_microvm, io_engine):
         )
 
     # Updates to `path_on_host` with an invalid path are not allowed.
-    expected_msg = f"Unable to patch the block device: MMIO Device manager error: Virtio backend error: Error manipulating the backing file: No such file or directory (os error 2) {drive_path} Please verify the request arguments"
+    expected_msg = f"Error manipulating the backing file: No such file or directory (os error 2) {drive_path}"
     with pytest.raises(RuntimeError, match=re.escape(expected_msg)):
         test_microvm.api.drive.patch(drive_id="scratch", path_on_host=drive_path)