From dd8687aebbae67e1fcf9a4c2b1063c2ecbd60d27 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Wed, 25 Jun 2025 13:49:05 +0200 Subject: [PATCH] vmm: add enum PayloadConfigError validation to improve error reporting Currently, the following scenarios are supported by Cloud Hypervisor to bootstrap a VM: 1. provide firmware 2. provide kernel 3. provide kernel + cmdline 4. provide kernel + initrd 5. provide kernel + cmdline + initrd As the difference between `--firmware` and `--kernel` is not very clear currently, especially as both use/support a Xen PVH entry, adding this helps to identify the cause of misconfiguration. Signed-off-by: Philipp Schuster On-behalf-of: SAP philipp.schuster@sap.com --- README.md | 14 +++++++++++++- vmm/src/config.rs | 17 +++++++++++------ vmm/src/vm.rs | 17 ++++++----------- vmm/src/vm_config.rs | 45 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 4609903d0..fdb18255f 100644 --- a/README.md +++ b/README.md @@ -153,7 +153,7 @@ interface will be enabled as per `network-config` details. $ sudo setcap cap_net_admin+ep ./cloud-hypervisor $ ./create-cloud-init.sh $ ./cloud-hypervisor \ - --kernel ./hypervisor-fw \ + --firmware ./hypervisor-fw \ --disk path=focal-server-cloudimg-amd64.raw path=/tmp/ubuntu-cloudinit.img \ --cpus boot=4 \ --memory size=1024M \ @@ -175,6 +175,18 @@ $ ./cloud-hypervisor \ --console off ``` +## Booting: `--firmware` vs `--kernel` + +The following scenarios are supported by Cloud Hypervisor to bootstrap a VM, i.e., +to load a payload/bootitem(s): + +- Provide firmware +- Provide kernel \[+ cmdline\]\ [+ initrd\] + +Please note that our Cloud Hypervisor firmware (`hypervisor-fw`) has a Xen PVH +boot entry, therefore it can also be booted via the `--kernel` parameter, as +seen in some examples. + ### Custom Kernel and Disk Image #### Building your Kernel diff --git a/vmm/src/config.rs b/vmm/src/config.rs index 1977c8728..06523761a 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -181,9 +181,6 @@ pub enum Error { #[derive(Debug, PartialEq, Eq, Error)] pub enum ValidationError { - /// No kernel specified - #[error("No kernel specified")] - KernelMissing, /// Missing file value for console #[error("Path missing when using file console mode")] ConsoleFileMissing, @@ -356,6 +353,8 @@ pub enum ValidationError { /// Invalid Ivshmem backend file path #[error("Invalid ivshmem backend file path")] InvalidIvshmemPath, + #[error("Payload configuration is not bootable")] + PayloadError(#[from] PayloadConfigError), } type ValidationResult = std::result::Result; @@ -2507,9 +2506,13 @@ impl VmConfig { pub fn validate(&mut self) -> ValidationResult> { let mut id_list = BTreeSet::new(); + // Is the payload configuration bootable? self.payload - .as_ref() - .ok_or(ValidationError::KernelMissing)?; + .as_mut() + .ok_or(ValidationError::PayloadError( + PayloadConfigError::MissingBootitem, + ))? + .validate()?; #[cfg(feature = "tdx")] { @@ -4216,7 +4219,9 @@ mod tests { invalid_config.payload = None; assert_eq!( invalid_config.validate(), - Err(ValidationError::KernelMissing) + Err(ValidationError::PayloadError( + PayloadConfigError::MissingBootitem + )) ); let mut invalid_config = valid_config.clone(); diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 7e8851194..347b39dc3 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -317,9 +317,6 @@ pub enum Error { #[error("Error joining kernel loading thread")] KernelLoadThreadJoin(std::boxed::Box), - #[error("Payload configuration is not bootable")] - InvalidPayload, - #[cfg(all(target_arch = "x86_64", feature = "guest_debug"))] #[error("Error coredumping VM")] Coredump(#[source] GuestDebuggableError), @@ -1217,7 +1214,7 @@ impl Vm { Self::load_firmware(&firmware, memory_manager)?; arch::layout::UEFI_START } - _ => return Err(Error::InvalidPayload), + _ => unreachable!("Unsupported boot configuration: programming error from 'PayloadConfigError::validate()'"), }; Ok(EntryPoint { entry_addr }) @@ -1267,7 +1264,7 @@ impl Vm { Self::load_firmware(&firmware, memory_manager)?; arch::layout::UEFI_START } - _ => return Err(Error::InvalidPayload), + _ => unreachable!("Unsupported boot configuration: programming error from 'PayloadConfigError::validate()'"), }; Ok(EntryPoint { entry_addr }) @@ -1385,19 +1382,17 @@ impl Vm { match ( &payload.firmware, &payload.kernel, - &payload.initramfs, - &payload.cmdline, ) { - (Some(firmware), None, None, None) => { + (Some(firmware), None) => { let firmware = File::open(firmware).map_err(Error::FirmwareFile)?; Self::load_kernel(firmware, None, memory_manager) } - (None, Some(kernel), _, _) => { + (None, Some(kernel)) => { let kernel = File::open(kernel).map_err(Error::KernelFile)?; let cmdline = Self::generate_cmdline(payload)?; Self::load_kernel(kernel, Some(cmdline), memory_manager) } - _ => Err(Error::InvalidPayload), + _ => unreachable!("Unsupported boot configuration: programming error from 'PayloadConfigError::validate()'"), } } @@ -1415,7 +1410,7 @@ impl Vm { let kernel = File::open(kernel).map_err(Error::KernelFile)?; Self::load_kernel(None, Some(kernel), memory_manager) } - _ => Err(Error::InvalidPayload), + _ => unreachable!("Unsupported boot configuration: programming error from 'PayloadConfigError::validate()'"), } } diff --git a/vmm/src/vm_config.rs b/vmm/src/vm_config.rs index 50841eeed..cf1f61e05 100644 --- a/vmm/src/vm_config.rs +++ b/vmm/src/vm_config.rs @@ -10,6 +10,7 @@ use std::{fs, result}; use net_util::MacAddr; use serde::{Deserialize, Serialize}; +use thiserror::Error; use virtio_devices::RateLimiterConfig; use crate::landlock::LandlockError; @@ -705,6 +706,21 @@ pub struct NumaConfig { pub pci_segments: Option>, } +/// Errors describing a misconfigured payload, i.e., a configuration that +/// can't be booted by Cloud Hypervisor. +/// +/// This typically is the case for invalid combinations of cmdline, kernel, +/// firmware, and initrd. +#[derive(Debug, Error, PartialEq, Eq)] +pub enum PayloadConfigError { + /// Specifying a kernel is not supported when a firmware is provided. + #[error("Specifying a kernel is not supported when a firmware is provided")] + FirmwarePlusOtherPayloads, + /// No bootitem provided: neither firmware nor kernel. + #[error("No bootitem provided: neither firmware nor kernel")] + MissingBootitem, +} + #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] pub struct PayloadConfig { #[serde(default)] @@ -796,6 +812,35 @@ impl FromStr for FwCfgItemList { } } +impl PayloadConfig { + /// Validates the payload config. + /// + /// Succeeds if Cloud Hypervisor will be able to boot the configuration. + /// Further, warns for some odd configurations. + pub fn validate(&mut self) -> Result<(), PayloadConfigError> { + match (&self.firmware, &self.kernel) { + (Some(_firmware), Some(_kernel)) => Err(PayloadConfigError::FirmwarePlusOtherPayloads), + (Some(_firmware), None) => { + if self.cmdline.is_some() { + log::warn!("Ignoring cmdline parameter as firmware is provided as the payload"); + self.cmdline = None; + } + if self.initramfs.is_some() { + log::warn!( + "Ignoring initramfs parameter as firmware is provided as the payload" + ); + self.initramfs = None; + } + Ok(()) + } + (None, Some(_kernel)) => Ok(()), + (None, None) => Err(PayloadConfigError::MissingBootitem), + }?; + + Ok(()) + } +} + impl ApplyLandlock for PayloadConfig { fn apply_landlock(&self, landlock: &mut Landlock) -> LandlockResult<()> { // Payload only needs read access