block: qcow: Share the synchronous read path between the two engines

Both engines walked the same cluster mappings to serve a read from
allocated clusters, the backing file, or zero fill. In the uring
engine this walk was a separate helper, scatter_read_sync, reached
only when the read was not a single contiguous allocated extent. In
the sync engine the same loop sat directly in the read path. Move
scatter_read_sync into common.rs and call it from both. The uring
contiguous fast path stays in resolve_read, so those reads still
offload to the ring.

Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>

block/src/formats/qcow/common.rs

@@ -16,7 +16,9 @@ use std::sync::Arc;
 use vmm_sys_util::write_zeroes::{PunchHole, WriteZeroesAt};
 
 use super::decoder::Decoder;
-use super::metadata::{BackingRead, ClusterWriteMapping, DeallocAction, QcowMetadata};
+use super::metadata::{
+    BackingRead, ClusterReadMapping, ClusterWriteMapping, DeallocAction, QcowMetadata,
+};
 use super::qcow_raw_file::QcowRawFile;
 use crate::async_io::{AsyncIoError, AsyncIoOperation, AsyncIoResult};
 
@@ -158,6 +160,78 @@ pub(super) fn cow_write_sync(
     Ok(())
 }
 
+/// Reads cluster mappings synchronously into an owned operation, filling
+/// holes, decompressing, and reading from the backing file as each
+/// mapping requires.
+pub(super) fn scatter_read_sync(
+    mappings: Vec<ClusterReadMapping>,
+    op: &mut AsyncIoOperation,
+    data_file: &QcowRawFile,
+    backing_file: &Option<Arc<dyn BackingRead>>,
+    cluster_size: u64,
+    decoder: &dyn Decoder,
+) -> AsyncIoResult<()> {
+    let mut buf_offset = 0usize;
+    for mapping in mappings {
+        match mapping {
+            ClusterReadMapping::Zero { length } => {
+                op.fill_zeroes_at(buf_offset, length as usize)
+                    .map_err(AsyncIoError::ReadVectored)?;
+                buf_offset += length as usize;
+            }
+            ClusterReadMapping::Allocated {
+                offset: host_offset,
+                length,
+            } => {
+                let len = length as usize;
+                let mut buf = vec![0u8; len];
+                data_file
+                    .file()
+                    .read_exact_at(&mut buf, host_offset)
+                    .map_err(AsyncIoError::ReadVectored)?;
+                op.write_bytes_at(buf_offset, &buf)
+                    .map_err(AsyncIoError::ReadVectored)?;
+                buf_offset += len;
+            }
+            ClusterReadMapping::Compressed {
+                host_offset,
+                compressed_size,
+                cluster_offset,
+                length,
+            } => {
+                let mut compressed = vec![0u8; compressed_size];
+                data_file
+                    .file()
+                    .read_exact_at(&mut compressed, host_offset)
+                    .map_err(AsyncIoError::ReadVectored)?;
+                let decompressed = decompress_cluster(&compressed, cluster_size as usize, decoder)
+                    .map_err(AsyncIoError::ReadVectored)?;
+                op.write_bytes_at(
+                    buf_offset,
+                    &decompressed[cluster_offset..cluster_offset + length],
+                )
+                .map_err(AsyncIoError::ReadVectored)?;
+                buf_offset += length;
+            }
+            ClusterReadMapping::Backing {
+                offset: backing_offset,
+                length,
+            } => {
+                let mut buf = vec![0u8; length as usize];
+                backing_file
+                    .as_ref()
+                    .unwrap()
+                    .read_at(backing_offset, &mut buf)
+                    .map_err(AsyncIoError::ReadVectored)?;
+                op.write_bytes_at(buf_offset, &buf)
+                    .map_err(AsyncIoError::ReadVectored)?;
+                buf_offset += length as usize;
+            }
+        }
+    }
+    Ok(())
+}
+
 #[cfg(test)]
 pub(crate) mod unit_tests {
     use std::fs::File;

block/src/formats/qcow/engine_sync.rs

@@ -4,14 +4,13 @@
 //
 // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
 
-use std::os::unix::fs::FileExt;
 use std::sync::Arc;
 
 use vmm_sys_util::eventfd::EventFd;
 
-use super::common::{cow_write_sync, deallocate_range_result, decompress_cluster};
+use super::common::{cow_write_sync, deallocate_range_result, scatter_read_sync};
 use super::decoder::Decoder;
-use super::metadata::{BackingRead, ClusterReadMapping, QcowMetadata};
+use super::metadata::{BackingRead, QcowMetadata};
 use super::qcow_raw_file::QcowRawFile;
 use crate::async_io::{
     AsyncIo, AsyncIoCompletion, AsyncIoError, AsyncIoOperation, AsyncIoResult, CompletionCommon,
@@ -56,65 +55,14 @@ impl QcowSync {
             .map_clusters_for_read(address, total_len, has_backing)
             .map_err(AsyncIoError::ReadVectored)?;
 
-        let mut buf_offset = 0usize;
+        scatter_read_sync(
-        for mapping in mappings {
+            mappings,
-            match mapping {
+            op,
-                ClusterReadMapping::Zero { length } => {
+            &self.data_file,
-                    op.fill_zeroes_at(buf_offset, length as usize)
+            &self.backing_file,
-                        .map_err(AsyncIoError::ReadVectored)?;
+            self.cluster_size,
-                    buf_offset += length as usize;
+            &*self.decoder,
-                }
+        )?;
-                ClusterReadMapping::Allocated {
-                    offset: host_offset,
-                    length,
-                } => {
-                    let len = length as usize;
-                    let mut buf = vec![0u8; len];
-                    self.data_file
-                        .file()
-                        .read_exact_at(&mut buf, host_offset)
-                        .map_err(AsyncIoError::ReadVectored)?;
-                    op.write_bytes_at(buf_offset, &buf)
-                        .map_err(AsyncIoError::ReadVectored)?;
-                    buf_offset += len;
-                }
-                ClusterReadMapping::Compressed {
-                    host_offset,
-                    compressed_size,
-                    cluster_offset,
-                    length,
-                } => {
-                    let mut compressed = vec![0u8; compressed_size];
-                    self.data_file
-                        .file()
-                        .read_exact_at(&mut compressed, host_offset)
-                        .map_err(AsyncIoError::ReadVectored)?;
-                    let decompressed =
-                        decompress_cluster(&compressed, self.cluster_size as usize, &*self.decoder)
-                            .map_err(AsyncIoError::ReadVectored)?;
-                    op.write_bytes_at(
-                        buf_offset,
-                        &decompressed[cluster_offset..cluster_offset + length],
-                    )
-                    .map_err(AsyncIoError::ReadVectored)?;
-                    buf_offset += length;
-                }
-                ClusterReadMapping::Backing {
-                    offset: backing_offset,
-                    length,
-                } => {
-                    let mut buf = vec![0u8; length as usize];
-                    self.backing_file
-                        .as_ref()
-                        .unwrap()
-                        .read_at(backing_offset, &mut buf)
-                        .map_err(AsyncIoError::ReadVectored)?;
-                    op.write_bytes_at(buf_offset, &buf)
-                        .map_err(AsyncIoError::ReadVectored)?;
-                    buf_offset += length as usize;
-                }
-            }
-        }
 
         Ok(total_len)
     }
@@ -208,7 +156,7 @@ mod unit_tests {
     use crate::formats::qcow;
     use crate::formats::qcow::common::apply_dealloc_action;
     use crate::formats::qcow::common::unit_tests::compress_allocated_clusters;
-    use crate::formats::qcow::metadata::DeallocAction;
+    use crate::formats::qcow::metadata::{ClusterReadMapping, DeallocAction};
     use crate::formats::qcow::{
         BackingFileConfig, Error as QcowError, ImageType, QcowDisk, QcowHeader, QcowTempDisk,
     };

block/src/formats/qcow/engine_uring.rs

@@ -9,13 +9,12 @@
 //! QCOW2 async disk backend.
 
 use std::io;
-use std::os::unix::fs::FileExt;
 use std::os::unix::io::AsRawFd;
 use std::sync::Arc;
 
 use vmm_sys_util::eventfd::EventFd;
 
-use super::common::{cow_write_sync, deallocate_range_result, decompress_cluster};
+use super::common::{cow_write_sync, deallocate_range_result, scatter_read_sync};
 use super::decoder::Decoder;
 use super::metadata::{BackingRead, ClusterReadMapping, QcowMetadata};
 use super::qcow_raw_file::QcowRawFile;
@@ -277,80 +276,9 @@ impl QcowAsync {
             return Ok(Some(*host_offset));
         }
 
-        Self::scatter_read_sync(mappings, op, data_file, backing_file, cluster_size, decoder)?;
+        scatter_read_sync(mappings, op, data_file, backing_file, cluster_size, decoder)?;
         Ok(None)
     }
-
-    /// Scatter-read cluster mappings synchronously into an owned operation.
-    fn scatter_read_sync(
-        mappings: Vec<ClusterReadMapping>,
-        op: &mut AsyncIoOperation,
-        data_file: &QcowRawFile,
-        backing_file: &Option<Arc<dyn BackingRead>>,
-        cluster_size: u64,
-        decoder: &dyn Decoder,
-    ) -> AsyncIoResult<()> {
-        let mut buf_offset = 0usize;
-        for mapping in mappings {
-            match mapping {
-                ClusterReadMapping::Zero { length } => {
-                    op.fill_zeroes_at(buf_offset, length as usize)
-                        .map_err(AsyncIoError::ReadVectored)?;
-                    buf_offset += length as usize;
-                }
-                ClusterReadMapping::Allocated {
-                    offset: host_offset,
-                    length,
-                } => {
-                    let len = length as usize;
-                    let mut buf = vec![0u8; len];
-                    data_file
-                        .file()
-                        .read_exact_at(&mut buf, host_offset)
-                        .map_err(AsyncIoError::ReadVectored)?;
-                    op.write_bytes_at(buf_offset, &buf)
-                        .map_err(AsyncIoError::ReadVectored)?;
-                    buf_offset += len;
-                }
-                ClusterReadMapping::Compressed {
-                    host_offset,
-                    compressed_size,
-                    cluster_offset,
-                    length,
-                } => {
-                    let mut compressed = vec![0u8; compressed_size];
-                    data_file
-                        .file()
-                        .read_exact_at(&mut compressed, host_offset)
-                        .map_err(AsyncIoError::ReadVectored)?;
-                    let decompressed =
-                        decompress_cluster(&compressed, cluster_size as usize, decoder)
-                            .map_err(AsyncIoError::ReadVectored)?;
-                    op.write_bytes_at(
-                        buf_offset,
-                        &decompressed[cluster_offset..cluster_offset + length],
-                    )
-                    .map_err(AsyncIoError::ReadVectored)?;
-                    buf_offset += length;
-                }
-                ClusterReadMapping::Backing {
-                    offset: backing_offset,
-                    length,
-                } => {
-                    let mut buf = vec![0u8; length as usize];
-                    backing_file
-                        .as_ref()
-                        .unwrap()
-                        .read_at(backing_offset, &mut buf)
-                        .map_err(AsyncIoError::ReadVectored)?;
-                    op.write_bytes_at(buf_offset, &buf)
-                        .map_err(AsyncIoError::ReadVectored)?;
-                    buf_offset += length as usize;
-                }
-            }
-        }
-        Ok(())
-    }
 }
 
 #[cfg(test)]