fix(bmc-explorer): skip boot options for B4240V BlueField-4#3053
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. |
Summary by CodeRabbit
WalkthroughAdds a B4240V BlueField4 variant recognized by explorer detection logic and mock hardware, introducing a ChangesBlueField4 B4240V and DGX VR NVL
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Test as Test/Caller
participant HMI as HostMachineInfo
participant DgxVrNvl as DgxVrNvl
participant BF4 as Bluefield4 (Mode::B4240V)
Test->>HMI: system_config(callbacks)
HMI->>HMI: match hardware_type == NvidiaDgxVr
HMI->>DgxVrNvl: dgx_vr_nvl()
DgxVrNvl->>BF4: host_nic()
BF4-->>DgxVrNvl: NIC identity/MAC (B4240V attrs)
DgxVrNvl->>DgxVrNvl: build boot options (disk + PXE per NIC)
DgxVrNvl-->>HMI: computer_system::Config
HMI-->>Test: system_config result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Treat B4240V chassis as BlueField-4 so exploration avoids fetching BootOptions when Redfish returns Members: null. Add DGX VR NVL mock support with B4240V metadata to cover the new variant. Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>
6410a41 to
f183a80
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/bmc-mock/src/hw/dgx_vr_nvl.rs (1)
63-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify boot-option construction for the single fixed NIC.
DgxVrNvlalways carries exactly onedpu(per themachine_info.rs::dgx_vr_nvl()constructor andfixed_number_of_dpu() == Some(1)), yet the network boot option is built via[&self.dpu.host_nic()].into_iter().enumerate().map(...). This array/enumerate/map indirection adds complexity without benefit, since there is exactly one item to process. Consider building the single boot option directly and chaining it withstd::iter::once, mirroring the disk boot option above it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/bmc-mock/src/hw/dgx_vr_nvl.rs` around lines 63 - 96, The boot option assembly in DgxVrNvl is overcomplicated for a single fixed NIC, since the constructor and fixed_number_of_dpu() guarantee only one host NIC exists. Simplify the network boot-option creation by removing the array/enumerate/map chain and building that single BootOption directly, then chain it with the existing std::iter::once disk option so the intent matches the one-device model.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/bmc-mock/src/hw/dgx_vr_nvl.rs`:
- Around line 63-96: The boot option assembly in DgxVrNvl is overcomplicated for
a single fixed NIC, since the constructor and fixed_number_of_dpu() guarantee
only one host NIC exists. Simplify the network boot-option creation by removing
the array/enumerate/map chain and building that single BootOption directly, then
chain it with the existing std::iter::once disk option so the intent matches the
one-device model.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d368f971-b9f7-4e94-975b-61ee56957a5f
📒 Files selected for processing (8)
crates/bmc-explorer/src/chassis.rscrates/bmc-explorer/tests/bluefield4_explore.rscrates/bmc-mock/src/hw/bluefield4.rscrates/bmc-mock/src/hw/dgx_vr_nvl.rscrates/bmc-mock/src/hw/mod.rscrates/bmc-mock/src/lib.rscrates/bmc-mock/src/machine_info.rscrates/bmc-mock/src/test_support/mod.rs
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
Treat B4240V chassis as BlueField-4 so exploration avoids fetching BootOptions when Redfish returns Members: null. Add DGX VR NVL mock support with B4240V metadata to cover the new variant.
Related issues
Type of Change
Breaking Changes
Testing
Additional Notes