From 3f20fd075930f8dd0d3ee60b30979249f6c23e38 Mon Sep 17 00:00:00 2001 From: Anatol Belski Date: Fri, 19 Jun 2026 22:24:30 +0200 Subject: [PATCH] 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 --- block/src/aligned_buffer.rs | 18 ++--- block/src/formats/qcow/worker/async_uring.rs | 82 ++++---------------- block/src/formats/qcow/worker/sync.rs | 53 ++++--------- 3 files changed, 37 insertions(+), 116 deletions(-) diff --git a/block/src/aligned_buffer.rs b/block/src/aligned_buffer.rs index 232fb9ef9..b62f9698b 100644 --- a/block/src/aligned_buffer.rs +++ b/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 = (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 = (offset as usize..offset as usize + len) .map(|i| (i % 251) as u8) @@ -230,7 +224,7 @@ mod tests { let data: Vec = (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 = (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]; diff --git a/block/src/formats/qcow/worker/async_uring.rs b/block/src/formats/qcow/worker/async_uring.rs index c76cea324..24dab966d 100644 --- a/block/src/formats/qcow/worker/async_uring.rs +++ b/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>, 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, } @@ -61,9 +55,6 @@ impl QcowAsync { sparse: bool, ring_depth: u32, ) -> io::Result { - 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) -> 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> { @@ -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( - mappings, - op, - data_file, - backing_file, - alignment, - cluster_size, - decoder, - )?; + Self::scatter_read_sync(mappings, op, data_file, backing_file, cluster_size, decoder)?; Ok(None) } @@ -375,7 +349,6 @@ impl QcowAsync { op: &mut AsyncIoOperation, data_file: &QcowRawFile, backing_file: &Option>, - 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 abuf = AlignedBuffer::new(host_offset, len, alignment) - .map_err(AsyncIoError::ReadVectored)?; - abuf.read_exact_from(data_file.file()) - .map_err(AsyncIoError::ReadVectored)?; - op.write_bytes_at(buf_offset, abuf.as_slice()) - .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)?; - } + 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>, - 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 abuf = AlignedBuffer::new(host_offset, count, alignment) - .map_err(AsyncIoError::WriteVectored)?; - abuf.read_exact_from(data_file.file()) - .map_err(AsyncIoError::WriteVectored)?; - op.read_bytes_at(buf_offset, abuf.as_mut_slice()) - .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)?; - } + 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; diff --git a/block/src/formats/qcow/worker/sync.rs b/block/src/formats/qcow/worker/sync.rs index a7171600f..56732b058 100644 --- a/block/src/formats/qcow/worker/sync.rs +++ b/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>, sparse: bool, - /// O_DIRECT alignment requirement (0 = no alignment needed). - alignment: usize, cluster_size: u64, decoder: Arc, eventfd: EventFd, @@ -42,7 +39,6 @@ impl QcowSync { backing_file: Option>, 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 abuf = AlignedBuffer::new(host_offset, len, self.alignment) - .map_err(AsyncIoError::ReadVectored)?; - abuf.read_exact_from(self.data_file.file()) - .map_err(AsyncIoError::ReadVectored)?; - op.write_bytes_at(buf_offset, abuf.as_slice()) - .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)?; - } + 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 abuf = AlignedBuffer::new(host_offset, count, self.alignment) - .map_err(AsyncIoError::WriteVectored)?; - abuf.read_exact_from(self.data_file.file()) - .map_err(AsyncIoError::WriteVectored)?; - op.read_bytes_at(buf_offset, abuf.as_mut_slice()) - .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)?; - } + 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;