From 6f2357c14ed83582fc085bc74acfe3a270e6320e Mon Sep 17 00:00:00 2001 From: Rob Bradford Date: Sun, 15 Feb 2026 11:25:53 +0000 Subject: [PATCH] vmm: Improve resiliency of image type handling Add an image_type to DiskConfig to specify the image type. If none is specified autodetect the image type but disable potentially unsafe behaviour in the QCOW2 backend by disabling the backing file support. If the image type is autodetected then fix it in the config so that it will be persistant across reboots and migrations/snapshot & restores. This also handles the case where the image type was not specified as part of the disk configuration. Signed-off-by: Rob Bradford --- block/src/lib.rs | 37 ++++++++++- cloud-hypervisor/src/main.rs | 14 ++-- cloud-hypervisor/tests/integration.rs | 81 ++++++++++++++++++----- vmm/src/api/openapi/cloud-hypervisor.yaml | 3 + vmm/src/config.rs | 21 +++++- vmm/src/device_manager.rs | 44 ++++++++++-- vmm/src/vm_config.rs | 3 + 7 files changed, 173 insertions(+), 30 deletions(-) diff --git a/block/src/lib.rs b/block/src/lib.rs index 0d248fb20..d8ae27647 100644 --- a/block/src/lib.rs +++ b/block/src/lib.rs @@ -30,12 +30,13 @@ pub mod vhdx_sync; use std::alloc::{Layout, alloc_zeroed, dealloc}; use std::collections::VecDeque; -use std::fmt::Debug; +use std::fmt::{self, Debug}; use std::fs::File; use std::io::{self, IoSlice, IoSliceMut, Read, Seek, SeekFrom, Write}; use std::os::linux::fs::MetadataExt; use std::os::unix::io::AsRawFd; use std::path::Path; +use std::str::FromStr; use std::time::Instant; use std::{cmp, result}; @@ -1005,12 +1006,44 @@ pub trait AsyncAdaptor { } } -#[derive(PartialEq, Eq, Debug)] +#[derive(Serialize, Deserialize, Clone, Copy, Debug, PartialEq, Eq, Default)] pub enum ImageType { FixedVhd, Qcow2, Raw, Vhdx, + #[default] + Unknown, +} + +impl fmt::Display for ImageType { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ImageType::FixedVhd => write!(f, "vhd"), + ImageType::Qcow2 => write!(f, "qcow2"), + ImageType::Raw => write!(f, "raw"), + ImageType::Vhdx => write!(f, "vhdx"), + ImageType::Unknown => write!(f, "unknown"), + } + } +} + +pub enum ImageTypeParseError { + InvalidValue(String), +} + +impl FromStr for ImageType { + type Err = ImageTypeParseError; + + fn from_str(s: &str) -> Result { + match s.to_lowercase().as_str() { + "vhd" => Ok(ImageType::FixedVhd), + "qcow2" => Ok(ImageType::Qcow2), + "raw" => Ok(ImageType::Raw), + "vhdx" => Ok(ImageType::Vhdx), + _ => Err(ImageTypeParseError::InvalidValue(s.to_string())), + } + } } const QCOW_MAGIC: u32 = 0x5146_49fb; diff --git a/cloud-hypervisor/src/main.rs b/cloud-hypervisor/src/main.rs index 3abc382f3..1d78b400b 100644 --- a/cloud-hypervisor/src/main.rs +++ b/cloud-hypervisor/src/main.rs @@ -1199,14 +1199,14 @@ mod unit_tests { "--kernel", "/path/to/kernel", "--disk", - "path=/path/to/disk/1", + "path=/path/to/disk/1,image_type=raw", "path=/path/to/disk/2", ], r#"{ "payload": {"kernel": "/path/to/kernel"}, "disks": [ - {"path": "/path/to/disk/1"}, - {"path": "/path/to/disk/2"} + {"path": "/path/to/disk/1", "image_type": "Raw"}, + {"path": "/path/to/disk/2", "image_type": "Unknown"} ] }"#, true, @@ -1217,8 +1217,8 @@ mod unit_tests { "--kernel", "/path/to/kernel", "--disk", - "path=/path/to/disk/1", - "path=/path/to/disk/2", + "path=/path/to/disk/1,image_type=raw", + "path=/path/to/disk/2,image_type=qcow2", ], r#"{ "payload": {"kernel": "/path/to/kernel"}, @@ -1280,8 +1280,8 @@ mod unit_tests { r#"{ "payload": {"kernel": "/path/to/kernel"}, "disks": [ - {"path": "/path/to/disk/1", "rate_limit_group": "group0"}, - {"path": "/path/to/disk/2", "rate_limit_group": "group0"} + {"path": "/path/to/disk/1", "rate_limit_group": "group0", "image_type": "Unknown"}, + {"path": "/path/to/disk/2", "rate_limit_group": "group0", "image_type": "Unknown"} ], "rate_limit_groups": [ {"id": "group0", "rate_limiter_config": {"bandwidth": {"size": 1000, "one_time_burst": 0, "refill_time": 100}}} diff --git a/cloud-hypervisor/tests/integration.rs b/cloud-hypervisor/tests/integration.rs index 3d54b4bd7..1aad1a372 100644 --- a/cloud-hypervisor/tests/integration.rs +++ b/cloud-hypervisor/tests/integration.rs @@ -2627,6 +2627,8 @@ mod common_parallel { use std::io::{self, SeekFrom}; use std::process::Command; + use block::ImageType; + use crate::*; #[test] @@ -3186,7 +3188,7 @@ mod common_parallel { guest.disk_config.disk(DiskType::CloudInit).unwrap() ) .as_str(), - format!("path={test_disk_path},pci_segment=15").as_str(), + format!("path={test_disk_path},pci_segment=15,image_type=raw").as_str(), ]) .capture_output() .default_net(); @@ -3423,6 +3425,7 @@ mod common_parallel { disable_aio: bool, verify_os_disk: bool, backing_files: bool, + image_type: ImageType, ) { let disk_config = UbuntuDiskConfig::new(image_name.to_string()); let guest = Guest::new(Box::new(disk_config)); @@ -3449,9 +3452,9 @@ mod common_parallel { .args([ "--disk", format!( - "path={},backing_files={}", + "path={},backing_files={},image_type={image_type}", guest.disk_config.disk(DiskType::OperatingSystem).unwrap(), - if backing_files { "on"} else {"off"} + if backing_files { "on"} else {"off"}, ) .as_str(), format!( @@ -3530,17 +3533,17 @@ mod common_parallel { #[test] fn test_virtio_block_io_uring() { - _test_virtio_block(FOCAL_IMAGE_NAME, false, true, false, false); + _test_virtio_block(FOCAL_IMAGE_NAME, false, true, false, false, ImageType::Raw); } #[test] fn test_virtio_block_aio() { - _test_virtio_block(FOCAL_IMAGE_NAME, true, false, false, false); + _test_virtio_block(FOCAL_IMAGE_NAME, true, false, false, false, ImageType::Raw); } #[test] fn test_virtio_block_sync() { - _test_virtio_block(FOCAL_IMAGE_NAME, true, true, false, false); + _test_virtio_block(FOCAL_IMAGE_NAME, true, true, false, false, ImageType::Raw); } fn run_qemu_img(path: &std::path::Path, args: &[&str]) -> std::process::Output { @@ -3770,17 +3773,38 @@ mod common_parallel { #[test] fn test_virtio_block_qcow2() { - _test_virtio_block(JAMMY_IMAGE_NAME_QCOW2, false, false, true, false); + _test_virtio_block( + JAMMY_IMAGE_NAME_QCOW2, + false, + false, + true, + false, + ImageType::Qcow2, + ); } #[test] fn test_virtio_block_qcow2_zlib() { - _test_virtio_block(JAMMY_IMAGE_NAME_QCOW2_ZLIB, false, false, true, false); + _test_virtio_block( + JAMMY_IMAGE_NAME_QCOW2_ZLIB, + false, + false, + true, + false, + ImageType::Qcow2, + ); } #[test] fn test_virtio_block_qcow2_zstd() { - _test_virtio_block(JAMMY_IMAGE_NAME_QCOW2_ZSTD, false, false, true, false); + _test_virtio_block( + JAMMY_IMAGE_NAME_QCOW2_ZSTD, + false, + false, + true, + false, + ImageType::Qcow2, + ); } #[test] @@ -3791,6 +3815,7 @@ mod common_parallel { false, true, true, + ImageType::Qcow2, ); } @@ -3802,6 +3827,7 @@ mod common_parallel { false, true, true, + ImageType::Qcow2, ); } @@ -3813,6 +3839,7 @@ mod common_parallel { false, true, true, + ImageType::Qcow2, ); } @@ -3889,7 +3916,7 @@ mod common_parallel { guest.disk_config.disk(DiskType::CloudInit).unwrap() ), &format!( - "path={},num_queues=8,backing_files={}", + "path={},num_queues=8,backing_files={},image_type=qcow2", test_image_path.to_str().unwrap(), if initial_backing_checksum.is_some() { "on" @@ -4696,7 +4723,14 @@ mod common_parallel { .output() .expect("Expect generating VHD image from RAW image"); - _test_virtio_block(FOCAL_IMAGE_NAME_VHD, false, false, false, false); + _test_virtio_block( + FOCAL_IMAGE_NAME_VHD, + false, + false, + false, + false, + ImageType::FixedVhd, + ); } #[test] @@ -4720,7 +4754,14 @@ mod common_parallel { .output() .expect("Expect generating dynamic VHDx image from RAW image"); - _test_virtio_block(FOCAL_IMAGE_NAME_VHDX, false, false, true, false); + _test_virtio_block( + FOCAL_IMAGE_NAME_VHDX, + false, + false, + true, + false, + ImageType::Vhdx, + ); } #[test] @@ -5785,7 +5826,7 @@ mod common_parallel { guest.disk_config.disk(DiskType::CloudInit).unwrap() ) .as_str(), - format!("path={}", vfio_disk_path.to_str().unwrap()).as_str(), + format!("path={},image_type=raw", vfio_disk_path.to_str().unwrap()).as_str(), format!("path={},iommu=on,readonly=true", blk_file_path.to_str().unwrap()).as_str(), ]) .args([ @@ -7364,7 +7405,12 @@ mod common_parallel { guest.disk_config.disk(DiskType::CloudInit).unwrap() ) .as_str(), - format!("path={},num_queues=4", test_disk_path.to_str().unwrap()).as_str(), + format!( + "path={},num_queues=4,image_type={}", + test_disk_path.to_str().unwrap(), + format_name.to_lowercase() + ) + .as_str(), ]) .default_net() .capture_output() @@ -7600,7 +7646,12 @@ mod common_parallel { guest.disk_config.disk(DiskType::CloudInit).unwrap() ) .as_str(), - format!("path={},num_queues=4", test_disk_path.to_str().unwrap()).as_str(), + format!( + "path={},num_queues=4,image_type={}", + test_disk_path.to_str().unwrap(), + format_name.to_lowercase() + ) + .as_str(), ]) .default_net() .capture_output() diff --git a/vmm/src/api/openapi/cloud-hypervisor.yaml b/vmm/src/api/openapi/cloud-hypervisor.yaml index 01c1b9ed9..359a7d9af 100644 --- a/vmm/src/api/openapi/cloud-hypervisor.yaml +++ b/vmm/src/api/openapi/cloud-hypervisor.yaml @@ -947,6 +947,9 @@ components: sparse: type: boolean default: true + image_type: + type: enum ["FixedVhd", "Qcow2", "Raw", "Vhdx"] + NetConfig: type: object diff --git a/vmm/src/config.rs b/vmm/src/config.rs index 16089e558..a4339c27b 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -10,6 +10,7 @@ use std::path::PathBuf; use std::result; use std::str::FromStr; +use block::ImageType; use clap::ArgMatches; use log::{debug, warn}; use option_parser::{ @@ -1096,7 +1097,8 @@ impl DiskConfig { ops_size=,ops_one_time_burst=,ops_refill_time=,\ id=,pci_segment=,rate_limit_group=,\ queue_affinity=,\ - serial=,backing_files=on|off,sparse=on|off"; + serial=,backing_files=on|off,sparse=on|off,\ + image_type="; pub fn parse(disk: &str) -> Result { let mut parser = OptionParser::new(); @@ -1123,7 +1125,9 @@ impl DiskConfig { .add("rate_limit_group") .add("queue_affinity") .add("backing_files") - .add("sparse"); + .add("sparse") + .add("image_type"); + parser.parse(disk).map_err(Error::ParseDisk)?; let path = parser.get("path").map(PathBuf::from); @@ -1208,12 +1212,22 @@ impl DiskConfig { }) .collect() }); + let backing_files = parser .convert::("backing_files") .map_err(Error::ParseDisk)? .unwrap_or(Toggle(false)) .0; + let image_type = if vhost_socket.is_none() { + parser + .convert::("image_type") + .map_err(Error::ParseDisk)? + .unwrap_or(ImageType::Unknown) + } else { + ImageType::Unknown + }; + let bw_tb_config = if bw_size != 0 && bw_refill_time != 0 { Some(TokenBucketConfig { size: bw_size, @@ -1265,6 +1279,7 @@ impl DiskConfig { queue_affinity, backing_files, sparse, + image_type, }) } @@ -3516,6 +3531,7 @@ mod unit_tests { queue_affinity: None, backing_files: false, sparse: true, + image_type: ImageType::Unknown, } } @@ -3538,6 +3554,7 @@ mod unit_tests { path: None, vhost_socket: Some(String::from("/tmp/sock")), vhost_user: true, + image_type: ImageType::Unknown, ..disk_fixture() } ); diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 842f529c8..d12317d8b 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -677,6 +677,15 @@ pub enum DeviceManagerError { /// Disk resizing failed. #[error("Disk resize error")] DiskResize(#[source] virtio_devices::block::Error), + + /// Disk image type does not match expected type. + #[error( + "Disk image type does not match expected type: specified = {specified}, detected = {detected}" + )] + DiskImageTypeMismatch { + specified: ImageType, + detected: ImageType, + }, } pub type DeviceManagerResult = result::Result; @@ -2665,14 +2674,40 @@ impl DeviceManager { .clone(), ) .map_err(DeviceManagerError::Disk)?; - let image_type = - detect_image_type(&mut file).map_err(DeviceManagerError::DetectImageType)?; - if image_type != ImageType::Qcow2 && disk_cfg.backing_files { + let detected_image_type = + detect_image_type(&mut file).map_err(DeviceManagerError::DetectImageType)?; + if disk_cfg.image_type == ImageType::Unknown { + warn!( + "No image_type specified - detected as {detected_image_type}. \ + Configuration updated to persist type across reboots and migrations." + ); + + if detected_image_type != ImageType::Raw { + warn!( + "Non-raw image type detected. In the future it will be necessary \ + to specify image_type for non-raw files." + ); + } + + if detected_image_type == ImageType::Qcow2 && disk_cfg.backing_files { + warn!("QCOW2 image type autodetected. Disabling backing files"); + disk_cfg.backing_files = false; + } + + disk_cfg.image_type = detected_image_type; + } else if disk_cfg.image_type != detected_image_type { + return Err(DeviceManagerError::DiskImageTypeMismatch { + specified: disk_cfg.image_type, + detected: detected_image_type, + }); + } + + if disk_cfg.image_type != ImageType::Qcow2 && disk_cfg.backing_files { warn!("Enabling backing_files option only applies for QCOW2 files"); } - let image = match image_type { + let image = match disk_cfg.image_type { ImageType::FixedVhd => { // Use asynchronous backend relying on io_uring if the // syscalls are supported. @@ -2749,6 +2784,7 @@ impl DeviceManager { .map_err(DeviceManagerError::CreateFixedVhdxDiskSync)?, ) as Box } + ImageType::Unknown => unreachable!(), }; let rate_limit_group = diff --git a/vmm/src/vm_config.rs b/vmm/src/vm_config.rs index 407d4e491..5de8c3145 100644 --- a/vmm/src/vm_config.rs +++ b/vmm/src/vm_config.rs @@ -8,6 +8,7 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use std::{fs, result}; +use block::ImageType; use log::{debug, warn}; use net_util::MacAddr; use serde::{Deserialize, Serialize}; @@ -288,6 +289,8 @@ pub struct DiskConfig { pub backing_files: bool, #[serde(default = "default_diskconfig_sparse")] pub sparse: bool, + #[serde(default)] + pub image_type: ImageType, } impl ApplyLandlock for DiskConfig {