Enhance usability of CPU template helper tool

CHANGELOG.md

@@ -45,6 +45,10 @@ and this project adheres to
   use memfd to back guest memory if a vhost-user-blk device is configured,
   otherwise use anonymous private memory. This is because serving page faults of
   shared memory used by memfd is slower and may impact workloads.
+- [#4492](https://github.com/firecracker-microvm/firecracker/pull/4492): Changed
+  `--config` parameter of `cpu-template-helper` optional. Users no longer need
+  to prepare kernel, rootfs and Firecracker configuration files to use
+  `cpu-template-helper`.
 
 ### Fixed
 

docs/cpu_templates/cpu-template-helper.md

@@ -16,7 +16,9 @@ This command dumps guest CPU configuration in the custom CPU template JSON
 format.
 
 ```
-cpu-template-helper template dump --config <firecracker-config> --output <cpu-config>
+cpu-template-helper template dump \
+    --output <cpu-config> \
+    [--config <firecracker-config>]
 ```
 
 Users can utilize this as an entry point of a custom CPU template creation to
@@ -68,7 +70,8 @@ This command verifies that the given custom CPU template is applied correctly.
 
 ```
 cpu-template-helper template verify \
-    --config <firecracker-config>
+    --template <cpu-template>
+    [--config <firecracker-config>]
 ```
 
 Firecracker modifies the guest CPU configuration after the CPU template is
@@ -77,8 +80,9 @@ not set the given configuration. Since Firecracker does not check them at
 runtime, it is required to ensure that these situations don't happen with their
 custom CPU templates before deploying it.
 
-The command uses the same configuration file as Firecracker and the path to the
-custom CPU template file should be specified in the "cpu-config" field.
+When a template is specified both through `--template` and in Firecracker
+configuration file provided via `--config`, the template specified with
+`--template` takes precedence.
 
 > **Note** This command does not ensure that the contents of the template are
 > sensible. Thus, users need to make sure that the template does not have any
@@ -93,8 +97,8 @@ information that could affect the validity of custom CPU templates.
 
 ```
 cpu-template-helper fingerprint dump \
-    --config <firecracker-config> \
-    --output <output-path>
+    --output <output-path> \
+    [--config <firecracker-config>]
 ```
 
 Keeping the underlying hardware and software stack updated is essential for

src/cpu-template-helper/Cargo.toml

@@ -18,12 +18,10 @@ log-instrument = { path = "../log-instrument", optional = true }
 serde = { version = "1.0.196", features = ["derive"] }
 serde_json = "1.0.113"
 thiserror = "1.0.57"
+vmm-sys-util = "0.12.1"
 
 vmm = { path = "../vmm" }
 
-[dev-dependencies]
-utils = { path = "../utils" }
-
 [features]
 tracing = ["log-instrument", "vmm/tracing"]
 

src/cpu-template-helper/src/main.rs

@@ -5,7 +5,7 @@
 use std::path::PathBuf;
 
 use clap::{Parser, Subcommand, ValueEnum};
-use vmm::cpu_config::templates::{CustomCpuTemplate, GetCpuTemplate, GetCpuTemplateError};
+use vmm::cpu_config::templates::{GetCpuTemplate, GetCpuTemplateError};
 
 use crate::utils::UtilsError;
 
@@ -58,7 +58,7 @@
     Dump {
         /// Path of firecracker config file.
         #[arg(short, long, value_name = "PATH")]
-        config: PathBuf,
+        config: Option<PathBuf>,
         /// Path of output file.
         #[arg(short, long, value_name = "PATH", default_value = "cpu_config.json")]
         output: PathBuf,
@@ -74,19 +74,22 @@
     },
     /// Verify that the given CPU template file is applied as intended.
     Verify {
-        /// Path of firecracker config file specifying the target CPU template.
+        /// Path of firecracker config file.
+        #[arg(short, long, value_name = "PATH")]
+        config: Option<PathBuf>,
+        /// Path of the target CPU template.
         #[arg(short, long, value_name = "PATH")]
-        config: PathBuf,
+        template: Option<PathBuf>,
     },
 }
 
 #[derive(Debug, Subcommand)]
 enum FingerprintOperation {
     /// Dump fingerprint consisting of host-related information and guest CPU config.
     Dump {
-        /// Path of fingerprint config file.
+        /// Path of firecracker config file.
         #[arg(short, long, value_name = "PATH")]
-        config: PathBuf,
+        config: Option<PathBuf>,
         /// Path of output file.
         #[arg(short, long, value_name = "PATH", default_value = "fingerprint.json")]
         output: PathBuf,
@@ -115,8 +118,8 @@
     match cli.command {
         Command::Template(op) => match op {
             TemplateOperation::Dump { config, output } => {
-                let config = read_to_string(config)?;
-                let (vmm, _) = utils::build_microvm_from_config(&config)?;
+                let config = config.map(read_to_string).transpose()?;
+                let (vmm, _) = utils::build_microvm_from_config(config, None)?;
 
                 let cpu_config = template::dump::dump(vmm)?;
 
@@ -127,7 +130,7 @@
                 let mut templates = Vec::with_capacity(paths.len());
                 for path in &paths {
                     let template_json = read_to_string(path)?;
-                    let template: CustomCpuTemplate = serde_json::from_str(&template_json)?;
+                    let template = serde_json::from_str(&template_json)?;
                     templates.push(template);
                 }
 
@@ -139,9 +142,16 @@
                     write(path, template_json)?;
                 }
             }
-            TemplateOperation::Verify { config } => {
-                let config = read_to_string(config)?;
-                let (vmm, vm_resources) = utils::build_microvm_from_config(&config)?;
+            TemplateOperation::Verify { config, template } => {
+                let config = config.map(read_to_string).transpose()?;
+                let template = match template {
+                    Some(path) => {
+                        let template_json = read_to_string(path)?;
+                        Some(serde_json::from_str(&template_json)?)
+                    }
+                    None => None,
+                };
+                let (vmm, vm_resources) = utils::build_microvm_from_config(config, template)?;
 
                 let cpu_template = vm_resources
                     .vm_config
@@ -155,8 +165,8 @@
         },
         Command::Fingerprint(op) => match op {
             FingerprintOperation::Dump { config, output } => {
-                let config = read_to_string(config)?;
-                let (vmm, _) = utils::build_microvm_from_config(&config)?;
+                let config = config.map(read_to_string).transpose()?;
+                let (vmm, _) = utils::build_microvm_from_config(config, None)?;
 
                 let fingerprint = fingerprint::dump::dump(vmm)?;
 
@@ -169,9 +179,9 @@
                 filters,
             } => {
                 let prev_json = read_to_string(prev)?;
-                let prev: fingerprint::Fingerprint = serde_json::from_str(&prev_json)?;
+                let prev = serde_json::from_str(&prev_json)?;
                 let curr_json = read_to_string(curr)?;
-                let curr: fingerprint::Fingerprint = serde_json::from_str(&curr_json)?;
+                let curr = serde_json::from_str(&curr_json)?;
                 fingerprint::compare::compare(prev, curr, filters)?;
             }
         },
@@ -195,70 +205,10 @@
 mod tests {
     use std::io::Write;
 
-    use ::utils::tempfile::TempFile;
-    use vmm::utilities::mock_resources::kernel_image_path;
+    use vmm_sys_util::tempfile::TempFile;
 
     use super::*;
 
-    pub fn generate_config(kernel_image_path: &str, rootfs_path: &str) -> String {
-        format!(
-            r#"{{
-                "boot-source": {{
-                    "kernel_image_path": "{}"
-                }},
-                "drives": [
-                    {{
-                        "drive_id": "rootfs",
-                        "path_on_host": "{}",
-                        "is_root_device": true,
-                        "is_read_only": false
-                    }}
-                ]
-            }}"#,
-            kernel_image_path, rootfs_path,
-        )
-    }
-
-    pub fn generate_config_with_template(
-        kernel_image_path: &str,
-        rootfs_path: &str,
-        cpu_template_path: &str,
-    ) -> String {
-        format!(
-            r#"{{
-                "boot-source": {{
-                    "kernel_image_path": "{}"
-                }},
-                "drives": [
-                    {{
-                        "drive_id": "rootfs",
-                        "path_on_host": "{}",
-                        "is_root_device": true,
-                        "is_read_only": false
-                    }}
-                ],
-                "cpu-config": "{}"
-            }}"#,
-            kernel_image_path, rootfs_path, cpu_template_path,
-        )
-    }
-
-    fn generate_config_file(
-        kernel_image_path: &str,
-        rootfs_path: &str,
-        cpu_template_path: Option<&str>,
-    ) -> TempFile {
-        let config = match cpu_template_path {
-            Some(cpu_template_path) => {
-                generate_config_with_template(kernel_image_path, rootfs_path, cpu_template_path)
-            }
-            None => generate_config(kernel_image_path, rootfs_path),
-        };
-        let config_file = TempFile::new().unwrap();
-        config_file.as_file().write_all(config.as_bytes()).unwrap();
-        config_file
-    }
-
     // Sample modifiers for x86_64 that should work correctly as a CPU template and a guest CPU
     // config.
     // * CPUID leaf 0x0 / subleaf 0x0 / register eax indicates the maximum input EAX value for basic
@@ -334,21 +284,11 @@
 
     #[test]
     fn test_template_dump_command() {
-        let kernel_image_path = kernel_image_path(None);
-        let rootfs_file = TempFile::new().unwrap();
-        let config_file = generate_config_file(
-            &kernel_image_path,
-            rootfs_file.as_path().to_str().unwrap(),
-            None,
-        );
         let output_file = TempFile::new().unwrap();
-
         let args = vec![
             "cpu-template-helper",
             "template",
             "dump",
-            "--config",
-            config_file.as_path().to_str().unwrap(),
             "--output",
             output_file.as_path().to_str().unwrap(),
         ];
@@ -374,21 +314,13 @@
 
     #[test]
     fn test_template_verify_command() {
-        let kernel_image_path = kernel_image_path(None);
-        let rootfs_file = TempFile::new().unwrap();
         let template_file = generate_sample_template();
-        let config_file = generate_config_file(
-            &kernel_image_path,
-            rootfs_file.as_path().to_str().unwrap(),
-            Some(template_file.as_path().to_str().unwrap()),
-        );
-
         let args = vec![
             "cpu-template-helper",
             "template",
             "verify",
-            "--config",
-            config_file.as_path().to_str().unwrap(),
+            "--template",
+            template_file.as_path().to_str().unwrap(),
         ];
         let cli = Cli::parse_from(args);
 
@@ -397,21 +329,11 @@
 
     #[test]
     fn test_fingerprint_dump_command() {
-        let kernel_image_path = kernel_image_path(None);
-        let rootfs_file = TempFile::new().unwrap();
-        let config_file = generate_config_file(
-            &kernel_image_path,
-            rootfs_file.as_path().to_str().unwrap(),
-            None,
-        );
         let output_file = TempFile::new().unwrap();
-
         let args = vec![
             "cpu-template-helper",
             "fingerprint",
             "dump",
-            "--config",
-            config_file.as_path().to_str().unwrap(),
             "--output",
             output_file.as_path().to_str().unwrap(),
         ];

src/cpu-template-helper/src/template/dump/mod.rs

@@ -32,21 +32,12 @@ pub fn dump(vmm: Arc<Mutex<Vmm>>) -> Result<CustomCpuTemplate, DumpError> {
 
 #[cfg(test)]
 mod tests {
-    use utils::tempfile::TempFile;
-    use vmm::utilities::mock_resources::kernel_image_path;
-
     use super::*;
-    use crate::tests::generate_config;
     use crate::utils::build_microvm_from_config;
 
     #[test]
     fn test_dump() {
-        let kernel_image_path = kernel_image_path(None);
-        let tmp_file = TempFile::new().unwrap();
-        let valid_config =
-            generate_config(&kernel_image_path, tmp_file.as_path().to_str().unwrap());
-        let (vmm, _) = build_microvm_from_config(&valid_config).unwrap();
-
+        let (vmm, _) = build_microvm_from_config(None, None).unwrap();
         dump(vmm).unwrap();
     }
 }

src/cpu-template-helper/src/utils/mock_kernel/Makefile

@@ -0,0 +1,17 @@
+# This is used to create a mock kernel to be embedded in CPU template helper tool.
+# It requires a valid ELF file for x86_64 and a file containing a valid kernel header for aarch64.
+
+ARCH := $(shell uname -m)
+
+.PHONY: clean
+
+$(ARCH).bin: $(ARCH).c
+ifeq ($(ARCH),x86_64)
+	gcc -nostdlib -fno-asynchronous-unwind-tables -s -o $(ARCH).bin $(ARCH).c
+else
+	gcc $(ARCH).c
+	./a.out
+endif
+
+clean:
+	rm -rf a.out $(ARCH).bin