diff --git a/block/src/fcntl.rs b/block/src/fcntl.rs index 0cb2ee552b..d5f5d9a0b6 100644 --- a/block/src/fcntl.rs +++ b/block/src/fcntl.rs @@ -24,8 +24,6 @@ use thiserror::Error; #[derive(Error, Debug)] pub enum LockError { /// The file is already locked. - /// - /// A call to [`get_lock_state`] can help to identify the reason. #[error("The file is already locked")] AlreadyLocked, /// IO error. @@ -43,12 +41,36 @@ enum FcntlArg<'a> { } /// Wrapper for [`libc::fcntl`] that properly sets the function arguments. -fn fcntl(fd: RawFd, arg: FcntlArg) -> libc::c_int { - // SAFETY: We use a valid FD. - unsafe { - match arg { - FcntlArg::F_OFD_SETLK(flock) => libc::fcntl(fd, libc::F_OFD_SETLK, flock), - FcntlArg::F_OFD_GETLK(flock) => libc::fcntl(fd, libc::F_OFD_GETLK, flock), +fn fcntl(fd: RawFd, mut arg: FcntlArg) -> Result<(), LockError> { + loop { + // SAFETY: + // - `F_OFD_SETLK` and `F_OFD_GETLK` fcntl calls handle invalid file descriptors. + // - `F_OFD_SETLK` does not modify `flock`. + // - `F_OFD_GETLK` uses a mutable pointer to `flock`. + let result = unsafe { + match &mut arg { + FcntlArg::F_OFD_SETLK(flock) => { + libc::fcntl(fd, libc::F_OFD_SETLK, *flock as *const libc::flock) + } + FcntlArg::F_OFD_GETLK(flock) => { + libc::fcntl(fd, libc::F_OFD_GETLK, *flock as *mut libc::flock) + } + } + }; + match result { + 0 => return Ok(()), + -1 => { + let io_error = io::Error::last_os_error(); + let errno = io_error.raw_os_error().unwrap(); + match errno { + // See man page for error code: + // + libc::EAGAIN | libc::EACCES => return Err(LockError::AlreadyLocked), + libc::EINTR => continue, + _ => return Err(LockError::Io(io_error)), + } + } + val => panic!("Unexpected return value from fcntl(): {val}"), } } } @@ -74,33 +96,15 @@ impl LockType { } } -/// Describes the current state of a lock. -#[derive(Debug)] -pub enum LockState { - /// No lock set. - Unlocked, - /// Locked for reading (non-exclusive). - SharedRead, - /// Locked for writing (exclusive mode). - ExclusiveWrite, -} +/// Amount of bytes by which the first lock is offset from the start of the file. +const QEMU_LOCK_OFFSET: u64 = 100; +/// Amount of bytes by which the first unshared lock is offset from the start of the file. +const QEMU_UNSHARE_LOCK_OFFSET: u64 = 200; -impl LockState { - fn new(value: libc::c_int) -> Self { - const F_UNLCK: libc::c_int = libc::F_UNLCK as libc::c_int; - const F_WRLCK: libc::c_int = libc::F_WRLCK as libc::c_int; - const F_RDLCK: libc::c_int = libc::F_RDLCK as libc::c_int; - match value { - F_UNLCK => Self::Unlocked, - F_WRLCK => Self::ExclusiveWrite, - F_RDLCK => Self::SharedRead, - // This is so unlikely that we want to avoid the complexity of - // coping with this error case. Can only fail if either Linux - // is broken or memory is messed up. - other => panic!("Unexpected lock state: {other}"), - } - } -} +/// Read permission lock index for QEMU. +const QEMU_CONSISTENT_READ_BYTE: u64 = 0; +/// Write permission lock index for QEMU. +const QEMU_WRITE_BYTE: u64 = 1; /// The granularity of the advisory lock. /// @@ -123,20 +127,210 @@ impl LockState { pub enum LockGranularity { WholeFile, ByteRange(u64 /* from, inclusive */, u64 /* len */), + QemuCompatible, } impl LockGranularity { - const fn l_start(self) -> u64 { + const fn l_len(self) -> u64 { match self { - LockGranularity::WholeFile => 0, - LockGranularity::ByteRange(start, _) => start, + LockGranularity::WholeFile => 0, /* EOF */ + LockGranularity::ByteRange(_, len) => len, + // QEMU uses multiple one byte long locks. + LockGranularity::QemuCompatible => 1, } } - const fn l_len(self) -> u64 { + /// Internal implementation of [`Self::try_acquire_lock`] for [`LockGranularity::WholeFile`] and + /// [`LockGranularity::ByteRange`]. + fn try_acquire_lock_file( + self, + file: &Fd, + lock_type: LockType, + l_start: u64, + ) -> Result<(), LockError> { + let flock = self.flock(lock_type.to_libc_val(), l_start); + + fcntl(file.as_raw_fd(), FcntlArg::F_OFD_SETLK(&flock)) + } + + /// Releases all locks not required for `lock_type`. + /// + /// Used to roll back a lock acquisition attempt to a previously acquired lock. + fn release_unneeded_locks_qemu( + self, + file: &Fd, + lock_type: LockType, + ) -> Result<(), LockError> { + let flocks = match lock_type { + LockType::Unlock => vec![ + LockGranularity::QemuCompatible + .flock(libc::F_UNLCK, QEMU_LOCK_OFFSET + QEMU_CONSISTENT_READ_BYTE), + LockGranularity::QemuCompatible + .flock(libc::F_UNLCK, QEMU_LOCK_OFFSET + QEMU_WRITE_BYTE), + LockGranularity::QemuCompatible.flock( + libc::F_UNLCK, + QEMU_UNSHARE_LOCK_OFFSET + QEMU_CONSISTENT_READ_BYTE, + ), + LockGranularity::QemuCompatible + .flock(libc::F_UNLCK, QEMU_UNSHARE_LOCK_OFFSET + QEMU_WRITE_BYTE), + ], + LockType::Write => vec![], + LockType::Read => vec![ + LockGranularity::QemuCompatible + .flock(libc::F_UNLCK, QEMU_LOCK_OFFSET + QEMU_WRITE_BYTE), + ], + }; + + let mut first_error = None; + for flock in flocks { + if let Err(error) = fcntl(file.as_raw_fd(), FcntlArg::F_OFD_SETLK(&flock)) { + first_error.get_or_insert(error); + } + } + if let Some(first_error) = first_error { + return Err(first_error); + } + Ok(()) + } + + /// Internal implementation of [`Self::try_acquire_lock`] for [`LockGranularity::QemuCompatible`]. + fn try_acquire_lock_qemu( + self, + file: &Fd, + lock_type: LockType, + current_lock_status: LockType, + ) -> Result<(), LockError> { + let flocks = match lock_type { + LockType::Unlock => vec![ + LockGranularity::QemuCompatible + .flock(libc::F_UNLCK, QEMU_LOCK_OFFSET + QEMU_CONSISTENT_READ_BYTE), + LockGranularity::QemuCompatible + .flock(libc::F_UNLCK, QEMU_LOCK_OFFSET + QEMU_WRITE_BYTE), + LockGranularity::QemuCompatible.flock( + libc::F_UNLCK, + QEMU_UNSHARE_LOCK_OFFSET + QEMU_CONSISTENT_READ_BYTE, + ), + LockGranularity::QemuCompatible + .flock(libc::F_UNLCK, QEMU_UNSHARE_LOCK_OFFSET + QEMU_WRITE_BYTE), + ], + LockType::Write => vec![ + LockGranularity::QemuCompatible + .flock(libc::F_RDLCK, QEMU_LOCK_OFFSET + QEMU_CONSISTENT_READ_BYTE), + LockGranularity::QemuCompatible + .flock(libc::F_RDLCK, QEMU_LOCK_OFFSET + QEMU_WRITE_BYTE), + LockGranularity::QemuCompatible + .flock(libc::F_RDLCK, QEMU_UNSHARE_LOCK_OFFSET + QEMU_WRITE_BYTE), + ], + LockType::Read => vec![ + LockGranularity::QemuCompatible + .flock(libc::F_RDLCK, QEMU_LOCK_OFFSET + QEMU_CONSISTENT_READ_BYTE), + LockGranularity::QemuCompatible + .flock(libc::F_RDLCK, QEMU_UNSHARE_LOCK_OFFSET + QEMU_WRITE_BYTE), + ], + }; + + for flock in flocks { + if let Err(error) = fcntl(file.as_raw_fd(), FcntlArg::F_OFD_SETLK(&flock)) { + if let LockType::Unlock = lock_type { + return Err(error); + } + let _ = self.release_unneeded_locks_qemu(file, current_lock_status); + return Err(error); + } + } + + if let Err(error) = self.check_lock_success_qemu(file, lock_type) { + let _ = self.release_unneeded_locks_qemu(file, current_lock_status); + return Err(error); + } + Ok(()) + } + + /// Tries to acquire a lock using [`fcntl`] with respect to the given + /// parameters. + /// + /// Please note that `fcntl()` OFD locks are **advisory locks**, which do not + /// prevent to `open()` a file if a lock is already placed. + /// + /// # Parameters + /// - `file`: The file to acquire a lock for [`LockType`]. The file's state will + /// be logically mutated, but not technically. + /// - `lock_type`: The [`LockType`] + /// - `current_lock_status`: Already held locks on this `file`. + /// Used for [`LockGranularity::QemuCompatible`] to roll back to if locking fails. + pub fn try_acquire_lock( + self, + file: &Fd, + lock_type: LockType, + current_lock_status: LockType, + ) -> Result<(), LockError> { match self { - LockGranularity::WholeFile => 0, /* EOF */ - LockGranularity::ByteRange(_, len) => len, + LockGranularity::WholeFile => self.try_acquire_lock_file(file, lock_type, 0), + LockGranularity::ByteRange(start, _) => { + self.try_acquire_lock_file(file, lock_type, start) + } + LockGranularity::QemuCompatible => { + self.try_acquire_lock_qemu(file, lock_type, current_lock_status) + } + } + } + + /// Clears a lock. + /// + /// # Parameters + /// - `file`: The file to clear all locks for [`LockType`]. + pub fn clear_lock(self, file: &Fd) -> Result<(), LockError> { + self.try_acquire_lock(file, LockType::Unlock, LockType::Unlock) + } + + /// Checks whether any conflicting locks are set. + /// + /// Returns an error if a conflicting lock is set. + fn check_lock_success_qemu( + &self, + file: &Fd, + lock_type: LockType, + ) -> Result<(), LockError> { + let flocks = match lock_type { + LockType::Unlock => vec![], + LockType::Write => vec![ + LockGranularity::QemuCompatible.flock( + libc::F_WRLCK, + QEMU_UNSHARE_LOCK_OFFSET + QEMU_CONSISTENT_READ_BYTE, + ), + LockGranularity::QemuCompatible + .flock(libc::F_WRLCK, QEMU_UNSHARE_LOCK_OFFSET + QEMU_WRITE_BYTE), + LockGranularity::QemuCompatible + .flock(libc::F_WRLCK, QEMU_LOCK_OFFSET + QEMU_WRITE_BYTE), + ], + LockType::Read => vec![ + LockGranularity::QemuCompatible.flock( + libc::F_WRLCK, + QEMU_UNSHARE_LOCK_OFFSET + QEMU_CONSISTENT_READ_BYTE, + ), + LockGranularity::QemuCompatible + .flock(libc::F_WRLCK, QEMU_LOCK_OFFSET + QEMU_WRITE_BYTE), + ], + }; + + for mut flock in flocks { + fcntl(file.as_raw_fd(), FcntlArg::F_OFD_GETLK(&mut flock))?; + + if flock.l_type as libc::c_int != libc::F_UNLCK { + return Err(LockError::AlreadyLocked); + } + } + Ok(()) + } + + /// Returns a [`struct@libc::flock`] structure. + const fn flock(self, lock_type: libc::c_int, l_start: u64) -> libc::flock { + libc::flock { + l_type: lock_type as libc::c_short, + l_whence: libc::SEEK_SET as libc::c_short, + l_start: l_start as libc::c_long, + l_len: self.l_len() as libc::c_long, + l_pid: 0, /* filled by callee */ } } } @@ -149,15 +343,17 @@ impl LockGranularity { #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, serde::Deserialize, serde::Serialize)] pub enum LockGranularityChoice { /// Byte-range lock covering [0, size). - #[default] ByteRange, /// Whole-file lock (l_start=0, l_len=0) - original OFD whole-file lock behavior. Full, + /// Locking scheme that mimics QEMU's marker byte based locking scheme. + #[default] + QemuCompatible, } /// Error returned when parsing a [`LockGranularityChoice`] from a string. #[derive(Error, Debug)] -#[error("Invalid lock granularity value: {0}, expected 'byte-range' or 'full'")] +#[error("Invalid lock granularity value: {0}, expected 'byte-range', 'full' or 'qemu-compatible'")] pub struct LockGranularityParseError(String); impl FromStr for LockGranularityChoice { @@ -167,91 +363,8 @@ impl FromStr for LockGranularityChoice { match s { "byte-range" => Ok(LockGranularityChoice::ByteRange), "full" => Ok(LockGranularityChoice::Full), + "qemu-compatible" => Ok(Self::QemuCompatible), _ => Err(LockGranularityParseError(s.to_owned())), } } } - -/// Returns a [`struct@libc::flock`] structure for the whole file. -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: granularity.l_start() as libc::c_long, - l_len: granularity.l_len() as libc::c_long, - l_pid: 0, /* filled by callee */ - } -} - -/// Tries to acquire a lock using [`fcntl`] with respect to the given -/// parameters. -/// -/// Please note that `fcntl()` OFD locks are **advisory locks**, which do not -/// prevent to `open()` a file if a lock is already placed. -/// -/// # Parameters -/// - `file`: The file to acquire a lock for [`LockType`]. The file's state will -/// be logically mutated, but not technically. -/// - `lock_type`: The [`LockType`] -/// - `granularity`: The [`LockGranularity`]. -pub fn try_acquire_lock( - file: &Fd, - lock_type: LockType, - granularity: LockGranularity, -) -> Result<(), LockError> { - let flock = get_flock(lock_type, granularity); - - loop { - let res = fcntl(file.as_raw_fd(), FcntlArg::F_OFD_SETLK(&flock)); - match res { - 0 => return Ok(()), - -1 => { - let io_error = io::Error::last_os_error(); - let errno = io_error.raw_os_error().unwrap(); - match errno { - // See man page for error code: - // - libc::EAGAIN | libc::EACCES => return Err(LockError::AlreadyLocked), - libc::EINTR => continue, - _ => return Err(LockError::Io(io_error)), - } - } - val => panic!("Unexpected return value from fcntl(): {val}"), - } - } -} - -/// Clears a lock. -/// -/// # Parameters -/// - `file`: The file to clear all locks for [`LockType`]. -/// - `granularity`: The [`LockGranularity`]. -pub fn clear_lock(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. -/// - `granularity`: The [`LockGranularity`]. -pub fn get_lock_state( - file: &Fd, - granularity: LockGranularity, -) -> Result { - let mut flock = get_flock(LockType::Write, granularity); - let res = fcntl(file.as_raw_fd(), FcntlArg::F_OFD_GETLK(&mut flock)); - match res { - 0 => { - let state = flock.l_type as libc::c_int; - let state = LockState::new(state); - Ok(state) - } - -1 => { - let io_error = io::Error::last_os_error(); - Err(LockError::Io(io_error)) - } - val => panic!("Unexpected return value from fcntl(): {val}"), - } -} diff --git a/docs/disk_locking.md b/docs/disk_locking.md index adb8956a08..e205031e3a 100644 --- a/docs/disk_locking.md +++ b/docs/disk_locking.md @@ -24,11 +24,22 @@ The `lock_granularity` parameter controls how the lock is placed on the disk image: ``` +--disk path=/bar.img,lock_granularity=qemu-compatible --disk path=/foo.img,lock_granularity=byte-range --disk path=/bar.img,lock_granularity=full ``` -### `byte-range` (default) +### `qemu-compatible` (default) + +Mimics QEMU's file locking behavior. Only locks marker-bytes to express +QEMU's file locking semantics. + +For read-only disks, this translates to QEMU's `BLK_PERM_CONSISTENT_READ` +with `BLK_PERM_WRITE` unshared. +For read-write disks, this translates to QEMU's `BLK_PERM_CONSISTENT_READ` +and `BLK_PERM_WRITE` with `BLK_PERM_WRITE` unshared. + +### `byte-range` Locks the byte range `[0, physical_file_size)`. The physical file size is evaluated once at startup; if the file grows after the lock is diff --git a/virtio-devices/src/block.rs b/virtio-devices/src/block.rs index 261091e494..edb3650ab0 100644 --- a/virtio-devices/src/block.rs +++ b/virtio-devices/src/block.rs @@ -23,10 +23,10 @@ use anyhow::anyhow; use block::async_io::{AsyncIo, AsyncIoError}; use block::disk_file::AsyncFullDiskFile; use block::error::BlockError; -use block::fcntl::{LockError, LockGranularity, LockGranularityChoice, LockType, get_lock_state}; +use block::fcntl::{LockError, LockGranularity, LockGranularityChoice, LockType}; use block::{ ExecuteAsync, ExecuteError, MAX_DISCARD_WRITE_ZEROES_SEG, Request, RequestType, - VirtioBlockConfig, build_serial, fcntl, + VirtioBlockConfig, build_serial, }; use event_monitor::event; use log::{debug, error, info, warn}; @@ -771,6 +771,9 @@ pub struct Block { queue_affinity: BTreeMap>, disable_sector0_writes: bool, lock_granularity_choice: LockGranularityChoice, + /// The current lock status. + // There is no way to query what locks are already held, so we need to cache that information. + held_lock: LockType, device_status: Arc, active_request_count: Arc, draining_active_requests: Arc, @@ -937,6 +940,7 @@ impl Block { queue_affinity, disable_sector0_writes, lock_granularity_choice: lock_granularity, + held_lock: LockType::Unlock, device_status: Arc::new(AtomicU8::new(0)), active_request_count: Arc::new(AtomicUsize::new(0)), draining_active_requests: Arc::new(AtomicBool::new(false)), @@ -1001,6 +1005,7 @@ impl Block { } } } + LockGranularityChoice::QemuCompatible => LockGranularity::QemuCompatible, } } @@ -1017,21 +1022,22 @@ impl Block { self.disk_path.display() ); let fd = self.disk_image.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 { - error!("Can't get {lock_type:?} lock for {} as there is already a {current_lock:?} lock", self.disk_path.display()); - } else { - error!("Can't get {lock_type:?} lock for {}, but also can't determine the current lock state", self.disk_path.display()); - } - Error::LockDiskImage { - path: self.disk_path.clone(), - error, - lock_type, - } - })?; + granularity + .try_acquire_lock(&fd, lock_type, self.held_lock) + .map_err(|error| { + error!( + "Cannot acquire {lock_type:?} lock for disk image: id={},path={},granularity={granularity:?}", + self.id, + self.disk_path.display() + ); + + Error::LockDiskImage { + path: self.disk_path.clone(), + error, + lock_type, + } + })?; + self.held_lock = lock_type; info!( "Acquired {lock_type:?} lock for disk image id={},path={}", self.id, @@ -1048,11 +1054,15 @@ impl Block { // Should we remove the Result to simplify the error propagation on // higher levels? let fd = self.disk_image.fd(); - fcntl::clear_lock(&fd, granularity).map_err(|error| Error::LockDiskImage { - path: self.disk_path.clone(), - error, - lock_type: LockType::Unlock, - }) + granularity + .clear_lock(&fd) + .map_err(|error| Error::LockDiskImage { + path: self.disk_path.clone(), + error, + lock_type: LockType::Unlock, + })?; + self.held_lock = LockType::Unlock; + Ok(()) } fn state(&self) -> BlockState { diff --git a/vmm/src/api/openapi/cloud-hypervisor.yaml b/vmm/src/api/openapi/cloud-hypervisor.yaml index fd5ccec531..ad612b9514 100644 --- a/vmm/src/api/openapi/cloud-hypervisor.yaml +++ b/vmm/src/api/openapi/cloud-hypervisor.yaml @@ -983,8 +983,8 @@ components: LockGranularity: type: string - enum: [ByteRange, Full] - default: ByteRange + enum: [ByteRange, Full, QemuCompatible] + default: QemuCompatible DiskConfig: type: object diff --git a/vmm/src/config.rs b/vmm/src/config.rs index 29cf8de56f..c0ab9aaa6f 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -1380,7 +1380,7 @@ impl DiskConfig { rate_limit_group=,\ queue_affinity=,\ serial=,backing_files=on|off,sparse=on|off,\ - image_type=,lock_granularity=byte-range|full"; + image_type=,lock_granularity=byte-range|full|qemu-compatible"; pub fn parse(disk: &str) -> Result { let mut parser = OptionParser::new(); @@ -4222,6 +4222,13 @@ mod unit_tests { ..disk_fixture() } ); + assert_eq!( + DiskConfig::parse("path=/path/to_file,lock_granularity=qemu-compatible")?, + DiskConfig { + lock_granularity: LockGranularityChoice::QemuCompatible, + ..disk_fixture() + } + ); assert_eq!( DiskConfig::parse("path=/path/to_file,queue_affinity=[0@[1],1@[2],2@[3,4],3@[5-8]]")?, DiskConfig {