virtio-blk: support async discard operations

Add async discard execution for virtio-blk drives when the discard
feature is enabled.

Regular file backends use IORING_OP_FALLOCATE with hole punching.
Block-device backends use BLOCK_URING_CMD_DISCARD through
IORING_OP_URING_CMD when the host kernel supports it.

Require async fsync support during io_uring initialization, document the
async discard support matrix, and return VIRTIO_BLK_S_UNSUPP for guest
DISCARD requests when the discard feature was not negotiated.

Signed-off-by: Jonas Savulionis <jonas@esnet.lt>

Author: bacarrdy

docs/api_requests/block-discard.md

@@ -9,15 +9,20 @@ Discard is configured per virtio-block device through the `discard` field in the
 
 ## Supported configuration
 
-Discard is currently supported only for writable virtio-block devices using the
-`Sync` IO engine. It is not supported for:
+Discard is currently supported only for writable virtio-block devices. It is not
+supported for:
 
 - read-only drives;
-- `Async` IO engine drives;
 - vhost-user block devices.
 
 For regular backing files, Firecracker uses hole punching. For block-device
-backends, Firecracker uses `BLKDISCARD`.
+backends, Firecracker uses `BLKDISCARD` with the `Sync` IO engine and
+`BLOCK_URING_CMD_DISCARD` with the `Async` IO engine.
+
+When discard is enabled with the `Async` IO engine, regular backing files
+require host support for `IORING_OP_FALLOCATE`. Block-device backing stores
+additionally require host support for `BLOCK_URING_CMD_DISCARD`, which is
+available starting with Linux 6.12.
 
 ## Example configuration
 
@@ -31,7 +36,7 @@ curl --unix-socket ${socket} -i \
              \"path_on_host\": \"${drive_path}\",
              \"is_root_device\": true,
              \"is_read_only\": false,
