From 71b9c88e4c15d1db147f9327de49a624b1f3d2bd Mon Sep 17 00:00:00 2001 From: Matt Kurjanowicz Date: Thu, 9 Oct 2025 12:35:59 -0700 Subject: [PATCH 1/4] draft: Add support for NVMe storage in Hyper-V VMM tests --- Cargo.lock | 2 + petri/src/requirements.rs | 21 ++++ petri/src/vm/hyperv/hyperv.psm1 | 66 ++++++++++- petri/src/vm/hyperv/mod.rs | 86 ++++++++++++-- petri/src/vm/hyperv/powershell.rs | 101 +++++++++++++++++ petri/src/vm/hyperv/vm.rs | 28 ++++- petri/src/vm/mod.rs | 20 +++- petri/src/vm/openvmm/construct.rs | 20 +--- vmm_tests/vmm_test_macros/src/lib.rs | 32 ++++++ vmm_tests/vmm_tests/Cargo.toml | 2 + .../vmm_tests/tests/tests/x86_64/storage.rs | 106 ++++++++++++++++++ 11 files changed, 450 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7d303f2de1..e3f785aa1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9283,6 +9283,7 @@ version = "0.0.0" dependencies = [ "anyhow", "disk_backend_resources", + "disk_vhd1", "futures", "get_resources", "guid", @@ -9317,6 +9318,7 @@ dependencies = [ "vm_resource", "vmgs_resources", "vmm_test_macros", + "vtl2_settings_proto", "zerocopy 0.8.25", ] diff --git a/petri/src/requirements.rs b/petri/src/requirements.rs index 2923ad8d3c..2fa2308d12 100644 --- a/petri/src/requirements.rs +++ b/petri/src/requirements.rs @@ -63,6 +63,8 @@ pub struct HostContext { pub vendor: Vendor, /// Execution environment pub execution_environment: ExecutionEnvironment, + /// Whether the host supports NVMe storage using Hyper-V as the VMM + pub supports_hyperv_nvme_storage: bool, } impl HostContext { @@ -140,6 +142,21 @@ impl HostContext { } }; + let supports_hyperv_nvme_storage = { + #[cfg(windows)] + { + crate::vm::hyperv::powershell::run_check_vm_host_supports_hyperv_storage() + .await + .unwrap_or(false) // Assume false if we can't query + } + #[cfg(not(windows))] + { + // Not checked for non-Windows platforms, so just assume that the feature + // is supported. + true + } + }; + Self { vm_host_info, vendor, @@ -148,6 +165,7 @@ impl HostContext { } else { ExecutionEnvironment::Baremetal }, + supports_hyperv_nvme_storage, } } } @@ -158,6 +176,8 @@ pub enum TestRequirement { ExecutionEnvironment(ExecutionEnvironment), /// Vendor requirement. Vendor(Vendor), + /// Supports NVMe storage requirement. + SupportsNvmeStorage, /// Isolation requirement. Isolation(IsolationType), /// Logical AND of two requirements. @@ -174,6 +194,7 @@ impl TestRequirement { match self { TestRequirement::ExecutionEnvironment(env) => context.execution_environment == *env, TestRequirement::Vendor(vendor) => context.vendor == *vendor, + TestRequirement::SupportsNvmeStorage => context.supports_hyperv_nvme_storage, TestRequirement::Isolation(isolation_type) => { if let Some(vm_host_info) = &context.vm_host_info { match isolation_type { diff --git a/petri/src/vm/hyperv/hyperv.psm1 b/petri/src/vm/hyperv/hyperv.psm1 index 05da76c755..acc553c4ff 100644 --- a/petri/src/vm/hyperv/hyperv.psm1 +++ b/petri/src/vm/hyperv/hyperv.psm1 @@ -464,4 +464,68 @@ function Get-GuestStateFile $guestStateFile = $vssd.GuestStateFile return "$guestStateDataRoot\$guestStateFile" -} \ No newline at end of file +} + +function Set-Vtl2Settings { + [CmdletBinding(DefaultParameterSetName = "ByName")] + param ( + [Parameter(Position = 0, Mandatory = $true, ParameterSetName = "ByGuid")] + [Guid] $VmId, + + [Parameter(Mandatory = $true)] + [ValidateNotNullOrEmpty()] + [string]$Namespace, + + [Parameter(Mandatory = $true)] + [string]$SettingsFile, + + [string]$ClientName = 'Petri' + ) + + $settingsContent = Get-Content -Raw -Path $SettingsFile + + $guestManagement = Get-VmGuestManagementService + + $options = New-Object Microsoft.Management.Infrastructure.Options.CimOperationOptions + $options.SetCustomOption("ClientName", $ClientName, $false) + + Write-Host "aaaa" + + # Parameter - VmId + $p1 = [Microsoft.Management.Infrastructure.CimMethodParameter]::Create("VmId", $VmId.ToString(), [Microsoft.Management.Infrastructure.cimtype]::String, [Microsoft.Management.Infrastructure.CimFlags]::In) + + Write-Host "bbbb" + + # Parameter - Namespace + $p2 = [Microsoft.Management.Infrastructure.CimMethodParameter]::Create("Namespace", $Namespace, [Microsoft.Management.Infrastructure.cimtype]::String, [Microsoft.Management.Infrastructure.CimFlags]::In) + + Write-Host "cccc" + + # Parameter - Settings + # The input is a byte buffer with the size prepended. + # Size is a uint32 in network byte order (i.e. Big Endian) + # Size includes the size itself and the payload. + + $bytes = [system.Text.Encoding]::UTF8.GetBytes($settingsContent) + + $header = [System.BitConverter]::GetBytes([uint32]($bytes.Length + 4)) + if ([System.BitConverter]::IsLittleEndian) { + [System.Array]::Reverse($header) + } + $bytes = $header + $bytes + + $p3 = [Microsoft.Management.Infrastructure.CimMethodParameter]::Create("Settings", $bytes, [Microsoft.Management.Infrastructure.cimtype]::UInt8Array, [Microsoft.Management.Infrastructure.CimFlags]::In) + + $result = $guestManagement | Invoke-CimMethod -MethodName GetManagementVtlSettings -Arguments @{"VmId" = $VmId.ToString(); "Namespace" = $Namespace } | + Trace-CimMethodExecution -CimInstance $guestManagement -MethodName "GetManagementVtlSettings" + $updateId = $result.CurrentUpdateId + + $p4 = [Microsoft.Management.Infrastructure.CimMethodParameter]::Create("CurrentUpdateId", $updateId, [Microsoft.Management.Infrastructure.cimtype]::UInt64, [Microsoft.Management.Infrastructure.CimFlags]::In) + + $params = New-Object Microsoft.Management.Infrastructure.CimMethodParametersCollection + $params.Add($p1); $params.Add($p2); $params.Add($p3); $params.Add($p4) + + $cimSession = New-CimSession + $cimSession.InvokeMethod("root\virtualization\v2", $guestManagement, "SetManagementVtlSettings", $params, $options) | + Trace-CimMethodExecution -CimInstance $guestManagement -MethodName "SetManagementVtlSettings" | Out-Null +} diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index b7c6cffc67..5c74fdcff1 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/petri/src/vm/hyperv/mod.rs @@ -30,6 +30,7 @@ use crate::hyperv::powershell::HyperVSecureBootTemplate; use crate::kmsg_log_task; use crate::openhcl_diag::OpenHclDiagHandler; use crate::vm::append_cmdline; +use crate::vm::append_log_params_to_cmdline; use anyhow::Context; use async_trait::async_trait; use get_resources::ged::FirmwareEvent; @@ -69,9 +70,37 @@ pub struct HyperVPetriRuntime { is_isolated: bool, } +pub struct HyperVPetriConfig { + pub initial_vtl2_settings: Option, +} + +impl HyperVPetriRuntime { + pub async fn add_openhcl_nvme_storage>( + &self, + instance_id: Option<&guid::Guid>, + vhd_paths: &[P], + ) -> anyhow::Result<()> { + for vhd_path in vhd_paths { + acl_for_vm(vhd_path, Some(*self.vm.vmid()), VmFileAccess::FullControl) + .context("grant VM access to VHD")?; + } + + self.vm + .add_openhcl_nvme_storage(instance_id, vhd_paths) + .await + } + + pub async fn set_base_vtl2_settings( + &self, + settings: &vtl2_settings_proto::Vtl2Settings, + ) -> anyhow::Result<()> { + self.vm.set_base_vtl2_settings(settings).await + } +} + #[async_trait] impl PetriVmmBackend for HyperVPetriBackend { - type VmmConfig = (); + type VmmConfig = HyperVPetriConfig; type VmRuntime = HyperVPetriRuntime; fn check_compat(firmware: &Firmware, arch: MachineArch) -> bool { @@ -94,10 +123,6 @@ impl PetriVmmBackend for HyperVPetriBackend { modify_vmm_config: Option Self::VmmConfig + Send>, resources: &PetriVmResources, ) -> anyhow::Result { - if modify_vmm_config.is_some() { - panic!("specified modify_vmm_config, but that is not supported for hyperv"); - } - let PetriVmConfig { name, arch, @@ -394,7 +419,7 @@ impl PetriVmmBackend for HyperVPetriBackend { // Hyper-V (e.g., if it is in a WSL filesystem). let igvm_file = temp_dir.path().join("igvm.bin"); fs_err::copy(src_igvm_file, &igvm_file).context("failed to copy igvm file")?; - acl_read_for_vm(&igvm_file, Some(*vm.vmid())) + acl_for_vm(&igvm_file, Some(*vm.vmid()), VmFileAccess::Read) .context("failed to set ACL for igvm file")?; // TODO: only increase VTL2 memory on debug builds @@ -410,9 +435,11 @@ impl PetriVmmBackend for HyperVPetriBackend { ) .await?; - if let Some(command_line) = command_line { - vm.set_vm_firmware_command_line(command_line).await?; - } + let mut command_line = command_line.clone(); + append_log_params_to_cmdline(&mut command_line); + + vm.set_vm_firmware_command_line(command_line.unwrap()) + .await?; vm.set_vmbus_redirect(*vmbus_redirect).await?; @@ -492,6 +519,21 @@ impl PetriVmmBackend for HyperVPetriBackend { hyperv_serial_log_task(driver.clone(), serial_pipe_path, serial_log_file), )); + // todo mattkur + let initial_vtl2_settings = if let Some(f) = modify_vmm_config { + f(HyperVPetriConfig { + initial_vtl2_settings: None, + }) + .initial_vtl2_settings + } else { + None + }; + + if let Some(settings) = initial_vtl2_settings { + tracing::info!(?settings, "applying initial VTL2 settings"); + vm.set_base_vtl2_settings(&settings).await?; + } + vm.start().await?; Ok(HyperVPetriRuntime { @@ -616,17 +658,36 @@ impl PetriVmRuntime for HyperVPetriRuntime { } } -fn acl_read_for_vm(path: &Path, id: Option) -> anyhow::Result<()> { +/// The type of access to grant the VM for a file. +#[expect(missing_docs)] +enum VmFileAccess { + Read, + FullControl, +} + +/// The Hyper-V security model requires that the VM be granted explicit access to any +/// resources assigned. Hyper-V takes care of this when you use the admin facing +/// PowerShell cmdlets and some WMI flows. But, some tests use lower level APIs that don't +/// do this automatically. +fn acl_for_vm>( + path: P, + id: Option, + access: VmFileAccess, +) -> anyhow::Result<()> { let sid_arg = format!( - "NT VIRTUAL MACHINE\\{name}:R", + "NT VIRTUAL MACHINE\\{name}:{perm}", name = if let Some(id) = id { format!("{id:X}") } else { "Virtual Machines".to_string() + }, + perm = match access { + VmFileAccess::Read => "R", + VmFileAccess::FullControl => "F", } ); let output = std::process::Command::new("icacls.exe") - .arg(path) + .arg(path.as_ref()) .arg("/grant") .arg(sid_arg) .output() @@ -635,6 +696,7 @@ fn acl_read_for_vm(path: &Path, id: Option) -> anyhow::Result<()> { let stderr = String::from_utf8_lossy(&output.stderr); anyhow::bail!("icacls failed: {stderr}"); } + Ok(()) } diff --git a/petri/src/vm/hyperv/powershell.rs b/petri/src/vm/hyperv/powershell.rs index 2616ce3a8f..34300cdda2 100644 --- a/petri/src/vm/hyperv/powershell.rs +++ b/petri/src/vm/hyperv/powershell.rs @@ -16,6 +16,7 @@ use powershell_builder::PowerShellBuilder; use serde::Deserialize; use serde::Serialize; use std::ffi::OsStr; +use std::io::Write; use std::path::Path; use std::path::PathBuf; use std::str::FromStr; @@ -1092,3 +1093,103 @@ pub async fn run_get_guest_state_file(vmid: &Guid, ps_mod: &Path) -> anyhow::Res Ok(PathBuf::from(output)) } + +/// Queries whether the host supports Hyper-V NVMe storage for VMs. +/// +/// This is true if there are Microsoft-internal utilties installed on the +/// host that support this feature. +/// FUTURE: If sufficient environments exist, we can do this using DDA'd NVMe +/// devices. +pub async fn run_check_vm_host_supports_hyperv_storage() -> anyhow::Result { + let output: String = run_host_cmd( + PowerShellBuilder::new() + .cmdlet("Test-Path") + .arg( + "Path", + "c:\\OpenvmmCI\\MicrosoftInternalTestHelpers\\MicrosoftInternalTestHelpers.psm1", + ) + .next() + .cmdlet("Import-Module") + .positional( + "c:\\OpenvmmCI\\MicrosoftInternalTestHelpers\\MicrosoftInternalTestHelpers.psm1", + ) + .next() + .cmdlet("Test-MicrosoftInternalVmNvmeStorage") + .finish() + .build(), + ) + .await + .context("run_check_vm_host_supports_hyperv_storage")?; + + Ok(output.trim().eq_ignore_ascii_case("true")) +} + +/// Queries whether the host supports Hyper-V NVMe storage for VMs. +/// +/// This is true if there are Microsoft-internal utilties installed on the +/// host that support this feature. +/// +/// TODO: Check that the resource is successfully added to the VM. +/// e.g. if there is a signature issue, it seems that this call completes +/// yet there was an error. +pub async fn run_configure_microsoft_internal_nvme_storage>( + vmid: &Guid, + instance_id: Option<&Guid>, + target_vtl: u32, + disk_paths: &[P], +) -> anyhow::Result<()> { + run_host_cmd( + PowerShellBuilder::new() + .cmdlet("Import-Module") + .positional( + "c:\\OpenvmmCI\\MicrosoftInternalTestHelpers\\MicrosoftInternalTestHelpers.psm1", + ) + .next() + .cmdlet("Add-MicrosoftInternalVmNvmeStorage") + .arg("VmId", vmid) + .arg("TargetVtl", target_vtl) + .arg( + "DiskPaths", + ps::Array::new( + disk_paths + .iter() + .map(|p| p.as_ref().to_str().expect("path is valid &str")), + ), + ) + .arg_opt("Vsid", instance_id) + .finish() + .build(), + ) + .await + .map(|_| ()) + .context("run_configure_microsoft_internal_nvme_storage") +} + +pub async fn run_set_base_vtl2_settings( + vmid: &Guid, + ps_mod: &Path, + vtl2_settings: &vtl2_settings_proto::Vtl2Settings, +) -> anyhow::Result<()> { + let mut tempfile = tempfile::NamedTempFile::new().context("creating tempfile")?; + tempfile + .write_all(serde_json::to_string(vtl2_settings)?.as_bytes()) + .context("writing settings to tempfile")?; + + tracing::info!(?tempfile, ?vtl2_settings, ?vmid, "set base vtl2 settings"); + + run_host_cmd( + PowerShellBuilder::new() + .cmdlet("Import-Module") + .positional(ps_mod) + .next() + .cmdlet("Set-Vtl2Settings") + .arg("VmId", vmid) + .arg("SettingsFile", tempfile.path()) + .arg("Namespace", "Base") + .finish() + .build(), + ) + .await + .map(|_| ()) + .context("set_base_vtl2_settings") +} diff --git a/petri/src/vm/hyperv/vm.rs b/petri/src/vm/hyperv/vm.rs index bd6d51ca7f..90879cf418 100644 --- a/petri/src/vm/hyperv/vm.rs +++ b/petri/src/vm/hyperv/vm.rs @@ -484,11 +484,12 @@ impl HyperVVM { } /// Sets the VM firmware command line. - pub async fn set_vm_firmware_command_line( + pub async fn set_vm_firmware_command_line>( &self, - openhcl_command_line: &str, + openhcl_command_line: S, ) -> anyhow::Result<()> { - powershell::run_set_vm_command_line(&self.vmid, &self.ps_mod, openhcl_command_line).await + powershell::run_set_vm_command_line(&self.vmid, &self.ps_mod, openhcl_command_line.as_ref()) + .await } /// Enable VMBusRelay @@ -515,6 +516,27 @@ impl HyperVVM { pub async fn get_guest_state_file(&self) -> anyhow::Result { powershell::run_get_guest_state_file(&self.vmid, &self.ps_mod).await } + + pub async fn add_openhcl_nvme_storage>( + &self, + instance_id: Option<&Guid>, + vhd_paths: &[P], + ) -> anyhow::Result<()> { + powershell::run_configure_microsoft_internal_nvme_storage( + &self.vmid, + instance_id, + 2, + vhd_paths, + ) + .await + } + + pub async fn set_base_vtl2_settings( + &self, + settings: &vtl2_settings_proto::Vtl2Settings, + ) -> anyhow::Result<()> { + powershell::run_set_base_vtl2_settings(&self.vmid, &self.ps_mod, settings).await + } } impl Drop for HyperVVM { diff --git a/petri/src/vm/mod.rs b/petri/src/vm/mod.rs index 4847a7f11d..b1c41e455b 100644 --- a/petri/src/vm/mod.rs +++ b/petri/src/vm/mod.rs @@ -323,7 +323,7 @@ impl PetriVmBuilder { let mut tasks = Vec::new(); { - const TIMEOUT_DURATION_MINUTES: u64 = 7; + const TIMEOUT_DURATION_MINUTES: u64 = 20; // DO NOT MERGE const TIMER_DURATION: Duration = Duration::from_secs(TIMEOUT_DURATION_MINUTES * 60); let log_source = resources.log_source.clone(); let inspect_task = @@ -1795,6 +1795,24 @@ fn append_cmdline(cmd: &mut Option, add_cmd: impl AsRef) { } } +fn append_log_params_to_cmdline(cmd: &mut Option) { + // Forward OPENVMM_LOG and OPENVMM_SHOW_SPANS to OpenHCL if they're set. + let openhcl_tracing = + if let Ok(x) = std::env::var("OPENVMM_LOG").or_else(|_| std::env::var("HVLITE_LOG")) { + format!("OPENVMM_LOG={x}") + } else { + "OPENVMM_LOG=debug".to_owned() + }; + let openhcl_show_spans = if let Ok(x) = std::env::var("OPENVMM_SHOW_SPANS") { + format!("OPENVMM_SHOW_SPANS={x}") + } else { + "OPENVMM_SHOW_SPANS=true".to_owned() + }; + + append_cmdline(cmd, openhcl_tracing); + append_cmdline(cmd, openhcl_show_spans); +} + async fn save_inspect( name: &str, inspect: std::pin::Pin> + Send>>, diff --git a/petri/src/vm/openvmm/construct.rs b/petri/src/vm/openvmm/construct.rs index 720d1d3dd4..baf1856219 100644 --- a/petri/src/vm/openvmm/construct.rs +++ b/petri/src/vm/openvmm/construct.rs @@ -30,6 +30,7 @@ use crate::linux_direct_serial_agent::LinuxDirectSerialAgent; use crate::openvmm::BOOT_NVME_INSTANCE; use crate::openvmm::memdiff_vmgs; use crate::vm::append_cmdline; +use crate::vm::append_log_params_to_cmdline; use crate::vtl2_settings::ControllerType; use crate::vtl2_settings::Vtl2LunBuilder; use crate::vtl2_settings::Vtl2StorageBackingDeviceBuilder; @@ -587,19 +588,6 @@ impl PetriVmConfigSetupCore<'_> { } fn load_firmware(&self) -> anyhow::Result { - // Forward OPENVMM_LOG and OPENVMM_SHOW_SPANS to OpenHCL if they're set. - let openhcl_tracing = - if let Ok(x) = std::env::var("OPENVMM_LOG").or_else(|_| std::env::var("HVLITE_LOG")) { - format!("OPENVMM_LOG={x}") - } else { - "OPENVMM_LOG=debug".to_owned() - }; - let openhcl_show_spans = if let Ok(x) = std::env::var("OPENVMM_SHOW_SPANS") { - format!("OPENVMM_SHOW_SPANS={x}") - } else { - "OPENVMM_SHOW_SPANS=true".to_owned() - }; - Ok(match (self.arch, &self.firmware) { (MachineArch::X86_64, Firmware::LinuxDirect { kernel, initrd }) => { let kernel = File::open(kernel.clone()) @@ -697,10 +685,8 @@ impl PetriVmConfigSetupCore<'_> { let mut cmdline = command_line.clone(); - append_cmdline( - &mut cmdline, - format!("panic=-1 reboot=triple {openhcl_tracing} {openhcl_show_spans}"), - ); + append_cmdline(&mut cmdline, format!("panic=-1 reboot=triple")); + append_log_params_to_cmdline(&mut cmdline); let isolated = match self.firmware { Firmware::OpenhclLinuxDirect { .. } => { diff --git a/vmm_tests/vmm_test_macros/src/lib.rs b/vmm_tests/vmm_test_macros/src/lib.rs index 61402b6a20..b98db497a7 100644 --- a/vmm_tests/vmm_test_macros/src/lib.rs +++ b/vmm_tests/vmm_test_macros/src/lib.rs @@ -627,6 +627,19 @@ pub fn openvmm_test( .into() } +/// Same options as `vmm_test`, but only for Hyper-V tests +#[proc_macro_attribute] +pub fn hyperv_test( + attr: proc_macro::TokenStream, + item: proc_macro::TokenStream, +) -> proc_macro::TokenStream { + let args = parse_macro_input!(attr as Args); + let item = parse_macro_input!(item as ItemFn); + make_vmm_test(args, item, Some(Vmm::HyperV), true) + .unwrap_or_else(|err| err.to_compile_error()) + .into() +} + /// Same options as `vmm_test`, but only for OpenVMM tests and without using pipette in VTL0. #[proc_macro_attribute] pub fn openvmm_test_no_agent( @@ -809,6 +822,25 @@ fn build_requirements(config: &Config, name: &str) -> Option { }; } + // Special case for NVMe tests on Hyper-V + let is_nvme = matches!( + config.firmware, + Firmware::OpenhclUefi(OpenhclUefiOptions { nvme: true, .. }, _) + ) || name.contains("add_openhcl_nvme_storage"); + if is_hyperv && is_nvme { + let hyperv_nvme_requirement_expr = + quote!(::petri::requirements::TestRequirement::SupportsNvmeStorage); + requirement_expr = match requirement_expr { + Some(existing) => Some(quote!( + ::petri::requirements::TestRequirement::And( + Box::new(#existing), + Box::new(#hyperv_nvme_requirement_expr) + ) + )), + None => Some(hyperv_nvme_requirement_expr), + }; + } + if requirement_expr.is_some() { Some(quote!(::petri::requirements::TestCaseRequirements::new(#requirement_expr))) } else { diff --git a/vmm_tests/vmm_tests/Cargo.toml b/vmm_tests/vmm_tests/Cargo.toml index 668c1b54d7..3c9dd009e0 100644 --- a/vmm_tests/vmm_tests/Cargo.toml +++ b/vmm_tests/vmm_tests/Cargo.toml @@ -26,6 +26,7 @@ petri.workspace = true get_resources.workspace = true hvlite_defs.workspace = true disk_backend_resources.workspace = true +disk_vhd1.workspace = true hyperv_ic_resources.workspace = true net_backend_resources.workspace = true nvme_resources.workspace = true @@ -35,6 +36,7 @@ scsidisk_resources.workspace = true storvsp_resources.workspace = true virtio_resources.workspace = true vm_resource.workspace = true +vtl2_settings_proto.workspace = true guid.workspace = true kmsg.workspace = true diff --git a/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs b/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs index 70a4852dfd..b0b1e72ab8 100644 --- a/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs +++ b/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs @@ -18,6 +18,8 @@ use mesh::rpc::RpcSend; use nvme_resources::NamespaceDefinition; use nvme_resources::NvmeControllerHandle; use petri::PetriVmBuilder; +#[cfg(windows)] +use petri::hyperv::HyperVPetriBackend; use petri::openvmm::OpenVmmPetriBackend; use petri::pipette::PipetteClient; use petri::pipette::cmd; @@ -36,7 +38,11 @@ use storvsp_resources::ScsiControllerHandle; use storvsp_resources::ScsiDeviceAndPath; use storvsp_resources::ScsiPath; use vm_resource::IntoResource; +#[cfg(windows)] +use vmm_test_macros::hyperv_test; use vmm_test_macros::openvmm_test; +#[cfg(windows)] +use vtl2_settings_proto::Vtl2Settings; /// Create a VPCI device config for an NVMe controller assigned to VTL2, with a single namespace. /// The namespace will be backed by either a file or a ramdisk, depending on whether @@ -619,3 +625,103 @@ async fn openhcl_linux_storvsp_dvd_nvme( Ok(()) } + +#[cfg(windows)] +#[hyperv_test(openhcl_uefi_x64(vhd(ubuntu_2504_server_x64)))] +pub async fn add_openhcl_nvme_storage( + config: PetriVmBuilder, +) -> anyhow::Result<()> { + let vtl0_instance_id = Guid::new_random(); + let vtl2_instance_id = Guid::new_random(); + const VHD_SIZE: u64 = 200 * 1024 * 1024; // 200 MiB + + // todo: generate a better temp file name + let temp_dir = tempfile::tempdir()?; + let vhd_path = temp_dir.path().join("test.vhd"); + let file = File::options() + .create(true) + .create_new(true) + .truncate(true) + .read(true) + .write(true) + .open(&vhd_path) + .context("create file")?; + + file.set_len(VHD_SIZE).context("set file length")?; + disk_vhd1::Vhd1Disk::make_fixed(&file).context("make fixed")?; + + let initial_vtl2_settings = Vtl2Settings { + version: vtl2_settings_proto::vtl2_settings_base::Version::V1.into(), + dynamic: Some(vtl2_settings_proto::Vtl2SettingsDynamic { + storage_controllers: vec![ + Vtl2StorageControllerBuilder::scsi() + .with_instance_id(vtl0_instance_id) + .build(), + ], + ..Default::default() + }), + fixed: None, + namespace_settings: Default::default(), + }; + + // This is a bit ugly; ideally the HyperV backend would munge and set any + // VTL2 settings to the modify call (just like happens for OpenVMM). + // For now, just take the expedient path to get this test stood up. + let mut vtl2_settings = initial_vtl2_settings.clone(); + + let (mut vm, agent) = config + .with_vmbus_redirect(true) + .modify_backend(move |b| { + assert!(b.initial_vtl2_settings.is_none()); + petri::hyperv::HyperVPetriConfig { + initial_vtl2_settings: Some(initial_vtl2_settings), + } + }) + .run() + .await?; + + // Runtime add the NVMe device and update the VTL2 settings to + // include it. + // + // This _could_ happen before the VM is started, but do it at runtime + // for test simplicity. + + vm.backend() + .add_openhcl_nvme_storage(Some(&vtl2_instance_id), &[vhd_path.to_str().unwrap()]) + .await?; + + vtl2_settings.dynamic.as_mut().unwrap().storage_controllers[0] + .luns + .push( + Vtl2LunBuilder::disk() + .with_location(0) + .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( + ControllerType::Nvme, + vtl2_instance_id, + 1, + )) + .build(), + ); + vm.backend().set_base_vtl2_settings(&vtl2_settings).await?; + + match vm.inspect_openhcl("vm/nvme/devices", None, None).await { + Err(e) => tracing::error!(?e, "Failed to inspect NVMe devices"), + Ok(devices) => tracing::info!(devices = %devices.json(), "NVMe devices"), + } + + test_storage_linux( + &agent, + vec![ExpectedGuestDevice { + controller_guid: vtl0_instance_id, + lun: 0, + disk_size_sectors: (VHD_SIZE / 512) as usize, + friendly_name: "nvme".to_string(), + }], + ) + .await?; + + agent.power_off().await?; + vm.wait_for_clean_teardown().await?; + + Ok(()) +} From 4d4b0930b2fb276f1dbc96e4d7139429a34b8bc8 Mon Sep 17 00:00:00 2001 From: "Matt LaFayette (Kurjanowicz)" Date: Sat, 11 Oct 2025 13:59:35 +0000 Subject: [PATCH 2/4] minor cleanups --- petri/src/vm/hyperv/hyperv.psm1 | 6 --- petri/src/vm/hyperv/mod.rs | 16 ++++++-- petri/src/vm/hyperv/powershell.rs | 37 +++++++++++-------- petri/src/vm/hyperv/vm.rs | 21 +++++++---- petri/src/vm/mod.rs | 2 +- vmm_tests/vmm_test_macros/src/lib.rs | 2 +- .../vmm_tests/tests/tests/x86_64/storage.rs | 8 ++-- 7 files changed, 53 insertions(+), 39 deletions(-) diff --git a/petri/src/vm/hyperv/hyperv.psm1 b/petri/src/vm/hyperv/hyperv.psm1 index acc553c4ff..7714e6c6e6 100644 --- a/petri/src/vm/hyperv/hyperv.psm1 +++ b/petri/src/vm/hyperv/hyperv.psm1 @@ -489,18 +489,12 @@ function Set-Vtl2Settings { $options = New-Object Microsoft.Management.Infrastructure.Options.CimOperationOptions $options.SetCustomOption("ClientName", $ClientName, $false) - Write-Host "aaaa" - # Parameter - VmId $p1 = [Microsoft.Management.Infrastructure.CimMethodParameter]::Create("VmId", $VmId.ToString(), [Microsoft.Management.Infrastructure.cimtype]::String, [Microsoft.Management.Infrastructure.CimFlags]::In) - Write-Host "bbbb" - # Parameter - Namespace $p2 = [Microsoft.Management.Infrastructure.CimMethodParameter]::Create("Namespace", $Namespace, [Microsoft.Management.Infrastructure.cimtype]::String, [Microsoft.Management.Infrastructure.CimFlags]::In) - Write-Host "cccc" - # Parameter - Settings # The input is a byte buffer with the size prepended. # Size is a uint32 in network byte order (i.e. Big Endian) diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index 5c74fdcff1..44895e25eb 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/petri/src/vm/hyperv/mod.rs @@ -70,14 +70,21 @@ pub struct HyperVPetriRuntime { is_isolated: bool, } +/// Additional configuration for a Hyper-V VM. pub struct HyperVPetriConfig { + /// VTL2 settings to configure on the VM before petri powers it on. pub initial_vtl2_settings: Option, } impl HyperVPetriRuntime { - pub async fn add_openhcl_nvme_storage>( + /// Adds an NVMe device to the VM, with a set of NVMe namespaces. Each + /// namespace is backed by one of the supplied VHDs. The namespaces will be + /// added in-order. That is, the first entry in the list of `vhd_paths` will + /// have `nsid` `1`, the second entry will have `nsid` `2`, and so on. + pub async fn add_nvme_device>( &self, instance_id: Option<&guid::Guid>, + target_vtl: u32, vhd_paths: &[P], ) -> anyhow::Result<()> { for vhd_path in vhd_paths { @@ -86,10 +93,12 @@ impl HyperVPetriRuntime { } self.vm - .add_openhcl_nvme_storage(instance_id, vhd_paths) + .add_nvme_device(instance_id, target_vtl, vhd_paths) .await } + /// Set the VTL2 settings in the `Base` namespace (fixed settings, storage + /// settings, etc). pub async fn set_base_vtl2_settings( &self, settings: &vtl2_settings_proto::Vtl2Settings, @@ -519,7 +528,6 @@ impl PetriVmmBackend for HyperVPetriBackend { hyperv_serial_log_task(driver.clone(), serial_pipe_path, serial_log_file), )); - // todo mattkur let initial_vtl2_settings = if let Some(f) = modify_vmm_config { f(HyperVPetriConfig { initial_vtl2_settings: None, @@ -659,7 +667,7 @@ impl PetriVmRuntime for HyperVPetriRuntime { } /// The type of access to grant the VM for a file. -#[expect(missing_docs)] +#[allow(missing_docs)] enum VmFileAccess { Read, FullControl, diff --git a/petri/src/vm/hyperv/powershell.rs b/petri/src/vm/hyperv/powershell.rs index 34300cdda2..c63d8e987f 100644 --- a/petri/src/vm/hyperv/powershell.rs +++ b/petri/src/vm/hyperv/powershell.rs @@ -1096,7 +1096,7 @@ pub async fn run_get_guest_state_file(vmid: &Guid, ps_mod: &Path) -> anyhow::Res /// Queries whether the host supports Hyper-V NVMe storage for VMs. /// -/// This is true if there are Microsoft-internal utilties installed on the +/// This is true if there are Microsoft-internal utilities installed on the /// host that support this feature. /// FUTURE: If sufficient environments exist, we can do this using DDA'd NVMe /// devices. @@ -1106,12 +1106,12 @@ pub async fn run_check_vm_host_supports_hyperv_storage() -> anyhow::Result .cmdlet("Test-Path") .arg( "Path", - "c:\\OpenvmmCI\\MicrosoftInternalTestHelpers\\MicrosoftInternalTestHelpers.psm1", + "c:\\OpenVMMCI\\MicrosoftInternalTestHelpers\\MicrosoftInternalTestHelpers.psm1", ) .next() .cmdlet("Import-Module") .positional( - "c:\\OpenvmmCI\\MicrosoftInternalTestHelpers\\MicrosoftInternalTestHelpers.psm1", + "c:\\OpenVMMCI\\MicrosoftInternalTestHelpers\\MicrosoftInternalTestHelpers.psm1", ) .next() .cmdlet("Test-MicrosoftInternalVmNvmeStorage") @@ -1124,15 +1124,12 @@ pub async fn run_check_vm_host_supports_hyperv_storage() -> anyhow::Result Ok(output.trim().eq_ignore_ascii_case("true")) } -/// Queries whether the host supports Hyper-V NVMe storage for VMs. -/// -/// This is true if there are Microsoft-internal utilties installed on the -/// host that support this feature. +/// Adds an NVMe device to the VM using Microsoft-internal utilities. /// -/// TODO: Check that the resource is successfully added to the VM. -/// e.g. if there is a signature issue, it seems that this call completes -/// yet there was an error. -pub async fn run_configure_microsoft_internal_nvme_storage>( +/// TODO FUTURE: Check that the resource is successfully added to the VM. e.g. +/// if there is a signature issue, it seems that this call completes yet there +/// was an error. +pub async fn run_add_nvme>( vmid: &Guid, instance_id: Option<&Guid>, target_vtl: u32, @@ -1142,7 +1139,7 @@ pub async fn run_configure_microsoft_internal_nvme_storage>( PowerShellBuilder::new() .cmdlet("Import-Module") .positional( - "c:\\OpenvmmCI\\MicrosoftInternalTestHelpers\\MicrosoftInternalTestHelpers.psm1", + "c:\\OpenVMMCI\\MicrosoftInternalTestHelpers\\MicrosoftInternalTestHelpers.psm1", ) .next() .cmdlet("Add-MicrosoftInternalVmNvmeStorage") @@ -1153,7 +1150,7 @@ pub async fn run_configure_microsoft_internal_nvme_storage>( ps::Array::new( disk_paths .iter() - .map(|p| p.as_ref().to_str().expect("path is valid &str")), + .map(|p| p.as_ref().to_str().expect("path can be converted to &str")), ), ) .arg_opt("Vsid", instance_id) @@ -1162,20 +1159,30 @@ pub async fn run_configure_microsoft_internal_nvme_storage>( ) .await .map(|_| ()) - .context("run_configure_microsoft_internal_nvme_storage") + .context("run_add_nvme") } +/// Sets the VTL2 settings for a VM that exist in the `Base` namespace. +/// +/// This should include the fixed VTL2 settings, as well as any storage +/// settings. +/// +/// TODO FUTURE: Detect if the settings should be in `json` or `protobuf` format +/// based on what is already there (or let the caller specify explicitly so that +/// we can test the handling of both deserializers). pub async fn run_set_base_vtl2_settings( vmid: &Guid, ps_mod: &Path, vtl2_settings: &vtl2_settings_proto::Vtl2Settings, ) -> anyhow::Result<()> { + // Pass the settings via a file to avoid challenges escaping the string across + // the command line. let mut tempfile = tempfile::NamedTempFile::new().context("creating tempfile")?; tempfile .write_all(serde_json::to_string(vtl2_settings)?.as_bytes()) .context("writing settings to tempfile")?; - tracing::info!(?tempfile, ?vtl2_settings, ?vmid, "set base vtl2 settings"); + tracing::debug!(?tempfile, ?vtl2_settings, ?vmid, "set base vtl2 settings"); run_host_cmd( PowerShellBuilder::new() diff --git a/petri/src/vm/hyperv/vm.rs b/petri/src/vm/hyperv/vm.rs index 90879cf418..6643378b3f 100644 --- a/petri/src/vm/hyperv/vm.rs +++ b/petri/src/vm/hyperv/vm.rs @@ -517,20 +517,25 @@ impl HyperVVM { powershell::run_get_guest_state_file(&self.vmid, &self.ps_mod).await } - pub async fn add_openhcl_nvme_storage>( + /// Add an NVMe device to the Hyper-V VM. (Microsoft-internal) + /// + /// TODO FUTURE: Abstract this out such that it can take an identifier for a + /// device, rather than a set of VHD paths (at that point it could be used + /// on real hardware). Such work could leverage NVMe DDA, which is availabe + /// outside of the Microsoft-internal environment (but then you have to have + /// spare NVMe devices on the system where you're running this). + /// Tracked by #2151. + pub async fn add_nvme_device>( &self, instance_id: Option<&Guid>, + target_vtl: u32, vhd_paths: &[P], ) -> anyhow::Result<()> { - powershell::run_configure_microsoft_internal_nvme_storage( - &self.vmid, - instance_id, - 2, - vhd_paths, - ) - .await + powershell::run_add_nvme(&self.vmid, instance_id, target_vtl, vhd_paths).await } + /// Set the VTL2 settings in the `Base` namespace (fixed settings, storage + /// settings, etc). pub async fn set_base_vtl2_settings( &self, settings: &vtl2_settings_proto::Vtl2Settings, diff --git a/petri/src/vm/mod.rs b/petri/src/vm/mod.rs index b1c41e455b..a261f78d5d 100644 --- a/petri/src/vm/mod.rs +++ b/petri/src/vm/mod.rs @@ -323,7 +323,7 @@ impl PetriVmBuilder { let mut tasks = Vec::new(); { - const TIMEOUT_DURATION_MINUTES: u64 = 20; // DO NOT MERGE + const TIMEOUT_DURATION_MINUTES: u64 = 10; const TIMER_DURATION: Duration = Duration::from_secs(TIMEOUT_DURATION_MINUTES * 60); let log_source = resources.log_source.clone(); let inspect_task = diff --git a/vmm_tests/vmm_test_macros/src/lib.rs b/vmm_tests/vmm_test_macros/src/lib.rs index b98db497a7..f577672f89 100644 --- a/vmm_tests/vmm_test_macros/src/lib.rs +++ b/vmm_tests/vmm_test_macros/src/lib.rs @@ -826,7 +826,7 @@ fn build_requirements(config: &Config, name: &str) -> Option { let is_nvme = matches!( config.firmware, Firmware::OpenhclUefi(OpenhclUefiOptions { nvme: true, .. }, _) - ) || name.contains("add_openhcl_nvme_storage"); + ) || name.contains("add_nvme_device"); if is_hyperv && is_nvme { let hyperv_nvme_requirement_expr = quote!(::petri::requirements::TestRequirement::SupportsNvmeStorage); diff --git a/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs b/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs index b0b1e72ab8..31faca9c37 100644 --- a/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs +++ b/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs @@ -628,9 +628,7 @@ async fn openhcl_linux_storvsp_dvd_nvme( #[cfg(windows)] #[hyperv_test(openhcl_uefi_x64(vhd(ubuntu_2504_server_x64)))] -pub async fn add_openhcl_nvme_storage( - config: PetriVmBuilder, -) -> anyhow::Result<()> { +pub async fn add_nvme_device(config: PetriVmBuilder) -> anyhow::Result<()> { let vtl0_instance_id = Guid::new_random(); let vtl2_instance_id = Guid::new_random(); const VHD_SIZE: u64 = 200 * 1024 * 1024; // 200 MiB @@ -653,6 +651,8 @@ pub async fn add_openhcl_nvme_storage( let initial_vtl2_settings = Vtl2Settings { version: vtl2_settings_proto::vtl2_settings_base::Version::V1.into(), dynamic: Some(vtl2_settings_proto::Vtl2SettingsDynamic { + // You can't add a _controller_ at runtime, so make sure to add it + // here before the test powers on the VM. storage_controllers: vec![ Vtl2StorageControllerBuilder::scsi() .with_instance_id(vtl0_instance_id) @@ -687,7 +687,7 @@ pub async fn add_openhcl_nvme_storage( // for test simplicity. vm.backend() - .add_openhcl_nvme_storage(Some(&vtl2_instance_id), &[vhd_path.to_str().unwrap()]) + .add_nvme_device(Some(&vtl2_instance_id), 2, &[vhd_path.to_str().unwrap()]) .await?; vtl2_settings.dynamic.as_mut().unwrap().storage_controllers[0] From dec67b2f56df9d6c683a5aaec785beea22add62c Mon Sep 17 00:00:00 2001 From: "Matt LaFayette (Kurjanowicz)" Date: Sat, 11 Oct 2025 14:02:48 +0000 Subject: [PATCH 3/4] more clippy --- petri/src/vm/openvmm/construct.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/petri/src/vm/openvmm/construct.rs b/petri/src/vm/openvmm/construct.rs index baf1856219..e3d8ded506 100644 --- a/petri/src/vm/openvmm/construct.rs +++ b/petri/src/vm/openvmm/construct.rs @@ -685,7 +685,7 @@ impl PetriVmConfigSetupCore<'_> { let mut cmdline = command_line.clone(); - append_cmdline(&mut cmdline, format!("panic=-1 reboot=triple")); + append_cmdline(&mut cmdline, "panic=-1 reboot=triple"); append_log_params_to_cmdline(&mut cmdline); let isolated = match self.firmware { From 5a9cec430a709834557d5fef6c3cde006effa770 Mon Sep 17 00:00:00 2001 From: Matt LaFayette Date: Tue, 14 Oct 2025 10:14:50 -0700 Subject: [PATCH 4/4] post merge fixups --- petri/src/vm/hyperv/mod.rs | 1 - petri/src/vm/mod.rs | 18 ------------------ petri/src/vm/openvmm/construct.rs | 1 - 3 files changed, 20 deletions(-) diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index 955d5f8e8d..1ab2233b3e 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/petri/src/vm/hyperv/mod.rs @@ -30,7 +30,6 @@ use crate::hyperv::powershell::HyperVSecureBootTemplate; use crate::kmsg_log_task; use crate::openhcl_diag::OpenHclDiagHandler; use crate::vm::append_cmdline; -use crate::vm::append_log_params_to_cmdline; use anyhow::Context; use async_trait::async_trait; use get_resources::ged::FirmwareEvent; diff --git a/petri/src/vm/mod.rs b/petri/src/vm/mod.rs index 595aa1ef43..158565fb96 100644 --- a/petri/src/vm/mod.rs +++ b/petri/src/vm/mod.rs @@ -1874,24 +1874,6 @@ fn append_cmdline(cmd: &mut Option, add_cmd: impl AsRef) { } } -fn append_log_params_to_cmdline(cmd: &mut Option) { - // Forward OPENVMM_LOG and OPENVMM_SHOW_SPANS to OpenHCL if they're set. - let openhcl_tracing = - if let Ok(x) = std::env::var("OPENVMM_LOG").or_else(|_| std::env::var("HVLITE_LOG")) { - format!("OPENVMM_LOG={x}") - } else { - "OPENVMM_LOG=debug".to_owned() - }; - let openhcl_show_spans = if let Ok(x) = std::env::var("OPENVMM_SHOW_SPANS") { - format!("OPENVMM_SHOW_SPANS={x}") - } else { - "OPENVMM_SHOW_SPANS=true".to_owned() - }; - - append_cmdline(cmd, openhcl_tracing); - append_cmdline(cmd, openhcl_show_spans); -} - async fn save_inspect( name: &str, inspect: std::pin::Pin> + Send>>, diff --git a/petri/src/vm/openvmm/construct.rs b/petri/src/vm/openvmm/construct.rs index 42db8d0d5f..05b00d1d9c 100644 --- a/petri/src/vm/openvmm/construct.rs +++ b/petri/src/vm/openvmm/construct.rs @@ -30,7 +30,6 @@ use crate::linux_direct_serial_agent::LinuxDirectSerialAgent; use crate::openvmm::BOOT_NVME_INSTANCE; use crate::openvmm::memdiff_vmgs; use crate::vm::append_cmdline; -use crate::vm::append_log_params_to_cmdline; use crate::vtl2_settings::ControllerType; use crate::vtl2_settings::Vtl2LunBuilder; use crate::vtl2_settings::Vtl2StorageBackingDeviceBuilder;