From 84700001d4a29981947a7c837bd28ab96d0556c7 Mon Sep 17 00:00:00 2001 From: Leander Kohler Date: Mon, 13 Jul 2026 11:26:34 +0200 Subject: [PATCH 1/3] virtio-devices: generalize lock granularity Calculate advisory lock granularity for an explicitly supplied disk backend and path instead of always using Block::disk_image. This lets mirror destinations reuse the configured locking policy. On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler --- virtio-devices/src/block.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/virtio-devices/src/block.rs b/virtio-devices/src/block.rs index edb3650ab0..2756b20e5f 100644 --- a/virtio-devices/src/block.rs +++ b/virtio-devices/src/block.rs @@ -13,7 +13,7 @@ use std::collections::{BTreeMap, HashMap, VecDeque}; use std::num::Wrapping; use std::ops::Deref; use std::os::unix::io::AsRawFd; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, AtomicU8, AtomicU64, AtomicUsize, Ordering}; use std::sync::{Arc, Barrier}; use std::time::{Duration, Instant}; @@ -979,16 +979,20 @@ impl Block { has_feature(self.features(), VIRTIO_BLK_F_RO.into()) } - /// Returns the granularity for the advisory lock for this disk. - fn lock_granularity(&mut self) -> LockGranularity { + /// Returns the configured advisory lock granularity for `disk_image`. + fn lock_granularity( + &self, + disk_image: &dyn AsyncFullDiskFile, + disk_path: &Path, + ) -> LockGranularity { match self.lock_granularity_choice { LockGranularityChoice::Full => LockGranularity::WholeFile, LockGranularityChoice::ByteRange => { // Byte range lock covering [0, max(logical, physical)) // logical > physical for sparse files, physical > logical // for small dense files due to filesystem block rounding. - let logical = self.disk_image.logical_size(); - let physical = self.disk_image.physical_size(); + let logical = disk_image.logical_size(); + let physical = disk_image.physical_size(); match (logical, physical) { (Ok(l), Ok(p)) => LockGranularity::ByteRange(0, max(l, p)), (Ok(l), Err(_)) => LockGranularity::ByteRange(0, l), @@ -998,7 +1002,7 @@ impl Block { warn!( "Can't get disk size for id={},path={}, falling back to {:?}: error: {e}", self.id, - self.disk_path.display(), + disk_path.display(), fallback ); fallback @@ -1015,7 +1019,7 @@ impl Block { true => LockType::Read, false => LockType::Write, }; - let granularity = self.lock_granularity(); + let granularity = self.lock_granularity(self.disk_image.as_ref(), &self.disk_path); debug!( "Attempting to acquire {lock_type:?} lock for disk image: id={},path={},granularity={granularity:?}", self.id, @@ -1048,7 +1052,7 @@ impl Block { /// Releases the advisory lock held for the corresponding disk image. pub fn unlock_image(&mut self) -> Result<()> { - let granularity = self.lock_granularity(); + let granularity = self.lock_granularity(self.disk_image.as_ref(), &self.disk_path); // It is very unlikely that this fails; // Should we remove the Result to simplify the error propagation on From 80b335d4b89d6974903bcb3a1dc8829f532544ab Mon Sep 17 00:00:00 2001 From: Leander Kohler Date: Mon, 13 Jul 2026 11:29:09 +0200 Subject: [PATCH 2/3] virtio-devices: generalize disk locking Extract advisory lock acquisition into a helper that accepts a disk backend, path, requested mode, and current mode. Keep try_lock_image as the source-disk wrapper. This enables the generalized use of `try_lock_disk_image` for other disk images within the same Block device, which occurs whenever we mirror a disk image. On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler --- virtio-devices/src/block.rs | 45 +++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/virtio-devices/src/block.rs b/virtio-devices/src/block.rs index 2756b20e5f..b674d4fad6 100644 --- a/virtio-devices/src/block.rs +++ b/virtio-devices/src/block.rs @@ -1013,43 +1013,60 @@ impl Block { } } - /// Tries to set an advisory lock for the corresponding disk image. - pub fn try_lock_image(&mut self) -> Result<()> { - let lock_type = match self.read_only() { - true => LockType::Read, - false => LockType::Write, - }; - let granularity = self.lock_granularity(self.disk_image.as_ref(), &self.disk_path); + /// Acquires an advisory lock for an arbitrary disk backend. + fn try_lock_disk_image( + &self, + disk_image: &dyn AsyncFullDiskFile, + disk_path: &Path, + lock_type: LockType, + current_lock: LockType, + ) -> Result<()> { + let granularity = self.lock_granularity(disk_image, disk_path); debug!( "Attempting to acquire {lock_type:?} lock for disk image: id={},path={},granularity={granularity:?}", self.id, - self.disk_path.display() + disk_path.display() ); - let fd = self.disk_image.fd(); + let fd = disk_image.fd(); granularity - .try_acquire_lock(&fd, lock_type, self.held_lock) + .try_acquire_lock(&fd, lock_type, current_lock) .map_err(|error| { error!( "Cannot acquire {lock_type:?} lock for disk image: id={},path={},granularity={granularity:?}", self.id, - self.disk_path.display() + disk_path.display() ); Error::LockDiskImage { - path: self.disk_path.clone(), + path: disk_path.to_path_buf(), error, lock_type, } })?; - self.held_lock = lock_type; info!( "Acquired {lock_type:?} lock for disk image id={},path={}", self.id, - self.disk_path.display() + disk_path.display() ); Ok(()) } + /// Tries to set an advisory lock for the corresponding disk image. + pub fn try_lock_image(&mut self) -> Result<()> { + let lock_type = match self.read_only() { + true => LockType::Read, + false => LockType::Write, + }; + self.try_lock_disk_image( + self.disk_image.as_ref(), + &self.disk_path, + lock_type, + self.held_lock, + )?; + self.held_lock = lock_type; + Ok(()) + } + /// Releases the advisory lock held for the corresponding disk image. pub fn unlock_image(&mut self) -> Result<()> { let granularity = self.lock_granularity(self.disk_image.as_ref(), &self.disk_path); From 383d0ddce1e1c8e2ea6b34e87f468edc1baa062e Mon Sep 17 00:00:00 2001 From: Leander Kohler Date: Mon, 20 Jul 2026 10:24:18 +0200 Subject: [PATCH 3/3] block: release stale QEMU lock after downgrade Downgrading a QEMU-compatible lock from write to read leaves the write marker byte locked. try_acquire_lock_qemu() only releases unneeded bytes when rolling back after a failed acquisition, so a successful downgrade keeps the write marker in place. This blocks mirroring of read-only disks. The destination acquires a write lock while data is copied and must downgrade to the source's read-only lock when the mirror completes. The stale write marker prevents another reader from locking the image. Release the bytes that the new lock type does not need once the conflict checks succeed, and add a test that downgrades a write lock to a read lock and verifies that another reader can still acquire the image. On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler --- block/src/fcntl.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/block/src/fcntl.rs b/block/src/fcntl.rs index f5cb626c00..7184fa8182 100644 --- a/block/src/fcntl.rs +++ b/block/src/fcntl.rs @@ -248,6 +248,8 @@ impl LockGranularity { let _ = self.release_unneeded_locks_qemu(file, current_lock_status); return Err(error); } + + self.release_unneeded_locks_qemu(file, lock_type)?; Ok(()) } @@ -369,3 +371,31 @@ impl FromStr for LockGranularityChoice { } } } + +#[cfg(test)] +mod tests { + use std::fs::OpenOptions; + + use vmm_sys_util::tempfile::TempFile; + + use super::{LockGranularity, LockType}; + + #[test] + fn qemu_lock_downgrade_allows_another_reader() { + let disk = TempFile::new().unwrap(); + let other_reader = OpenOptions::new() + .read(true) + .write(true) + .open(disk.as_path()) + .unwrap(); + let lock = LockGranularity::QemuCompatible; + + lock.try_acquire_lock(disk.as_file(), LockType::Write, LockType::Unlock) + .unwrap(); + lock.try_acquire_lock(disk.as_file(), LockType::Read, LockType::Write) + .unwrap(); + + lock.try_acquire_lock(&other_reader, LockType::Read, LockType::Unlock) + .unwrap(); + } +}