diff --git a/src/firecracker/src/api_server/request/machine_configuration.rs b/src/firecracker/src/api_server/request/machine_configuration.rs index 8db1f20b2c9..39ed8d24561 100644 --- a/src/firecracker/src/api_server/request/machine_configuration.rs +++ b/src/firecracker/src/api_server/request/machine_configuration.rs @@ -127,7 +127,7 @@ mod tests { vcpu_count: Some(8), mem_size_mib: Some(1024), smt: Some(false), - cpu_template: Some(StaticCpuTemplate::None), + cpu_template: Some(None), track_dirty_pages: Some(false), }; assert_eq!( @@ -167,7 +167,7 @@ mod tests { vcpu_count: Some(8), mem_size_mib: Some(1024), smt: Some(false), - cpu_template: Some(StaticCpuTemplate::T2), + cpu_template: Some(Some(StaticCpuTemplate::T2)), track_dirty_pages: Some(true), }; assert_eq!( diff --git a/src/vmm/src/cpu_config/aarch64/custom_cpu_template.rs b/src/vmm/src/cpu_config/aarch64/custom_cpu_template.rs index d6cb0806c21..750688b3576 100644 --- a/src/vmm/src/cpu_config/aarch64/custom_cpu_template.rs +++ b/src/vmm/src/cpu_config/aarch64/custom_cpu_template.rs @@ -24,7 +24,6 @@ impl GetCpuTemplate for Option { CpuTemplateType::Static(template) => match template { // TODO: Check if the CPU model is Neoverse-V1. StaticCpuTemplate::V1N1 => Ok(Cow::Owned(v1n1::v1n1())), - other => Err(GetCpuTemplateError::InvalidStaticCpuTemplate(*other)), }, }, None => Ok(Cow::Owned(CustomCpuTemplate::default())), diff --git a/src/vmm/src/cpu_config/aarch64/static_cpu_templates/mod.rs b/src/vmm/src/cpu_config/aarch64/static_cpu_templates/mod.rs index 88159e82e2f..abe20d6bf0a 100644 --- a/src/vmm/src/cpu_config/aarch64/static_cpu_templates/mod.rs +++ b/src/vmm/src/cpu_config/aarch64/static_cpu_templates/mod.rs @@ -3,29 +3,24 @@ use serde::{Deserialize, Serialize}; +use crate::cpu_config::templates::CpuTemplateType; + /// Module with V1N1 CPU template for aarch64 pub mod v1n1; /// Templates available for configuring the supported ARM CPU types. -#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub enum StaticCpuTemplate { - // Needed for compatibility - /// Empty0 - Empty0, - // Needed for compatibility - /// Empty1 - Empty1, /// Template to mask Neoverse-V1 as Neoverse-N1 V1N1, - /// No CPU template is used. - #[default] - None, } -impl StaticCpuTemplate { - /// Check if no template specified - pub fn is_none(&self) -> bool { - self == &StaticCpuTemplate::None +impl Into> for &CpuTemplateType { + fn into(self) -> Option { + match self { + CpuTemplateType::Custom(_) => None, + CpuTemplateType::Static(template) => Some(*template), + } } } @@ -33,8 +28,6 @@ impl std::fmt::Display for StaticCpuTemplate { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { match self { StaticCpuTemplate::V1N1 => write!(f, "V1N1"), - StaticCpuTemplate::None => write!(f, "None"), - _ => write!(f, "None"), } } } diff --git a/src/vmm/src/cpu_config/templates.rs b/src/vmm/src/cpu_config/templates.rs index 984fd8dcac2..18d8827775f 100644 --- a/src/vmm/src/cpu_config/templates.rs +++ b/src/vmm/src/cpu_config/templates.rs @@ -61,28 +61,6 @@ pub enum CpuTemplateType { Static(StaticCpuTemplate), } -// This conversion is only used for snapshot, but the static CPU template -// information has not been saved into snapshot since v1.1. -impl From<&Option> for StaticCpuTemplate { - fn from(value: &Option) -> Self { - match value { - Some(CpuTemplateType::Static(template)) => *template, - Some(CpuTemplateType::Custom(_)) | None => StaticCpuTemplate::None, - } - } -} - -// This conversion is used when converting `&VmConfig` to `MachineConfig` to -// respond `GET /machine-config` and `GET /vm`. -impl From<&CpuTemplateType> for StaticCpuTemplate { - fn from(value: &CpuTemplateType) -> Self { - match value { - CpuTemplateType::Static(template) => *template, - CpuTemplateType::Custom(_) => StaticCpuTemplate::None, - } - } -} - impl<'a> TryFrom<&'a [u8]> for CustomCpuTemplate { type Error = serde_json::Error; diff --git a/src/vmm/src/cpu_config/x86_64/custom_cpu_template.rs b/src/vmm/src/cpu_config/x86_64/custom_cpu_template.rs index 11b89c9dcae..eb4272fd3fe 100644 --- a/src/vmm/src/cpu_config/x86_64/custom_cpu_template.rs +++ b/src/vmm/src/cpu_config/x86_64/custom_cpu_template.rs @@ -68,9 +68,6 @@ impl GetCpuTemplate for Option { } Ok(Cow::Owned(t2a::t2a())) } - StaticCpuTemplate::None => { - Err(InvalidStaticCpuTemplate(StaticCpuTemplate::None)) - } } } }, @@ -342,21 +339,6 @@ mod tests { } } - #[test] - fn test_get_cpu_template_with_none_static_template() { - // Test `get_cpu_template()` when no static CPU template is provided. - // `InvalidStaticCpuTemplate` error should be returned because it is no longer valid and - // was replaced with `None` of `Option`. - let cpu_template = Some(CpuTemplateType::Static(StaticCpuTemplate::None)); - assert_eq!( - cpu_template.get_cpu_template().unwrap_err(), - GetCpuTemplateError::InvalidStaticCpuTemplate(StaticCpuTemplate::None) - ); - - // Test the Display for StaticCpuTemplate - assert_eq!(format!("{}", StaticCpuTemplate::None), "None"); - } - #[test] fn test_get_cpu_template_with_custom_template() { // Test `get_cpu_template()` when a custom CPU template is provided. The borrowed diff --git a/src/vmm/src/cpu_config/x86_64/static_cpu_templates/mod.rs b/src/vmm/src/cpu_config/x86_64/static_cpu_templates/mod.rs index db976ed9658..12dac35463a 100644 --- a/src/vmm/src/cpu_config/x86_64/static_cpu_templates/mod.rs +++ b/src/vmm/src/cpu_config/x86_64/static_cpu_templates/mod.rs @@ -4,6 +4,8 @@ use derive_more::Display; use serde::{Deserialize, Serialize}; +use crate::cpu_config::templates::CpuTemplateType; + /// Module with C3 CPU template for x86_64 pub mod c3; /// Module with T2 CPU template for x86_64 @@ -17,7 +19,7 @@ pub mod t2s; /// Template types available for configuring the x86 CPU features that map /// to EC2 instances. -#[derive(Debug, Default, Display, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Display, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub enum StaticCpuTemplate { /// C3 Template. #[display(fmt = "C3")] @@ -28,10 +30,6 @@ pub enum StaticCpuTemplate { /// T2S Template. #[display(fmt = "T2S")] T2S, - /// No CPU template is used. - #[default] - #[display(fmt = "None")] - None, /// T2CL Template. #[display(fmt = "T2CL")] T2CL, @@ -40,10 +38,12 @@ pub enum StaticCpuTemplate { T2A, } -impl StaticCpuTemplate { - /// Check if no template specified - pub fn is_none(&self) -> bool { - self == &StaticCpuTemplate::None +impl From<&CpuTemplateType> for Option { + fn from(val: &CpuTemplateType) -> Self { + match val { + CpuTemplateType::Custom(_) => None, + CpuTemplateType::Static(template) => Some(*template), + } } } diff --git a/src/vmm/src/persist.rs b/src/vmm/src/persist.rs index 47a2e5e9591..e58924fff56 100644 --- a/src/vmm/src/persist.rs +++ b/src/vmm/src/persist.rs @@ -51,7 +51,7 @@ pub struct VmInfo { /// smt information pub smt: bool, /// CPU template type - pub cpu_template: StaticCpuTemplate, + pub cpu_template: Option, /// Boot source information. pub boot_source: BootSourceConfig, } @@ -61,7 +61,7 @@ impl From<&VmResources> for VmInfo { 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), + cpu_template: value.vm_config.cpu_template.as_ref().and_then(Into::into), boot_source: value.boot_source_config().clone(), } } diff --git a/src/vmm/src/resources.rs b/src/vmm/src/resources.rs index 8ad98875db0..fc5cd07aadc 100644 --- a/src/vmm/src/resources.rs +++ b/src/vmm/src/resources.rs @@ -1332,9 +1332,9 @@ mod tests { mem_size_mib: Some(512), smt: Some(true), #[cfg(target_arch = "x86_64")] - cpu_template: Some(StaticCpuTemplate::T2), + cpu_template: Some(Some(StaticCpuTemplate::T2)), #[cfg(target_arch = "aarch64")] - cpu_template: Some(StaticCpuTemplate::V1N1), + cpu_template: Some(Some(StaticCpuTemplate::V1N1)), track_dirty_pages: Some(false), }; diff --git a/src/vmm/src/rpc_interface.rs b/src/vmm/src/rpc_interface.rs index ea3579b17f6..2f8adc64703 100644 --- a/src/vmm/src/rpc_interface.rs +++ b/src/vmm/src/rpc_interface.rs @@ -864,7 +864,7 @@ mod tests { use super::*; use crate::cpu_config::templates::test_utils::build_test_template; - use crate::cpu_config::templates::{CpuTemplateType, StaticCpuTemplate}; + use crate::cpu_config::templates::CpuTemplateType; use crate::devices::virtio::balloon::{BalloonConfig, BalloonError}; use crate::devices::virtio::block_common::CacheType; use crate::devices::virtio::rng::EntropyError; @@ -1071,7 +1071,7 @@ mod tests { 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), + cpu_template: value.vm_config.cpu_template.as_ref().and_then(Into::into), boot_source: value.boot_source_config().clone(), } } diff --git a/src/vmm/src/vmm_config/machine_config.rs b/src/vmm/src/vmm_config/machine_config.rs index a36f39f0216..0c7dba69087 100644 --- a/src/vmm/src/vmm_config/machine_config.rs +++ b/src/vmm/src/vmm_config/machine_config.rs @@ -90,13 +90,29 @@ pub struct MachineConfigUpdate { )] 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")] - pub cpu_template: Option, + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "some_option" + )] + // Needs to be wrapped in two options: The outer indicates whether the `cpu_template` + // field should be updated at all (with `None` meaning "leave as is"), and the inner + // indicating that, if an update should happen, whether a static template should be + // used or not. + pub cpu_template: Option>, /// Enables or disables dirty page tracking. Enabling allows incremental snapshots. #[serde(skip_serializing_if = "Option::is_none")] pub track_dirty_pages: Option, } +fn some_option<'de, T, D>(deserializer: D) -> Result>, D::Error> +where + T: Deserialize<'de>, + D: serde::Deserializer<'de>, +{ + Option::::deserialize(deserializer).map(Some) +} + impl MachineConfigUpdate { /// Checks if the update request contains any data. /// Returns `true` if all fields are set to `None` which means that there is nothing @@ -121,7 +137,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: Some(cfg.cpu_template), track_dirty_pages: Some(cfg.track_dirty_pages), } } @@ -179,10 +195,7 @@ impl VmConfig { self.mem_size_mib = mem_size_mib; if let Some(cpu_template) = update.cpu_template { - self.cpu_template = match cpu_template { - StaticCpuTemplate::None => None, - other => Some(CpuTemplateType::Static(other)), - }; + self.cpu_template = cpu_template.map(CpuTemplateType::Static); } if let Some(track_dirty_pages) = update.track_dirty_pages { @@ -211,7 +224,10 @@ impl From<&VmConfig> for MachineConfig { 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()), + cpu_template: value + .cpu_template + .as_ref() + .and_then(|template| template.into()), track_dirty_pages: value.track_dirty_pages, } }