refactor: moved CacheType out of Block to share it with vhost_user_blk

CacheType is shared between VirtioBlock and VhostUserBlock.

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

Author: ShadowCurse

src/vmm/src/builder.rs

@@ -990,14 +990,15 @@ pub mod tests {
 
     use super::*;
     use crate::arch::DeviceType;
+    use crate::devices::virtio::block_common::CacheType;
     use crate::devices::virtio::rng::device::ENTROPY_DEV_ID;
     use crate::devices::virtio::vsock::{TYPE_VSOCK, VSOCK_DEV_ID};
     use crate::devices::virtio::{TYPE_BALLOON, TYPE_BLOCK, TYPE_RNG};
     use crate::mmds::data_store::{Mmds, MmdsVersion};
     use crate::mmds::ns::MmdsNetworkStack;
     use crate::vmm_config::balloon::{BalloonBuilder, BalloonDeviceConfig, BALLOON_DEV_ID};
     use crate::vmm_config::boot_source::DEFAULT_KERNEL_CMDLINE;
-    use crate::vmm_config::drive::{BlockBuilder, BlockDeviceConfig, CacheType, FileEngineType};
+    use crate::vmm_config::drive::{BlockBuilder, BlockDeviceConfig, FileEngineType};
     use crate::vmm_config::entropy::{EntropyDeviceBuilder, EntropyDeviceConfig};
     use crate::vmm_config::net::{NetBuilder, NetworkInterfaceConfig};
     use crate::vmm_config::vsock::tests::default_config;

src/vmm/src/device_manager/persist.rs

@@ -655,7 +655,7 @@ mod tests {
 
     use super::*;
     use crate::builder::tests::*;
-    use crate::devices::virtio::block::CacheType;
+    use crate::devices::virtio::block_common::CacheType;
     use crate::devices::virtio::net::persist::NetConfigSpaceState;
     use crate::resources::VmmConfig;
     use crate::vmm_config::balloon::BalloonDeviceConfig;

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

@@ -20,40 +20,27 @@ use utils::eventfd::EventFd;
 use utils::kernel_version::{min_kernel_version_for_io_uring, KernelVersion};
 use utils::u64_to_usize;
 
-use super::super::device::{DeviceState, VirtioDevice};
-use super::super::queue::Queue;
-use super::super::{ActivateError, TYPE_BLOCK};
 use super::io::async_io;
 use super::request::*;
 use super::{
     io as block_io, VirtioBlockError, BLOCK_CONFIG_SPACE_SIZE, BLOCK_QUEUE_SIZES, SECTOR_SHIFT,
     SECTOR_SIZE,
 };
 use crate::devices::virtio::block::block_metrics::{BlockDeviceMetrics, BlockMetricsPerDevice};
-use crate::devices::virtio::device::{IrqTrigger, IrqType};
+use crate::devices::virtio::block_common::CacheType;
+use crate::devices::virtio::device::{DeviceState, IrqTrigger, IrqType, VirtioDevice};
 use crate::devices::virtio::gen::virtio_blk::{
     VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_RO, VIRTIO_BLK_ID_BYTES, VIRTIO_F_VERSION_1,
 };
 use crate::devices::virtio::gen::virtio_ring::VIRTIO_RING_F_EVENT_IDX;
+use crate::devices::virtio::queue::Queue;
+use crate::devices::virtio::{ActivateError, TYPE_BLOCK};
 use crate::logger::{error, warn, IncMetric};
 use crate::rate_limiter::{BucketUpdate, RateLimiter};
 use crate::vmm_config::drive::BlockDeviceConfig;
 use crate::vmm_config::RateLimiterConfig;
 use crate::vstate::memory::GuestMemoryMmap;
 
-/// Configuration options for disk caching.
-#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Deserialize, Serialize)]
-pub enum CacheType {
-    /// Flushing mechanic will be advertised to the guest driver, but
-    /// the operation will be a noop.
-    #[default]
-    Unsafe,
-    /// Flushing mechanic will be advertised to the guest driver and
-    /// flush requests coming from the guest will be performed using
-    /// `fsync`.
-    Writeback,
-}
-
 /// The engine file type, either Sync or Async (through io_uring).
 #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)]
 pub enum FileEngineType {

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

@@ -13,10 +13,11 @@ pub mod test_utils;
 
 use vm_memory::GuestMemoryError;
 
-pub use self::device::{CacheType, VirtioBlock};
+pub use self::device::VirtioBlock;
 pub use self::event_handler::*;
 pub use self::request::*;
 use super::queue::FIRECRACKER_MAX_QUEUE_SIZE;
+pub use crate::devices::virtio::block_common::CacheType;
 
 /// Size of config space for block device.
 pub const BLOCK_CONFIG_SPACE_SIZE: usize = 8;

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

@@ -24,37 +24,6 @@ use crate::rate_limiter::persist::RateLimiterState;
 use crate::rate_limiter::RateLimiter;
 use crate::vstate::memory::GuestMemoryMmap;
 
-/// Holds info about block's cache type. Gets saved in snapshot.
-// NOTICE: Any changes to this structure require a snapshot version bump.
-#[derive(Clone, Copy, Debug, PartialEq, Eq, Versionize)]
-pub enum CacheTypeState {
-    /// Flushing mechanic will be advertised to the guest driver, but
-    /// the operation will be a noop.
-    Unsafe,
-    /// Flushing mechanic will be advertised to the guest driver and
-    /// flush requests coming from the guest will be performed using
-    /// `fsync`.
-    Writeback,
-}
-
-impl From<CacheType> for CacheTypeState {
-    fn from(cache_type: CacheType) -> Self {
-        match cache_type {
-            CacheType::Unsafe => CacheTypeState::Unsafe,
-            CacheType::Writeback => CacheTypeState::Writeback,
-        }
-    }
-}
-
-impl From<CacheTypeState> for CacheType {
-    fn from(cache_type_state: CacheTypeState) -> Self {
-        match cache_type_state {
-            CacheTypeState::Unsafe => CacheType::Unsafe,
-            CacheTypeState::Writeback => CacheType::Writeback,
-        }
-    }
-}
-
 /// Holds info about block's file engine type. Gets saved in snapshot.
 // NOTICE: Any changes to this structure require a snapshot version bump.
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Versionize)]
@@ -93,7 +62,7 @@ pub struct VirtioBlockState {
     id: String,
     partuuid: Option<String>,
     #[version(start = 2, default_fn = "default_cache_type_flush")]
-    cache_type: CacheTypeState,
+    cache_type: CacheType,
     root_device: bool,
     disk_path: String,
     virtio_state: VirtioDeviceState,
@@ -103,8 +72,8 @@ pub struct VirtioBlockState {
 }
 
 impl VirtioBlockState {
-    fn default_cache_type_flush(_source_version: u16) -> CacheTypeState {
-        CacheTypeState::Unsafe
+    fn default_cache_type_flush(_source_version: u16) -> CacheType {
+        CacheType::Unsafe
     }
 }
 
@@ -125,7 +94,7 @@ impl Persist<'_> for VirtioBlock {
         VirtioBlockState {
             id: self.id.clone(),
             partuuid: self.partuuid.clone(),
-            cache_type: CacheTypeState::from(self.cache_type()),
+            cache_type: self.cache_type(),
             root_device: self.root_device,
             disk_path: self.disk.file_path().clone(),
             virtio_state: VirtioDeviceState::from_device(self),
@@ -223,36 +192,6 @@ mod tests {
     use crate::devices::virtio::device::VirtioDevice;
     use crate::devices::virtio::test_utils::default_mem;
 
-    #[test]
-    fn test_cache_type_state_from() {
-        assert_eq!(
-            CacheTypeState::Unsafe,
-            CacheTypeState::from(CacheType::Unsafe)
-        );
-        assert_eq!(
-            CacheTypeState::Writeback,
-            CacheTypeState::from(CacheType::Writeback)
-        );
-    }
-
-    #[test]
-    fn test_cache_type_state_into() {
-        assert_eq!(CacheType::Unsafe, CacheTypeState::Unsafe.into());
-        assert_eq!(CacheType::Writeback, CacheTypeState::Writeback.into());
-    }
-
-    #[test]
-    fn test_default_cache_type_flush() {
-        assert_eq!(
-            VirtioBlockState::default_cache_type_flush(2),
-            CacheTypeState::Unsafe
-        );
-        assert_eq!(
-            VirtioBlockState::default_cache_type_flush(3),
-            CacheTypeState::Unsafe
-        );
-    }
-
     #[test]
     fn test_cache_semantic_ser() {
         // We create the backing file here so that it exists for the whole lifetime of the test.

src/vmm/src/devices/virtio/block_common.rs

@@ -0,0 +1,19 @@
+// Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+use serde::{Deserialize, Serialize};
+use versionize::{VersionMap, Versionize, VersionizeError, VersionizeResult};
+use versionize_derive::Versionize;
+
+/// Configuration options for disk caching.
+// NOTICE: Any changes to this structure require a snapshot version bump.
+#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Deserialize, Serialize, Versionize)]
+pub enum CacheType {
+    /// Flushing mechanic not will be advertised to the guest driver
+    #[default]
+    Unsafe,
+    /// Flushing mechanic will be advertised to the guest driver and
+    /// flush requests coming from the guest will be performed using
+    /// `fsync`.
+    Writeback,
+}

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

@@ -12,6 +12,7 @@ use std::io::Error as IOError;
 
 pub mod balloon;
 pub mod block;
+pub mod block_common;
 pub mod device;
 pub mod gen;
 pub mod iovec;

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

@@ -16,7 +16,7 @@ use vhost::vhost_user::message::*;
 use vhost::vhost_user::VhostUserFrontend;
 
 use super::{VhostUserBlockError, NUM_QUEUES, QUEUE_SIZE};
-use crate::devices::virtio::block::CacheType;
+use crate::devices::virtio::block_common::CacheType;
 use crate::devices::virtio::device::{DeviceState, IrqTrigger, VirtioDevice};
 use crate::devices::virtio::gen::virtio_blk::{
     VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_RO, VIRTIO_F_VERSION_1,

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

@@ -9,7 +9,7 @@ use versionize_derive::Versionize;
 
 use super::device::VhostUserBlock;
 use super::VhostUserBlockError;
-use crate::devices::virtio::block::persist::CacheTypeState;
+use crate::devices::virtio::block_common::CacheType;
 use crate::devices::virtio::persist::VirtioDeviceState;
 use crate::vstate::memory::GuestMemoryMmap;
 
@@ -19,7 +19,7 @@ use crate::vstate::memory::GuestMemoryMmap;
 pub struct VhostUserBlockState {
     id: String,
     partuuid: Option<String>,
-    cache_type: CacheTypeState,
+    cache_type: CacheType,
     root_device: bool,
     socket_path: String,
     vu_acked_protocol_features: u64,

src/vmm/src/persist.rs

@@ -588,8 +588,8 @@ mod tests {
     };
     #[cfg(target_arch = "aarch64")]
     use crate::construct_kvm_mpidrs;
+    use crate::devices::virtio::block_common::CacheType;
     use crate::vmm_config::balloon::BalloonDeviceConfig;
-    use crate::vmm_config::drive::CacheType;
     use crate::vmm_config::net::NetworkInterfaceConfig;
     use crate::vmm_config::vsock::tests::default_config;
     use crate::Vmm;