virtio-blk: support async discard operations

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

Author: bacarrdy

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. Hosts running Linux 7.x or newer satisfy this requirement.
+
 ## Performance considerations
 
 The performance is strictly tied to the host kernel version. The gathered data

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,
@@ -264,7 +265,7 @@ impl From<VirtioBlockConfig> for BlockDeviceConfig {
             cache_type: value.cache_type,
 
             is_read_only: Some(value.is_read_only),
-            discard: Some(value.discard),
+            discard: value.discard.then_some(true),
             path_on_host: Some(value.path_on_host),
             rate_limiter: value.rate_limiter,
             file_engine_type: Some(value.file_engine_type),
@@ -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
@@ -813,16 +811,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 +893,11 @@ 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]

src/vmm/src/devices/virtio/block/virtio/io/async_io.rs

@@ -1,9 +1,11 @@
 // Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
+use std::ffi::CStr;
 use std::fmt::Debug;
 use std::fs::File;
 use std::os::fd::RawFd;
+use std::os::unix::fs::FileTypeExt;
 use std::os::unix::io::AsRawFd;
 
 use vm_memory::GuestMemoryError;
@@ -21,6 +23,8 @@ use crate::vstate::memory::{GuestAddress, GuestMemory, GuestMemoryExtension, Gue
 pub enum AsyncIoError {
     /// Not implemented
     NotImplemented,
+    /// Discard is not supported with this async backend on the host kernel.
+    DiscardUnsupported,
     /// IO: {0}
     IO(std::io::Error),
     /// IoUring: {0}
@@ -40,6 +44,13 @@ pub struct AsyncFileEngine {
     file: File,
     ring: IoUring<WrappedRequest>,
     completion_evt: EventFd,
+    discard_op: Option<AsyncDiscardOp>,
+}
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+enum AsyncDiscardOp {
+    BlockUringCmd,
+    Fallocate,
 }
 
 #[derive(Debug)]
@@ -70,48 +81,135 @@ impl WrappedRequest {
 }
 
 impl AsyncFileEngine {
+    const BLOCK_URING_CMD_DISCARD: u32 = 0x1200;
+    const FALLOC_FL_KEEP_SIZE: u32 = 0x01;
+    const FALLOC_FL_PUNCH_HOLE: u32 = 0x02;
+    const MIN_BLOCK_URING_DISCARD_KERNEL: (u32, u32) = (6, 12);
+
     fn new_ring(
         file: &File,
         completion_fd: RawFd,
+        discard_op: Option<AsyncDiscardOp>,
     ) -> Result<IoUring<WrappedRequest>, IoUringError> {
-        IoUring::new(
+        let mut restrictions = vec![
+            // Make sure we only allow operations on pre-registered fds.
+            Restriction::RequireFixedFds,
+            // Allowlist of opcodes.
+            Restriction::AllowOpCode(OpCode::Read),
+            Restriction::AllowOpCode(OpCode::Write),
+            Restriction::AllowOpCode(OpCode::Fsync),
+        ];
+        let mut required_ops = vec![OpCode::Read, OpCode::Write];
+        match discard_op {
+            Some(AsyncDiscardOp::Fallocate) => {
+                restrictions.push(Restriction::AllowOpCode(OpCode::Fallocate));
+                required_ops.push(OpCode::Fallocate);
+            }
+            Some(AsyncDiscardOp::BlockUringCmd) => {
+                restrictions.push(Restriction::AllowOpCode(OpCode::UringCmd));
+                required_ops.push(OpCode::UringCmd);
+            }
+            None => {}
+        }
+
+        IoUring::new_with_required_ops(
             u32::from(IO_URING_NUM_ENTRIES),
             vec![file],
-            vec![
-                // Make sure we only allow operations on pre-registered fds.
-                Restriction::RequireFixedFds,
-                // Allowlist of opcodes.
-                Restriction::AllowOpCode(OpCode::Read),
-                Restriction::AllowOpCode(OpCode::Write),
-                Restriction::AllowOpCode(OpCode::Fsync),
-            ],
+            restrictions,
             Some(completion_fd),
+            &required_ops,
         )
     }
 
-    pub fn from_file(file: File) -> Result<AsyncFileEngine, AsyncIoError> {
+    pub fn from_file(file: File, discard: bool) -> Result<AsyncFileEngine, AsyncIoError> {
         log_dev_preview_warning("Async file IO", Option::None);
 
         let completion_evt = EventFd::new(libc::EFD_NONBLOCK).map_err(AsyncIoError::EventFd)?;
-        let ring =
-            Self::new_ring(&file, completion_evt.as_raw_fd()).map_err(AsyncIoError::IoUring)?;
+        let discard_op = Self::discard_op(&file, discard)?;
+        let ring = Self::new_ring(&file, completion_evt.as_raw_fd(), discard_op)
+            .map_err(AsyncIoError::IoUring)?;
 
         Ok(AsyncFileEngine {
             file,
             ring,
             completion_evt,
+            discard_op,
         })
     }
 
     pub fn update_file(&mut self, file: File) -> Result<(), AsyncIoError> {
-        let ring = Self::new_ring(&file, self.completion_evt.as_raw_fd())
+        let discard_op = Self::discard_op(&file, self.discard_op.is_some())?;
+        let ring = Self::new_ring(&file, self.completion_evt.as_raw_fd(), discard_op)
             .map_err(AsyncIoError::IoUring)?;
 
-        self.file = file;
         self.ring = ring;
+        self.file = file;
+        self.discard_op = discard_op;
         Ok(())
     }
 
+    fn discard_op(file: &File, discard: bool) -> Result<Option<AsyncDiscardOp>, AsyncIoError> {
+        if !discard {
+            return Ok(None);
+        }
+
+        if file
+            .metadata()
+            .map_err(AsyncIoError::IO)?
+            .file_type()
+            .is_block_device()
+        {
+            // BLOCK_URING_CMD_DISCARD is introduced for block devices in Linux 6.12.
+            // IORING_OP_URING_CMD probing alone is not enough because older kernels can
+            // support uring commands for other file operations.
+            if !Self::host_kernel_at_least(Self::MIN_BLOCK_URING_DISCARD_KERNEL)
+                .map_err(AsyncIoError::IO)?
+            {
+                return Err(AsyncIoError::DiscardUnsupported);
+            }
+            Ok(Some(AsyncDiscardOp::BlockUringCmd))
+        } else {
+            Ok(Some(AsyncDiscardOp::Fallocate))
+        }
+    }
+
+    fn host_kernel_at_least((major, minor): (u32, u32)) -> Result<bool, std::io::Error> {
+        // SAFETY: An all-zeroed value for `libc::utsname` is valid.
+        let mut name: libc::utsname = unsafe { std::mem::zeroed() };
+        // SAFETY: The passed arg is a valid mutable reference of `libc::utsname`.
+        let ret = unsafe { libc::uname(&mut name) };
+        if ret != 0 {
+            return Err(std::io::Error::last_os_error());
+        }
+
+        // SAFETY: The fields of `libc::utsname` are terminated by a null byte.
+        let release = unsafe { CStr::from_ptr(name.release.as_ptr()) }
+            .to_string_lossy()
+            .into_owned();
+        Self::kernel_release_at_least(&release, (major, minor)).ok_or_else(|| {
+            std::io::Error::new(std::io::ErrorKind::InvalidData, "invalid kernel release")
+        })
+    }
+
+    fn parse_kernel_release(release: &str) -> Option<(u32, u32)> {
+        let mut parts = release
+            .split(|ch: char| !ch.is_ascii_digit() && ch != '.')
+            .next()
+            .unwrap_or("")
+            .split('.');
+
+        let host_major = parts.next()?.parse::<u32>().ok()?;
+        let host_minor = parts.next()?.parse::<u32>().ok()?;
+
+        Some((host_major, host_minor))
+    }
+
+    fn kernel_release_at_least(release: &str, (major, minor): (u32, u32)) -> Option<bool> {
+        let (host_major, host_minor) = Self::parse_kernel_release(release)?;
+
+        Some(host_major > major || (host_major == major && host_minor >= minor))
+    }
+
     #[cfg(test)]
     pub fn file(&self) -> &File {
         &self.file
@@ -200,8 +298,42 @@ impl AsyncFileEngine {
             })
     }
 
-    pub fn discard(&mut self, _range: (u64, u64)) -> Result<u32, AsyncIoError> {
-        Err(AsyncIoError::NotImplemented)
+    pub fn push_discard(
+        &mut self,
+        range: (u64, u64),
+        req: PendingRequest,
+    ) -> Result<(), RequestError<AsyncIoError>> {
+        let wrapped_user_data = WrappedRequest::new(req);
+        let (offset, len) = range;
+        let operation = match self.discard_op {
+            Some(AsyncDiscardOp::Fallocate) => Operation::fallocate(
+                0,
+                Self::FALLOC_FL_KEEP_SIZE | Self::FALLOC_FL_PUNCH_HOLE,
+                offset,
+                len,
+                wrapped_user_data,
+            ),
+            Some(AsyncDiscardOp::BlockUringCmd) => Operation::block_discard(
+                0,
+                Self::BLOCK_URING_CMD_DISCARD,
+                offset,
+                len,
+                wrapped_user_data,
+            ),
+            None => {
+                return Err(RequestError {
+                    req: wrapped_user_data.req,
+                    error: AsyncIoError::NotImplemented,
+                });
+            }
+        };
+
+        self.ring
+            .push(operation)
+            .map_err(|(io_uring_error, data)| RequestError {
+                req: data.req,
+                error: AsyncIoError::IoUring(io_uring_error),
+            })
     }
 
     pub fn kick_submission_queue(&mut self) -> Result<(), AsyncIoError> {
@@ -254,3 +386,42 @@ impl AsyncFileEngine {
         Ok(cqe)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use vmm_sys_util::tempfile::TempFile;
+
+    use super::*;
+
+    #[test]
+    fn test_kernel_release_at_least() {
+        assert_eq!(
+            AsyncFileEngine::kernel_release_at_least("6.11.0-1018-aws", (6, 12)),
+            Some(false)
+        );
+        assert_eq!(
+            AsyncFileEngine::kernel_release_at_least("6.12.0-1020-aws", (6, 12)),
+            Some(true)
+        );
+        assert_eq!(
+            AsyncFileEngine::kernel_release_at_least("6.17.0-29-generic", (6, 12)),
+            Some(true)
+        );
+        assert_eq!(
+            AsyncFileEngine::kernel_release_at_least("7.0.2-6-pve", (6, 12)),
+            Some(true)
+        );
+        assert_eq!(
+            AsyncFileEngine::kernel_release_at_least("not-a-kernel", (6, 12)),
+            None
+        );
+    }
+
+    #[test]
+    fn test_discard_regular_file_uses_fallocate() {
+        let file = TempFile::new().unwrap().into_file();
+        let engine = AsyncFileEngine::from_file(file, true).unwrap();
+
+        assert_eq!(engine.discard_op, Some(AsyncDiscardOp::Fallocate));
+    }
+}