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 <anbelski@linux.microsoft.com>

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;

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<usize> {
         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);

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 {