diff --git a/block/src/qcow/backing.rs b/block/src/qcow/backing.rs index 754618f13..6b8448861 100644 --- a/block/src/qcow/backing.rs +++ b/block/src/qcow/backing.rs @@ -11,9 +11,10 @@ use std::os::fd::{AsFd, AsRawFd, BorrowedFd, OwnedFd}; use std::sync::Arc; use crate::error::{BlockError, BlockErrorKind, BlockResult, ErrorOp}; +use crate::qcow::decoder::Decoder; use crate::qcow::metadata::{BackingRead, ClusterReadMapping, QcowMetadata}; use crate::qcow::{BackingFile, BackingKind, Error as QcowError}; -use crate::qcow_common::pread_exact; +use crate::qcow_common::{decompress_cluster, pread_alloc, pread_exact}; /// Raw backing file using pread64 on a duplicated fd. pub(crate) struct RawBacking { @@ -52,6 +53,8 @@ pub(crate) struct Qcow2Backing { pub(crate) metadata: Arc, pub(crate) data_fd: OwnedFd, pub(crate) backing_file: Option>, + pub(crate) cluster_size: u64, + pub(crate) decoder: Arc, } // SAFETY: All reads go through QcowMetadata which uses RwLock @@ -104,10 +107,22 @@ impl Qcow2Backing { )?; buf_offset += length as usize; } - ClusterReadMapping::Compressed { data } => { - let len = data.len(); - buf[buf_offset..buf_offset + len].copy_from_slice(&data); - buf_offset += len; + ClusterReadMapping::Compressed { + host_offset, + compressed_size, + cluster_offset, + length, + } => { + let compressed = + pread_alloc(self.data_fd.as_raw_fd(), host_offset, compressed_size)?; + let decompressed = decompress_cluster( + &compressed, + self.cluster_size as usize, + &*self.decoder, + )?; + buf[buf_offset..buf_offset + length] + .copy_from_slice(&decompressed[cluster_offset..cluster_offset + length]); + buf_offset += length; } ClusterReadMapping::Backing { offset: backing_offset, @@ -152,8 +167,11 @@ pub fn shared_backing_from(bf: BackingFile) -> BlockResult> } BackingKind::Qcow { inner, backing } => { let data_fd = dup_fd(inner.raw_file.as_fd())?; + let metadata = Arc::new(QcowMetadata::new(*inner)); Ok(Arc::new(Qcow2Backing { - metadata: Arc::new(QcowMetadata::new(*inner)), + cluster_size: metadata.cluster_size(), + decoder: metadata.decoder(), + metadata, data_fd, backing_file: backing.map(|bf| shared_backing_from(*bf)).transpose()?, })) diff --git a/block/src/qcow/decoder.rs b/block/src/qcow/decoder.rs index f1237aae7..f9510baf9 100644 --- a/block/src/qcow/decoder.rs +++ b/block/src/qcow/decoder.rs @@ -19,7 +19,7 @@ pub enum Error { pub type Result = std::result::Result; /// Generic trait for decoding zlib/zstd formats -pub trait Decoder { +pub trait Decoder: Send + Sync { fn decode(&self, input: &[u8], output: &mut [u8]) -> Result; } diff --git a/block/src/qcow/metadata.rs b/block/src/qcow/metadata.rs index 37bb7847b..d7c38c9fc 100644 --- a/block/src/qcow/metadata.rs +++ b/block/src/qcow/metadata.rs @@ -19,10 +19,11 @@ use std::cmp::min; use std::io::{self, Seek}; use std::mem; -use std::sync::RwLock; +use std::sync::{Arc, RwLock}; use libc::{EINVAL, EIO}; +use super::decoder::Decoder; use super::qcow_raw_file::QcowRawFile; use super::refcount::RefCount; use super::util::{ @@ -51,13 +52,16 @@ pub enum ClusterReadMapping { /// bounded by cluster boundary and guest request. Allocated { offset: u64, length: u64 }, - /// The cluster is compressed. The decompressed data is returned inline - /// because decompression is a CPU only operation that was done under the - /// write lock to access the raw compressed bytes from disk. - /// - /// The data field contains exactly the bytes the guest requested, already - /// sliced from the decompressed cluster. - Compressed { data: Vec }, + /// The cluster is compressed. The host file offset and compressed byte + /// count are extracted from the L2 entry under the read lock. The caller + /// reads the compressed data with pread on its own fd, decompresses + /// into a cluster sized buffer, then slices the requested range. + Compressed { + host_offset: u64, + compressed_size: usize, + cluster_offset: usize, + length: usize, + }, /// The cluster is not allocated in this layer but may exist in a backing /// file. The caller should delegate to the backing file at the given @@ -112,6 +116,7 @@ pub enum DeallocAction { /// write lock, so contention stays low and queues scale. pub struct QcowMetadata { inner: RwLock, + decoder: Arc, } /// The actual metadata state, accessible only through the RwLock. @@ -132,6 +137,7 @@ pub(crate) struct QcowState { impl QcowMetadata { pub(crate) fn new(inner: QcowState) -> Self { QcowMetadata { + decoder: Arc::from(inner.header.get_decoder()), inner: RwLock::new(inner), } } @@ -333,6 +339,11 @@ impl QcowMetadata { pub fn cluster_size(&self) -> u64 { self.inner.read().unwrap().raw_file.cluster_size() } + + /// Returns the shared decoder matching the image compression type. + pub fn decoder(&self) -> Arc { + Arc::clone(&self.decoder) + } } impl QcowState { @@ -373,10 +384,19 @@ impl QcowState { let l2_index = self.l2_table_index(address) as usize; let l2_entry = l2_table[l2_index]; - // Compressed entries require disk I/O for decompression - can't do - // that under a read lock. Fall through to the write lock path. + // Compressed entries: extract layout from L2 entry under read lock. + // The caller reads and decompresses on its own fd without holding + // the metadata lock. if l2_entry_is_compressed(l2_entry) { - return Ok(None); + let (host_offset, compressed_size) = + l2_entry_compressed_cluster_layout(l2_entry, self.header.cluster_bits); + let cluster_offset = self.raw_file.cluster_offset(address) as usize; + return Ok(Some(ClusterReadMapping::Compressed { + host_offset, + compressed_size, + cluster_offset, + length: count, + })); } if l2_entry_is_empty(l2_entry) { @@ -439,17 +459,14 @@ impl QcowState { if l2_entry_is_empty(l2_entry) { Ok(self.unallocated_read_mapping(address, count, has_backing_file)) } else if l2_entry_is_compressed(l2_entry) { - // Under write lock we can do I/O for decompression - let decompressed = self.decompress_l2_cluster(l2_entry)?; - let start = self.raw_file.cluster_offset(address) as usize; - let end = start - .checked_add(count) - .ok_or_else(|| io::Error::from_raw_os_error(EINVAL))?; - if end > decompressed.len() { - return Err(io::Error::from_raw_os_error(EINVAL)); - } + let (host_offset, compressed_size) = + l2_entry_compressed_cluster_layout(l2_entry, self.header.cluster_bits); + let cluster_offset = self.raw_file.cluster_offset(address) as usize; Ok(ClusterReadMapping::Compressed { - data: decompressed[start..end].to_vec(), + host_offset, + compressed_size, + cluster_offset, + length: count, }) } else if l2_entry_is_zero(l2_entry) { // Match original QcowFile::file_read semantics where zero flagged diff --git a/block/src/qcow/mod.rs b/block/src/qcow/mod.rs index 8c0abb5da..3e629bf57 100644 --- a/block/src/qcow/mod.rs +++ b/block/src/qcow/mod.rs @@ -57,6 +57,7 @@ use crate::qcow::qcow_raw_file::{BeUint, QcowRawFile}; pub use crate::qcow::raw_file::RawFile; use crate::qcow::refcount::RefCount; use crate::qcow::vec_cache::{CacheMap, Cacheable, VecCache}; +use crate::qcow_common::decompress_cluster; #[sorted] #[derive(Debug, Error)] @@ -322,8 +323,26 @@ impl BackingFile { .file_mut() .read_exact(&mut buf[pos..pos + length as usize])?; } - ClusterReadMapping::Compressed { data } => { - buf[pos..pos + data.len()].copy_from_slice(&data); + ClusterReadMapping::Compressed { + host_offset, + compressed_size, + cluster_offset, + length, + } => { + let mut compressed = vec![0u8; compressed_size]; + inner + .raw_file + .file_mut() + .seek(SeekFrom::Start(host_offset))?; + inner.raw_file.file_mut().read_exact(&mut compressed)?; + let decompressed = decompress_cluster( + &compressed, + cluster_size as usize, + &*inner.header.get_decoder(), + )?; + buf[pos..pos + length].copy_from_slice( + &decompressed[cluster_offset..cluster_offset + length], + ); } ClusterReadMapping::Backing { offset: backing_off, diff --git a/block/src/qcow_async.rs b/block/src/qcow_async.rs index a0f470674..cdcd548f3 100644 --- a/block/src/qcow_async.rs +++ b/block/src/qcow_async.rs @@ -21,14 +21,15 @@ use vmm_sys_util::write_zeroes::{PunchHole, WriteZeroesAt}; use crate::async_io::{AsyncIo, AsyncIoError, AsyncIoResult, BorrowedDiskFd, DiskFileError}; use crate::error::{BlockError, BlockErrorKind, BlockResult, ErrorOp}; use crate::qcow::backing::shared_backing_from; +use crate::qcow::decoder::Decoder; use crate::qcow::metadata::{ BackingRead, ClusterReadMapping, ClusterWriteMapping, DeallocAction, QcowMetadata, }; use crate::qcow::qcow_raw_file::QcowRawFile; use crate::qcow::{MAX_NESTING_DEPTH, RawFile, parse_qcow}; use crate::qcow_common::{ - AlignedBuf, aligned_pread, aligned_pwrite, gather_from_iovecs_into, pread_exact, pwrite_all, - scatter_to_iovecs, zero_fill_iovecs, + AlignedBuf, aligned_pread, aligned_pwrite, decompress_cluster, gather_from_iovecs_into, + pread_alloc, pread_exact, pwrite_all, scatter_to_iovecs, zero_fill_iovecs, }; use crate::{BatchRequest, RequestType, SECTOR_SIZE, disk_file}; @@ -178,6 +179,7 @@ pub struct QcowAsync { /// I/O alignment for the AsyncIo trait (at least SECTOR_SIZE). io_alignment: u64, cluster_size: u64, + decoder: Arc, io_uring: IoUring, eventfd: EventFd, completion_list: VecDeque<(u64, i32)>, @@ -199,6 +201,7 @@ impl QcowAsync { Ok(QcowAsync { cluster_size: metadata.cluster_size(), + decoder: metadata.decoder(), metadata, data_file, backing_file, @@ -253,6 +256,8 @@ impl AsyncIo for QcowAsync { iovecs, total_len, self.alignment, + self.cluster_size, + &*self.decoder, )? { let fd = self.data_file.as_raw_fd(); let (submitter, mut sq, _) = self.io_uring.split(); @@ -396,6 +401,8 @@ impl AsyncIo for QcowAsync { &req.iovecs, total_len, self.alignment, + self.cluster_size, + &*self.decoder, )? { let fd = self.data_file.as_raw_fd(); // SAFETY: fd is valid and iovecs point to valid guest memory. @@ -462,6 +469,7 @@ impl QcowAsync { /// Returns `Some(host_offset)` if the entire read falls within a single /// allocated cluster (fast path). Otherwise handles the read /// synchronously via `scatter_read_sync` and returns `None`. + #[allow(clippy::too_many_arguments)] fn resolve_read( metadata: &QcowMetadata, data_file: &QcowRawFile, @@ -470,6 +478,8 @@ impl QcowAsync { iovecs: &[libc::iovec], total_len: usize, alignment: usize, + cluster_size: u64, + decoder: &dyn Decoder, ) -> AsyncIoResult> { let has_backing = backing_file.is_some(); let mappings = metadata @@ -494,7 +504,15 @@ impl QcowAsync { return Ok(Some(*host_offset)); } - Self::scatter_read_sync(mappings, iovecs, data_file, backing_file, alignment)?; + Self::scatter_read_sync( + mappings, + iovecs, + data_file, + backing_file, + alignment, + cluster_size, + decoder, + )?; Ok(None) } @@ -505,6 +523,8 @@ impl QcowAsync { data_file: &QcowRawFile, backing_file: &Option>, alignment: usize, + cluster_size: u64, + decoder: &dyn Decoder, ) -> AsyncIoResult<()> { let mut buf_offset = 0usize; for mapping in mappings { @@ -542,11 +562,27 @@ impl QcowAsync { } buf_offset += len; } - ClusterReadMapping::Compressed { data } => { - let len = data.len(); + ClusterReadMapping::Compressed { + host_offset, + compressed_size, + cluster_offset, + length, + } => { + let compressed = + pread_alloc(data_file.as_raw_fd(), host_offset, compressed_size) + .map_err(AsyncIoError::ReadVectored)?; + let decompressed = + decompress_cluster(&compressed, cluster_size as usize, decoder) + .map_err(AsyncIoError::ReadVectored)?; // SAFETY: iovecs point to valid guest memory buffers. - unsafe { scatter_to_iovecs(iovecs, buf_offset, &data) }; - buf_offset += len; + unsafe { + scatter_to_iovecs( + iovecs, + buf_offset, + &decompressed[cluster_offset..cluster_offset + length], + ); + } + buf_offset += length; } ClusterReadMapping::Backing { offset: backing_offset, diff --git a/block/src/qcow_sync.rs b/block/src/qcow_sync.rs index fd2c61143..d33a2fd6a 100644 --- a/block/src/qcow_sync.rs +++ b/block/src/qcow_sync.rs @@ -16,14 +16,16 @@ use crate::async_io::{AsyncIo, AsyncIoError, AsyncIoResult, BorrowedDiskFd, Disk use crate::disk_file; use crate::error::{BlockError, BlockErrorKind, BlockResult, ErrorOp}; use crate::qcow::backing::shared_backing_from; +use crate::qcow::decoder::Decoder; use crate::qcow::metadata::{ BackingRead, ClusterReadMapping, ClusterWriteMapping, DeallocAction, QcowMetadata, }; use crate::qcow::qcow_raw_file::QcowRawFile; use crate::qcow::{MAX_NESTING_DEPTH, RawFile, parse_qcow}; use crate::qcow_common::{ - AlignedBuf, aligned_pread, aligned_pwrite, gather_from_iovecs, gather_from_iovecs_into, - pread_exact, pwrite_all, scatter_to_iovecs, zero_fill_iovecs, + AlignedBuf, aligned_pread, aligned_pwrite, decompress_cluster, gather_from_iovecs, + gather_from_iovecs_into, pread_alloc, pread_exact, pwrite_all, scatter_to_iovecs, + zero_fill_iovecs, }; pub struct QcowDiskSync { @@ -157,6 +159,7 @@ pub struct QcowSync { /// O_DIRECT alignment requirement (0 = no alignment needed). alignment: usize, cluster_size: u64, + decoder: Arc, eventfd: EventFd, completion_list: VecDeque<(u64, i32)>, } @@ -171,6 +174,7 @@ impl QcowSync { let alignment = data_file.file().alignment(); QcowSync { cluster_size: metadata.cluster_size(), + decoder: metadata.decoder(), metadata, data_file, backing_file, @@ -239,11 +243,27 @@ impl AsyncIo for QcowSync { } buf_offset += len; } - ClusterReadMapping::Compressed { data } => { - let len = data.len(); + ClusterReadMapping::Compressed { + host_offset, + compressed_size, + cluster_offset, + length, + } => { + let compressed = + pread_alloc(self.data_file.as_raw_fd(), host_offset, compressed_size) + .map_err(AsyncIoError::ReadVectored)?; + let decompressed = + decompress_cluster(&compressed, self.cluster_size as usize, &*self.decoder) + .map_err(AsyncIoError::ReadVectored)?; // SAFETY: iovecs point to valid guest memory buffers - unsafe { scatter_to_iovecs(iovecs, buf_offset, &data) }; - buf_offset += len; + unsafe { + scatter_to_iovecs( + iovecs, + buf_offset, + &decompressed[cluster_offset..cluster_offset + length], + ); + } + buf_offset += length; } ClusterReadMapping::Backing { offset: backing_offset,