From eeb71091db3f9d4d4b2a5f79a0eac2cba12c719e Mon Sep 17 00:00:00 2001 From: Anatol Belski Date: Tue, 28 Jul 2026 14:32:47 +0200 Subject: [PATCH] block: qcow: Share the deallocation path between the two engines The sync and uring qcow engines each carried an identical apply_dealloc_action plus punch_hole and write_zeroes bodies that differed only by a flag and the completion sink. Move that shared deallocation logic into common.rs as apply_dealloc_action and deallocate_range_result. Each engine now calls it and injects the completion through its own queue. Signed-off-by: Anatol Belski --- block/src/formats/qcow/common.rs | 67 +++++++++++++ block/src/formats/qcow/engine_sync.rs | 133 ++++++------------------- block/src/formats/qcow/engine_uring.rs | 128 +++++------------------- 3 files changed, 122 insertions(+), 206 deletions(-) diff --git a/block/src/formats/qcow/common.rs b/block/src/formats/qcow/common.rs index d4f9f25db..b78d8fef6 100644 --- a/block/src/formats/qcow/common.rs +++ b/block/src/formats/qcow/common.rs @@ -10,7 +10,12 @@ use std::io; +use vmm_sys_util::write_zeroes::{PunchHole, WriteZeroesAt}; + use super::decoder::Decoder; +use super::metadata::{BackingRead, DeallocAction, QcowMetadata}; +use super::qcow_raw_file::QcowRawFile; +use crate::async_io::AsyncIoError; /// Decompress a full QCOW2 cluster from compressed data. /// @@ -32,6 +37,68 @@ pub(super) fn decompress_cluster( Ok(decompressed) } +/// Applies one deallocation action to the data file and refcount table. +pub(super) fn apply_dealloc_action( + metadata: &QcowMetadata, + data_file: &mut QcowRawFile, + action: &DeallocAction, +) -> io::Result<()> { + match action { + DeallocAction::PunchHole { + host_offset, + length, + } => { + data_file.file_mut().punch_hole(*host_offset, *length)?; + metadata.complete_punch_hole(*host_offset); + Ok(()) + } + DeallocAction::WriteZeroes { + host_offset, + length, + } => data_file + .file_mut() + .write_zeroes_at(*host_offset, *length) + .map(|_| ()), + } +} + +/// Deallocates a byte range and returns the completion result, 0 on +/// success or a negative errno on the first failing action. +pub(super) fn deallocate_range_result( + metadata: &QcowMetadata, + data_file: &mut QcowRawFile, + offset: u64, + length: usize, + sparse: bool, + write_zeroes: bool, + backing_file: Option<&dyn BackingRead>, +) -> i32 { + let wrap_error: fn(io::Error) -> AsyncIoError = if write_zeroes { + AsyncIoError::WriteZeroes + } else { + AsyncIoError::PunchHole + }; + let result = metadata + .deallocate_bytes(offset, length, sparse, write_zeroes, backing_file) + .and_then(|actions| { + let mut first_error = None; + for action in &actions { + if let Err(e) = apply_dealloc_action(metadata, data_file, action) { + first_error.get_or_insert(e); + } + } + first_error.map_or(Ok(()), Err) + }) + .map_err(wrap_error); + match result { + Ok(()) => 0, + Err(AsyncIoError::PunchHole(e) | AsyncIoError::WriteZeroes(e)) => { + -e.raw_os_error().unwrap_or(libc::EIO) + } + Err(_) => -libc::EIO, + } +} + #[cfg(test)] pub(crate) mod unit_tests { use std::fs::File; diff --git a/block/src/formats/qcow/engine_sync.rs b/block/src/formats/qcow/engine_sync.rs index 9b3b7f766..96fb70e0a 100644 --- a/block/src/formats/qcow/engine_sync.rs +++ b/block/src/formats/qcow/engine_sync.rs @@ -5,18 +5,14 @@ // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause use std::cmp::min; -use std::io; use std::os::unix::fs::FileExt; use std::sync::Arc; use vmm_sys_util::eventfd::EventFd; -use vmm_sys_util::write_zeroes::{PunchHole, WriteZeroesAt}; -use super::common::decompress_cluster; +use super::common::{deallocate_range_result, decompress_cluster}; use super::decoder::Decoder; -use super::metadata::{ - BackingRead, ClusterReadMapping, ClusterWriteMapping, DeallocAction, QcowMetadata, -}; +use super::metadata::{BackingRead, ClusterReadMapping, ClusterWriteMapping, QcowMetadata}; use super::qcow_raw_file::QcowRawFile; use crate::async_io::{ AsyncIo, AsyncIoCompletion, AsyncIoError, AsyncIoOperation, AsyncIoResult, CompletionCommon, @@ -51,29 +47,6 @@ impl QcowSync { } } - fn apply_dealloc_action(&mut self, action: &DeallocAction) -> io::Result<()> { - match action { - DeallocAction::PunchHole { - host_offset, - length, - } => { - self.data_file - .file_mut() - .punch_hole(*host_offset, *length)?; - self.metadata.complete_punch_hole(*host_offset); - Ok(()) - } - DeallocAction::WriteZeroes { - host_offset, - length, - } => self - .data_file - .file_mut() - .write_zeroes_at(*host_offset, *length) - .map(|_| ()), - } - } - fn read_operation(&mut self, op: &mut AsyncIoOperation) -> AsyncIoResult { let address = op.offset() as u64; let total_len = op.total_len(); @@ -231,83 +204,33 @@ impl AsyncIo for QcowSync { } fn punch_hole(&mut self, offset: u64, length: u64, user_data: u64) -> AsyncIoResult<()> { - let result = self - .metadata - .deallocate_bytes( - offset, - length as usize, - self.sparse, - false, - self.backing_file.as_deref(), - ) - .and_then(|actions| { - let mut first_error = None; - for action in &actions { - if let Err(e) = self.apply_dealloc_action(action) { - first_error.get_or_insert(e); - } - } - first_error.map_or(Ok(()), Err) - }) - .map_err(AsyncIoError::PunchHole); - - match result { - Ok(()) => { - self.completions - .complete(AsyncIoCompletion::new(user_data, 0, None)); - Ok(()) - } - Err(e) => { - let errno = if let AsyncIoError::PunchHole(ref io_err) = e { - -io_err.raw_os_error().unwrap_or(libc::EIO) - } else { - -libc::EIO - }; - self.completions - .complete(AsyncIoCompletion::new(user_data, errno, None)); - Ok(()) - } - } + let result = deallocate_range_result( + &self.metadata, + &mut self.data_file, + offset, + length as usize, + self.sparse, + false, + self.backing_file.as_deref(), + ); + self.completions + .complete(AsyncIoCompletion::new(user_data, result, None)); + Ok(()) } fn write_zeroes(&mut self, offset: u64, length: u64, user_data: u64) -> AsyncIoResult<()> { - let result = self - .metadata - .deallocate_bytes( - offset, - length as usize, - self.sparse, - true, - self.backing_file.as_deref(), - ) - .and_then(|actions| { - let mut first_error = None; - for action in &actions { - if let Err(e) = self.apply_dealloc_action(action) { - first_error.get_or_insert(e); - } - } - first_error.map_or(Ok(()), Err) - }) - .map_err(AsyncIoError::WriteZeroes); - - match result { - Ok(()) => { - self.completions - .complete(AsyncIoCompletion::new(user_data, 0, None)); - Ok(()) - } - Err(e) => { - let errno = if let AsyncIoError::WriteZeroes(ref io_err) = e { - -io_err.raw_os_error().unwrap_or(libc::EIO) - } else { - -libc::EIO - }; - self.completions - .complete(AsyncIoCompletion::new(user_data, errno, None)); - Ok(()) - } - } + let result = deallocate_range_result( + &self.metadata, + &mut self.data_file, + offset, + length as usize, + self.sparse, + true, + self.backing_file.as_deref(), + ); + self.completions + .complete(AsyncIoCompletion::new(user_data, result, None)); + Ok(()) } } @@ -329,7 +252,9 @@ mod unit_tests { use crate::disk_file::{AsyncDiskFile, DiskSize, MetadataSync, Resizable}; use crate::error::BlockErrorKind; 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::{ BackingFileConfig, Error as QcowError, ImageType, QcowDisk, QcowHeader, QcowTempDisk, }; @@ -2311,7 +2236,7 @@ mod unit_tests { // Queue A resumes. Completing the old action must not touch the new // L2, and only now may the old data cluster enter unref_clusters. - aio.apply_dealloc_action(&actions[0]).unwrap(); + apply_dealloc_action(&aio.metadata, &mut aio.data_file, &actions[0]).unwrap(); metadata.flush().unwrap(); drop(aio); drop(metadata); diff --git a/block/src/formats/qcow/engine_uring.rs b/block/src/formats/qcow/engine_uring.rs index 1c08c40a1..1335b550c 100644 --- a/block/src/formats/qcow/engine_uring.rs +++ b/block/src/formats/qcow/engine_uring.rs @@ -15,13 +15,10 @@ use std::os::unix::io::AsRawFd; use std::sync::Arc; use vmm_sys_util::eventfd::EventFd; -use vmm_sys_util::write_zeroes::{PunchHole, WriteZeroesAt}; -use super::common::decompress_cluster; +use super::common::{deallocate_range_result, decompress_cluster}; use super::decoder::Decoder; -use super::metadata::{ - BackingRead, ClusterReadMapping, ClusterWriteMapping, DeallocAction, QcowMetadata, -}; +use super::metadata::{BackingRead, ClusterReadMapping, ClusterWriteMapping, QcowMetadata}; use super::qcow_raw_file::QcowRawFile; use crate::async_io::{ AsyncIo, AsyncIoCompletion, AsyncIoError, AsyncIoOperation, AsyncIoResult, UringDataIo, @@ -66,29 +63,6 @@ impl QcowAsync { }) } - fn apply_dealloc_action(&mut self, action: &DeallocAction) -> io::Result<()> { - match action { - DeallocAction::PunchHole { - host_offset, - length, - } => { - self.data_file - .file_mut() - .punch_hole(*host_offset, *length)?; - self.metadata.complete_punch_hole(*host_offset); - Ok(()) - } - DeallocAction::WriteZeroes { - host_offset, - length, - } => self - .data_file - .file_mut() - .write_zeroes_at(*host_offset, *length) - .map(|_| ()), - } - } - fn async_error_result(error: &AsyncIoError) -> i32 { let io_error = match error { AsyncIoError::ReadVectored(e) @@ -196,83 +170,33 @@ impl AsyncIo for QcowAsync { } fn punch_hole(&mut self, offset: u64, length: u64, user_data: u64) -> AsyncIoResult<()> { - let result = self - .metadata - .deallocate_bytes( - offset, - length as usize, - self.sparse, - false, - self.backing_file.as_deref(), - ) - .and_then(|actions| { - let mut first_error = None; - for action in &actions { - if let Err(e) = self.apply_dealloc_action(action) { - first_error.get_or_insert(e); - } - } - first_error.map_or(Ok(()), Err) - }) - .map_err(AsyncIoError::PunchHole); - - match result { - Ok(()) => { - self.data_io - .inject_completion(AsyncIoCompletion::new(user_data, 0, None)); - Ok(()) - } - Err(e) => { - let errno = if let AsyncIoError::PunchHole(ref io_err) = e { - -io_err.raw_os_error().unwrap_or(libc::EIO) - } else { - -libc::EIO - }; - self.data_io - .inject_completion(AsyncIoCompletion::new(user_data, errno, None)); - Ok(()) - } - } + let result = deallocate_range_result( + &self.metadata, + &mut self.data_file, + offset, + length as usize, + self.sparse, + false, + self.backing_file.as_deref(), + ); + self.data_io + .inject_completion(AsyncIoCompletion::new(user_data, result, None)); + Ok(()) } fn write_zeroes(&mut self, offset: u64, length: u64, user_data: u64) -> AsyncIoResult<()> { - let result = self - .metadata - .deallocate_bytes( - offset, - length as usize, - self.sparse, - true, - self.backing_file.as_deref(), - ) - .and_then(|actions| { - let mut first_error = None; - for action in &actions { - if let Err(e) = self.apply_dealloc_action(action) { - first_error.get_or_insert(e); - } - } - first_error.map_or(Ok(()), Err) - }) - .map_err(AsyncIoError::WriteZeroes); - - match result { - Ok(()) => { - self.data_io - .inject_completion(AsyncIoCompletion::new(user_data, 0, None)); - Ok(()) - } - Err(e) => { - let errno = if let AsyncIoError::WriteZeroes(ref io_err) = e { - -io_err.raw_os_error().unwrap_or(libc::EIO) - } else { - -libc::EIO - }; - self.data_io - .inject_completion(AsyncIoCompletion::new(user_data, errno, None)); - Ok(()) - } - } + let result = deallocate_range_result( + &self.metadata, + &mut self.data_file, + offset, + length as usize, + self.sparse, + true, + self.backing_file.as_deref(), + ); + self.data_io + .inject_completion(AsyncIoCompletion::new(user_data, result, None)); + Ok(()) } fn batch_requests_enabled(&self) -> bool {