feat: updated BlockDeviceConfig with vhost-user-block parameters

Updated `BlockDeviceConfig` with vhost-user-block specific field.
Updated methods to convert `BlockDeviceConfig` into
`VirtIOBlockConfig` or `VhostUserBlockConfig`.
Updated integration tests with new ordering of fields in the
`BlockDeviceConfig`.

Signed-off-by: Egor Lazarchuk <[email protected]>

Author: ShadowCurse

src/vmm/src/builder.rs

@@ -1122,24 +1122,31 @@ pub mod tests {
     ) -> Vec<TempFile> {
         let mut block_dev_configs = BlockBuilder::new();
         let mut block_files = Vec::new();
-        for custom_block_cfg in &custom_block_cfgs {
+        for custom_block_cfg in custom_block_cfgs {
             block_files.push(TempFile::new().unwrap());
+
             let block_device_config = BlockDeviceConfig {
                 drive_id: String::from(&custom_block_cfg.drive_id),
-                path_on_host: block_files
-                    .last()
-                    .unwrap()
-                    .as_path()
-                    .to_str()
-                    .unwrap()
-                    .to_string(),
+                partuuid: custom_block_cfg.partuuid,
                 is_root_device: custom_block_cfg.is_root_device,
-                partuuid: custom_block_cfg.partuuid.clone(),
-                is_read_only: custom_block_cfg.is_read_only,
                 cache_type: custom_block_cfg.cache_type,
+
+                is_read_only: Some(custom_block_cfg.is_read_only),
+                path_on_host: Some(
+                    block_files
+                        .last()
+                        .unwrap()
+                        .as_path()
+                        .to_str()
+                        .unwrap()
+                        .to_string(),
+                ),
                 rate_limiter: None,
                 file_engine_type: FileEngineType::default(),
+
+                socket: None,
             };
+
             block_dev_configs.insert(block_device_config).unwrap();
         }
 

src/vmm/src/device_manager/persist.rs

@@ -869,13 +869,14 @@ mod tests {
   "drives": [
     {{
       "drive_id": "root",
-      "path_on_host": "{}",
-      "is_root_device": true,
       "partuuid": null,
-      "is_read_only": true,
+      "is_root_device": true,
       "cache_type": "Unsafe",
+      "is_read_only": true,
+      "path_on_host": "{}",
       "rate_limiter": null,
-      "io_engine": "Sync"
+      "io_engine": "Sync",
+      "socket": null
     }}
   ],
   "boot-source": {{

src/vmm/src/devices/virtio/block/device.rs

@@ -53,20 +53,15 @@ pub enum CacheType {
 }
 
 /// The engine file type, either Sync or Async (through io_uring).
-#[derive(Debug, PartialEq, Eq, Deserialize, Serialize)]
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)]
 pub enum FileEngineType {
     /// Use an Async engine, based on io_uring.
     Async,
     /// Use a Sync engine, based on blocking system calls.
+    #[default]
     Sync,
 }
 
-impl Default for FileEngineType {
-    fn default() -> Self {
-        Self::Sync
-    }
-}
-
 impl FileEngineType {
     /// Whether the Async engine is supported on the current host kernel.
     pub fn is_supported(&self) -> Result<bool, utils::kernel_version::Error> {
@@ -203,22 +198,23 @@ impl DiskProperties {
 pub struct VirtioBlockConfig {
     /// Unique identifier of the drive.
     pub drive_id: String,
-    /// Path of the backing file on the host
-    pub path_on_host: String,
+    /// Part-UUID. Represents the unique id of the boot partition of this device. It is
+    /// optional and it will be used only if the `is_root_device` field is true.
+    pub partuuid: Option<String>,
     /// If set to true, it makes the current device the root block device.
     /// Setting this flag to true will mount the block device in the
     /// guest under /dev/vda unless the partuuid is present.
     pub is_root_device: bool,
-    /// Part-UUID. Represents the unique id of the boot partition of this device. It is
-    /// optional and it will be used only if the `is_root_device` field is true.
-    pub partuuid: Option<String>,
-    /// If set to true, the drive is opened in read-only mode. Otherwise, the
-    /// drive is opened as read-write.
-    pub is_read_only: bool,
     /// If set to true, the drive will ignore flush requests coming from
     /// the guest driver.
     #[serde(default)]
     pub cache_type: CacheType,
+
+    /// If set to true, the drive is opened in read-only mode. Otherwise, the
+    /// drive is opened as read-write.
+    pub is_read_only: bool,
+    /// Path of the backing file on the host
+    pub path_on_host: String,
     /// Rate Limiter for I/O operations.
     pub rate_limiter: Option<RateLimiterConfig>,
     /// The type of IO engine used by the device.
@@ -227,17 +223,24 @@ pub struct VirtioBlockConfig {
     pub file_engine_type: FileEngineType,
 }
 
-impl From<BlockDeviceConfig> for VirtioBlockConfig {
-    fn from(value: BlockDeviceConfig) -> Self {
-        Self {
-            drive_id: value.drive_id,
-            path_on_host: value.path_on_host,
-            is_root_device: value.is_root_device,
-            partuuid: value.partuuid,
-            is_read_only: value.is_read_only,
-            cache_type: value.cache_type,
-            rate_limiter: value.rate_limiter,
-            file_engine_type: value.file_engine_type,
+impl TryFrom<&BlockDeviceConfig> for VirtioBlockConfig {
+    type Error = VirtioBlockError;
+
+    fn try_from(value: &BlockDeviceConfig) -> Result<Self, Self::Error> {
+        if value.path_on_host.is_some() && value.socket.is_none() {
+            Ok(Self {
+                drive_id: value.drive_id.clone(),
+                partuuid: value.partuuid.clone(),
+                is_root_device: value.is_root_device,
+                cache_type: value.cache_type,
+
+                is_read_only: value.is_read_only.unwrap_or(false),
+                path_on_host: value.path_on_host.as_ref().unwrap().clone(),
+                rate_limiter: value.rate_limiter,
+                file_engine_type: value.file_engine_type,
+            })
+        } else {
+            Err(VirtioBlockError::Config)
         }
     }
 }
@@ -246,13 +249,16 @@ impl From<VirtioBlockConfig> for BlockDeviceConfig {
     fn from(value: VirtioBlockConfig) -> Self {
         Self {
             drive_id: value.drive_id,
-            path_on_host: value.path_on_host,
-            is_root_device: value.is_root_device,
             partuuid: value.partuuid,
-            is_read_only: value.is_read_only,
+            is_root_device: value.is_root_device,
             cache_type: value.cache_type,
+
+            is_read_only: Some(value.is_read_only),
+            path_on_host: Some(value.path_on_host),
             rate_limiter: value.rate_limiter,
             file_engine_type: value.file_engine_type,
+
+            socket: None,
         }
     }
 }
@@ -363,7 +369,7 @@ impl VirtioBlock {
         }
     }
 
-    /// Process a single event in the VirtIO queue.
+    /// Process a single event in the Virtio queue.
     ///
     /// This function is called by the event manager when the guest notifies us
     /// about new buffers in the queue.
@@ -747,6 +753,54 @@ mod tests {
     use crate::rate_limiter::TokenType;
     use crate::vstate::memory::{Address, Bytes, GuestAddress};
 
+    #[test]
+    fn test_from_config() {
+        let block_config = BlockDeviceConfig {
+            drive_id: "".to_string(),
+            partuuid: None,
+            is_root_device: false,
+            cache_type: CacheType::Unsafe,
+
+            is_read_only: Some(true),
+            path_on_host: Some("path".to_string()),
+            rate_limiter: None,
+            file_engine_type: Default::default(),
+
+            socket: None,
+        };
+        assert!(VirtioBlockConfig::try_from(&block_config).is_ok());
+
+        let block_config = BlockDeviceConfig {
+            drive_id: "".to_string(),
+            partuuid: None,
+            is_root_device: false,
+            cache_type: CacheType::Unsafe,
+
+            is_read_only: None,
+            path_on_host: None,
+            rate_limiter: None,
+            file_engine_type: Default::default(),
+
+            socket: Some("sock".to_string()),
+        };
+        assert!(VirtioBlockConfig::try_from(&block_config).is_err());
+
+        let block_config = BlockDeviceConfig {
+            drive_id: "".to_string(),
+            partuuid: None,
+            is_root_device: false,
+            cache_type: CacheType::Unsafe,
+
+            is_read_only: Some(true),
+            path_on_host: Some("path".to_string()),
+            rate_limiter: None,
+            file_engine_type: Default::default(),
+
+            socket: Some("sock".to_string()),
+        };
+        assert!(VirtioBlockConfig::try_from(&block_config).is_err());
+    }
+
     #[test]
     fn test_disk_backing_file_helper() {
         let num_sectors = 2;

src/vmm/src/devices/virtio/block/mod.rs

@@ -35,6 +35,8 @@ pub const IO_URING_NUM_ENTRIES: u16 = 128;
 /// Errors the block device can trigger.
 #[derive(Debug)]
 pub enum VirtioBlockError {
+    /// Can not create config
+    Config,
     /// Guest gave us too few descriptors in a descriptor chain.
     DescriptorChainTooShort,
     /// Guest gave us a descriptor that was too short to use.

src/vmm/src/devices/virtio/vhost_user_block/device.rs

@@ -16,7 +16,6 @@ use vhost::vhost_user::message::*;
 use vhost::vhost_user::VhostUserFrontend;
 
 use super::{VhostUserBlockError, NUM_QUEUES, QUEUE_SIZE};
-use crate::devices::virtio::block::device::FileEngineType;
 use crate::devices::virtio::gen::virtio_blk::{
     VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_RO, VIRTIO_F_VERSION_1,
 };
@@ -47,18 +46,30 @@ pub struct VhostUserBlockConfig {
     /// If set to true, the drive will ignore flush requests coming from
     /// the guest driver.
     pub cache_type: CacheType,
+
     /// Socket path of the vhost-user process
     pub socket: String,
 }
 
-impl From<BlockDeviceConfig> for VhostUserBlockConfig {
-    fn from(value: BlockDeviceConfig) -> Self {
-        Self {
-            drive_id: value.drive_id,
-            is_root_device: value.is_root_device,
-            partuuid: value.partuuid,
-            cache_type: value.cache_type,
-            socket: Default::default(),
+impl TryFrom<&BlockDeviceConfig> for VhostUserBlockConfig {
+    type Error = VhostUserBlockError;
+
+    fn try_from(value: &BlockDeviceConfig) -> Result<Self, Self::Error> {
+        if value.socket.is_some()
+            && value.is_read_only.is_none()
+            && value.path_on_host.is_none()
+            && value.rate_limiter.is_none()
+        {
+            Ok(Self {
+                drive_id: value.drive_id.clone(),
+                partuuid: value.partuuid.clone(),
+                is_root_device: value.is_root_device,
+                cache_type: value.cache_type,
+
+                socket: value.socket.as_ref().unwrap().clone(),
+            })
+        } else {
+            Err(VhostUserBlockError::Config)
         }
     }
 }
@@ -67,13 +78,16 @@ impl From<VhostUserBlockConfig> for BlockDeviceConfig {
     fn from(value: VhostUserBlockConfig) -> Self {
         Self {
             drive_id: value.drive_id,
-            path_on_host: Default::default(),
-            is_root_device: value.is_root_device,
             partuuid: value.partuuid,
-            is_read_only: false,
+            is_root_device: value.is_root_device,
             cache_type: value.cache_type,
+
+            is_read_only: None,
+            path_on_host: None,
             rate_limiter: None,
-            file_engine_type: FileEngineType::default(),
+            file_engine_type: Default::default(),
+
+            socket: Some(value.socket),
         }
     }
 }
@@ -280,3 +294,56 @@ impl VirtioDevice for VhostUserBlock {
         self.device_state.is_activated()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_from_config() {
+        let block_config = BlockDeviceConfig {
+            drive_id: "".to_string(),
+            partuuid: None,
+            is_root_device: false,
+            cache_type: CacheType::Unsafe,
+
+            is_read_only: None,
+            path_on_host: None,
+            rate_limiter: None,
+            file_engine_type: Default::default(),
+
+            socket: Some("sock".to_string()),
+        };
+        assert!(VhostUserBlockConfig::try_from(&block_config).is_ok());
+
+        let block_config = BlockDeviceConfig {
+            drive_id: "".to_string(),
+            partuuid: None,
+            is_root_device: false,
+            cache_type: CacheType::Unsafe,
+
+            is_read_only: Some(true),
+            path_on_host: Some("path".to_string()),
+            rate_limiter: None,
+            file_engine_type: Default::default(),
+
+            socket: None,
+        };
+        assert!(VhostUserBlockConfig::try_from(&block_config).is_err());
+
+        let block_config = BlockDeviceConfig {
+            drive_id: "".to_string(),
+            partuuid: None,
+            is_root_device: false,
+            cache_type: CacheType::Unsafe,
+
+            is_read_only: Some(true),
+            path_on_host: Some("path".to_string()),
+            rate_limiter: None,
+            file_engine_type: Default::default(),
+
+            socket: Some("sock".to_string()),
+        };
+        assert!(VhostUserBlockConfig::try_from(&block_config).is_err());
+    }
+}

src/vmm/src/devices/virtio/vhost_user_block/mod.rs

@@ -17,6 +17,8 @@ pub const QUEUE_SIZE: u16 = 256;
 /// Vhost-user block device error.
 #[derive(Debug, displaydoc::Display)]
 pub enum VhostUserBlockError {
+    /// Cannot create config
+    Config,
     /// Persistence error: {0:?}
     Persist(crate::devices::virtio::persist::PersistError),
     /// Vhost-user error: {0}