block: removed virtio flush feature

Signed-off-by: George Pisaltu <[email protected]>

Author: georgepisaltu

CHANGELOG.md

@@ -6,6 +6,7 @@
 
 - Changed Docker images repository from DockerHub to Amazon ECR.
 - Fixed off-by-one error in virtio-block descriptor address validation.
+- Removed the `flush` capability from the block device
 
 ## [0.24.0]
 

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

@@ -162,7 +162,7 @@ impl Block {
     ) -> io::Result<Block> {
         let disk_properties = DiskProperties::new(disk_image_path, is_disk_read_only)?;
 
-        let mut avail_features = (1u64 << VIRTIO_F_VERSION_1) | (1u64 << VIRTIO_BLK_F_FLUSH);
+        let mut avail_features = 1u64 << VIRTIO_F_VERSION_1;
 
         if is_disk_read_only {
             avail_features |= 1u64 << VIRTIO_BLK_F_RO;
@@ -513,7 +513,7 @@ pub(crate) mod tests {
 
         assert_eq!(block.device_type(), TYPE_BLOCK);
 
-        let features: u64 = (1u64 << VIRTIO_F_VERSION_1) | (1u64 << VIRTIO_BLK_F_FLUSH);
+        let features: u64 = 1u64 << VIRTIO_F_VERSION_1;
 
         assert_eq!(block.avail_features_by_page(0), features as u32);
         assert_eq!(block.avail_features_by_page(1), (features >> 32) as u32);
@@ -708,7 +708,7 @@ pub(crate) mod tests {
         let status_addr = GuestAddress(vq.dtable[2].addr.get());
 
         // Currently only VIRTIO_BLK_T_IN, VIRTIO_BLK_T_OUT,
-        // VIRTIO_BLK_T_FLUSH and VIRTIO_BLK_T_GET_ID  are supported.
+        // VIRTIO_BLK_T_GET_ID  are supported.
         // Generate an unsupported request.
         let request_header = RequestHeader::new(42, 0);
         mem.write_obj::<RequestHeader>(request_header, request_type_addr)
@@ -907,50 +907,6 @@ pub(crate) mod tests {
         }
     }
 
-    #[test]
-    fn test_flush() {
-        let mut block = default_block();
-        let mem = default_mem();
-        let vq = VirtQueue::new(GuestAddress(0), &mem, 16);
-        set_queue(&mut block, 0, vq.create_queue());
-        block.activate(mem.clone()).unwrap();
-        initialize_virtqueue(&vq);
-
-        let request_type_addr = GuestAddress(vq.dtable[0].addr.get());
-        let status_addr = GuestAddress(vq.dtable[2].addr.get());
-
-        // Flush completes successfully without a data descriptor.
-        {
-            vq.dtable[0].next.set(2);
-
-            mem.write_obj::<u32>(VIRTIO_BLK_T_FLUSH, request_type_addr)
-                .unwrap();
-
-            invoke_handler_for_queue_event(&mut block);
-            assert_eq!(vq.used.idx.get(), 1);
-            assert_eq!(vq.used.ring[0].get().id, 0);
-            assert_eq!(vq.used.ring[0].get().len, 1);
-            assert_eq!(mem.read_obj::<u32>(status_addr).unwrap(), VIRTIO_BLK_S_OK);
-        }
-
-        // Flush completes successfully even with a data descriptor.
-        {
-            vq.used.idx.set(0);
-            set_queue(&mut block, 0, vq.create_queue());
-            vq.dtable[0].next.set(1);
-
-            mem.write_obj::<u32>(VIRTIO_BLK_T_FLUSH, request_type_addr)
-                .unwrap();
-
-            invoke_handler_for_queue_event(&mut block);
-            assert_eq!(vq.used.idx.get(), 1);
-            assert_eq!(vq.used.ring[0].get().id, 0);
-            // status byte length.
-            assert_eq!(vq.used.ring[0].get().len, 1);
-            assert_eq!(mem.read_obj::<u32>(status_addr).unwrap(), VIRTIO_BLK_S_OK);
-        }
-    }
-
     #[test]
     fn test_get_device_id() {
         let mut block = default_block();

src/devices/src/virtio/block/request.rs

@@ -6,7 +6,7 @@
 // found in the THIRD-PARTY file.
 
 use std::convert::From;
-use std::io::{self, Seek, SeekFrom, Write};
+use std::io::{self, Seek, SeekFrom};
 use std::result;
 
 use logger::{IncMetric, METRICS};
@@ -20,7 +20,6 @@ use super::{Error, SECTOR_SHIFT, SECTOR_SIZE};
 #[derive(Debug)]
 pub enum ExecuteError {
     BadRequest(Error),
-    Flush(io::Error),
     Read(GuestMemoryError),
     Seek(io::Error),
     Write(GuestMemoryError),
@@ -31,7 +30,6 @@ impl ExecuteError {
     pub fn status(&self) -> u32 {
         match *self {
             ExecuteError::BadRequest(_) => VIRTIO_BLK_S_IOERR,
-            ExecuteError::Flush(_) => VIRTIO_BLK_S_IOERR,
             ExecuteError::Read(_) => VIRTIO_BLK_S_IOERR,
             ExecuteError::Seek(_) => VIRTIO_BLK_S_IOERR,
             ExecuteError::Write(_) => VIRTIO_BLK_S_IOERR,
@@ -44,7 +42,6 @@ impl ExecuteError {
 pub enum RequestType {
     In,
     Out,
-    Flush,
     GetDeviceID,
     Unsupported(u32),
 }
@@ -54,7 +51,6 @@ impl From<u32> for RequestType {
         match value {
             VIRTIO_BLK_T_IN => RequestType::In,
             VIRTIO_BLK_T_OUT => RequestType::Out,
-            VIRTIO_BLK_T_FLUSH => RequestType::Flush,
             VIRTIO_BLK_T_GET_ID => RequestType::GetDeviceID,
             t => RequestType::Unsupported(t),
         }
@@ -137,11 +133,7 @@ impl Request {
             .ok_or(Error::DescriptorChainTooShort)?;
 
         if !desc.has_next() {
-            status_desc = desc;
-            // Only flush requests are allowed to skip the data descriptor.
-            if req.request_type != RequestType::Flush {
-                return Err(Error::DescriptorChainTooShort);
-            }
+            return Err(Error::DescriptorChainTooShort);
         } else {
             data_desc = desc;
             status_desc = data_desc
@@ -214,13 +206,6 @@ impl Request {
                     0
                 })
                 .map_err(ExecuteError::Write),
-            RequestType::Flush => diskfile
-                .flush()
-                .map(|_| {
-                    METRICS.block.flush_count.inc();
-                    0
-                })
-                .map_err(ExecuteError::Flush),
             RequestType::GetDeviceID => {
                 let disk_id = disk.image_id();
                 if (self.data_len as usize) < disk_id.len() {
@@ -253,7 +238,6 @@ mod tests {
         let supported_request_types = vec![
             VIRTIO_BLK_T_IN,
             VIRTIO_BLK_T_OUT,
-            VIRTIO_BLK_T_FLUSH,
             VIRTIO_BLK_T_GET_ID,
         ];
 
@@ -276,7 +260,6 @@ mod tests {
     fn test_request_type_from() {
         assert_eq!(RequestType::from(VIRTIO_BLK_T_IN), RequestType::In);
         assert_eq!(RequestType::from(VIRTIO_BLK_T_OUT), RequestType::Out);
-        assert_eq!(RequestType::from(VIRTIO_BLK_T_FLUSH), RequestType::Flush);
         assert_eq!(
             RequestType::from(VIRTIO_BLK_T_GET_ID),
             RequestType::GetDeviceID
@@ -290,10 +273,6 @@ mod tests {
             ExecuteError::BadRequest(Error::InvalidOffset).status(),
             VIRTIO_BLK_S_IOERR
         );
-        assert_eq!(
-            ExecuteError::Flush(io::Error::from_raw_os_error(42)).status(),
-            VIRTIO_BLK_S_IOERR
-        );
         assert_eq!(
             ExecuteError::Read(GuestMemoryError::InvalidBackendAddress).status(),
             VIRTIO_BLK_S_IOERR

tests/integration_tests/build/test_coverage.py

@@ -23,7 +23,7 @@
 # this contains the frequency while on AMD it does not.
 # Checkout the cpuid crate. In the future other
 # differences may appear.
-COVERAGE_DICT = {"Intel": 85.44, "AMD": 84.77, "ARM": 83.6}
+COVERAGE_DICT = {"Intel": 85.44, "AMD": 84.77, "ARM": 83.55}
 
 PROC_MODEL = proc.proc_type()