rfc: petri: hyperv backed with emulated nvme #2137
Conversation
mattkur
force-pushed
the
hyperv-nvme-storage
branch
from
bcd45c7 to
71b9c88
Compare
mattkur
changed the title
There was a problem hiding this comment.
Pull Request Overview
This RFC introduces support for NVMe storage in Hyper-V VMs using Microsoft-internal utilities, enabling VTL2 storage emulation that can be accessed by VTL0 guests. This bridges a testing gap between OpenVMM and Hyper-V backends in the Petri testing framework.
- Adds host capability detection for Microsoft-internal NVMe storage support
- Implements runtime NVMe device addition and VTL2 settings configuration
- Refactors command line parameter handling to be shared between OpenVMM and Hyper-V backends
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs | Adds new test case for OpenHCL NVMe storage on Hyper-V |
| vmm_tests/vmm_tests/Cargo.toml | Adds dependencies for VHD handling and VTL2 settings |
| vmm_tests/vmm_test_macros/src/lib.rs | Adds hyperv_test macro and NVMe storage requirements detection |
| petri/src/vm/openvmm/construct.rs | Refactors command line parameter handling to use shared function |
| petri/src/vm/mod.rs | Adds shared function for appending log parameters and increases timeout |
| petri/src/vm/hyperv/vm.rs | Adds methods for NVMe storage and VTL2 settings management |
| petri/src/vm/hyperv/powershell.rs | Implements PowerShell commands for NVMe storage and VTL2 settings |
| petri/src/vm/hyperv/mod.rs | Updates backend to support NVMe configuration and shared command line handling |
| petri/src/vm/hyperv/hyperv.psm1 | Adds PowerShell function for setting VTL2 settings |
| petri/src/requirements.rs | Adds host capability detection for NVMe storage support |
|
|
||
| { | ||
| const TIMEOUT_DURATION_MINUTES: u64 = 7; | ||
| const TIMEOUT_DURATION_MINUTES: u64 = 10; |
There was a problem hiding this comment.
todo Don't merge a difference here...
| let is_nvme = matches!( | ||
| config.firmware, | ||
| Firmware::OpenhclUefi(OpenhclUefiOptions { nvme: true, .. }, _) | ||
| ) || name.contains("add_nvme_device"); |
There was a problem hiding this comment.
Ideally we wouldn't be depending on the name, we want to avoid that as much as possible. Is there any other bit we can look for?
If not, this is something that can probably be addressed with future planned refactorings.
There was a problem hiding this comment.
Maybe we could add macro syntax for additional requirements, the same as we have one for additional artifacts.
There was a problem hiding this comment.
Right. For now: I think we can just look for nvme as part of the name to simplify this. The current match is set up to catch any of the existing magic (where you add [nvme] into the dependency requirements), or based on certain test names.
I'm certainly not allergic to adding more test macros here, but I'd like to keep the first PR tactical, and to leverage stuff that comes from Microsoft internal tooling. Other future improvements will be for petri to set up NVMe redirection for Hyper-V just like it does for openvmm, for example.
We'd also expand this to the MANA emulator, for example.
There was a problem hiding this comment.
I'm not saying add a whole new macro, just add additional syntax to the current one, similar to the [] stuff we do for additional artifact dependencies, but instead for additional host requirements. We could use that for servicing too potentially. I don't think it'd be a huge lift, but up to you if we want to punt.
| { | ||
| // Not checked for non-Windows platforms, so just assume that the feature | ||
| // is supported. | ||
| true |
There was a problem hiding this comment.
shouldn't we default to false?
| .cmdlet("Test-Path") | ||
| .arg( | ||
| "Path", | ||
| "c:\\OpenVMMCI\\MicrosoftInternalTestHelpers\\MicrosoftInternalTestHelpers.psm1", |
There was a problem hiding this comment.
Where do these come from?
There was a problem hiding this comment.
This is the crux of the approach: those need to be latent on the CI machines. These come from our internal repositories (although I am still churning on our CI infra).
There was a problem hiding this comment.
hmmm, not a huge fan of anything that makes our CI setup less easily reproducible...
There was a problem hiding this comment.
Understood. I have heartache about this as well. This conversation I should take offline with you.
|
Split out much of the non-nvme specific stuff to #2171 |
In doing #2137, I realized that I had done almost all the work required to do scsi-to-scsi relay for the Hyper-V backend. petri: - Add infra to set VTL2 settings for a Hyper-V test backend. - Return the SCSI controller VSID when one is added (required to reference that controller in VTL2 settings...). - Add a `hyperv_test` macro, so that the test can get access to the Hyper-V Petri backend (like `openvmm_test`). vmm_tests: - Write a test that uses this infra.
2 participants
One of the gaps between the HyperV and OpenVMM Petri backends is the ability to show NVMe storage to VTL2 (or, really, any storage to VTL2) and translate that storage to the VTL0 guest. This work in progress change aims to remove that. Unfortunately, Hyper-V does not support any NVMe emulators, but Microsoft has some ability to do this using Microsoft-proprietary methods. Coverage for this scenario in CI is so important that I think it worth it to leverage those methods, even though doing so is only viable for CI (where Microsoft controls the test environment) and for engineers with access to those Microsoft-internal mechanisms.
TODO: