From b68e7f3d911ca308c4458fa757618f876fc0d96e Mon Sep 17 00:00:00 2001 From: CMGS Date: Sun, 19 Jul 2026 14:45:55 +0800 Subject: [PATCH] block: flush cached qcow2 metadata on device pause The qcow2 backend caches L2 table and refcount updates in memory and only writes them back on a guest-initiated flush, clean shutdown or drop. A paused VM therefore leaves the on-disk image without the mappings for any cluster allocated since the last guest flush: the data clusters are present in the file, but nothing references them. Anything that reads the image while the VM is paused sees those writes as missing. Copying the disk alongside vm.snapshot (the documented snapshot workflow) captures a stale image, and live migration releases the disk locks after pausing so the destination reopens the file with the same stale metadata. In both cases writes the guest has completed, and may later read back, silently disappear. Add a MetadataSync capability trait with a no-op default, fold it into FullDiskFile, implement it for the qcow2 backend as a metadata cache flush, and call it from the virtio-block pause path after in-flight requests have drained. Pause is the quiesce point both flows rely on, and it is a cold path, so the extra flush does not affect runtime I/O. Reproduced by writing to a qcow2 disk from the guest with O_DIRECT and no explicit flush, pausing the VM and copying the image: qemu-img map on the copy shows no mapped clusters and reads return zeros. With this change the copy contains every completed write. A unit test covers the same sequence at the format level: a completed write is invisible to a fresh reader until sync_metadata, and visible after. Signed-off-by: CMGS --- block/src/disk_file.rs | 35 ++++++++++++++++++---- block/src/formats/qcow/engine_sync.rs | 43 ++++++++++++++++++++++++++- block/src/formats/qcow/mod.rs | 8 +++++ block/src/formats/raw/mod.rs | 2 ++ block/src/formats/vhd/mod.rs | 2 ++ block/src/formats/vhdx/mod.rs | 2 ++ block/src/io/async_io.rs | 3 ++ virtio-devices/src/block.rs | 8 +++++ 8 files changed, 96 insertions(+), 7 deletions(-) diff --git a/block/src/disk_file.rs b/block/src/disk_file.rs index 7f044ea7e..f354457a4 100644 --- a/block/src/disk_file.rs +++ b/block/src/disk_file.rs @@ -12,6 +12,7 @@ //! - [`Geometry`] - sector/cluster geometry (default 512B) //! - [`SparseCapable`] - sparse and zero flag support //! - [`Resizable`] - online resize +//! - [`MetadataSync`] - flush of format metadata cached in memory //! //! [`DiskFile`] is a supertrait that bundles the universal capabilities //! (`DiskSize` + `Geometry`). [`FullDiskFile`] adds all optional @@ -25,7 +26,7 @@ //! FullDiskFile: AsyncDiskFile: //! DiskFile + PhysicalSize + DiskFile + Unpin //! DiskFd + SparseCapable + try_clone, create_async_io -//! Resizable +//! Resizable + MetadataSync //! \ / //! AsyncFullDiskFile: FullDiskFile + AsyncDiskFile //! ``` @@ -91,6 +92,22 @@ pub trait Resizable: Send + Debug { fn resize(&mut self, size: u64) -> BlockResult<()>; } +/// Flush of format metadata cached in memory. +/// +/// Default is a no-op for formats that keep no metadata cache +/// (e.g. raw, fixed vhd). +pub trait MetadataSync: Send + Debug { + /// Flushes format metadata cached in memory (e.g. qcow2 L2/refcount + /// tables) to the underlying file. + /// + /// Called on device pause so that an externally copied or reopened + /// image is self-consistent without requiring a guest-initiated + /// flush. + fn sync_metadata(&self) -> BlockResult<()> { + Ok(()) + } +} + /// Supertrait bundling universal disk capabilities. /// /// Every disk format implements `DiskSize` and `Geometry`. @@ -101,14 +118,20 @@ pub trait DiskFile: DiskSize + Geometry + Sync {} /// Full capability disk file trait. /// /// Bundles all optional capabilities on top of [`DiskFile`]: -/// file descriptor access, physical size, sparse operations, and resize. -/// Used by consumers that need feature negotiation without async I/O -/// (e.g. vhost user block). -pub trait FullDiskFile: DiskFile + PhysicalSize + DiskFd + SparseCapable + Resizable {} +/// file descriptor access, physical size, sparse operations, resize, +/// and metadata sync. Used by consumers that need feature negotiation +/// without async I/O (e.g. vhost user block). +pub trait FullDiskFile: + DiskFile + PhysicalSize + DiskFd + SparseCapable + Resizable + MetadataSync +{ +} /// Blanket implementation: any type implementing all constituent traits /// automatically satisfies [`FullDiskFile`]. -impl FullDiskFile for T {} +impl FullDiskFile + for T +{ +} /// Extended disk file trait for virtio queue workers. /// diff --git a/block/src/formats/qcow/engine_sync.rs b/block/src/formats/qcow/engine_sync.rs index 44b6a327c..e406f7bbe 100644 --- a/block/src/formats/qcow/engine_sync.rs +++ b/block/src/formats/qcow/engine_sync.rs @@ -318,7 +318,7 @@ mod unit_tests { use super::*; use crate::aligned_file::AlignedFile; use crate::async_io::{AsyncIoCompletion, OwnedIoBuffer}; - use crate::disk_file::{AsyncDiskFile, DiskSize, Resizable}; + use crate::disk_file::{AsyncDiskFile, DiskSize, MetadataSync, Resizable}; use crate::error::BlockErrorKind; use crate::formats::qcow; use crate::formats::qcow::common::unit_tests::compress_allocated_clusters; @@ -582,6 +582,47 @@ mod unit_tests { ); } + // sync_metadata must make completed writes visible to a fresh reader of + // the file while the writing disk stays open. Device pause relies on + // this so snapshot copies and migration reopen a self-consistent image + // without a guest-initiated flush. + #[test] + fn write_visible_after_sync_metadata_and_reopen() { + const CL: u64 = 65536; + let virtual_size = 512 * 1024 * 1024; + let (temp, disk) = create_disk_with_data(virtual_size, &[], 0, false, false); + let pattern = vec![0xA5u8; CL as usize]; + async_write(&disk, 0, &pattern); + + let reopen = || { + QcowDisk::new( + temp.as_file().try_clone().unwrap(), + false, + false, + false, + false, + ) + .unwrap() + }; + + // Without the flush the L2 mapping exists only in the writer's + // in-memory cache: a fresh reader sees the cluster unallocated. + let stale = reopen(); + assert_eq!( + async_read(&stale, 0, CL as usize), + vec![0u8; CL as usize], + "write leaked to disk without a metadata flush; test is vacuous", + ); + + disk.sync_metadata().unwrap(); + let fresh = reopen(); + assert_eq!( + async_read(&fresh, 0, CL as usize), + pattern, + "write not visible after sync_metadata and reopen", + ); + } + #[test] fn test_qcow_sync_rejects_out_of_bounds_allocated_l2_entry_on_read() { let data = vec![0x5a; 4096]; diff --git a/block/src/formats/qcow/mod.rs b/block/src/formats/qcow/mod.rs index f3b5950f9..a7ad66c20 100644 --- a/block/src/formats/qcow/mod.rs +++ b/block/src/formats/qcow/mod.rs @@ -294,6 +294,14 @@ impl disk_file::Resizable for QcowDisk { } } +impl disk_file::MetadataSync for QcowDisk { + fn sync_metadata(&self) -> BlockResult<()> { + self.metadata + .flush() + .map_err(|e| BlockError::new(BlockErrorKind::Io, DiskFileError::SyncMetadata(e))) + } +} + impl disk_file::DiskFile for QcowDisk {} impl disk_file::AsyncDiskFile for QcowDisk { diff --git a/block/src/formats/raw/mod.rs b/block/src/formats/raw/mod.rs index 4729ce392..f65573776 100644 --- a/block/src/formats/raw/mod.rs +++ b/block/src/formats/raw/mod.rs @@ -132,6 +132,8 @@ impl disk_file::Resizable for RawDisk { } } +impl disk_file::MetadataSync for RawDisk {} + impl disk_file::DiskFile for RawDisk {} impl disk_file::AsyncDiskFile for RawDisk { diff --git a/block/src/formats/vhd/mod.rs b/block/src/formats/vhd/mod.rs index eb46574d9..c087e5542 100644 --- a/block/src/formats/vhd/mod.rs +++ b/block/src/formats/vhd/mod.rs @@ -104,6 +104,8 @@ impl disk_file::Resizable for VhdDisk { } } +impl disk_file::MetadataSync for VhdDisk {} + impl disk_file::DiskFile for VhdDisk {} impl disk_file::AsyncDiskFile for VhdDisk { diff --git a/block/src/formats/vhdx/mod.rs b/block/src/formats/vhdx/mod.rs index ea15156bf..53f16f307 100644 --- a/block/src/formats/vhdx/mod.rs +++ b/block/src/formats/vhdx/mod.rs @@ -102,6 +102,8 @@ impl disk_file::Resizable for VhdxDisk { } } +impl disk_file::MetadataSync for VhdxDisk {} + impl disk_file::DiskFile for VhdxDisk {} impl disk_file::AsyncDiskFile for VhdxDisk { diff --git a/block/src/io/async_io.rs b/block/src/io/async_io.rs index abad01787..ffe3a3ec8 100644 --- a/block/src/io/async_io.rs +++ b/block/src/io/async_io.rs @@ -41,6 +41,9 @@ pub enum DiskFileError { /// Resize failed #[error("Resize failed")] ResizeError(#[source] io::Error), + /// Flushing cached metadata failed + #[error("Flushing cached metadata failed")] + SyncMetadata(#[source] io::Error), #[error("Failed cloning disk file")] Clone(#[source] io::Error), } diff --git a/virtio-devices/src/block.rs b/virtio-devices/src/block.rs index 135b43934..c48421c2f 100644 --- a/virtio-devices/src/block.rs +++ b/virtio-devices/src/block.rs @@ -1327,6 +1327,14 @@ impl Pausable for Block { let result = self .wait_for_active_requests() .map_err(MigratableError::Pause) + .and_then(|()| { + // Flush cached format metadata (e.g. qcow2 L2/refcount tables) so + // the on-disk image is self-consistent while paused: snapshot + // copies and migration disk-lock handoff read the file directly. + self.disk_image.sync_metadata().map_err(|e| { + MigratableError::Pause(anyhow::Error::new(e).context("sync disk metadata")) + }) + }) .and_then(|()| self.common.pause()); self.draining_active_requests.store(false, Ordering::SeqCst);