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 <rbradford@meta.com>
This commit is contained in:
Rob Bradford
2026-02-15 11:25:53 +00:00
committed by Bo Chen
parent e36096db3e
commit 6f2357c14e
7 changed files with 173 additions and 30 deletions

View File

@@ -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<Self, Self::Err> {
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;

View File

@@ -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}}}

View File

@@ -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()

View File

@@ -947,6 +947,9 @@ components:
sparse:
type: boolean
default: true
image_type:
type: enum ["FixedVhd", "Qcow2", "Raw", "Vhdx"]
NetConfig:
type: object

View File

@@ -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=<io_ops>,ops_one_time_burst=<io_ops>,ops_refill_time=<ms>,\
id=<device_id>,pci_segment=<segment_id>,rate_limit_group=<group_id>,\
queue_affinity=<list_of_queue_indices_with_their_associated_cpuset>,\
serial=<serial_number>,backing_files=on|off,sparse=on|off";
serial=<serial_number>,backing_files=on|off,sparse=on|off,\
image_type=<raw,qcow2,vhd,vhdx>";
pub fn parse(disk: &str) -> Result<Self> {
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::<Toggle>("backing_files")
.map_err(Error::ParseDisk)?
.unwrap_or(Toggle(false))
.0;
let image_type = if vhost_socket.is_none() {
parser
.convert::<ImageType>("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()
}
);

View File

@@ -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<T> = result::Result<T, DeviceManagerError>;
@@ -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<dyn DiskFile>
}
ImageType::Unknown => unreachable!(),
};
let rate_limit_group =

View File

@@ -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 {