[Block] Add option to configure block device flush

CHANGELOG.md

@@ -6,6 +6,7 @@
 
 - Added devtool build `--ssh-keys` flag to support fetching from private
   git repositories.
+- Added option to configure block device flush.
 
 ### Changed
 

docs/api_requests/block-caching.md

@@ -0,0 +1,70 @@
+# Block device caching strategies
+
+Firecracker offers the possiblity of choosing the block device caching
+strategy. Caching strategy affects the path data written from inside the
+microVM takes to the host persistent storage.
+
+## How it works
+
+When installing a block device through a PUT /drives API call, users can choose
+the caching strategy by inserting a `cache_type` field in the JSON body of the
+request. The available cache types are:
+
+- `Unsafe`
+- `Writeback`
+
+### Unsafe mode (default)
+
+When configuring the block caching strategy to `Unsafe`, the device will
+advertise the VirtIO `flush` feature to the guest driver. If negotiated when
+activating the device, the guest driver will be able to send flush requests
+to the device, but the device will just acknowledge the request without
+actually performing any flushing on the host side. The data which was not
+yet committed to disk will continue to reside in the host page cache.
+
+### Writeback mode
+
+When configuring the block caching strategy to `Writeback`, the device will
+advertise the VirtIO `flush` feature to the guest driver. If negotiated when
+activating the device, the guest driver will be able to send flush requests
+to the device. When the device executes a flush request, it will perform an
+`fsync` syscall on the backing block file, committing all data in the host
+page cache to disk.
+
+## Supported use cases
+
+The caching strategy should be used in order to make a trade-off:
+
+- `Unsafe`
+  - enhances performance as fewer syscalls and IO operations are performed when
+    running workloads
+  - sacrifices data integrity in situations where the host simply loses the
+    contents of the page cache without committing them to the backing storage
+    (such as a power outage)
+  - recommended for use cases with ephemeral storage, such as serverless
+    environments
+- `Writeback`
+  - ensures that once a flush request was acknowledged by the host, the data
+    is committed to the backing storage
+  - sacrifices performance, from boot time increases to greater
+    emulation-related latencies when running workloads
+  - recommended for use cases with low power environments, such as embedded
+    environments
+
+## How to configure it
+
+Example sequence that configures a block device with a caching strategy:
+
+```bash
+curl --unix-socket ${socket} -i \
+     -X PUT "http://localhost/drives/dummy" \
+     -H "accept: application/json" \
+     -H "Content-Type: application/json" \
+     -d "{
+             \"drive_id\": \"dummy\",
+             \"path_on_host\": \"${drive_path}\",
+             \"is_root_device\": false,
+             \"is_read_only\": false,
+             \"cache_type\": \"Writeback\"
+         }"
+```

src/api_server/src/parsed_request.rs

@@ -643,6 +643,7 @@ pub(crate) mod tests {
             \"is_root_device\": true, \
             \"partuuid\": \"string\", \
             \"is_read_only\": true, \
+            \"cache_type\": \"Unsafe\", \
             \"rate_limiter\": { \
                 \"bandwidth\": { \
                     \"size\": 0, \

src/api_server/src/request/drive.rs

@@ -216,20 +216,33 @@ mod tests {
         assert!(parse_put_drive(&Body::new("invalid_payload"), None).is_err());
         assert!(parse_put_drive(&Body::new("invalid_payload"), Some(&"id")).is_err());
 
-        // PATCH with invalid fields.
+        // PUT with invalid fields.
         let body = r#"{
                 "drive_id": "bar",
                 "is_read_only": false
               }"#;
         assert!(parse_put_drive(&Body::new(body), Some(&"2")).is_err());
 
-        // PATCH with invalid types on fields. Adding a drive_id as number instead of string.
+        // PUT with missing all optional fields.
+        let body = r#"{
+            "drive_id": "1000",
+            "path_on_host": "dummy",
+            "is_root_device": true,
+            "is_read_only": true
+        }"#;
+        assert!(parse_put_drive(&Body::new(body), Some(&"1000")).is_ok());
+
+        // PUT with invalid types on fields. Adding a drive_id as number instead of string.
+        assert!(parse_put_drive(&Body::new(body), Some(&"foo")).is_err());
+
+        // PUT with the complete configuration.
         let body = r#"{
                 "drive_id": "1000",
                 "path_on_host": "dummy",
                 "is_root_device": true,
                 "partuuid": "string",
                 "is_read_only": true,
+                "cache_type": "Unsafe",
                 "rate_limiter": {
                     "bandwidth": {
                         "size": 0,
@@ -244,7 +257,5 @@ mod tests {
                 }
             }"#;
         assert!(parse_put_drive(&Body::new(body), Some(&"1000")).is_ok());
-
-        assert!(parse_put_drive(&Body::new(body), Some(&"foo")).is_err());
     }
 }

src/api_server/swagger/firecracker.yaml

@@ -753,6 +753,11 @@ definitions:
     properties:
       drive_id:
         type: string
+      cache_type:
+        type: string
+        description:
+          Represents the caching strategy for the block device.
+        default: "Unsafe"
       is_read_only:
         type: boolean
       is_root_device:

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

