vmm-tests: correctly request vhost dependency, update mapping#3261
vmm-tests: correctly request vhost dependency, update mapping#3261chris-oo wants to merge 2 commits intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the vmm vhost-user blk device test to request the OpenVMM vhost binary as an explicit artifact dependency and adjusts Flowey’s artifact→build mapping so the correct build is triggered.
Changes:
- Add an OpenVMM vhost artifact dependency to the Linux-only
vhost_user_blk_devicetest and use the resolved artifact path at runtime. - Update Flowey artifact selection mapping to request building
openvmm_vhostwhen the corresponding Linux vhost artifacts are selected.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| vmm_tests/vmm_tests/tests/tests/multiarch.rs | Switch vhost binary discovery from “known path” lookup to an explicit resolved artifact dependency passed via the test macro. |
| flowey/flowey_lib_hvlite/src/artifact_to_build_mapping.rs | Map Linux vhost artifact selections to build.openvmm_vhost = true so the dependency is built. |
8c3fdcb to
5b168f5
Compare
| async fn vhost_user_blk_device<T>( | ||
| config: PetriVmBuilder<OpenVmmPetriBackend>, | ||
| _extra_deps: (), | ||
| extra_deps: (petri::ResolvedArtifact<T>,), |
There was a problem hiding this comment.
The new generic parameter name T is ambiguous here because it represents the resolved artifact type (not a backend, unlike other T usages in this module). Renaming it to something more specific (e.g., VhostArtifact) would make the signature easier to understand and reduce confusion with other generics used in this file.
This seemed to have been merged after another refactor, but this correctly expresses the dependency used by the vhost test.