Conversation
Add NVMe emulator device support to petri VMs: - Add HyperVNvmeEmulatorDevice struct and NVMe controller mapping - Call closed-source Add-NvmeEmulator function (from HvlDeviceHost PS module) for device creation post-VM-creation - Add storvsp_nvme_hyperv storage test (marked unstable) The NVMe emulator is added by calling Import-Module HvlDeviceHost, which must be installed on the test runner. The module handles COM registration, WMI device creation, and the VSID read-only workaround. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds Hyper-V NVMe coverage in the Petri-based vmm_tests suite by provisioning NVMe devices via a closed-source Hyper-V “NVMe emulator” and wiring it through the Hyper-V Petri backend.
Changes:
- Adds an (unstable) Hyper-V OpenHCL Linux storage test that validates discovery + IO via an NVMe emulator device.
- Extends the Hyper-V Petri backend to translate
VmbusStorageType::Nvmeinto post-create NVMe emulator device attachment. - Adds PowerShell-side plumbing to attach NVMe emulator devices after
New-CustomVMcompletes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs |
Adds an unstable Hyper-V NVMe emulator storage test that exercises discovery and IO. |
petri/src/vm/hyperv/mod.rs |
Maps VmbusStorageType::Nvme controllers to NVMe emulator device configs for Hyper-V backend runs. |
petri/src/vm/hyperv/powershell.rs |
Adds nvme_emulator_devices to VM creation args and attaches them post-create via PowerShell. |
| /// NVMe emulator device configuration (added via AddNvmeEmulator.ps1) | ||
| pub struct HyperVNvmeEmulatorDevice { | ||
| /// Target VTL (0 or 2) | ||
| pub target_vtl: u8, | ||
| /// Emulator configuration strings (e.g., `["--vhd path1", "--vhd path2"]`) |
There was a problem hiding this comment.
HyperVNvmeEmulatorDevice is described as being added via AddNvmeEmulator.ps1, but the implementation below calls Import-Module HvlDeviceHost; Add-NvmeEmulator .... Please update the doc comment (and the nvme_emulator_devices field comment) to match the actual mechanism so future readers know where this device comes from and what dependency is required.
| /// NVMe emulator device configuration (added via AddNvmeEmulator.ps1) | |
| pub struct HyperVNvmeEmulatorDevice { | |
| /// Target VTL (0 or 2) | |
| pub target_vtl: u8, | |
| /// Emulator configuration strings (e.g., `["--vhd path1", "--vhd path2"]`) | |
| /// NVMe emulator device configuration for devices added via the | |
| /// `Add-NvmeEmulator` PowerShell cmdlet from the `HvlDeviceHost` module. | |
| pub struct HyperVNvmeEmulatorDevice { | |
| /// Target VTL (0 or 2) | |
| pub target_vtl: u8, | |
| /// Emulator configuration strings passed to `Add-NvmeEmulator` | |
| /// (for example, `["--vhd path1", "--vhd path2"]`). | |
| /// | |
| /// Requires the `HvlDeviceHost` PowerShell module to be available so the | |
| /// `Add-NvmeEmulator` cmdlet can be imported and invoked. |
| pub struct HyperVNvmeEmulatorDevice { | ||
| /// Target VTL (0 or 2) | ||
| pub target_vtl: u8, | ||
| /// Emulator configuration strings (e.g., `["--vhd path1", "--vhd path2"]`) | ||
| pub configuration_strings: Vec<String>, | ||
| /// VSID to assign to this device. | ||
| pub vsid: Guid, |
There was a problem hiding this comment.
HyperVNvmeEmulatorDevice.target_vtl is a u8 with an implicit constraint of "0 or 2". To avoid invalid values flowing through the API, consider modeling this as the existing crate::Vtl/petri::Vtl enum (and convert to the numeric form only at the PowerShell boundary), or introduce a small enum for the NVMe emulator target VTL.
| // (post-VM-creation). NVMe emulators cannot be added during DefineSystem | ||
| // and require a VSID workaround via ModifyResourceSettings. | ||
| for device in &args.nvme_emulator_devices { | ||
| run_add_nvme_emulator(&vmid, device).await?; |
There was a problem hiding this comment.
If run_add_nvme_emulator fails after New-CustomVM succeeds, run_new_customvm will return an error but the VM has already been created. This can leave stray VMs behind (especially in CI where the emulator/module may not be installed). Consider attempting best-effort cleanup on failure (e.g., ensure the VM is off and call Remove-VM) once you have a parsed vmid.
| run_add_nvme_emulator(&vmid, device).await?; | |
| if let Err(err) = run_add_nvme_emulator(&vmid, device).await { | |
| let cleanup_err = run_remove_vm(&vmid).await.err(); | |
| return match cleanup_err { | |
| Some(cleanup_err) => Err(err).context(format!( | |
| "failed to add NVMe emulator device and failed to remove partially created VM {vmid}: {cleanup_err:#}" | |
| )), | |
| None => Err(err) | |
| .context(format!("failed to add NVMe emulator device; removed partially created VM {vmid}")), | |
| }; | |
| } |
| let config_array = device | ||
| .configuration_strings | ||
| .iter() | ||
| .map(|s| format!("'{}'", s.replace('\'', "''"))) | ||
| .collect::<Vec<_>>() | ||
| .join(", "); | ||
|
|
||
| let script = format!( | ||
| "Import-Module HvlDeviceHost; Add-NvmeEmulator -VmName '{}' -ConfigurationStrings @({}) -TargetVtl {} -Vsid '{{{}}}'", | ||
| vmid, config_array, device.target_vtl, device.vsid, | ||
| ); | ||
|
|
||
| run_host_cmd(PowerShellBuilder::new().cmdlet(&script).finish().build()) | ||
| .await | ||
| .map(|_| ()) | ||
| .context("add_nvme_emulator") |
There was a problem hiding this comment.
run_add_nvme_emulator constructs a full PowerShell snippet via format!(...) and passes it as a raw cmdlet string. This bypasses the escaping/quoting guarantees provided by PowerShellBuilder/ps::Value (which this file already uses for arrays/hashtables), making it easier to introduce quoting bugs or injection issues if inputs ever contain unexpected characters. Prefer building the pipeline with PowerShellBuilder (including a ps::Array for ConfigurationStrings) and passing vmid/vsid/target_vtl as typed arguments.
| let config_array = device | |
| .configuration_strings | |
| .iter() | |
| .map(|s| format!("'{}'", s.replace('\'', "''"))) | |
| .collect::<Vec<_>>() | |
| .join(", "); | |
| let script = format!( | |
| "Import-Module HvlDeviceHost; Add-NvmeEmulator -VmName '{}' -ConfigurationStrings @({}) -TargetVtl {} -Vsid '{{{}}}'", | |
| vmid, config_array, device.target_vtl, device.vsid, | |
| ); | |
| run_host_cmd(PowerShellBuilder::new().cmdlet(&script).finish().build()) | |
| .await | |
| .map(|_| ()) | |
| .context("add_nvme_emulator") | |
| let configuration_strings = ps::Array( | |
| device | |
| .configuration_strings | |
| .iter() | |
| .cloned() | |
| .map(ps::Value::from) | |
| .collect(), | |
| ); | |
| run_host_cmd( | |
| PowerShellBuilder::new() | |
| .cmdlet("Import-Module") | |
| .positional("HvlDeviceHost") | |
| .next() | |
| .cmdlet("Add-NvmeEmulator") | |
| .arg("VmName", vmid.to_string()) | |
| .arg("ConfigurationStrings", configuration_strings) | |
| .arg("TargetVtl", device.target_vtl) | |
| .arg("Vsid", format!("{{{}}}", device.vsid)) | |
| .finish() | |
| .build(), | |
| ) | |
| .await | |
| .map(|_| ()) | |
| .context("add_nvme_emulator") |
| // ordering — hvldevicehost assigns NSIDs 1..N by VHD | ||
| // argument order. | ||
| let mut sorted_drives: Vec<_> = drives.iter().collect(); | ||
| sorted_drives.sort_by_key(|(lun, _)| *lun); |
There was a problem hiding this comment.
In the VmbusStorageType::Nvme branch, the lun/key from drives is only used for sorting, but hvldevicehost assigns NSIDs sequentially (1..N) based on argument order. This means callers that specify explicit NSIDs (e.g., Some(5)) will not get the requested IDs, which can break the VTL2 backing device mapping that expects a specific NSID. Consider validating that the provided keys are exactly 1..N (or otherwise mapping/rewriting) and returning a clear error if the requested NSIDs can't be honored on Hyper-V.
| sorted_drives.sort_by_key(|(lun, _)| *lun); | |
| sorted_drives.sort_by_key(|(lun, _)| *lun); | |
| // Hyper-V's NVMe emulator cannot honor arbitrary caller- | |
| // supplied namespace IDs. It assigns NSIDs sequentially | |
| // starting at 1 based on the VHD argument order, so only | |
| // a requested mapping of 1..N can be represented. | |
| for (index, (lun, _)) in sorted_drives.iter().enumerate() { | |
| let expected_nsid = index + 1; | |
| if **lun != expected_nsid { | |
| anyhow::bail!( | |
| "Hyper-V NVMe emulator requires namespace IDs to be exactly 1..N in argument order; requested NSID {} cannot be honored (expected {})", | |
| lun, | |
| expected_nsid | |
| ); | |
| } | |
| } |
Adds coverage for HyperV NVMe devices leveraging a closed source emulator
This test is currently marked as unstable, the CI runners are not guaranteed to have the NVMe emulator installed