block: qcow: Drop own O_DIRECT alignment handling

The qcow workers carried their own O_DIRECT alignment requirement
and bounced unaligned cluster accesses through AlignedBuffer. Now
that the data file is an AlignedFile that handles O_DIRECT
transparently, the qcow layer can read and write through plain
buffers and let AlignedFile perform the aligned bounce.

Remove the alignment field and the per cluster AlignedBuffer RMW
branches from both the sync and async workers. The async io_uring
fast path still needs to avoid submitting unaligned guest iovecs
under O_DIRECT, so gate it on is_direct rather than on a stored
alignment value.

Drop the QcowAsync alignment override so it reports the trait
default sector size, matching QcowSync. qcow never submits guest
iovecs to the kernel under O_DIRECT, so reporting a larger value
only forced the request layer into an extra bounce buffer.

This adds one buffer copy per unaligned O_DIRECT cluster but moves
all alignment handling into a single place. The buffered path is
unchanged.

With qcow no longer the only caller, AlignedBuffer::read_exact_from
becomes dead code, so remove it and switch its tests to read_from.

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

block/src/aligned_buffer.rs

@@ -101,12 +101,6 @@ impl AlignedBuffer {
         unsafe { slice::from_raw_parts_mut(self.ptr, self.aligned_len) }
     }
 
-    /// Read the full aligned region from `f` into this buffer.
-    pub fn read_exact_from(&mut self, f: &impl FileExt) -> io::Result<()> {
-        let offset = self.aligned_offset;
-        f.read_exact_at(self.full_mut_slice(), offset)
-    }
-
     /// Read into the buffer from `f`, tolerating a short read at EOF.
     ///
     /// Returns the number of caller-logical bytes now valid in `as_slice()`,
