virtio-devices, block: generalize disk locking

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();
+    }
+}

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
@@ -1009,46 +1013,63 @@ 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();
+    /// 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();
+        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