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 {