feat: Adding VR hardware type to BMC explorer#2910
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
martinraumann
left a comment
martinraumann
left a comment
There was a problem hiding this comment.
Can we add a VR scrape-backed mock/test, or link the source for the VR Redfish assumptions this PR relies on?
The new behavior depends on VR-specific details that are not otherwise verifiable in this diff: NVIDIA / VR NVL72 detection, HGX_GPU_ chassis IDs for PCIe discovery, /MAC(...)/IPv4(...)/Uri() boot-option paths, the expected BIOS attrs, and the EmbeddedUefiShell = Disabled infinite-boot polarity.
Without that fixture/source, a wrong chassis prefix or boot path shape could make VR setup validation silently incomplete.
| let expected_bios_attrs: &[hw::BiosAttr] = match hw_type { | ||
| hw::HwType::Gb200 => &hw::gb200::EXPECTED_BIOS_ATTRS, | ||
| hw::HwType::VeraRubin => &hw::vera_rubin::EXPECTED_BIOS_ATTRS, | ||
| _ => unreachable!(), |
There was a problem hiding this comment.
Can we avoid the inner match hw_type { ..., _ => unreachable!() } here?
This outer arm only matches Gb200 | VeraRubin, but Rust does not carry that narrowing into the inner match. If another variant is later added to the outer OR-pattern and not added here, this compiles and panics at runtime. A small helper like HwType::expected_bios_attrs() with an exhaustive match, or separate Gb200 / VeraRubin arms, would keep this checked by the compiler.
| chassis_id.starts_with("HGX_GPU_") | ||
| } | ||
|
|
||
| Some(hw::HwType::VeraRubin) => { |
There was a problem hiding this comment.
Do we have a Redfish scrape or fixture confirming that VR GPU chassis IDs use the HGX_GPU_ prefix? This filter controls which chassis contribute PCIe devices, so if VR uses a different ID scheme, PCIe discovery would come back empty/incomplete without an obvious error.
nit: once that assumption is confirmed, this VeraRubin arm can be folded into the identical GB300 multi-variant arm above.
3 participants
Adds Vera Rubin (VR) hardware type to bmc explorer. We will need further changes to libredfish and nvredfish for end to end VR compute tray ingestion.
Related issues
(https://jirasw.nvidia.com/browse/FORGE-8111)
Type of Change