From 25b270010fa136f378f321f199464e19d71a2bdc Mon Sep 17 00:00:00 2001 From: Anatol Belski Date: Tue, 28 Jul 2026 14:39:48 +0200 Subject: [PATCH] block: qcow: Share the synchronous write path between the two engines Both engines ran the same copy on write loop for a partial cluster write. In the uring engine it was a separate helper, cow_write_sync. In the sync engine the same loop sat directly in the write path. Move cow_write_sync into common.rs and call it from both. The uring write already runs synchronously, so its behavior does not change. Signed-off-by: Anatol Belski --- block/src/formats/qcow/common.rs | 63 ++++++++++++++++++++++- block/src/formats/qcow/engine_sync.rs | 70 +++++--------------------- block/src/formats/qcow/engine_uring.rs | 61 ++-------------------- 3 files changed, 76 insertions(+), 118 deletions(-) diff --git a/block/src/formats/qcow/common.rs b/block/src/formats/qcow/common.rs index b78d8fef6..5969b7e45 100644 --- a/block/src/formats/qcow/common.rs +++ b/block/src/formats/qcow/common.rs @@ -8,14 +8,17 @@ //! Shared helpers for QCOW2 sync and async backends. +use std::cmp::min; use std::io; +use std::os::unix::fs::FileExt; +use std::sync::Arc; use vmm_sys_util::write_zeroes::{PunchHole, WriteZeroesAt}; use super::decoder::Decoder; -use super::metadata::{BackingRead, DeallocAction, QcowMetadata}; +use super::metadata::{BackingRead, ClusterWriteMapping, DeallocAction, QcowMetadata}; use super::qcow_raw_file::QcowRawFile; -use crate::async_io::AsyncIoError; +use crate::async_io::{AsyncIoError, AsyncIoOperation, AsyncIoResult}; /// Decompress a full QCOW2 cluster from compressed data. /// @@ -99,6 +102,62 @@ pub(super) fn deallocate_range_result( } } +/// Writes an operation to the data file cluster by cluster, allocating +/// and copying up backing data as needed. Writes are synchronous because +/// the host offset is only known after the metadata allocation. +pub(super) fn cow_write_sync( + address: u64, + op: &AsyncIoOperation, + metadata: &QcowMetadata, + data_file: &QcowRawFile, + backing_file: &Option>, + cluster_size: u64, +) -> AsyncIoResult<()> { + let total_len = op.total_len(); + let mut buf_offset = 0usize; + + while buf_offset < total_len { + let curr_addr = address + buf_offset as u64; + let intra_offset = curr_addr & (cluster_size - 1); + let remaining_in_cluster = (cluster_size - intra_offset) as usize; + let count = min(total_len - buf_offset, remaining_in_cluster); + + let backing_data = if let Some(backing) = backing_file + .as_ref() + .filter(|_| intra_offset != 0 || count < cluster_size as usize) + { + let cluster_begin = curr_addr - intra_offset; + let mut data = vec![0u8; cluster_size as usize]; + backing + .read_at(cluster_begin, &mut data) + .map_err(AsyncIoError::WriteVectored)?; + Some(data) + } else { + None + }; + + let mapping = metadata + .map_cluster_for_write(curr_addr, backing_data) + .map_err(AsyncIoError::WriteVectored)?; + + match mapping { + ClusterWriteMapping::Allocated { + offset: host_offset, + } => { + 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; + } + Ok(()) +} + #[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 96fb70e0a..85f7d534a 100644 --- a/block/src/formats/qcow/engine_sync.rs +++ b/block/src/formats/qcow/engine_sync.rs @@ -4,15 +4,14 @@ // // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause -use std::cmp::min; use std::os::unix::fs::FileExt; use std::sync::Arc; use vmm_sys_util::eventfd::EventFd; -use super::common::{deallocate_range_result, decompress_cluster}; +use super::common::{cow_write_sync, deallocate_range_result, decompress_cluster}; use super::decoder::Decoder; -use super::metadata::{BackingRead, ClusterReadMapping, ClusterWriteMapping, QcowMetadata}; +use super::metadata::{BackingRead, ClusterReadMapping, QcowMetadata}; use super::qcow_raw_file::QcowRawFile; use crate::async_io::{ AsyncIo, AsyncIoCompletion, AsyncIoError, AsyncIoOperation, AsyncIoResult, CompletionCommon, @@ -119,58 +118,6 @@ impl QcowSync { Ok(total_len) } - - fn write_operation(&mut self, op: &AsyncIoOperation) -> AsyncIoResult { - let address = op.offset() as u64; - let total_len = op.total_len(); - let mut buf_offset = 0usize; - - while buf_offset < total_len { - let curr_addr = address + buf_offset as u64; - let intra_offset = curr_addr & (self.cluster_size - 1); - let remaining_in_cluster = (self.cluster_size - intra_offset) as usize; - let count = min(total_len - buf_offset, remaining_in_cluster); - - // Read backing data for COW if this is a partial cluster - // write to an unallocated cluster with a backing file. - let backing_data = if let Some(backing) = self - .backing_file - .as_ref() - .filter(|_| intra_offset != 0 || count < self.cluster_size as usize) - { - let cluster_begin = curr_addr - intra_offset; - let mut data = vec![0u8; self.cluster_size as usize]; - backing - .read_at(cluster_begin, &mut data) - .map_err(AsyncIoError::WriteVectored)?; - Some(data) - } else { - None - }; - - let mapping = self - .metadata - .map_cluster_for_write(curr_addr, backing_data) - .map_err(AsyncIoError::WriteVectored)?; - - match mapping { - ClusterWriteMapping::Allocated { - offset: host_offset, - } => { - 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; - } - - Ok(total_len) - } } impl AsyncIo for QcowSync { @@ -179,11 +126,18 @@ impl AsyncIo for QcowSync { } fn submit_data_operation(&mut self, mut op: AsyncIoOperation) -> AsyncIoResult<()> { - let is_read = op.is_read(); - let total_len = if is_read { + let total_len = if op.is_read() { self.read_operation(&mut op)? } else { - self.write_operation(&op)? + cow_write_sync( + op.offset() as u64, + &op, + &self.metadata, + &self.data_file, + &self.backing_file, + self.cluster_size, + )?; + op.total_len() }; self.completions .complete(AsyncIoCompletion::from_operation(op, total_len as i32)); diff --git a/block/src/formats/qcow/engine_uring.rs b/block/src/formats/qcow/engine_uring.rs index 1335b550c..81961c9dd 100644 --- a/block/src/formats/qcow/engine_uring.rs +++ b/block/src/formats/qcow/engine_uring.rs @@ -8,7 +8,6 @@ //! QCOW2 async disk backend. -use std::cmp::min; use std::io; use std::os::unix::fs::FileExt; use std::os::unix::io::AsRawFd; @@ -16,9 +15,9 @@ use std::sync::Arc; use vmm_sys_util::eventfd::EventFd; -use super::common::{deallocate_range_result, decompress_cluster}; +use super::common::{cow_write_sync, deallocate_range_result, decompress_cluster}; use super::decoder::Decoder; -use super::metadata::{BackingRead, ClusterReadMapping, ClusterWriteMapping, QcowMetadata}; +use super::metadata::{BackingRead, ClusterReadMapping, QcowMetadata}; use super::qcow_raw_file::QcowRawFile; use crate::async_io::{ AsyncIo, AsyncIoCompletion, AsyncIoError, AsyncIoOperation, AsyncIoResult, UringDataIo, @@ -118,7 +117,7 @@ impl QcowAsync { // write, L2 commit) with per request buffer lifetime tracking // and write ordering. let total_len = op.total_len(); - if let Err(e) = Self::cow_write_sync( + if let Err(e) = cow_write_sync( op.offset() as u64, &op, &self.metadata, @@ -352,60 +351,6 @@ impl QcowAsync { } Ok(()) } - - /// Write owned operation data cluster-by-cluster with COW from backing file. - fn cow_write_sync( - address: u64, - op: &AsyncIoOperation, - metadata: &QcowMetadata, - data_file: &QcowRawFile, - backing_file: &Option>, - cluster_size: u64, - ) -> AsyncIoResult<()> { - let total_len = op.total_len(); - let mut buf_offset = 0usize; - - while buf_offset < total_len { - let curr_addr = address + buf_offset as u64; - let intra_offset = curr_addr & (cluster_size - 1); - let remaining_in_cluster = (cluster_size - intra_offset) as usize; - let count = min(total_len - buf_offset, remaining_in_cluster); - - let backing_data = if let Some(backing) = backing_file - .as_ref() - .filter(|_| intra_offset != 0 || count < cluster_size as usize) - { - let cluster_begin = curr_addr - intra_offset; - let mut data = vec![0u8; cluster_size as usize]; - backing - .read_at(cluster_begin, &mut data) - .map_err(AsyncIoError::WriteVectored)?; - Some(data) - } else { - None - }; - - let mapping = metadata - .map_cluster_for_write(curr_addr, backing_data) - .map_err(AsyncIoError::WriteVectored)?; - - match mapping { - ClusterWriteMapping::Allocated { - offset: host_offset, - } => { - 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; - } - Ok(()) - } } #[cfg(test)]