-             \"io_engine\": \"Sync\",
+             \"io_engine\": \"Async\",
              \"discard\": true
          }"
 ```

docs/api_requests/block-io-engine.md

@@ -59,6 +59,12 @@ If a block device is configured with the `Async` io_engine on a host kernel
 older than 5.10.51, the API call will return a 400 Bad Request, with a
 suggestive error message.
 
+When the `discard` block option is enabled with the `Async` IO engine,
+Firecracker uses `IORING_OP_FALLOCATE` for regular backing files. For block
+device backing stores, async discard requires the kernel block uring discard
+command (`BLOCK_URING_CMD_DISCARD`), which is available starting with Linux
+6.12.
+
 ## Performance considerations
 
 The performance is strictly tied to the host kernel version. The gathered data

src/firecracker/swagger/firecracker.yaml

@@ -1243,7 +1243,9 @@ definitions:
         description:
           Enables virtio-blk discard support. When enabled, the guest can issue
           discard/TRIM requests for this drive. Only supported for writable
-          virtio-block devices using the Sync IO engine.
+          virtio-block devices. Async IO engine block-device backends require
+          host support for BLOCK_URING_CMD_DISCARD, available starting with
+          Linux 6.12.
           This field is optional for virtio-block config and should be omitted for vhost-user-block configuration.
         default: false
       path_on_host:

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

@@ -97,14 +97,15 @@ impl DiskProperties {
         disk_image_path: String,
         is_disk_read_only: bool,
         file_engine_type: FileEngineType,
+        discard: bool,
     ) -> Result<Self, VirtioBlockError> {
         let mut disk_image = Self::open_file(&disk_image_path, is_disk_read_only)?;
         let disk_size = Self::file_size(&disk_image_path, &mut disk_image)?;
         let image_id = Self::build_disk_image_id(&disk_image);
 
         Ok(Self {
             file_path: disk_image_path,
-            file_engine: FileEngine::from_file(disk_image, file_engine_type)
+            file_engine: FileEngine::from_file(disk_image, file_engine_type, discard)
                 .map_err(VirtioBlockError::FileEngine)?,
             nsectors: disk_size >> SECTOR_SHIFT,
             image_id,
@@ -319,14 +320,11 @@ impl VirtioBlock {
     ///
     /// The given file must be seekable and sizable.
     pub fn new(config: VirtioBlockConfig) -> Result<VirtioBlock, VirtioBlockError> {
-        if config.discard && config.file_engine_type == FileEngineType::Async {
-            return Err(VirtioBlockError::DiscardAsyncUnsupported);
-        }
-
         let disk_properties = DiskProperties::new(
             config.path_on_host,
             config.is_read_only,
             config.file_engine_type,
+            config.discard,
         )?;
 
         let rate_limiter = config
@@ -451,6 +449,7 @@ impl VirtioBlock {
                             head.index,
                             &active_state.mem,
                             &self.metrics,
+                            self.acked_features & (1u64 << VIRTIO_BLK_F_DISCARD) != 0,
                         )
                     }
                     Err(err) => {
@@ -745,9 +744,9 @@ mod tests {
     use crate::check_metric_after_block;
     use crate::devices::virtio::block::virtio::IO_URING_NUM_ENTRIES;
     use crate::devices::virtio::block::virtio::test_utils::{
-        default_block, read_blk_req_descriptors, set_queue, set_rate_limiter,
-        simulate_async_completion_event, simulate_queue_and_async_completion_events,
-        simulate_queue_event,
+        RequestDescriptorChain, default_block, read_blk_req_descriptors, set_queue,
+        set_rate_limiter, simulate_async_completion_event,
+        simulate_queue_and_async_completion_events, simulate_queue_event,
     };
     use crate::devices::virtio::queue::{VIRTQ_DESC_F_NEXT, VIRTQ_DESC_F_WRITE};
     use crate::devices::virtio::test_utils::{VirtQueue, default_interrupt, default_mem};
@@ -813,16 +812,20 @@ mod tests {
         f.as_file().set_len(size).unwrap();
 
         for engine in [FileEngineType::Sync, FileEngineType::Async] {
-            let disk_properties =
-                DiskProperties::new(String::from(f.as_path().to_str().unwrap()), true, engine)
-                    .unwrap();
+            let disk_properties = DiskProperties::new(
+                String::from(f.as_path().to_str().unwrap()),
+                true,
+                engine,
+                false,
+            )
+            .unwrap();
 
             assert_eq!(size, u64::from(SECTOR_SIZE) * num_sectors);
             assert_eq!(disk_properties.nsectors, num_sectors);
             // Testing `backing_file.virtio_block_disk_image_id()` implies
             // duplicating that logic in tests, so skipping it.
 
-            let res = DiskProperties::new("invalid-disk-path".to_string(), true, engine);
+            let res = DiskProperties::new("invalid-disk-path".to_string(), true, engine, false);
             assert!(
                 matches!(res, Err(VirtioBlockError::BackingFile(_, _))),
                 "{:?}",
@@ -891,10 +894,59 @@ mod tests {
             rate_limiter: None,
             file_engine_type: FileEngineType::Async,
         };
-        assert!(matches!(
-            VirtioBlock::new(async_config),
-            Err(VirtioBlockError::DiscardAsyncUnsupported)
-        ));
+        let block = VirtioBlock::new(async_config).unwrap();
+        assert_eq!(
+            block.avail_features & (1u64 << VIRTIO_BLK_F_DISCARD),
+            1u64 << VIRTIO_BLK_F_DISCARD
+        );
+    }
+
+    #[test]
+    fn test_discard_requires_negotiated_feature() {
+        for engine in [FileEngineType::Sync, FileEngineType::Async] {
+            let f = TempFile::new().unwrap();
+            f.as_file().set_len(0x1000).unwrap();
+            let config = VirtioBlockConfig {
+                drive_id: "test".to_string(),
+                path_on_host: f.as_path().to_str().unwrap().to_string(),
+                is_root_device: false,
+                partuuid: None,
+                is_read_only: false,
+                discard: true,
+                cache_type: CacheType::Unsafe,
+                rate_limiter: None,
+                file_engine_type: engine,
+            };
+            let mut block = VirtioBlock::new(config).unwrap();
+            let mem = default_mem();
+            let interrupt = default_interrupt();
+            let vq = VirtQueue::new(GuestAddress(0), &mem, 16);
+            set_queue(&mut block, 0, vq.create_queue());
+            block.activate(mem.clone(), interrupt).unwrap();
+
+            let chain = RequestDescriptorChain::new(&vq);
+            chain.set_header(RequestHeader::new(VIRTIO_BLK_T_DISCARD, 0));
+            chain.data_desc.flags.set(VIRTQ_DESC_F_NEXT);
+            chain.data_desc.len.set(16);
+
+            let mut discard_range = [0u8; 16];
+            discard_range[8..12].copy_from_slice(&1_u32.to_le_bytes());
+            mem.write_slice(&discard_range, GuestAddress(chain.data_desc.addr.get()))
+                .unwrap();
+
+            simulate_queue_event(&mut block, Some(true));
+
+            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!(
+                u32::from(
+                    mem.read_obj::<u8>(GuestAddress(chain.status_desc.addr.get()))
+                        .unwrap()
+                ),
+                VIRTIO_BLK_S_UNSUPP
+            );
+        }
     }
 
     #[test]