@@ -30,16 +30,41 @@ use super::{
 use crate::virtio::VIRTIO_MMIO_INT_CONFIG;
 use crate::Error as DeviceError;
 
+use serde::{Deserialize, Serialize};
+
+/// Configuration options for disk caching.
+#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
+pub enum CacheType {
+    /// 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 Default for CacheType {
+    fn default() -> CacheType {
+        CacheType::Unsafe
+    }
+}
+
 /// Helper object for setting up all `Block` fields derived from its backing file.
 pub(crate) struct DiskProperties {
+    cache_type: CacheType,
     file_path: String,
     file: File,
     nsectors: u64,
     image_id: Vec<u8>,
 }
 
 impl DiskProperties {
-    pub fn new(disk_image_path: String, is_disk_read_only: bool) -> io::Result<Self> {
+    pub fn new(
+        disk_image_path: String,
+        is_disk_read_only: bool,
+        cache_type: CacheType,
+    ) -> io::Result<Self> {
         let mut disk_image = OpenOptions::new()
             .read(true)
             .write(!is_disk_read_only)
@@ -57,6 +82,7 @@ impl DiskProperties {
         }
 
         Ok(Self {
+            cache_type,
             nsectors: disk_size >> SECTOR_SHIFT,
             image_id: Self::build_disk_image_id(&disk_image),
             file_path: disk_image_path,
@@ -121,6 +147,31 @@ impl DiskProperties {
         }
         config
     }
+
+    pub fn cache_type(&self) -> CacheType {
+        self.cache_type
+    }
+}
+
+impl Drop for DiskProperties {
+    fn drop(&mut self) {
+        match self.cache_type {
+            CacheType::Writeback => {
+                // flush() first to force any cached data out.
+                if self.file.flush().is_err() {
+                    error!("Failed to flush block data on drop.");
+                }
+                // Sync data out to physical media on host.
+                if self.file.sync_all().is_err() {
+                    error!("Failed to sync block data on drop.")
+                }
+                METRICS.block.flush_count.inc();
+            }
+            CacheType::Unsafe => {
+                // This is a noop.
+            }
+        };
+    }
 }
 
 /// Virtio device for exposing block level read/write operations on a host file.
@@ -155,12 +206,13 @@ impl Block {
     pub fn new(
         id: String,
         partuuid: Option<String>,
+        cache_type: CacheType,
         disk_image_path: String,
         is_disk_read_only: bool,
         is_disk_root: bool,
         rate_limiter: RateLimiter,
     ) -> io::Result<Block> {
-        let disk_properties = DiskProperties::new(disk_image_path, is_disk_read_only)?;
+        let disk_properties = DiskProperties::new(disk_image_path, is_disk_read_only, cache_type)?;
 
         let mut avail_features = (1u64 << VIRTIO_F_VERSION_1) | (1u64 << VIRTIO_BLK_F_FLUSH);
 
@@ -343,7 +395,8 @@ impl Block {
 
     /// Update the backing file and the config space of the block device.
     pub fn update_disk_image(&mut self, disk_image_path: String) -> io::Result<()> {
-        let disk_properties = DiskProperties::new(disk_image_path, self.is_read_only())?;
+        let disk_properties =
+            DiskProperties::new(disk_image_path, self.is_read_only(), self.cache_type())?;
         self.disk = disk_properties;
         self.config_space = self.disk.virtio_block_config_space();
 
@@ -380,6 +433,10 @@ impl Block {
     pub fn is_root_device(&self) -> bool {
         self.root_device
     }
+
+    pub fn cache_type(&self) -> CacheType {
+        self.disk.cache_type()
+    }
 }
 
 impl VirtioDevice for Block {
@@ -491,8 +548,12 @@ pub(crate) mod tests {
         let size = SECTOR_SIZE * num_sectors;
         f.as_file().set_len(size).unwrap();
 
-        let disk_properties =
-            DiskProperties::new(String::from(f.as_path().to_str().unwrap()), true).unwrap();
+        let disk_properties = DiskProperties::new(
+            String::from(f.as_path().to_str().unwrap()),
+            true,
+            CacheType::Unsafe,
+        )
+        .unwrap();
 
         assert_eq!(size, SECTOR_SIZE * num_sectors);
         assert_eq!(disk_properties.nsectors, num_sectors);
@@ -504,7 +565,9 @@ pub(crate) mod tests {
         // Testing `backing_file.virtio_block_disk_image_id()` implies
         // duplicating that logic in tests, so skipping it.
 
-        assert!(DiskProperties::new("invalid-disk-path".to_string(), true).is_err());
+        assert!(
+            DiskProperties::new("invalid-disk-path".to_string(), true, CacheType::Unsafe).is_err()
+        );
     }
 
     #[test]

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

@@ -7,7 +7,7 @@ pub mod persist;
 pub mod request;
 pub mod test_utils;
 
-pub use self::device::Block;
+pub use self::device::{Block, CacheType};
 pub use self::event_handler::*;
 pub use self::request::*;