Skip to content

Commit 78d45a3

Browse files
damanm24Daman Mulye
andauthored
vmm_tests: Fix build requirements function when parsing vmm_test macros (#2218)
The build_requirements function did not take into account all the possible combinations for a VmmBackend to be configured. This allowed for some discrepancy when generating the test case host capabilities requirements depending on how the vmm backend was being configured. This PR refactors the make_vmm_test so that resolving the vmm backend happens earlier in the function, and the resolved value is the one that is used throughout. --------- Co-authored-by: Daman Mulye <[email protected]>
1 parent 9da728a commit 78d45a3

File tree

1 file changed

+26
-22
lines changed
  • vmm_tests/vmm_test_macros/src

1 file changed

+26
-22
lines changed

vmm_tests/vmm_test_macros/src/lib.rs

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,12 @@ fn arch_to_tokens(arch: MachineArch) -> TokenStream {
9191
}
9292

9393
impl Config {
94-
fn name_prefix(&self, specific_vmm: Option<Vmm>) -> String {
94+
fn name_prefix(&self, resolved_vmm: Vmm) -> String {
9595
let arch_prefix = arch_to_str(self.arch);
9696

97-
let vmm_prefix = match (specific_vmm, self.vmm) {
98-
(_, Some(Vmm::OpenVmm)) | (Some(Vmm::OpenVmm), None) => "openvmm",
99-
(_, Some(Vmm::HyperV)) | (Some(Vmm::HyperV), None) => "hyperv",
100-
_ => "",
97+
let vmm_prefix = match resolved_vmm {
98+
Vmm::OpenVmm => "openvmm",
99+
Vmm::HyperV => "hyperv",
101100
};
102101

103102
let firmware_prefix = match &self.firmware {
@@ -675,10 +674,22 @@ fn make_vmm_test(
675674
let mut tests = TokenStream::new();
676675
// FUTURE: compute all this in code instead of in the macro.
677676
for config in args.configs {
678-
let name = format!("{}_{original_name}", config.name_prefix(specific_vmm));
677+
// Resolve the VMM backend early by combining specific_vmm and config.vmm
678+
let resolved_vmm = match (specific_vmm, config.vmm) {
679+
(Some(Vmm::HyperV), Some(Vmm::HyperV))
680+
| (Some(Vmm::HyperV), None)
681+
| (None, Some(Vmm::HyperV)) => Vmm::HyperV,
682+
(Some(Vmm::OpenVmm), Some(Vmm::OpenVmm))
683+
| (Some(Vmm::OpenVmm), None)
684+
| (None, Some(Vmm::OpenVmm)) => Vmm::OpenVmm,
685+
(None, None) => return Err(Error::new(config.span, "vmm must be specified")),
686+
_ => return Err(Error::new(config.span, "vmm mismatch")),
687+
};
688+
689+
let name = format!("{}_{original_name}", config.name_prefix(resolved_vmm));
679690

680-
// Build requirements based on the configuration
681-
let requirements = build_requirements(&config, &name);
691+
// Build requirements based on the configuration and resolved VMM
692+
let requirements = build_requirements(&config.firmware, &name, resolved_vmm);
682693
let requirements = if let Some(req) = requirements {
683694
quote! { Some(#req) }
684695
} else {
@@ -694,24 +705,17 @@ fn make_vmm_test(
694705
};
695706
let arch = arch_to_tokens(config.arch);
696707

697-
let (cfg_conditions, artifacts, petri_vm_config) = match (specific_vmm, config.vmm) {
698-
(Some(Vmm::HyperV), Some(Vmm::HyperV))
699-
| (Some(Vmm::HyperV), None)
700-
| (None, Some(Vmm::HyperV)) => (
708+
let (cfg_conditions, artifacts, petri_vm_config) = match resolved_vmm {
709+
Vmm::HyperV => (
701710
quote!(#[cfg(windows)]),
702711
quote!(::petri::PetriVmArtifacts::<::petri::hyperv::HyperVPetriBackend>),
703712
quote!(::petri::PetriVmBuilder::<::petri::hyperv::HyperVPetriBackend>),
704713
),
705-
706-
(Some(Vmm::OpenVmm), Some(Vmm::OpenVmm))
707-
| (Some(Vmm::OpenVmm), None)
708-
| (None, Some(Vmm::OpenVmm)) => (
714+
Vmm::OpenVmm => (
709715
quote!(),
710716
quote!(::petri::PetriVmArtifacts::<::petri::openvmm::OpenVmmPetriBackend>),
711717
quote!(::petri::PetriVmBuilder::<::petri::openvmm::OpenVmmPetriBackend>),
712718
),
713-
(None, None) => return Err(Error::new(config.span, "vmm must be specified")),
714-
_ => return Err(Error::new(config.span, "vmm mismatch")),
715719
};
716720

717721
let petri_vm_config = quote!(#petri_vm_config::new(&params, artifacts, &driver)?);
@@ -746,8 +750,8 @@ fn make_vmm_test(
746750
})
747751
}
748752

749-
// Helper to build requirements TokenStream for a config and specific_vmm
750-
fn build_requirements(config: &Config, name: &str) -> Option<TokenStream> {
753+
// Helper to build requirements TokenStream for firmware and resolved VMM
754+
fn build_requirements(firmware: &Firmware, name: &str, resolved_vmm: Vmm) -> Option<TokenStream> {
751755
let mut requirement_expr: Option<TokenStream> = None;
752756
let mut is_vbs = false;
753757
// Add isolation requirement if specified
@@ -757,7 +761,7 @@ fn build_requirements(config: &Config, name: &str) -> Option<TokenStream> {
757761
..
758762
},
759763
_,
760-
) = &config.firmware
764+
) = firmware
761765
{
762766
let isolation_requirement = match isolation {
763767
IsolationType::Vbs => {
@@ -803,7 +807,7 @@ fn build_requirements(config: &Config, name: &str) -> Option<TokenStream> {
803807
};
804808
}
805809

806-
let is_hyperv = config.vmm.is_some() && config.vmm == Some(Vmm::HyperV);
810+
let is_hyperv = resolved_vmm == Vmm::HyperV;
807811

808812
if is_hyperv && is_vbs {
809813
let hyperv_vbs_requirement_expr = quote!(

0 commit comments

Comments
 (0)