From 7c690ffec020a56acea587f52518ce9bc7fb3533 Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Fri, 6 Mar 2026 12:09:09 -0800 Subject: [PATCH] vmm: config: Expose disk lock granularity option Add a per-disk lock_granularity parameter that lets users choose between byte-range OFD locks and whole-file OFD locks: --disk path=/foo.img,lock_granularity=byte-range --disk path=/bar.img,lock_granularity=full Byte-range is the default and matches QEMU behavior, working best with storage backends where whole-file OFD locks are treated as mandatory. The full option restores the original whole-file locking for environments that depend on it. The LockGranularityChoice enum and its FromStr impl live in the block crate alongside the existing LockGranularity type. The Block device converts the user-facing choice to the internal LockGranularity at lock time, keeping device_manager.rs simple. Closes: #7553 Signed-off-by: Victor Vieux --- block/src/fcntl.rs | 33 ++++++++++++++++++ fuzz/fuzz_targets/block.rs | 2 ++ virtio-devices/src/block.rs | 41 +++++++++++++---------- vmm/src/api/openapi/cloud-hypervisor.yaml | 5 ++- vmm/src/config.rs | 26 ++++++++++++-- vmm/src/device_manager.rs | 1 + vmm/src/vm_config.rs | 3 ++ 7 files changed, 91 insertions(+), 20 deletions(-) diff --git a/block/src/fcntl.rs b/block/src/fcntl.rs index a2a684f32..23c6f9f16 100644 --- a/block/src/fcntl.rs +++ b/block/src/fcntl.rs @@ -16,6 +16,7 @@ use std::fmt::Debug; use std::io; use std::os::fd::{AsRawFd, RawFd}; +use std::str::FromStr; use thiserror::Error; @@ -140,6 +141,38 @@ impl LockGranularity { } } +/// User-facing choice for the lock granularity. +/// +/// This allows external management software to create snapshots of the disk +/// image. Without a byte-range lock, some NFS implementations may treat the +/// entire file as exclusively locked and prevent such operations (e.g. NetApp). +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, serde::Deserialize, serde::Serialize)] +#[serde(rename_all = "kebab-case")] +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, +} + +/// Error returned when parsing a [`LockGranularityChoice`] from a string. +#[derive(Error, Debug)] +#[error("Invalid lock granularity value: {0}, expected 'byte-range' or 'full'")] +pub struct LockGranularityParseError(String); + +impl FromStr for LockGranularityChoice { + type Err = LockGranularityParseError; + + fn from_str(s: &str) -> Result { + match s { + "byte-range" => Ok(LockGranularityChoice::ByteRange), + "full" => Ok(LockGranularityChoice::Full), + _ => 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 { diff --git a/fuzz/fuzz_targets/block.rs b/fuzz/fuzz_targets/block.rs index 7d1fbdf38..0ad9193fd 100644 --- a/fuzz/fuzz_targets/block.rs +++ b/fuzz/fuzz_targets/block.rs @@ -16,6 +16,7 @@ use std::sync::Arc; use std::{ffi, io}; use block::async_io::DiskFile; +use block::fcntl::LockGranularityChoice; use block::raw_sync::RawFileDiskSync; use libfuzzer_sys::{fuzz_target, Corpus}; use seccompiler::SeccompAction; @@ -69,6 +70,7 @@ fuzz_target!(|bytes: &[u8]| -> Corpus { queue_affinity, true, false, + LockGranularityChoice::default(), ) .unwrap(); diff --git a/virtio-devices/src/block.rs b/virtio-devices/src/block.rs index 9d09ab91a..9bb97d31c 100644 --- a/virtio-devices/src/block.rs +++ b/virtio-devices/src/block.rs @@ -19,7 +19,7 @@ use std::{io, result}; use anyhow::anyhow; use block::async_io::{AsyncIo, AsyncIoError, DiskFile, DiskFileError}; -use block::fcntl::{LockError, LockGranularity, LockType, get_lock_state}; +use block::fcntl::{LockError, LockGranularity, LockGranularityChoice, LockType, get_lock_state}; use block::{ ExecuteAsync, ExecuteError, Request, RequestType, VirtioBlockConfig, build_serial, fcntl, }; @@ -662,6 +662,7 @@ pub struct Block { serial: Vec, queue_affinity: BTreeMap>, disable_sector0_writes: bool, + lock_granularity_choice: LockGranularityChoice, } #[derive(Serialize, Deserialize)] @@ -692,6 +693,7 @@ impl Block { queue_affinity: BTreeMap>, sparse: bool, disable_sector0_writes: bool, + lock_granularity: LockGranularityChoice, ) -> io::Result { let (disk_nsectors, avail_features, acked_features, config, paused) = if let Some(state) = state { @@ -807,6 +809,7 @@ impl Block { serial, queue_affinity, disable_sector0_writes, + lock_granularity_choice: lock_granularity, }) } @@ -815,23 +818,27 @@ impl Block { } /// Returns the granularity for the advisory lock for this disk. - // TODO In future, we could add a `lock_granularity=` configuration to the CLI. - // For now, we stick to QEMU behavior. fn lock_granularity(&mut self) -> LockGranularity { - self.disk_image.physical_size().map_or_else( - // use a safe fallback - |e| { - let fallback = LockGranularity::WholeFile; - warn!( - "Can't get disk size for id={},path={}, falling back to {:?}: error: {e}", - self.id, - self.disk_path.display(), - fallback - ); - fallback - }, - |size| LockGranularity::ByteRange(0, size), - ) + match self.lock_granularity_choice { + LockGranularityChoice::Full => LockGranularity::WholeFile, + LockGranularityChoice::ByteRange => { + // Byte-range lock covering [0, size) + self.disk_image.physical_size().map_or_else( + // use a safe fallback + |e| { + let fallback = LockGranularity::WholeFile; + warn!( + "Can't get disk size for id={},path={}, falling back to {:?}: error: {e}", + self.id, + self.disk_path.display(), + fallback + ); + fallback + }, + |size| LockGranularity::ByteRange(0, size), + ) + } + } } /// Tries to set an advisory lock for the corresponding disk image. diff --git a/vmm/src/api/openapi/cloud-hypervisor.yaml b/vmm/src/api/openapi/cloud-hypervisor.yaml index c4f4b6acf..77b16e97f 100644 --- a/vmm/src/api/openapi/cloud-hypervisor.yaml +++ b/vmm/src/api/openapi/cloud-hypervisor.yaml @@ -980,7 +980,10 @@ components: image_type: type: string enum: [FixedVhd, Qcow2, Raw, Vhdx, Unknown] - + lock_granularity: + type: string + enum: [byte-range, full] + default: byte-range NetConfig: type: object diff --git a/vmm/src/config.rs b/vmm/src/config.rs index 46f344313..b4c04570f 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -1159,7 +1159,7 @@ impl DiskConfig { id=,pci_segment=,rate_limit_group=,\ queue_affinity=,\ serial=,backing_files=on|off,sparse=on|off,\ - image_type="; + image_type=,lock_granularity=byte-range|full"; pub fn parse(disk: &str) -> Result { let mut parser = OptionParser::new(); @@ -1187,7 +1187,8 @@ impl DiskConfig { .add("queue_affinity") .add("backing_files") .add("sparse") - .add("image_type"); + .add("image_type") + .add("lock_granularity"); parser.parse(disk).map_err(Error::ParseDisk)?; @@ -1289,6 +1290,11 @@ impl DiskConfig { ImageType::Unknown }; + let lock_granularity = parser + .convert::("lock_granularity") + .map_err(Error::ParseDisk)? + .unwrap_or_default(); + let bw_tb_config = if bw_size != 0 && bw_refill_time != 0 { Some(TokenBucketConfig { size: bw_size, @@ -1341,6 +1347,7 @@ impl DiskConfig { backing_files, sparse, image_type, + lock_granularity, }) } @@ -3800,6 +3807,7 @@ mod unit_tests { backing_files: false, sparse: true, image_type: ImageType::Unknown, + lock_granularity: LockGranularityChoice::default(), } } @@ -3871,6 +3879,20 @@ mod unit_tests { ..disk_fixture() } ); + assert_eq!( + DiskConfig::parse("path=/path/to_file,lock_granularity=full")?, + DiskConfig { + lock_granularity: LockGranularityChoice::Full, + ..disk_fixture() + } + ); + assert_eq!( + DiskConfig::parse("path=/path/to_file,lock_granularity=byte-range")?, + DiskConfig { + lock_granularity: LockGranularityChoice::ByteRange, + ..disk_fixture() + } + ); assert_eq!( DiskConfig::parse("path=/path/to_file,queue_affinity=[0@[1],1@[2],2@[3,4],3@[5-8]]")?, DiskConfig { diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 1ac9afe30..c3c5618bd 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -2853,6 +2853,7 @@ impl DeviceManager { queue_affinity, disk_cfg.sparse, disable_sector0_writes, + disk_cfg.lock_granularity, ) .map_err(DeviceManagerError::CreateVirtioBlock)?; diff --git a/vmm/src/vm_config.rs b/vmm/src/vm_config.rs index 33c2b23ac..d453ead2d 100644 --- a/vmm/src/vm_config.rs +++ b/vmm/src/vm_config.rs @@ -9,6 +9,7 @@ use std::str::FromStr; use std::{fs, result}; use block::ImageType; +pub use block::fcntl::LockGranularityChoice; use log::{debug, warn}; use net_util::MacAddr; use serde::{Deserialize, Serialize}; @@ -302,6 +303,8 @@ pub struct DiskConfig { pub sparse: bool, #[serde(default)] pub image_type: ImageType, + #[serde(default)] + pub lock_granularity: LockGranularityChoice, } impl ApplyLandlock for DiskConfig {