block: advisory locks: use byte-range locks to match QEMU behavior

Switch from a whole-file lock (0, 0 /* EOF */) to a byte-range lock
covering (0, disk.len()), i.e., enabling control over the lock's
granularity.

The granularity has significant implications in typical Cloud
deployments where disk image management software (e.g., Cinder as part
of OpenStack) may treat advisory locks for the whole file as mandatory
locks. This however prevents these software components from snapshotting
disk images while VMs are running. Therefore, it is a valid use case to
lock the whole byte range of a disk image without technically locking
the whole file - to get the best of both worlds.

This also brings CHVs behavior in line with QEMU.

Whole-file locks remain a valid use case and could be supported later.
This patch only provides the necessary groundwork; making it configurable
is out of scope for now.

Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com

Author: phip1611

block/src/fcntl.rs

@@ -101,13 +101,44 @@ impl LockState {
     }
 }
 
+/// The granularity of the advisory lock.
+///
+/// The granularity has significant implications in typical Cloud deployments
+/// where disk image management software (e.g., Cinder as part of OpenStack)
+/// may treat advisory locks for the whole file as mandatory locks. This
+/// however prevents these software components from snapshotting disk images
+/// while VMs are running. Therefore, it is a valid use case to lock the whole
+/// byte range of a disk image without technically locking the whole file -
+/// to get the best of both worlds.
+#[derive(Clone, Copy, Debug)]
+pub enum LockGranularity {
+    WholeFile,
+    ByteRange(u64 /* from, inclusive */, u64 /* len */),
+}
+
+impl LockGranularity {
+    const fn l_start(self) -> u64 {
+        match self {
+            LockGranularity::WholeFile => 0,
+            LockGranularity::ByteRange(start, _) => start,
+        }
+    }
+
+    const fn l_len(self) -> u64 {
+        match self {
+            LockGranularity::WholeFile => 0, /* EOF */
+            LockGranularity::ByteRange(_, len) => len,
+        }
+    }
+}
+
 /// Returns a [`struct@libc::flock`] structure for the whole file.
-const fn get_flock(lock_type: LockType) -> libc::flock {
+const fn get_flock(lock_type: LockType, granularity: LockGranularity) -> libc::flock {
     libc::flock {
         l_type: lock_type.to_libc_val() as libc::c_short,
         l_whence: libc::SEEK_SET as libc::c_short,
-        l_start: 0,
-        l_len: 0, /* EOF */
+        l_start: granularity.l_start() as libc::c_long,
+        l_len: granularity.l_len() as libc::c_long,
         l_pid: 0, /* filled by callee */
     }
 }
@@ -122,8 +153,13 @@ const fn get_flock(lock_type: LockType) -> libc::flock {
 /// - `file`: The file to acquire a lock for [`LockType`]. The file's state will
 ///   be logically mutated, but not technically.
 /// - `lock_type`: The [`LockType`]
-pub fn try_acquire_lock<Fd: AsRawFd>(file: Fd, lock_type: LockType) -> Result<(), LockError> {
-    let flock = get_flock(lock_type);
+/// - `granularity`: The [`LockGranularity`].
+pub fn try_acquire_lock<Fd: AsRawFd>(
+    file: Fd,
+    lock_type: LockType,
+    granularity: LockGranularity,
+) -> Result<(), LockError> {
+    let flock = get_flock(lock_type, granularity);
 
     let res = fcntl(file.as_raw_fd(), FcntlArg::F_OFD_SETLK(&flock));
     match res {
@@ -146,17 +182,22 @@ pub fn try_acquire_lock<Fd: AsRawFd>(file: Fd, lock_type: LockType) -> Result<()
 ///
 /// # Parameters
 /// - `file`: The file to clear all locks for [`LockType`].
-pub fn clear_lock<Fd: AsRawFd>(file: Fd) -> Result<(), LockError> {
-    try_acquire_lock(file, LockType::Unlock)
+/// - `granularity`: The [`LockGranularity`].
+pub fn clear_lock<Fd: AsRawFd>(file: Fd, granularity: LockGranularity) -> Result<(), LockError> {
+    try_acquire_lock(file, LockType::Unlock, granularity)
 }
 
 /// Returns the current lock state using [`fcntl`] with respect to the given
 /// parameters.
 ///
 /// # Parameters
 /// - `file`: The file for which to get the lock state.
-pub fn get_lock_state<Fd: AsRawFd>(file: Fd) -> Result<LockState, LockError> {
-    let mut flock = get_flock(LockType::Write);
+/// - `granularity`: The [`LockGranularity`].
+pub fn get_lock_state<Fd: AsRawFd>(
+    file: Fd,
+    granularity: LockGranularity,
+) -> Result<LockState, LockError> {
+    let mut flock = get_flock(LockType::Write, granularity);
     let res = fcntl(file.as_raw_fd(), FcntlArg::F_OFD_GETLK(&mut flock));
     match res {
         0 => {

virtio-devices/src/block.rs

@@ -19,7 +19,7 @@ use std::{io, result};
 
 use anyhow::anyhow;
 use block::async_io::{AsyncIo, AsyncIoError, DiskFile};
-use block::fcntl::{LockError, LockType, get_lock_state};
+use block::fcntl::{LockError, LockGranularity, LockType, get_lock_state};
 use block::{
     ExecuteAsync, ExecuteError, Request, RequestType, VirtioBlockConfig, build_serial, fcntl,
 };
@@ -789,9 +789,14 @@ impl Block {
             self.id,
             self.disk_path.display()
         );
+        let granularity = self
+            .disk_image
+            .size()
+            .map(|size| LockGranularity::ByteRange(0, size))
+            .unwrap_or(LockGranularity::WholeFile);
         let fd = self.disk_image.fd();
-        fcntl::try_acquire_lock(fd, lock_type).map_err(|error| {
-            let current_lock = get_lock_state(fd);
+        fcntl::try_acquire_lock(fd, lock_type, granularity).map_err(|error| {
+            let current_lock = get_lock_state(fd, granularity);
             // Don't propagate the error to the outside, as it is not useful at all. Instead,
             // we try to log additional help to the user.
             if let Ok(current_lock) = current_lock {
@@ -815,10 +820,16 @@ impl Block {
 
     /// Releases the advisory lock held for the corresponding disk image.
     pub fn unlock_image(&mut self) -> Result<()> {
+        let granularity = self
+            .disk_image
+            .size()
+            .map(|size| LockGranularity::ByteRange(0, size))
+            .unwrap_or(LockGranularity::WholeFile);
+
         // It is very unlikely that this fails;
         // Should we remove the Result to simplify the error propagation on
         // higher levels?
-        fcntl::clear_lock(self.disk_image.fd()).map_err(|error| Error::LockDiskImage {
+        fcntl::clear_lock(self.disk_image.fd(), granularity).map_err(|error| Error::LockDiskImage {
             path: self.disk_path.clone(),
             error,
             lock_type: LockType::Unlock,