From 53ee9ebb7769626207ca7b85d7fd3f375771236f Mon Sep 17 00:00:00 2001
From: Rob Bradford <rbradford@meta.com>
Date: Tue, 28 Jul 2026 18:20:52 +0100
Subject: [PATCH] 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>
---
 block/src/formats/qcow/backing.rs     |  6 ----
 block/src/formats/qcow/engine_sync.rs |  2 --
 block/src/formats/qcow/metadata.rs    |  6 ++++
 block/src/formats/qcow/mod.rs         | 43 +++++++++++++++++++++++----
 4 files changed, 43 insertions(+), 14 deletions(-)

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<Arc<dyn BackingRead>> {
     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<u64> {
         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() {