From d4e0a0bc2019034a2a12add7aac8f3235676aa5b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 7 Jul 2025 18:38:35 +0000 Subject: [PATCH 1/6] Implement VM name truncation to fix Hyper-V 100-character limit Co-authored-by: benhillis <17727402+benhillis@users.noreply.github.com> --- petri/src/vm/hyperv/mod.rs | 79 +++++++++++++++++++++++++++++++++++++- 1 file changed, 78 insertions(+), 1 deletion(-) diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index 9023a08b3a..acd49d37d6 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/petri/src/vm/hyperv/mod.rs @@ -6,6 +6,8 @@ pub mod powershell; pub mod vm; use vmsocket::VmAddress; use vmsocket::VmSocket; +use std::collections::hash_map::DefaultHasher; +use std::hash::{Hash, Hasher}; use super::ProcessorTopology; use crate::Firmware; @@ -231,6 +233,33 @@ impl PetriVmArtifactsHyperV { } impl PetriVmConfigHyperV { + /// Creates a valid Hyper-V VM name from a test name. + /// + /// Hyper-V limits VM names to 100 characters. If the test name exceeds this limit, + /// we truncate it and append a hash to maintain uniqueness. + fn create_vm_name(test_name: &str) -> String { + const MAX_VM_NAME_LENGTH: usize = 100; + + if test_name.len() <= MAX_VM_NAME_LENGTH { + return test_name.to_owned(); + } + + // Create a hash of the full name for uniqueness + let mut hasher = DefaultHasher::new(); + test_name.hash(&mut hasher); + let hash = hasher.finish(); + let hash_suffix = format!("-{:x}", hash); + + // Reserve space for the hash suffix + let max_prefix_length = MAX_VM_NAME_LENGTH - hash_suffix.len(); + + // Take the end of the test name to preserve the most specific part + let start_pos = test_name.len().saturating_sub(max_prefix_length); + let truncated = &test_name[start_pos..]; + + format!("{}{}", truncated, hash_suffix) + } + /// Create a new Hyper-V petri VM config pub fn new( params: &PetriTestParams<'_>, @@ -287,7 +316,7 @@ impl PetriVmConfigHyperV { let openhcl_igvm = igvm_artifact.cloned(); Ok(PetriVmConfigHyperV { - name: params.test_name.to_owned(), + name: Self::create_vm_name(params.test_name), arch, generation, guest_state_isolation_type, @@ -778,3 +807,51 @@ fn acl_read_for_vm(path: &Path, id: Option) -> anyhow::Result<()> { } Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_create_vm_name() { + // Test short names remain unchanged + let short_name = "test::short_name"; + assert_eq!(PetriVmConfigHyperV::create_vm_name(short_name), short_name); + + // Test long names are truncated to 100 characters + let long_name = "multiarch::openhcl_servicing::hyperv_openhcl_uefi_aarch64_ubuntu_2404_server_aarch64_openhcl_servicing"; + let vm_name = PetriVmConfigHyperV::create_vm_name(long_name); + assert!(vm_name.len() <= 100); + assert!(vm_name.len() == 100); // Should be exactly 100 for this case + + // Test different long names get different hashes + let long_name2 = "multiarch::openhcl_servicing::hyperv_openhcl_uefi_aarch64_ubuntu_2404_server_aarch64_openhcl_different"; + let vm_name2 = PetriVmConfigHyperV::create_vm_name(long_name2); + assert!(vm_name2.len() <= 100); + assert_ne!(vm_name, vm_name2); + + // Test the most specific part is preserved + assert!(vm_name.contains("openhcl_servicing")); + assert!(vm_name2.contains("openhcl_different")); + } + + #[test] + fn test_vm_name_exactly_100_chars() { + let exactly_100 = "a".repeat(100); + let result = PetriVmConfigHyperV::create_vm_name(&exactly_100); + assert_eq!(result, exactly_100); + assert_eq!(result.len(), 100); + } + + #[test] + fn test_vm_name_101_chars() { + let exactly_101 = "a".repeat(101); + let result = PetriVmConfigHyperV::create_vm_name(&exactly_101); + assert_eq!(result.len(), 100); + assert!(result.ends_with(&format!("-{:x}", { + let mut hasher = DefaultHasher::new(); + exactly_101.hash(&mut hasher); + hasher.finish() + }))); + } +} From 6bbbd2efca246f3719b98f3d0aa64cbd6b8df95f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 9 Jul 2025 22:47:39 +0000 Subject: [PATCH 2/6] Run cargo xtask fmt --fix to ensure consistent coding style Co-authored-by: benhillis <17727402+benhillis@users.noreply.github.com> --- petri/src/vm/hyperv/mod.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index acd49d37d6..692e52d046 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/petri/src/vm/hyperv/mod.rs @@ -4,10 +4,10 @@ mod hvc; pub mod powershell; pub mod vm; -use vmsocket::VmAddress; -use vmsocket::VmSocket; use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; +use vmsocket::VmAddress; +use vmsocket::VmSocket; use super::ProcessorTopology; use crate::Firmware; @@ -234,29 +234,29 @@ impl PetriVmArtifactsHyperV { impl PetriVmConfigHyperV { /// Creates a valid Hyper-V VM name from a test name. - /// + /// /// Hyper-V limits VM names to 100 characters. If the test name exceeds this limit, /// we truncate it and append a hash to maintain uniqueness. fn create_vm_name(test_name: &str) -> String { const MAX_VM_NAME_LENGTH: usize = 100; - + if test_name.len() <= MAX_VM_NAME_LENGTH { return test_name.to_owned(); } - + // Create a hash of the full name for uniqueness let mut hasher = DefaultHasher::new(); test_name.hash(&mut hasher); let hash = hasher.finish(); let hash_suffix = format!("-{:x}", hash); - + // Reserve space for the hash suffix let max_prefix_length = MAX_VM_NAME_LENGTH - hash_suffix.len(); - + // Take the end of the test name to preserve the most specific part let start_pos = test_name.len().saturating_sub(max_prefix_length); let truncated = &test_name[start_pos..]; - + format!("{}{}", truncated, hash_suffix) } From 2da6d11b017a6526f7282b1031ac963aacb92866 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Jul 2025 18:57:51 +0000 Subject: [PATCH 3/6] Address PR feedback: separate imports and improve VM name truncation Co-authored-by: benhillis <17727402+benhillis@users.noreply.github.com> --- petri/src/vm/hyperv/mod.rs | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index 692e52d046..99695d6869 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/petri/src/vm/hyperv/mod.rs @@ -4,8 +4,10 @@ mod hvc; pub mod powershell; pub mod vm; + use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; + use vmsocket::VmAddress; use vmsocket::VmSocket; @@ -253,10 +255,28 @@ impl PetriVmConfigHyperV { // Reserve space for the hash suffix let max_prefix_length = MAX_VM_NAME_LENGTH - hash_suffix.len(); - // Take the end of the test name to preserve the most specific part - let start_pos = test_name.len().saturating_sub(max_prefix_length); - let truncated = &test_name[start_pos..]; + // Split by :: and take full components from the end to avoid cutting words + let parts: Vec<&str> = test_name.split("::").collect(); + let mut result_parts = Vec::new(); + let mut current_length = 0; + + // Work backwards through the parts to preserve the most specific components + for part in parts.iter().rev() { + let part_with_separator = if result_parts.is_empty() { + part.len() + } else { + part.len() + 2 // Add 2 for "::" + }; + + if current_length + part_with_separator <= max_prefix_length { + result_parts.insert(0, *part); + current_length += part_with_separator; + } else { + break; + } + } + let truncated = result_parts.join("::"); format!("{}{}", truncated, hash_suffix) } @@ -818,11 +838,11 @@ mod tests { let short_name = "test::short_name"; assert_eq!(PetriVmConfigHyperV::create_vm_name(short_name), short_name); - // Test long names are truncated to 100 characters + // Test long names are truncated to 100 characters with full components let long_name = "multiarch::openhcl_servicing::hyperv_openhcl_uefi_aarch64_ubuntu_2404_server_aarch64_openhcl_servicing"; let vm_name = PetriVmConfigHyperV::create_vm_name(long_name); assert!(vm_name.len() <= 100); - assert!(vm_name.len() == 100); // Should be exactly 100 for this case + assert!(vm_name.contains("-")); // Should have hash suffix // Test different long names get different hashes let long_name2 = "multiarch::openhcl_servicing::hyperv_openhcl_uefi_aarch64_ubuntu_2404_server_aarch64_openhcl_different"; @@ -830,6 +850,10 @@ mod tests { assert!(vm_name2.len() <= 100); assert_ne!(vm_name, vm_name2); + // Test that full components are preserved (no partial components) + assert!(!vm_name.starts_with("::")); + assert!(!vm_name2.starts_with("::")); + // Test the most specific part is preserved assert!(vm_name.contains("openhcl_servicing")); assert!(vm_name2.contains("openhcl_different")); From 2cfdb15c63804a65112cbc1f4f032d416fa7f90c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Jul 2025 19:06:29 +0000 Subject: [PATCH 4/6] Fix import organization to follow Rust coding conventions Co-authored-by: benhillis <17727402+benhillis@users.noreply.github.com> --- petri/src/vm/hyperv/mod.rs | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index 99695d6869..239350b8f0 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/petri/src/vm/hyperv/mod.rs @@ -6,21 +6,13 @@ pub mod powershell; pub mod vm; use std::collections::hash_map::DefaultHasher; +use std::fs; use std::hash::{Hash, Hasher}; +use std::io::Write; +use std::path::Path; +use std::path::PathBuf; +use std::time::Duration; -use vmsocket::VmAddress; -use vmsocket::VmSocket; - -use super::ProcessorTopology; -use crate::Firmware; -use crate::IsolationType; -use crate::PetriLogSource; -use crate::PetriTestParams; -use crate::PetriVm; -use crate::PetriVmConfig; -use crate::ShutdownKind; -use crate::disk_image::AgentImage; -use crate::openhcl_diag::OpenHclDiagHandler; use anyhow::Context; use async_trait::async_trait; use get_resources::ged::FirmwareEvent; @@ -37,13 +29,21 @@ use petri_artifacts_common::tags::OsFlavor; use petri_artifacts_core::ArtifactResolver; use petri_artifacts_core::ResolvedArtifact; use pipette_client::PipetteClient; -use std::fs; -use std::io::Write; -use std::path::Path; -use std::path::PathBuf; -use std::time::Duration; use vm::HyperVVM; use vmm_core_defs::HaltReason; +use vmsocket::VmAddress; +use vmsocket::VmSocket; + +use super::ProcessorTopology; +use crate::Firmware; +use crate::IsolationType; +use crate::PetriLogSource; +use crate::PetriTestParams; +use crate::PetriVm; +use crate::PetriVmConfig; +use crate::ShutdownKind; +use crate::disk_image::AgentImage; +use crate::openhcl_diag::OpenHclDiagHandler; /// Hyper-V VM configuration and resources pub struct PetriVmConfigHyperV { From f79be7c91e1ad98dd422ad7631d6987c6d3e4526 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Jul 2025 19:37:27 +0000 Subject: [PATCH 5/6] Add test case for final component over 100 characters and fix edge case Co-authored-by: mattkur <13772048+mattkur@users.noreply.github.com> --- petri/src/vm/hyperv/mod.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index 239350b8f0..48090db4c5 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/petri/src/vm/hyperv/mod.rs @@ -276,6 +276,13 @@ impl PetriVmConfigHyperV { } } + // If no parts fit, take the last part and truncate it + if result_parts.is_empty() && !parts.is_empty() { + let last_part = parts.last().unwrap(); + let truncated_part = &last_part[..max_prefix_length.min(last_part.len())]; + result_parts.push(truncated_part); + } + let truncated = result_parts.join("::"); format!("{}{}", truncated, hash_suffix) } @@ -878,4 +885,34 @@ mod tests { hasher.finish() }))); } + + #[test] + fn test_vm_name_final_component_over_100_chars() { + // Test case where the final component itself is longer than 100 characters + let long_final_component = "a".repeat(120); + let test_name = format!("short::component::{}", long_final_component); + let result = PetriVmConfigHyperV::create_vm_name(&test_name); + + // Should still be exactly 100 characters + assert_eq!(result.len(), 100); + + // Should have a hash suffix + assert!(result.contains("-")); + + // Should handle the situation reasonably (not be empty or malformed) + assert!(!result.is_empty()); + assert!(!result.starts_with("-")); + + // Should contain part of the long component (truncated) + assert!(result.starts_with("a")); + + // Edge case: entire name is one very long component without separators + let very_long_name = "a".repeat(150); + let result2 = PetriVmConfigHyperV::create_vm_name(&very_long_name); + assert_eq!(result2.len(), 100); + assert!(result2.contains("-")); + assert!(!result2.is_empty()); + assert!(!result2.starts_with("-")); + assert!(result2.starts_with("a")); + } } From 4456d2a53ebcd1828c77054027261f02d719c41a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 14 Jul 2025 15:16:03 +0000 Subject: [PATCH 6/6] Fix import organization: separate grouped imports into individual use statements Co-authored-by: mattkur <13772048+mattkur@users.noreply.github.com> --- petri/src/vm/hyperv/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index 48090db4c5..61a3556eae 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/petri/src/vm/hyperv/mod.rs @@ -7,7 +7,8 @@ pub mod vm; use std::collections::hash_map::DefaultHasher; use std::fs; -use std::hash::{Hash, Hasher}; +use std::hash::Hash; +use std::hash::Hasher; use std::io::Write; use std::path::Path; use std::path::PathBuf;