From e737d61a0ac5d68cc25a4ac2ae411da25b16b487 Mon Sep 17 00:00:00 2001 From: doge Date: Thu, 23 Jul 2026 00:26:01 +0800 Subject: [PATCH] block: Propagate QCOW dealloc action errors to the guest apply_dealloc_action() discarded the result of the host punch-hole and write-zeroes operations, so a guest DISCARD or WRITE ZEROES request completed successfully even when the host operation failed. Return the error to the per-queue engine and complete the request with an error instead, in both the synchronous and io_uring QCOW engines. A failure does not abort the remaining actions of the request: they are still applied, and the first error is reported. Signed-off-by: doge --- block/src/formats/qcow/engine_sync.rs | 46 ++++++++++++++++---------- block/src/formats/qcow/engine_uring.rs | 45 +++++++++++++++---------- 2 files changed, 55 insertions(+), 36 deletions(-) diff --git a/block/src/formats/qcow/engine_sync.rs b/block/src/formats/qcow/engine_sync.rs index c9e3f81fb..4bc3e4b5c 100644 --- a/block/src/formats/qcow/engine_sync.rs +++ b/block/src/formats/qcow/engine_sync.rs @@ -5,6 +5,7 @@ // 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; @@ -50,23 +51,20 @@ impl QcowSync { } } - fn apply_dealloc_action(&mut self, action: &DeallocAction) { + fn apply_dealloc_action(&mut self, action: &DeallocAction) -> io::Result<()> { match action { DeallocAction::PunchHole { host_offset, length, - } => { - let _ = self.data_file.file_mut().punch_hole(*host_offset, *length); - } + } => self.data_file.file_mut().punch_hole(*host_offset, *length), DeallocAction::WriteZeroes { host_offset, length, - } => { - let _ = self - .data_file - .file_mut() - .write_zeroes_at(*host_offset, *length); - } + } => self + .data_file + .file_mut() + .write_zeroes_at(*host_offset, *length) + .map(|_| ()), } } @@ -236,13 +234,19 @@ impl AsyncIo for QcowSync { 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(actions) => { - for action in &actions { - self.apply_dealloc_action(action); - } + Ok(()) => { self.completions .complete(AsyncIoCompletion::new(user_data, 0, None)); Ok(()) @@ -270,13 +274,19 @@ impl AsyncIo for QcowSync { 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(actions) => { - for action in &actions { - self.apply_dealloc_action(action); - } + Ok(()) => { self.completions .complete(AsyncIoCompletion::new(user_data, 0, None)); Ok(()) diff --git a/block/src/formats/qcow/engine_uring.rs b/block/src/formats/qcow/engine_uring.rs index 7999dab23..98dd33641 100644 --- a/block/src/formats/qcow/engine_uring.rs +++ b/block/src/formats/qcow/engine_uring.rs @@ -66,23 +66,20 @@ impl QcowAsync { }) } - fn apply_dealloc_action(&mut self, action: &DeallocAction) { + fn apply_dealloc_action(&mut self, action: &DeallocAction) -> io::Result<()> { match action { DeallocAction::PunchHole { host_offset, length, - } => { - let _ = self.data_file.file_mut().punch_hole(*host_offset, *length); - } + } => self.data_file.file_mut().punch_hole(*host_offset, *length), DeallocAction::WriteZeroes { host_offset, length, - } => { - let _ = self - .data_file - .file_mut() - .write_zeroes_at(*host_offset, *length); - } + } => self + .data_file + .file_mut() + .write_zeroes_at(*host_offset, *length) + .map(|_| ()), } } @@ -202,13 +199,19 @@ impl AsyncIo for QcowAsync { 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(actions) => { - for action in &actions { - self.apply_dealloc_action(action); - } + Ok(()) => { self.data_io .inject_completion(AsyncIoCompletion::new(user_data, 0, None)); Ok(()) @@ -236,13 +239,19 @@ impl AsyncIo for QcowAsync { 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(actions) => { - for action in &actions { - self.apply_dealloc_action(action); - } + Ok(()) => { self.data_io .inject_completion(AsyncIoCompletion::new(user_data, 0, None)); Ok(())