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