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 {