diff --git a/CHANGELOG.md b/CHANGELOG.md index c993dfe8bb2..138ccd20079 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,10 @@ and this project adheres to - [#4916](https://github.com/firecracker-microvm/firecracker/pull/4916): Fixed `IovDeque` implementation to work with any host page size. This fixes virtio-net device on non 4K host kernels. +- [#4991](https://github.com/firecracker-microvm/firecracker/pull/4991): Fixed + `mem_size_mib` and `track_dirty_pages` being mandatory for all + `PATCH /machine-config` requests. Now, they can be omitted which leaves these + parts of the machine configuration unchanged. ## [1.10.1] diff --git a/src/cpu-template-helper/src/main.rs b/src/cpu-template-helper/src/main.rs index 84a127e353a..35b7ea22d82 100644 --- a/src/cpu-template-helper/src/main.rs +++ b/src/cpu-template-helper/src/main.rs @@ -161,7 +161,7 @@ fn run(cli: Cli) -> Result<(), HelperError> { let (vmm, vm_resources) = utils::build_microvm_from_config(config, template)?; let cpu_template = vm_resources - .vm_config + .machine_config .cpu_template .get_cpu_template()? .into_owned(); diff --git a/src/firecracker/examples/uffd/fault_all_handler.rs b/src/firecracker/examples/uffd/fault_all_handler.rs index 31ce68a97bc..5e9f49a3207 100644 --- a/src/firecracker/examples/uffd/fault_all_handler.rs +++ b/src/firecracker/examples/uffd/fault_all_handler.rs @@ -24,6 +24,7 @@ fn main() { let (stream, _) = listener.accept().expect("Cannot listen on UDS socket"); let mut runtime = Runtime::new(stream, file); + runtime.install_panic_hook(); runtime.run(|uffd_handler: &mut UffdHandler| { // Read an event from the userfaultfd. let event = uffd_handler diff --git a/src/firecracker/examples/uffd/uffd_utils.rs b/src/firecracker/examples/uffd/uffd_utils.rs index 52d33765bd8..37aa63c62a3 100644 --- a/src/firecracker/examples/uffd/uffd_utils.rs +++ b/src/firecracker/examples/uffd/uffd_utils.rs @@ -208,6 +208,43 @@ impl Runtime { } } + fn peer_process_credentials(&self) -> libc::ucred { + let mut creds: libc::ucred = libc::ucred { + pid: 0, + gid: 0, + uid: 0, + }; + let mut creds_size = size_of::() as u32; + let ret = unsafe { + libc::getsockopt( + self.stream.as_raw_fd(), + libc::SOL_SOCKET, + libc::SO_PEERCRED, + &mut creds as *mut _ as *mut _, + &mut creds_size as *mut libc::socklen_t, + ) + }; + if ret != 0 { + panic!("Failed to get peer process credentials"); + } + creds + } + + pub fn install_panic_hook(&self) { + let peer_creds = self.peer_process_credentials(); + + let default_panic_hook = std::panic::take_hook(); + std::panic::set_hook(Box::new(move |panic_info| { + let r = unsafe { libc::kill(peer_creds.pid, libc::SIGKILL) }; + + if r != 0 { + eprintln!("Failed to kill Firecracker process from panic hook"); + } + + default_panic_hook(panic_info); + })); + } + /// Polls the `UnixStream` and UFFD fds in a loop. /// When stream is polled, new uffd is retrieved. /// When uffd is polled, page fault is handled by diff --git a/src/firecracker/examples/uffd/valid_handler.rs b/src/firecracker/examples/uffd/valid_handler.rs index cfc5faf432c..6c681d932ac 100644 --- a/src/firecracker/examples/uffd/valid_handler.rs +++ b/src/firecracker/examples/uffd/valid_handler.rs @@ -24,6 +24,7 @@ fn main() { let (stream, _) = listener.accept().expect("Cannot listen on UDS socket"); let mut runtime = Runtime::new(stream, file); + runtime.install_panic_hook(); runtime.run(|uffd_handler: &mut UffdHandler| { // Read an event from the userfaultfd. let event = uffd_handler diff --git a/src/firecracker/src/api_server/parsed_request.rs b/src/firecracker/src/api_server/parsed_request.rs index 10405c156ec..41d625e9abe 100644 --- a/src/firecracker/src/api_server/parsed_request.rs +++ b/src/firecracker/src/api_server/parsed_request.rs @@ -163,8 +163,8 @@ impl ParsedRequest { info!("The request was executed successfully. Status code: 204 No Content."); Response::new(Version::Http11, StatusCode::NoContent) } - VmmData::MachineConfiguration(vm_config) => { - Self::success_response_with_data(vm_config) + VmmData::MachineConfiguration(machine_config) => { + Self::success_response_with_data(machine_config) } VmmData::MmdsValue(value) => Self::success_response_with_mmds_value(value), VmmData::BalloonConfig(balloon_config) => { diff --git a/src/firecracker/src/api_server/request/machine_configuration.rs b/src/firecracker/src/api_server/request/machine_configuration.rs index 871bbda5ecc..2409aa06cac 100644 --- a/src/firecracker/src/api_server/request/machine_configuration.rs +++ b/src/firecracker/src/api_server/request/machine_configuration.rs @@ -31,7 +31,8 @@ pub(crate) fn parse_put_machine_config(body: &Body) -> Result Result Result pub fn configure_system_for_boot( vmm: &mut Vmm, vcpus: &mut [Vcpu], - vm_config: &VmConfig, + machine_config: &MachineConfig, cpu_template: &CustomCpuTemplate, entry_addr: GuestAddress, initrd: &Option, @@ -793,8 +796,8 @@ pub fn configure_system_for_boot( let cpu_config = CpuConfiguration::apply_template(cpu_config, cpu_template)?; let vcpu_config = VcpuConfig { - vcpu_count: vm_config.vcpu_count, - smt: vm_config.smt, + vcpu_count: machine_config.vcpu_count, + smt: machine_config.smt, cpu_config, }; diff --git a/src/vmm/src/persist.rs b/src/vmm/src/persist.rs index 1feef41ec30..621d95d1e87 100644 --- a/src/vmm/src/persist.rs +++ b/src/vmm/src/persist.rs @@ -32,7 +32,7 @@ use crate::snapshot::Snapshot; use crate::utils::u64_to_usize; use crate::vmm_config::boot_source::BootSourceConfig; use crate::vmm_config::instance_info::InstanceInfo; -use crate::vmm_config::machine_config::{HugePageConfig, MachineConfigUpdate, VmConfigError}; +use crate::vmm_config::machine_config::{HugePageConfig, MachineConfigError, MachineConfigUpdate}; use crate::vmm_config::snapshot::{ CreateSnapshotParams, LoadSnapshotParams, MemBackendType, SnapshotType, }; @@ -61,11 +61,11 @@ pub struct VmInfo { impl From<&VmResources> for VmInfo { fn from(value: &VmResources) -> Self { Self { - mem_size_mib: value.vm_config.mem_size_mib as u64, - smt: value.vm_config.smt, - cpu_template: StaticCpuTemplate::from(&value.vm_config.cpu_template), + mem_size_mib: value.machine_config.mem_size_mib as u64, + smt: value.machine_config.smt, + cpu_template: StaticCpuTemplate::from(&value.machine_config.cpu_template), boot_source: value.boot_source.config.clone(), - huge_pages: value.vm_config.huge_pages, + huge_pages: value.machine_config.huge_pages, } } } @@ -422,11 +422,11 @@ pub fn restore_from_snapshot( .vcpu_states .len() .try_into() - .map_err(|_| VmConfigError::InvalidVcpuCount) + .map_err(|_| MachineConfigError::InvalidVcpuCount) .map_err(BuildMicrovmFromSnapshotError::VmUpdateConfig)?; vm_resources - .update_vm_config(&MachineConfigUpdate { + .update_machine_config(&MachineConfigUpdate { vcpu_count: Some(vcpu_count), mem_size_mib: Some(u64_to_usize(microvm_state.vm_info.mem_size_mib)), smt: Some(microvm_state.vm_info.smt), @@ -450,7 +450,7 @@ pub fn restore_from_snapshot( mem_backend_path, mem_state, track_dirty_pages, - vm_resources.vm_config.huge_pages, + vm_resources.machine_config.huge_pages, ) .map_err(RestoreFromSnapshotGuestMemoryError::File)?, None, @@ -462,7 +462,7 @@ pub fn restore_from_snapshot( // We enable the UFFD_FEATURE_EVENT_REMOVE feature only if a balloon device // is present in the microVM state. microvm_state.device_states.balloon_device.is_some(), - vm_resources.vm_config.huge_pages, + vm_resources.machine_config.huge_pages, ) .map_err(RestoreFromSnapshotGuestMemoryError::Uffd)?, }; diff --git a/src/vmm/src/resources.rs b/src/vmm/src/resources.rs index 0cde08a844d..d0c80789681 100644 --- a/src/vmm/src/resources.rs +++ b/src/vmm/src/resources.rs @@ -22,7 +22,7 @@ use crate::vmm_config::drive::*; use crate::vmm_config::entropy::*; use crate::vmm_config::instance_info::InstanceInfo; use crate::vmm_config::machine_config::{ - HugePageConfig, MachineConfig, MachineConfigUpdate, VmConfig, VmConfigError, + HugePageConfig, MachineConfig, MachineConfigError, MachineConfigUpdate, }; use crate::vmm_config::metrics::{init_metrics, MetricsConfig, MetricsConfigError}; use crate::vmm_config::mmds::{MmdsConfig, MmdsConfigError}; @@ -54,7 +54,7 @@ pub enum ResourcesError { /// Network device error: {0} NetDevice(#[from] NetworkInterfaceError), /// VM config error: {0} - VmConfig(#[from] VmConfigError), + MachineConfig(#[from] MachineConfigError), /// Vsock device error: {0} VsockDevice(#[from] VsockConfigError), /// Entropy device error: {0} @@ -93,7 +93,7 @@ pub struct VmmConfig { #[derive(Debug, Default)] pub struct VmResources { /// The vCpu and memory configuration for this microVM. - pub vm_config: VmConfig, + pub machine_config: MachineConfig, /// The boot source spec (contains both config and builder) for this microVM. pub boot_source: BootSource, /// The block devices. @@ -140,7 +140,7 @@ impl VmResources { }; if let Some(machine_config) = vmm_config.machine_config { let machine_config = MachineConfigUpdate::from(machine_config); - resources.update_vm_config(&machine_config)?; + resources.update_machine_config(&machine_config)?; } if let Some(cpu_config) = vmm_config.cpu_config { @@ -219,7 +219,7 @@ impl VmResources { SharedDeviceType::Balloon(balloon) => { self.balloon.set_device(balloon); - if self.vm_config.huge_pages != HugePageConfig::None { + if self.machine_config.huge_pages != HugePageConfig::None { return Err(ResourcesError::BalloonDevice(BalloonConfigError::HugePages)); } } @@ -238,16 +238,19 @@ impl VmResources { /// Add a custom CPU template to the VM resources /// to configure vCPUs. pub fn set_custom_cpu_template(&mut self, cpu_template: CustomCpuTemplate) { - self.vm_config.set_custom_cpu_template(cpu_template); + self.machine_config.set_custom_cpu_template(cpu_template); } /// Updates the configuration of the microVM. - pub fn update_vm_config(&mut self, update: &MachineConfigUpdate) -> Result<(), VmConfigError> { + pub fn update_machine_config( + &mut self, + update: &MachineConfigUpdate, + ) -> Result<(), MachineConfigError> { if update.huge_pages.is_some() && update.huge_pages != Some(HugePageConfig::None) { log_dev_preview_warning("Huge pages support", None); } - let updated = self.vm_config.update(update)?; + let updated = self.machine_config.update(update)?; // The VM cannot have a memory size smaller than the target size // of the balloon device, if present. @@ -256,23 +259,23 @@ impl VmResources { < self .balloon .get_config() - .map_err(|_| VmConfigError::InvalidVmState)? + .map_err(|_| MachineConfigError::InvalidVmState)? .amount_mib as usize { - return Err(VmConfigError::IncompatibleBalloonSize); + return Err(MachineConfigError::IncompatibleBalloonSize); } if self.balloon.get().is_some() && updated.huge_pages != HugePageConfig::None { - return Err(VmConfigError::BalloonAndHugePages); + return Err(MachineConfigError::BalloonAndHugePages); } if self.boot_source.config.initrd_path.is_some() && updated.huge_pages != HugePageConfig::None { - return Err(VmConfigError::InitrdAndHugePages); + return Err(MachineConfigError::InitrdAndHugePages); } - self.vm_config = updated; + self.machine_config = updated; Ok(()) } @@ -322,11 +325,11 @@ impl VmResources { ) -> Result<(), BalloonConfigError> { // The balloon cannot have a target size greater than the size of // the guest memory. - if config.amount_mib as usize > self.vm_config.mem_size_mib { + if config.amount_mib as usize > self.machine_config.mem_size_mib { return Err(BalloonConfigError::TooManyPagesRequested); } - if self.vm_config.huge_pages != HugePageConfig::None { + if self.machine_config.huge_pages != HugePageConfig::None { return Err(BalloonConfigError::HugePages); } @@ -339,7 +342,7 @@ impl VmResources { boot_source_cfg: BootSourceConfig, ) -> Result<(), BootSourceConfigError> { if boot_source_cfg.initrd_path.is_some() - && self.vm_config.huge_pages != HugePageConfig::None + && self.machine_config.huge_pages != HugePageConfig::None { return Err(BootSourceConfigError::HugePagesAndInitRd); } @@ -480,16 +483,16 @@ impl VmResources { // that would not be worth the effort. if vhost_user_device_used { GuestMemoryMmap::memfd_backed( - self.vm_config.mem_size_mib, - self.vm_config.track_dirty_pages, - self.vm_config.huge_pages, + self.machine_config.mem_size_mib, + self.machine_config.track_dirty_pages, + self.machine_config.huge_pages, ) } else { - let regions = crate::arch::arch_memory_regions(self.vm_config.mem_size_mib << 20); + let regions = crate::arch::arch_memory_regions(self.machine_config.mem_size_mib << 20); GuestMemoryMmap::from_raw_regions( ®ions, - self.vm_config.track_dirty_pages, - self.vm_config.huge_pages, + self.machine_config.track_dirty_pages, + self.machine_config.huge_pages, ) } } @@ -503,7 +506,7 @@ impl From<&VmResources> for VmmConfig { boot_source: resources.boot_source.config.clone(), cpu_config: None, logger: None, - machine_config: Some(MachineConfig::from(&resources.vm_config)), + machine_config: Some(resources.machine_config.clone()), metrics: None, mmds_config: resources.mmds_config(), net_devices: resources.net_builder.configs(), @@ -535,7 +538,7 @@ mod tests { BootConfig, BootSource, BootSourceConfig, DEFAULT_KERNEL_CMDLINE, }; use crate::vmm_config::drive::{BlockBuilder, BlockDeviceConfig}; - use crate::vmm_config::machine_config::{HugePageConfig, MachineConfig, VmConfigError}; + use crate::vmm_config::machine_config::{HugePageConfig, MachineConfig, MachineConfigError}; use crate::vmm_config::net::{NetBuilder, NetworkInterfaceConfig}; use crate::vmm_config::vsock::tests::default_config; use crate::vmm_config::RateLimiterConfig; @@ -608,7 +611,7 @@ mod tests { fn default_vm_resources() -> VmResources { VmResources { - vm_config: VmConfig::default(), + machine_config: MachineConfig::default(), boot_source: default_boot_cfg(), block: default_blocks(), vsock: Default::default(), @@ -821,7 +824,7 @@ mod tests { assert!( matches!( error, - ResourcesError::VmConfig(VmConfigError::InvalidMemorySize) + ResourcesError::MachineConfig(MachineConfigError::InvalidMemorySize) ), "{:?}", error @@ -1135,7 +1138,7 @@ mod tests { ) .unwrap(); assert_eq!( - vm_resources.vm_config.cpu_template, + vm_resources.machine_config.cpu_template, Some(CpuTemplateType::Custom(CustomCpuTemplate::default())) ); } @@ -1339,7 +1342,7 @@ mod tests { } #[test] - fn test_update_vm_config() { + fn test_update_machine_config() { let mut vm_resources = default_vm_resources(); let mut aux_vm_config = MachineConfigUpdate { vcpu_count: Some(32), @@ -1354,25 +1357,25 @@ mod tests { }; assert_ne!( - MachineConfigUpdate::from(MachineConfig::from(&vm_resources.vm_config)), + MachineConfigUpdate::from(vm_resources.machine_config.clone()), aux_vm_config ); - vm_resources.update_vm_config(&aux_vm_config).unwrap(); + vm_resources.update_machine_config(&aux_vm_config).unwrap(); assert_eq!( - MachineConfigUpdate::from(MachineConfig::from(&vm_resources.vm_config)), + MachineConfigUpdate::from(vm_resources.machine_config.clone()), aux_vm_config ); // Invalid vcpu count. aux_vm_config.vcpu_count = Some(0); assert_eq!( - vm_resources.update_vm_config(&aux_vm_config), - Err(VmConfigError::InvalidVcpuCount) + vm_resources.update_machine_config(&aux_vm_config), + Err(MachineConfigError::InvalidVcpuCount) ); aux_vm_config.vcpu_count = Some(33); assert_eq!( - vm_resources.update_vm_config(&aux_vm_config), - Err(VmConfigError::InvalidVcpuCount) + vm_resources.update_machine_config(&aux_vm_config), + Err(MachineConfigError::InvalidVcpuCount) ); // Check that SMT is not supported on aarch64, and that on x86_64 enabling it requires vcpu @@ -1380,29 +1383,29 @@ mod tests { aux_vm_config.smt = Some(true); #[cfg(target_arch = "aarch64")] assert_eq!( - vm_resources.update_vm_config(&aux_vm_config), - Err(VmConfigError::SmtNotSupported) + vm_resources.update_machine_config(&aux_vm_config), + Err(MachineConfigError::SmtNotSupported) ); aux_vm_config.vcpu_count = Some(3); #[cfg(target_arch = "x86_64")] assert_eq!( - vm_resources.update_vm_config(&aux_vm_config), - Err(VmConfigError::InvalidVcpuCount) + vm_resources.update_machine_config(&aux_vm_config), + Err(MachineConfigError::InvalidVcpuCount) ); aux_vm_config.vcpu_count = Some(32); #[cfg(target_arch = "x86_64")] - vm_resources.update_vm_config(&aux_vm_config).unwrap(); + vm_resources.update_machine_config(&aux_vm_config).unwrap(); aux_vm_config.smt = Some(false); // Invalid mem_size_mib. aux_vm_config.mem_size_mib = Some(0); assert_eq!( - vm_resources.update_vm_config(&aux_vm_config), - Err(VmConfigError::InvalidMemorySize) + vm_resources.update_machine_config(&aux_vm_config), + Err(MachineConfigError::InvalidMemorySize) ); // Incompatible mem_size_mib with balloon size. - vm_resources.vm_config.mem_size_mib = 128; + vm_resources.machine_config.mem_size_mib = 128; vm_resources .set_balloon_device(BalloonDeviceConfig { amount_mib: 100, @@ -1412,20 +1415,22 @@ mod tests { .unwrap(); aux_vm_config.mem_size_mib = Some(90); assert_eq!( - vm_resources.update_vm_config(&aux_vm_config), - Err(VmConfigError::IncompatibleBalloonSize) + vm_resources.update_machine_config(&aux_vm_config), + Err(MachineConfigError::IncompatibleBalloonSize) ); // mem_size_mib compatible with balloon size. aux_vm_config.mem_size_mib = Some(256); - vm_resources.update_vm_config(&aux_vm_config).unwrap(); + vm_resources.update_machine_config(&aux_vm_config).unwrap(); // mem_size_mib incompatible with huge pages configuration aux_vm_config.mem_size_mib = Some(129); aux_vm_config.huge_pages = Some(HugePageConfig::Hugetlbfs2M); assert_eq!( - vm_resources.update_vm_config(&aux_vm_config).unwrap_err(), - VmConfigError::InvalidMemorySize + vm_resources + .update_machine_config(&aux_vm_config) + .unwrap_err(), + MachineConfigError::InvalidMemorySize ); // mem_size_mib compatible with huge page configuration @@ -1433,7 +1438,7 @@ mod tests { // Remove the balloon device config that's added by `default_vm_resources` as it would // trigger the "ballooning incompatible with huge pages" check. vm_resources.balloon = BalloonBuilder::new(); - vm_resources.update_vm_config(&aux_vm_config).unwrap(); + vm_resources.update_machine_config(&aux_vm_config).unwrap(); } #[test] @@ -1474,7 +1479,7 @@ mod tests { let mut vm_resources = default_vm_resources(); vm_resources.balloon = BalloonBuilder::new(); vm_resources - .update_vm_config(&MachineConfigUpdate { + .update_machine_config(&MachineConfigUpdate { huge_pages: Some(HugePageConfig::Hugetlbfs2M), ..Default::default() }) diff --git a/src/vmm/src/rpc_interface.rs b/src/vmm/src/rpc_interface.rs index 60a046f7e89..82993fcafea 100644 --- a/src/vmm/src/rpc_interface.rs +++ b/src/vmm/src/rpc_interface.rs @@ -26,7 +26,7 @@ use crate::vmm_config::boot_source::{BootSourceConfig, BootSourceConfigError}; use crate::vmm_config::drive::{BlockDeviceConfig, BlockDeviceUpdateConfig, DriveError}; use crate::vmm_config::entropy::{EntropyDeviceConfig, EntropyDeviceError}; use crate::vmm_config::instance_info::InstanceInfo; -use crate::vmm_config::machine_config::{MachineConfig, MachineConfigUpdate, VmConfigError}; +use crate::vmm_config::machine_config::{MachineConfig, MachineConfigError, MachineConfigUpdate}; use crate::vmm_config::metrics::{MetricsConfig, MetricsConfigError}; use crate::vmm_config::mmds::{MmdsConfig, MmdsConfigError}; use crate::vmm_config::net::{ @@ -120,7 +120,7 @@ pub enum VmmAction { UpdateNetworkInterface(NetworkInterfaceUpdateConfig), /// Update the microVM configuration (memory & vcpu) using `VmUpdateConfig` as input. This /// action can only be called before the microVM has booted. - UpdateVmConfiguration(MachineConfigUpdate), + UpdateMachineConfiguration(MachineConfigUpdate), } /// Wrapper for all errors associated with VMM actions. @@ -145,7 +145,7 @@ pub enum VmmActionError { /// Logger error: {0} Logger(#[from] crate::logger::LoggerUpdateError), /// Machine config error: {0} - MachineConfig(#[from] VmConfigError), + MachineConfig(#[from] MachineConfigError), /// Metrics error: {0} Metrics(#[from] MetricsConfigError), #[from(ignore)] @@ -415,9 +415,9 @@ impl<'a> PrebootApiController<'a> { Ok(VmmData::FullVmConfig((&*self.vm_resources).into())) } GetMMDS => self.get_mmds(), - GetVmMachineConfig => Ok(VmmData::MachineConfiguration(MachineConfig::from( - &self.vm_resources.vm_config, - ))), + GetVmMachineConfig => Ok(VmmData::MachineConfiguration( + self.vm_resources.machine_config.clone(), + )), GetVmInstanceInfo => Ok(VmmData::InstanceInformation(self.instance_info.clone())), GetVmmVersion => Ok(VmmData::VmmVersion(self.instance_info.vmm_version.clone())), InsertBlockDevice(config) => self.insert_block_device(config), @@ -434,7 +434,7 @@ impl<'a> PrebootApiController<'a> { SetVsockDevice(config) => self.set_vsock_device(config), SetMmdsConfiguration(config) => self.set_mmds_config(config), StartMicroVm => self.start_microvm(), - UpdateVmConfiguration(config) => self.update_vm_config(config), + UpdateMachineConfiguration(config) => self.update_machine_config(config), SetEntropyDevice(config) => self.set_entropy_device(config), // Operations not allowed pre-boot. CreateSnapshot(_) @@ -502,10 +502,13 @@ impl<'a> PrebootApiController<'a> { .map_err(VmmActionError::MmdsConfig) } - fn update_vm_config(&mut self, cfg: MachineConfigUpdate) -> Result { + fn update_machine_config( + &mut self, + cfg: MachineConfigUpdate, + ) -> Result { self.boot_path = true; self.vm_resources - .update_vm_config(&cfg) + .update_machine_config(&cfg) .map(|()| VmmData::Empty) .map_err(VmmActionError::MachineConfig) } @@ -641,9 +644,9 @@ impl RuntimeApiController { .map_err(|err| VmmActionError::BalloonConfig(BalloonConfigError::from(err))), GetFullVmConfig => Ok(VmmData::FullVmConfig((&self.vm_resources).into())), GetMMDS => self.get_mmds(), - GetVmMachineConfig => Ok(VmmData::MachineConfiguration(MachineConfig::from( - &self.vm_resources.vm_config, - ))), + GetVmMachineConfig => Ok(VmmData::MachineConfiguration( + self.vm_resources.machine_config.clone(), + )), GetVmInstanceInfo => Ok(VmmData::InstanceInformation( self.vmm.lock().expect("Poisoned lock").instance_info(), )), @@ -686,7 +689,7 @@ impl RuntimeApiController { | SetMmdsConfiguration(_) | SetEntropyDevice(_) | StartMicroVm - | UpdateVmConfiguration(_) => Err(VmmActionError::OperationNotSupportedPostBoot), + | UpdateMachineConfiguration(_) => Err(VmmActionError::OperationNotSupportedPostBoot), } } @@ -753,7 +756,7 @@ impl RuntimeApiController { log_dev_preview_warning("Virtual machine snapshots", None); if create_params.snapshot_type == SnapshotType::Diff - && !self.vm_resources.vm_config.track_dirty_pages + && !self.vm_resources.machine_config.track_dirty_pages { return Err(VmmActionError::NotSupported( "Diff snapshots are not allowed on uVMs with dirty page tracking disabled." @@ -1254,7 +1257,7 @@ mod tests { network_interfaces: Vec::new(), }, ))); - check_unsupported(runtime_request(VmmAction::UpdateVmConfiguration( + check_unsupported(runtime_request(VmmAction::UpdateMachineConfiguration( MachineConfigUpdate::from(MachineConfig::default()), ))); check_unsupported(runtime_request(VmmAction::LoadSnapshot( diff --git a/src/vmm/src/test_utils/mock_resources/mod.rs b/src/vmm/src/test_utils/mock_resources/mod.rs index 9f4406ab280..f8485bf9678 100644 --- a/src/vmm/src/test_utils/mock_resources/mod.rs +++ b/src/vmm/src/test_utils/mock_resources/mod.rs @@ -81,12 +81,12 @@ impl MockVmResources { pub fn with_vm_config(mut self, vm_config: MachineConfig) -> Self { let machine_config = MachineConfigUpdate::from(vm_config); - self.0.update_vm_config(&machine_config).unwrap(); + self.0.update_machine_config(&machine_config).unwrap(); self } pub fn set_cpu_template(&mut self, cpu_template: CustomCpuTemplate) { - self.0.vm_config.set_custom_cpu_template(cpu_template); + self.0.machine_config.set_custom_cpu_template(cpu_template); } } diff --git a/src/vmm/src/vmm_config/machine_config.rs b/src/vmm/src/vmm_config/machine_config.rs index 8eee91c88be..092179f5ff5 100644 --- a/src/vmm/src/vmm_config/machine_config.rs +++ b/src/vmm/src/vmm_config/machine_config.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use std::fmt::Debug; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; use crate::cpu_config::templates::{CpuTemplateType, CustomCpuTemplate, StaticCpuTemplate}; @@ -15,7 +15,7 @@ pub const MAX_SUPPORTED_VCPUS: u8 = 32; /// Errors associated with configuring the microVM. #[rustfmt::skip] #[derive(Debug, thiserror::Error, displaydoc::Display, PartialEq, Eq)] -pub enum VmConfigError { +pub enum MachineConfigError { /// The memory size (MiB) is smaller than the previously set balloon device target size. IncompatibleBalloonSize, /// The memory size (MiB) is either 0, or not a multiple of the configured page size. @@ -103,8 +103,14 @@ pub struct MachineConfig { #[serde(default)] pub smt: bool, /// A CPU template that it is used to filter the CPU features exposed to the guest. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub cpu_template: Option, + // FIXME: once support for static CPU templates is removed, this field can be dropped altogether + #[serde( + default, + skip_serializing_if = "is_none_or_custom_template", + deserialize_with = "deserialize_static_template", + serialize_with = "serialize_static_template" + )] + pub cpu_template: Option, /// Enables or disables dirty page tracking. Enabling allows incremental snapshots. #[serde(default)] pub track_dirty_pages: bool, @@ -117,42 +123,78 @@ pub struct MachineConfig { pub gdb_socket_path: Option, } +fn is_none_or_custom_template(template: &Option) -> bool { + matches!(template, None | Some(CpuTemplateType::Custom(_))) +} + +fn deserialize_static_template<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + Option::::deserialize(deserializer) + .map(|maybe_template| maybe_template.map(CpuTemplateType::Static)) +} + +fn serialize_static_template( + template: &Option, + serializer: S, +) -> Result +where + S: Serializer, +{ + let Some(CpuTemplateType::Static(template)) = template else { + // We have a skip_serializing_if on the field + unreachable!() + }; + + template.serialize(serializer) +} + impl Default for MachineConfig { fn default() -> Self { - Self::from(&VmConfig::default()) + Self { + vcpu_count: 1, + mem_size_mib: DEFAULT_MEM_SIZE_MIB, + smt: false, + cpu_template: None, + track_dirty_pages: false, + huge_pages: HugePageConfig::None, + #[cfg(feature = "gdb")] + gdb_socket_path: None, + } } } /// Struct used in PATCH `/machine-config` API call. -/// Used to update `VmConfig` in `VmResources`. +/// Used to update `MachineConfig` in `VmResources`. /// This struct mirrors all the fields in `MachineConfig`. /// All fields are optional, but at least one needs to be specified. /// If a field is `Some(value)` then we assume an update is requested /// for that field. -#[derive(Clone, Default, Debug, PartialEq, Eq, Deserialize, Serialize)] +#[derive(Clone, Default, Debug, PartialEq, Eq, Deserialize)] #[serde(deny_unknown_fields)] pub struct MachineConfigUpdate { /// Number of vcpu to start. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub vcpu_count: Option, /// The memory size in MiB. - #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default)] pub mem_size_mib: Option, /// Enables or disabled SMT. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub smt: Option, /// A CPU template that it is used to filter the CPU features exposed to the guest. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub cpu_template: Option, /// Enables or disables dirty page tracking. Enabling allows incremental snapshots. - #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default)] pub track_dirty_pages: Option, /// Configures what page size Firecracker should use to back guest memory. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub huge_pages: Option, /// GDB socket address. #[cfg(feature = "gdb")] - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub gdb_socket_path: Option, } @@ -171,7 +213,7 @@ impl From for MachineConfigUpdate { vcpu_count: Some(cfg.vcpu_count), mem_size_mib: Some(cfg.mem_size_mib), smt: Some(cfg.smt), - cpu_template: cfg.cpu_template, + cpu_template: cfg.static_template(), track_dirty_pages: Some(cfg.track_dirty_pages), huge_pages: Some(cfg.huge_pages), #[cfg(feature = "gdb")] @@ -180,62 +222,52 @@ impl From for MachineConfigUpdate { } } -/// Configuration of the microvm. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct VmConfig { - /// Number of vcpu to start. - pub vcpu_count: u8, - /// The memory size in MiB. - pub mem_size_mib: usize, - /// Enables or disabled SMT. - pub smt: bool, - /// A CPU template that it is used to filter the CPU features exposed to the guest. - pub cpu_template: Option, - /// Enables or disables dirty page tracking. Enabling allows incremental snapshots. - pub track_dirty_pages: bool, - /// Configures what page size Firecracker should use to back guest memory. - pub huge_pages: HugePageConfig, - /// GDB socket address. - #[cfg(feature = "gdb")] - pub gdb_socket_path: Option, -} - -impl VmConfig { +impl MachineConfig { /// Sets cpu tempalte field to `CpuTemplateType::Custom(cpu_template)`. pub fn set_custom_cpu_template(&mut self, cpu_template: CustomCpuTemplate) { self.cpu_template = Some(CpuTemplateType::Custom(cpu_template)); } - /// Updates [`VmConfig`] with [`MachineConfigUpdate`]. + fn static_template(&self) -> Option { + match self.cpu_template { + Some(CpuTemplateType::Static(template)) => Some(template), + _ => None, + } + } + + /// Updates [`MachineConfig`] with [`MachineConfigUpdate`]. /// Mapping for cpu template update: /// StaticCpuTemplate::None -> None /// StaticCpuTemplate::Other -> Some(CustomCpuTemplate::Static(Other)), - /// Returns the updated `VmConfig` object. - pub fn update(&self, update: &MachineConfigUpdate) -> Result { + /// Returns the updated `MachineConfig` object. + pub fn update( + &self, + update: &MachineConfigUpdate, + ) -> Result { let vcpu_count = update.vcpu_count.unwrap_or(self.vcpu_count); let smt = update.smt.unwrap_or(self.smt); #[cfg(target_arch = "aarch64")] if smt { - return Err(VmConfigError::SmtNotSupported); + return Err(MachineConfigError::SmtNotSupported); } if vcpu_count == 0 || vcpu_count > MAX_SUPPORTED_VCPUS { - return Err(VmConfigError::InvalidVcpuCount); + return Err(MachineConfigError::InvalidVcpuCount); } // If SMT is enabled or is to be enabled in this call // only allow vcpu count to be 1 or even. if smt && vcpu_count > 1 && vcpu_count % 2 == 1 { - return Err(VmConfigError::InvalidVcpuCount); + return Err(MachineConfigError::InvalidVcpuCount); } let mem_size_mib = update.mem_size_mib.unwrap_or(self.mem_size_mib); let page_config = update.huge_pages.unwrap_or(self.huge_pages); if mem_size_mib == 0 || !page_config.is_valid_mem_size(mem_size_mib) { - return Err(VmConfigError::InvalidMemorySize); + return Err(MachineConfigError::InvalidMemorySize); } let cpu_template = match update.cpu_template { @@ -244,7 +276,7 @@ impl VmConfig { Some(other) => Some(CpuTemplateType::Static(other)), }; - Ok(VmConfig { + Ok(MachineConfig { vcpu_count, mem_size_mib, smt, @@ -257,32 +289,54 @@ impl VmConfig { } } -impl Default for VmConfig { - fn default() -> Self { - Self { - vcpu_count: 1, - mem_size_mib: DEFAULT_MEM_SIZE_MIB, - smt: false, +#[cfg(test)] +mod tests { + use crate::cpu_config::templates::{CpuTemplateType, CustomCpuTemplate, StaticCpuTemplate}; + use crate::vmm_config::machine_config::MachineConfig; + + // Ensure the special (de)serialization logic for the cpu_template field works: + // only static cpu templates can be specified via the machine-config endpoint, but + // we still cram custom cpu templates into the MachineConfig struct if they're set otherwise + // Ensure that during (de)serialization we preserve static templates, but we set custom + // templates to None + #[test] + fn test_serialize_machine_config() { + #[cfg(target_arch = "aarch64")] + const TEMPLATE: StaticCpuTemplate = StaticCpuTemplate::V1N1; + #[cfg(target_arch = "x86_64")] + const TEMPLATE: StaticCpuTemplate = StaticCpuTemplate::T2S; + + let mconfig = MachineConfig { cpu_template: None, - track_dirty_pages: false, - huge_pages: HugePageConfig::None, - #[cfg(feature = "gdb")] - gdb_socket_path: None, - } - } -} + ..Default::default() + }; -impl From<&VmConfig> for MachineConfig { - fn from(value: &VmConfig) -> Self { - Self { - vcpu_count: value.vcpu_count, - mem_size_mib: value.mem_size_mib, - smt: value.smt, - cpu_template: value.cpu_template.as_ref().map(|template| template.into()), - track_dirty_pages: value.track_dirty_pages, - huge_pages: value.huge_pages, - #[cfg(feature = "gdb")] - gdb_socket_path: value.gdb_socket_path.clone(), - } + let serialized = serde_json::to_string(&mconfig).unwrap(); + let deserialized = serde_json::from_str::(&serialized).unwrap(); + + assert!(deserialized.cpu_template.is_none()); + + let mconfig = MachineConfig { + cpu_template: Some(CpuTemplateType::Static(TEMPLATE)), + ..Default::default() + }; + + let serialized = serde_json::to_string(&mconfig).unwrap(); + let deserialized = serde_json::from_str::(&serialized).unwrap(); + + assert_eq!( + deserialized.cpu_template, + Some(CpuTemplateType::Static(TEMPLATE)) + ); + + let mconfig = MachineConfig { + cpu_template: Some(CpuTemplateType::Custom(CustomCpuTemplate::default())), + ..Default::default() + }; + + let serialized = serde_json::to_string(&mconfig).unwrap(); + let deserialized = serde_json::from_str::(&serialized).unwrap(); + + assert!(deserialized.cpu_template.is_none()); } } diff --git a/src/vmm/tests/integration_tests.rs b/src/vmm/tests/integration_tests.rs index 40eab05c4a4..a66f29e7f55 100644 --- a/src/vmm/tests/integration_tests.rs +++ b/src/vmm/tests/integration_tests.rs @@ -22,7 +22,7 @@ use vmm::vmm_config::balloon::BalloonDeviceConfig; use vmm::vmm_config::boot_source::BootSourceConfig; use vmm::vmm_config::drive::BlockDeviceConfig; use vmm::vmm_config::instance_info::{InstanceInfo, VmState}; -use vmm::vmm_config::machine_config::{MachineConfig, MachineConfigUpdate, VmConfig}; +use vmm::vmm_config::machine_config::{MachineConfig, MachineConfigUpdate}; use vmm::vmm_config::net::NetworkInterfaceConfig; use vmm::vmm_config::snapshot::{ CreateSnapshotParams, LoadSnapshotParams, MemBackendConfig, MemBackendType, SnapshotType, @@ -188,7 +188,7 @@ fn verify_create_snapshot(is_diff: bool) -> (TempFile, TempFile) { let (vmm, _) = create_vmm(Some(NOISY_KERNEL_IMAGE), is_diff, true); let resources = VmResources { - vm_config: VmConfig { + machine_config: MachineConfig { mem_size_mib: 1, track_dirty_pages: is_diff, ..Default::default() @@ -403,6 +403,7 @@ fn test_preboot_load_snap_disallowed_after_boot_resources() { }); verify_load_snap_disallowed_after_boot_resources(req, "SetVsockDevice"); - let req = VmmAction::UpdateVmConfiguration(MachineConfigUpdate::from(MachineConfig::default())); + let req = + VmmAction::UpdateMachineConfiguration(MachineConfigUpdate::from(MachineConfig::default())); verify_load_snap_disallowed_after_boot_resources(req, "SetVmConfiguration"); } diff --git a/tests/framework/microvm.py b/tests/framework/microvm.py index 278cb9ecd60..f93a0dabf19 100644 --- a/tests/framework/microvm.py +++ b/tests/framework/microvm.py @@ -310,6 +310,9 @@ def kill(self): if self.screen_pid: os.kill(self.screen_pid, signal.SIGKILL) except: + LOG.error( + "Failed to kill Firecracker Process. Did it already die (or did the UFFD handler process die and take it down)?" + ) LOG.error(self.log_data) raise