refactor: Eliminate unused {Static,Custom}CpuTemplate variants

They were needed for Versionize reasons, but with firecracker-microvm#4230 merged, we can
eliminate them, simplifying the code.

Signed-off-by: Patrick Roy <[email protected]>

Author: roypat

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!(

src/vmm/src/cpu_config/aarch64/custom_cpu_template.rs

@@ -24,7 +24,6 @@ impl GetCpuTemplate for Option<CpuTemplateType> {
                 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())),

src/vmm/src/cpu_config/aarch64/static_cpu_templates/mod.rs

@@ -3,38 +3,31 @@
 
 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<Option<StaticCpuTemplate>> for &CpuTemplateType {
+    fn into(self) -> Option<StaticCpuTemplate> {
+        match self {
+            CpuTemplateType::Custom(_) => None,
+            CpuTemplateType::Static(template) => Some(*template),
+        }
     }
 }
 
 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"),
         }
     }
 }

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<CpuTemplateType>> for StaticCpuTemplate {
-    fn from(value: &Option<CpuTemplateType>) -> 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;
 

src/vmm/src/cpu_config/x86_64/custom_cpu_template.rs

@@ -68,9 +68,6 @@ impl GetCpuTemplate for Option<CpuTemplateType> {
                             }
                             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<CpuTemplateType>`.
-        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

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<StaticCpuTemplate> {
+    fn from(val: &CpuTemplateType) -> Self {
+        match val {
+            CpuTemplateType::Custom(_) => None,
+            CpuTemplateType::Static(template) => Some(*template),
+        }
     }
 }
 

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<StaticCpuTemplate>,
     /// 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(),
         }
     }

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),
         };
 

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(),
             }
         }

src/vmm/src/vmm_config/machine_config.rs

@@ -90,13 +90,29 @@ pub struct MachineConfigUpdate {
     )]
     pub smt: Option<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<StaticCpuTemplate>,
+    #[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<Option<StaticCpuTemplate>>,
     /// Enables or disables dirty page tracking. Enabling allows incremental snapshots.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub track_dirty_pages: Option<bool>,
 }
 
+fn some_option<'de, T, D>(deserializer: D) -> Result<Option<Option<T>>, D::Error>
+where
+    T: Deserialize<'de>,
+    D: serde::Deserializer<'de>,
+{
+    Option::<T>::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<MachineConfig> 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,
         }
     }