@@ -168,7 +162,7 @@ mod tests {
         let alignment = 512;
 
         let mut abuf = AlignedBuffer::new(0, size, alignment).unwrap();
-        abuf.read_exact_from(tf.as_file()).unwrap();
+        abuf.read_from(tf.as_file()).unwrap();
 
         let expected: Vec<u8> = (0..size).map(|i| (i % 251) as u8).collect();
         assert_eq!(abuf.as_slice(), &expected[..]);
@@ -179,7 +173,7 @@ mod tests {
         let tf = create_pattern_file(512);
         let mut abuf = AlignedBuffer::new(100, 0, 512).unwrap();
 
-        abuf.read_exact_from(tf.as_file()).unwrap();
+        abuf.read_from(tf.as_file()).unwrap();
         abuf.write_to(tf.as_file()).unwrap();
 
         assert!(abuf.as_slice().is_empty());
@@ -195,7 +189,7 @@ mod tests {
         let offset = 100u64;
         let len = 200usize;
         let mut abuf = AlignedBuffer::new(offset, len, alignment).unwrap();
-        abuf.read_exact_from(tf.as_file()).unwrap();
+        abuf.read_from(tf.as_file()).unwrap();
 
         let expected: Vec<u8> = (offset as usize..offset as usize + len)
             .map(|i| (i % 251) as u8)
@@ -230,7 +224,7 @@ mod tests {
         let data: Vec<u8> = (0..len).map(|i| ((i + 1) % 239) as u8).collect();
 
         let mut abuf = AlignedBuffer::new(offset, len, alignment).unwrap();
-        abuf.read_exact_from(tf.as_file()).unwrap();
+        abuf.read_from(tf.as_file()).unwrap();
         abuf.as_mut_slice().copy_from_slice(&data);
         abuf.write_to(tf.as_file()).unwrap();
 
@@ -256,12 +250,12 @@ mod tests {
         let data: Vec<u8> = (0..len).map(|i| ((i + 1) % 239) as u8).collect();
 
         let mut abuf = AlignedBuffer::new(offset, len, alignment).unwrap();
-        abuf.read_exact_from(tf.as_file()).unwrap();
+        abuf.read_from(tf.as_file()).unwrap();
         abuf.as_mut_slice().copy_from_slice(&data);
         abuf.write_to(tf.as_file()).unwrap();
 
         let mut abuf = AlignedBuffer::new(offset, len, alignment).unwrap();
-        abuf.read_exact_from(tf.as_file()).unwrap();
+        abuf.read_from(tf.as_file()).unwrap();
         assert_eq!(abuf.as_slice(), &data[..]);
 
         let mut whole = vec![0u8; file_size];

block/src/formats/qcow/worker/async_uring.rs

@@ -8,7 +8,7 @@
 
 //! QCOW2 async disk backend.
 
-use std::cmp::{max, min};
+use std::cmp::min;
 use std::io;
 use std::os::unix::fs::FileExt;
 use std::os::unix::io::AsRawFd;
@@ -23,8 +23,6 @@ use super::internal::metadata::{
     BackingRead, ClusterReadMapping, ClusterWriteMapping, DeallocAction, QcowMetadata,
 };
 use super::internal::qcow_raw_file::QcowRawFile;
-use crate::SECTOR_SIZE;
-use crate::aligned_buffer::AlignedBuffer;
 use crate::async_io::{
     AsyncIo, AsyncIoCompletion, AsyncIoError, AsyncIoOperation, AsyncIoResult, UringDataIo,
 };
@@ -45,10 +43,6 @@ pub struct QcowAsync {
     data_file: QcowRawFile,
     backing_file: Option<Arc<dyn BackingRead>>,
     sparse: bool,
-    /// O_DIRECT alignment requirement (0 = no alignment needed).
-    alignment: usize,
-    /// I/O alignment for the AsyncIo trait (at least SECTOR_SIZE).
-    io_alignment: u64,
     cluster_size: u64,
     decoder: Arc<dyn Decoder>,
 }
@@ -61,9 +55,6 @@ impl QcowAsync {
         sparse: bool,
         ring_depth: u32,
     ) -> io::Result<Self> {
-        let alignment = data_file.file().alignment();
-        let io_alignment = max(alignment as u64, SECTOR_SIZE);
-
         Ok(QcowAsync {
             cluster_size: metadata.cluster_size(),
             decoder: metadata.decoder(),
@@ -72,8 +63,6 @@ impl QcowAsync {
             data_file,
             backing_file,
             sparse,
-            alignment,
-            io_alignment,
         })
     }
 
@@ -126,7 +115,6 @@ impl QcowAsync {
             op.offset() as u64,
             &mut op,
             total_len,
-            self.alignment,
             self.cluster_size,
             &*self.decoder,
         ) {
@@ -159,7 +147,6 @@ impl QcowAsync {
             &self.metadata,
             &self.data_file,
             &self.backing_file,
-            self.alignment,
             self.cluster_size,
         ) {
             return Err(Box::new((op, e)));
@@ -277,10 +264,6 @@ impl AsyncIo for QcowAsync {
         true
     }
 
-    fn alignment(&self) -> u64 {
-        self.io_alignment
-    }
-
     fn submit_batch_requests(&mut self, batch_request: Vec<AsyncIoOperation>) -> AsyncIoResult<()> {
         let mut async_reads = Vec::new();
 
@@ -330,7 +313,6 @@ impl QcowAsync {
         address: u64,
         op: &mut AsyncIoOperation,
         total_len: usize,
-        alignment: usize,
         cluster_size: u64,
         decoder: &dyn Decoder,
     ) -> AsyncIoResult<Option<u64>> {
@@ -346,7 +328,7 @@ impl QcowAsync {
         // Guest requests can be smaller (e.g. 512 byte UEFI reads on a
         // 4096 byte sector device), so O_DIRECT reads fall through to the
         // alignment aware synchronous path instead.
-        if alignment == 0
+        if !data_file.file().is_direct()
             && mappings.len() == 1
             && let ClusterReadMapping::Allocated {
                 offset: host_offset,
@@ -357,15 +339,7 @@ impl QcowAsync {
             return Ok(Some(*host_offset));
         }
 
-        Self::scatter_read_sync(
+        Self::scatter_read_sync(mappings, op, data_file, backing_file, cluster_size, decoder)?;
-            mappings,
-            op,
-            data_file,
-            backing_file,
-            alignment,
-            cluster_size,
-            decoder,
-        )?;
         Ok(None)
     }
 
@@ -375,7 +349,6 @@ impl QcowAsync {
         op: &mut AsyncIoOperation,
         data_file: &QcowRawFile,
         backing_file: &Option<Arc<dyn BackingRead>>,
-        alignment: usize,
         cluster_size: u64,
         decoder: &dyn Decoder,
     ) -> AsyncIoResult<()> {
@@ -392,22 +365,13 @@ impl QcowAsync {
                     length,
                 } => {
                     let len = length as usize;
-                    if alignment > 0 {
+                    let mut buf = vec![0u8; len];
-                        let mut abuf = AlignedBuffer::new(host_offset, len, alignment)
+                    data_file
-                            .map_err(AsyncIoError::ReadVectored)?;
+                        .file()
-                        abuf.read_exact_from(data_file.file())
+                        .read_exact_at(&mut buf, host_offset)
-                            .map_err(AsyncIoError::ReadVectored)?;
+                        .map_err(AsyncIoError::ReadVectored)?;
-                        op.write_bytes_at(buf_offset, abuf.as_slice())
+                    op.write_bytes_at(buf_offset, &buf)
-                            .map_err(AsyncIoError::ReadVectored)?;
+                        .map_err(AsyncIoError::ReadVectored)?;
-                    } else {
-                        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 {
@@ -457,7 +421,6 @@ impl QcowAsync {
         metadata: &QcowMetadata,
         data_file: &QcowRawFile,
         backing_file: &Option<Arc<dyn BackingRead>>,
-        alignment: usize,
         cluster_size: u64,
     ) -> AsyncIoResult<()> {
         let total_len = op.total_len();
@@ -491,24 +454,13 @@ impl QcowAsync {
                 ClusterWriteMapping::Allocated {
                     offset: host_offset,
                 } => {
-                    if alignment > 0 {
+                    let mut buf = vec![0u8; count];
-                        let mut abuf = AlignedBuffer::new(host_offset, count, alignment)
+                    op.read_bytes_at(buf_offset, &mut buf)
-                            .map_err(AsyncIoError::WriteVectored)?;
+                        .map_err(AsyncIoError::WriteVectored)?;
-                        abuf.read_exact_from(data_file.file())
+                    data_file
-                            .map_err(AsyncIoError::WriteVectored)?;
+                        .file()
-                        op.read_bytes_at(buf_offset, abuf.as_mut_slice())
+                        .write_all_at(&buf, host_offset)
-                            .map_err(AsyncIoError::WriteVectored)?;
+                        .map_err(AsyncIoError::WriteVectored)?;
-                        abuf.write_to(data_file.file())
-                            .map_err(AsyncIoError::WriteVectored)?;
-                    } else {
-                        let mut buf = vec![0u8; count];
-                        op.read_bytes_at(buf_offset, &mut buf)
-                            .map_err(AsyncIoError::WriteVectored)?;
-                        data_file
-                            .file()
-                            .write_all_at(&buf, host_offset)
-                            .map_err(AsyncIoError::WriteVectored)?;
-                    }
                 }
             }
             buf_offset += count;

block/src/formats/qcow/worker/sync.rs

@@ -18,7 +18,6 @@ use super::internal::metadata::{
     BackingRead, ClusterReadMapping, ClusterWriteMapping, DeallocAction, QcowMetadata,
 };
 use super::internal::qcow_raw_file::QcowRawFile;
-use crate::aligned_buffer::AlignedBuffer;
 use crate::async_io::{AsyncIo, AsyncIoCompletion, AsyncIoError, AsyncIoOperation, AsyncIoResult};
 
 pub struct QcowSync {
@@ -27,8 +26,6 @@ pub struct QcowSync {
     /// See the backing_file field on QcowDisk.
     backing_file: Option<Arc<dyn BackingRead>>,
     sparse: bool,
-    /// O_DIRECT alignment requirement (0 = no alignment needed).
-    alignment: usize,
     cluster_size: u64,
     decoder: Arc<dyn Decoder>,
     eventfd: EventFd,
@@ -42,7 +39,6 @@ impl QcowSync {
         backing_file: Option<Arc<dyn BackingRead>>,
         sparse: bool,
     ) -> Self {
-        let alignment = data_file.file().alignment();
         QcowSync {
             cluster_size: metadata.cluster_size(),
             decoder: metadata.decoder(),
@@ -50,7 +46,6 @@ impl QcowSync {
             data_file,
             backing_file,
             sparse,
-            alignment,
             eventfd: EventFd::new(libc::EFD_NONBLOCK)
                 .expect("Failed creating EventFd for QcowSync"),
             completion_list: VecDeque::new(),
@@ -100,22 +95,13 @@ impl QcowSync {
                     length,
                 } => {
                     let len = length as usize;
-                    if self.alignment > 0 {
+                    let mut buf = vec![0u8; len];
-                        let mut abuf = AlignedBuffer::new(host_offset, len, self.alignment)
+                    self.data_file
-                            .map_err(AsyncIoError::ReadVectored)?;
+                        .file()
-                        abuf.read_exact_from(self.data_file.file())
+                        .read_exact_at(&mut buf, host_offset)
-                            .map_err(AsyncIoError::ReadVectored)?;
+                        .map_err(AsyncIoError::ReadVectored)?;
-                        op.write_bytes_at(buf_offset, abuf.as_slice())
+                    op.write_bytes_at(buf_offset, &buf)
-                            .map_err(AsyncIoError::ReadVectored)?;
+                        .map_err(AsyncIoError::ReadVectored)?;
-                    } else {
-                        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 {
@@ -196,24 +182,13 @@ impl QcowSync {
                 ClusterWriteMapping::Allocated {
                     offset: host_offset,
                 } => {
-                    if self.alignment > 0 {
+                    let mut buf = vec![0u8; count];
-                        let mut abuf = AlignedBuffer::new(host_offset, count, self.alignment)
+                    op.read_bytes_at(buf_offset, &mut buf)
-                            .map_err(AsyncIoError::WriteVectored)?;
+                        .map_err(AsyncIoError::WriteVectored)?;
-                        abuf.read_exact_from(self.data_file.file())
+                    self.data_file
-                            .map_err(AsyncIoError::WriteVectored)?;
+                        .file()
-                        op.read_bytes_at(buf_offset, abuf.as_mut_slice())
+                        .write_all_at(&buf, host_offset)
-                            .map_err(AsyncIoError::WriteVectored)?;
+                        .map_err(AsyncIoError::WriteVectored)?;
-                        abuf.write_to(self.data_file.file())
-                            .map_err(AsyncIoError::WriteVectored)?;
-                    } else {
-                        let mut buf = vec![0u8; count];
-                        op.read_bytes_at(buf_offset, &mut buf)
-                            .map_err(AsyncIoError::WriteVectored)?;
-                        self.data_file
-                            .file()
-                            .write_all_at(&buf, host_offset)
-                            .map_err(AsyncIoError::WriteVectored)?;
-                    }
                 }
             }
             buf_offset += count;