diff --git a/Cargo.lock b/Cargo.lock index 62bbf709f..099bf6b5b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -421,6 +421,7 @@ version = "50.0.0" dependencies = [ "anyhow", "api_client", + "block", "clap", "dhat", "dirs", diff --git a/block/src/lib.rs b/block/src/lib.rs index 72210302a..afd4b0dd7 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}; @@ -788,12 +789,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/Cargo.toml b/cloud-hypervisor/Cargo.toml index 426a52263..c38ae2231 100644 --- a/cloud-hypervisor/Cargo.toml +++ b/cloud-hypervisor/Cargo.toml @@ -41,6 +41,7 @@ vmm-sys-util = { workspace = true } zbus = { version = "5.7.1", optional = true } [dev-dependencies] +block = { path = "../block" } dirs = { workspace = true } net_util = { path = "../net_util" } serde_json = { workspace = true } diff --git a/cloud-hypervisor/src/main.rs b/cloud-hypervisor/src/main.rs index d08293b6e..82d3ce05c 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 386e71a3f..77df84fb1 100644 --- a/cloud-hypervisor/tests/integration.rs +++ b/cloud-hypervisor/tests/integration.rs @@ -2526,6 +2526,8 @@ mod common_parallel { use std::fs::OpenOptions; use std::io::SeekFrom; + use block::ImageType; + use crate::*; #[test] @@ -3172,7 +3174,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(); @@ -3413,6 +3415,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)); @@ -3433,9 +3436,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!( @@ -3505,17 +3508,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); } /// Uses `qemu-img check` to verify disk image consistency. @@ -3551,17 +3554,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] @@ -3572,6 +3596,7 @@ mod common_parallel { false, true, true, + ImageType::Qcow2, ); } @@ -3583,6 +3608,7 @@ mod common_parallel { false, true, true, + ImageType::Qcow2, ); } @@ -3608,7 +3634,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] @@ -3632,7 +3665,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] @@ -4697,7 +4737,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([ diff --git a/vmm/src/api/openapi/cloud-hypervisor.yaml b/vmm/src/api/openapi/cloud-hypervisor.yaml index d10832101..aca4cc28e 100644 --- a/vmm/src/api/openapi/cloud-hypervisor.yaml +++ b/vmm/src/api/openapi/cloud-hypervisor.yaml @@ -944,6 +944,9 @@ components: backing_files: type: boolean default: false + image_type: + type: enum ["FixedVhd", "Qcow2", "Raw", "Vhdx"] + NetConfig: type: object diff --git a/vmm/src/config.rs b/vmm/src/config.rs index e1f9213d3..ed909362a 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::{ @@ -1093,7 +1094,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"; + serial=,backing_files=on|off,\ + image_type="; pub fn parse(disk: &str) -> Result { let mut parser = OptionParser::new(); @@ -1119,7 +1121,9 @@ impl DiskConfig { .add("serial") .add("rate_limit_group") .add("queue_affinity") - .add("backing_files"); + .add("backing_files") + .add("image_type"); + parser.parse(disk).map_err(Error::ParseDisk)?; let path = parser.get("path").map(PathBuf::from); @@ -1204,12 +1208,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, @@ -1255,6 +1269,7 @@ impl DiskConfig { serial, queue_affinity, backing_files, + image_type, }) } @@ -3423,6 +3438,7 @@ mod unit_tests { serial: None, queue_affinity: None, backing_files: false, + image_type: ImageType::Unknown, } } @@ -3445,6 +3461,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 9b2a32455..8ca636ab8 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -674,6 +674,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; @@ -2654,14 +2663,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. @@ -2725,6 +2760,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 a71cca7be..fc625c0c7 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}; @@ -286,6 +287,8 @@ pub struct DiskConfig { pub queue_affinity: Option>, #[serde(default)] pub backing_files: bool, + #[serde(default)] + pub image_type: ImageType, } impl ApplyLandlock for DiskConfig {