block: qcow: Only clear DIRTY bit when last QcowDisk destroyed

Multiple QcowDisk/Qcow2Backing can share references to the same
QcowMetadata via an Arc. Unfortunately the .shutdown() which clears the
DIRTY bit was being called when the first of those was dropped. Instead
move this to the drop of the metadata itself. Now only once all
references to the metadata are dropped then we can safely set the DIRTY
bit.

Assisted-by: Codex:GPT-5.6
Signed-off-by: Rob Bradford <rbradford@meta.com>
This commit is contained in:
Rob Bradford
2026-07-28 18:20:52 +01:00
parent 8268a4fd5b
commit 53ee9ebb77
4 changed files with 43 additions and 14 deletions

View File

@@ -139,12 +139,6 @@ impl Qcow2Backing {
} }
} }
impl Drop for Qcow2Backing {
fn drop(&mut self) {
self.metadata.shutdown();
}
}
/// Construct a thread safe backing file reader. /// Construct a thread safe backing file reader.
pub(super) fn shared_backing_from(bf: BackingFile) -> BlockResult<Arc<dyn BackingRead>> { pub(super) fn shared_backing_from(bf: BackingFile) -> BlockResult<Arc<dyn BackingRead>> {
let (kind, virtual_size) = bf.into_kind(); let (kind, virtual_size) = bf.into_kind();

View File

@@ -2016,7 +2016,6 @@ mod unit_tests {
assert!(result < 0, "host WriteZeroes failure must reach the guest"); assert!(result < 0, "host WriteZeroes failure must reach the guest");
drop(aio); drop(aio);
metadata.shutdown();
drop(metadata); drop(metadata);
let reopened = QcowDisk::new( let reopened = QcowDisk::new(
@@ -2314,7 +2313,6 @@ mod unit_tests {
// L2, and only now may the old data cluster enter unref_clusters. // L2, and only now may the old data cluster enter unref_clusters.
aio.apply_dealloc_action(&actions[0]).unwrap(); aio.apply_dealloc_action(&actions[0]).unwrap();
metadata.flush().unwrap(); metadata.flush().unwrap();
metadata.shutdown();
drop(aio); drop(aio);
drop(metadata); drop(metadata);

View File

@@ -389,6 +389,12 @@ impl QcowMetadata {
} }
} }
impl Drop for QcowMetadata {
fn drop(&mut self) {
self.shutdown();
}
}
impl QcowState { impl QcowState {
/// Fast path read mapping under read lock only. Returns None on cache /// Fast path read mapping under read lock only. Returns None on cache
/// miss. /// miss.

View File

@@ -240,12 +240,6 @@ impl QcowTempDisk {
} }
} }
impl Drop for QcowDisk {
fn drop(&mut self) {
self.metadata.shutdown();
}
}
impl disk_file::DiskSize for QcowDisk { impl disk_file::DiskSize for QcowDisk {
fn logical_size(&self) -> BlockResult<u64> { fn logical_size(&self) -> BlockResult<u64> {
Ok(self.metadata.virtual_size()) Ok(self.metadata.virtual_size())
@@ -349,6 +343,8 @@ impl disk_file::AsyncDiskFile for QcowDisk {
#[cfg(test)] #[cfg(test)]
mod unit_tests { mod unit_tests {
use std::os::unix::fs::FileExt;
use super::*; use super::*;
use crate::async_io::AsyncIo; use crate::async_io::AsyncIo;
use crate::disk_file::{AsyncDiskFile, DiskSize, PhysicalSize}; use crate::disk_file::{AsyncDiskFile, DiskSize, PhysicalSize};
@@ -362,6 +358,13 @@ mod unit_tests {
.into_file() .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] #[test]
fn new_sync_returns_correct_size() { fn new_sync_returns_correct_size() {
let file = make_qcow_file(); let file = make_qcow_file();
@@ -401,6 +404,34 @@ mod unit_tests {
assert_async_io_from_dyn(cloned.as_ref(), false); 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")] #[cfg(feature = "io_uring")]
#[test] #[test]
fn try_clone_preserves_io_uring_dispatch() { fn try_clone_preserves_io_uring_dispatch() {