diff --git a/block/src/formats/qcow/backing.rs b/block/src/formats/qcow/backing.rs index ced5cd39b..a149b2145 100644 --- a/block/src/formats/qcow/backing.rs +++ b/block/src/formats/qcow/backing.rs @@ -139,12 +139,6 @@ impl Qcow2Backing { } } -impl Drop for Qcow2Backing { - fn drop(&mut self) { - self.metadata.shutdown(); - } -} - /// Construct a thread safe backing file reader. pub(super) fn shared_backing_from(bf: BackingFile) -> BlockResult> { let (kind, virtual_size) = bf.into_kind(); diff --git a/block/src/formats/qcow/engine_sync.rs b/block/src/formats/qcow/engine_sync.rs index 488ebf7c4..9b3b7f766 100644 --- a/block/src/formats/qcow/engine_sync.rs +++ b/block/src/formats/qcow/engine_sync.rs @@ -2016,7 +2016,6 @@ mod unit_tests { assert!(result < 0, "host WriteZeroes failure must reach the guest"); drop(aio); - metadata.shutdown(); drop(metadata); let reopened = QcowDisk::new( @@ -2314,7 +2313,6 @@ mod unit_tests { // L2, and only now may the old data cluster enter unref_clusters. aio.apply_dealloc_action(&actions[0]).unwrap(); metadata.flush().unwrap(); - metadata.shutdown(); drop(aio); drop(metadata); diff --git a/block/src/formats/qcow/metadata.rs b/block/src/formats/qcow/metadata.rs index 1ce8145d7..2e7780e35 100644 --- a/block/src/formats/qcow/metadata.rs +++ b/block/src/formats/qcow/metadata.rs @@ -389,6 +389,12 @@ impl QcowMetadata { } } +impl Drop for QcowMetadata { + fn drop(&mut self) { + self.shutdown(); + } +} + impl QcowState { /// Fast path read mapping under read lock only. Returns None on cache /// miss. diff --git a/block/src/formats/qcow/mod.rs b/block/src/formats/qcow/mod.rs index a7ad66c20..3d09d27b3 100644 --- a/block/src/formats/qcow/mod.rs +++ b/block/src/formats/qcow/mod.rs @@ -240,12 +240,6 @@ impl QcowTempDisk { } } -impl Drop for QcowDisk { - fn drop(&mut self) { - self.metadata.shutdown(); - } -} - impl disk_file::DiskSize for QcowDisk { fn logical_size(&self) -> BlockResult { Ok(self.metadata.virtual_size()) @@ -349,6 +343,8 @@ impl disk_file::AsyncDiskFile for QcowDisk { #[cfg(test)] mod unit_tests { + use std::os::unix::fs::FileExt; + use super::*; use crate::async_io::AsyncIo; use crate::disk_file::{AsyncDiskFile, DiskSize, PhysicalSize}; @@ -362,6 +358,13 @@ mod unit_tests { .into_file() } + fn dirty_bit_is_set(file: &File) -> bool { + let mut buf = [0u8; 8]; + file.read_exact_at(&mut buf, header::V2_BARE_HEADER_SIZE as u64) + .unwrap(); + u64::from_be_bytes(buf) & IncompatFeatures::DIRTY.bits() != 0 + } + #[test] fn new_sync_returns_correct_size() { let file = make_qcow_file(); @@ -401,6 +404,34 @@ mod unit_tests { assert_async_io_from_dyn(cloned.as_ref(), false); } + #[test] + fn dropping_clone_does_not_clear_dirty_bit() { + let file = make_qcow_file(); + let disk = QcowDisk::new(file, false, false, true, false).unwrap(); + let cloned = disk.try_clone().unwrap(); + + drop(cloned); + + assert_ne!( + disk.metadata().header().incompatible_features & IncompatFeatures::DIRTY.bits(), + 0 + ); + } + + #[test] + fn async_io_clears_dirty_bit_when_last_metadata_owner_drops() { + let file = make_qcow_file(); + let inspect = file.try_clone().unwrap(); + let disk = QcowDisk::new(file, false, false, true, false).unwrap(); + let async_io = disk.create_async_io(1).unwrap(); + + drop(disk); + assert!(dirty_bit_is_set(&inspect)); + + drop(async_io); + assert!(!dirty_bit_is_set(&inspect)); + } + #[cfg(feature = "io_uring")] #[test] fn try_clone_preserves_io_uring_dispatch() {