Repository navigation
Conversation
M2Slot::from_chassis_location maps "M.2 East" to A and "M.2 West" to B, replacing the TryFrom<i64> impl that keyed on the PCIe slot numbers of the bridges above the M.2 sockets. Those numbers are internal to the board and may change across host OS versions; the chassis labels are what the hardware topology promises. On Gimlet and Cosmo the two mappings agree, so real sleds see no change. Synthetic M.2s named m2_0 and m2_1 are labelled East and West, so they now get slots A and B where before their offset PCIe slots matched neither.
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
The boot-storage-unit property says whether the host booted from A or B. Match that against each M.2 socket's label, as M2Slot already does for the slot sled-agent reports, rather than against a per-board table of PCIe slot numbers. Removes OxideSled::bootdisk_pcie_slots.
smklein
changed the title
|
@claude review |
There was a problem hiding this comment.
I reviewed the latest push and found no new bugs. Boot-disk detection now uses the chassis label, the same source as the reported slot. This changes how a sled finds its boot disk, and the PR notes it hasn't run on real hardware yet, so a human look would still be worthwhile.
What was reviewed:
classify_baynow setsis_boot_diskonly for an M.2 bay whose label maps throughfrom_chassis_locationto the boot storage unit. U.2 bays and unknown labels never qualify, and the new test covers each of these cases.boot_storage_unit_from_propertykeeps the 0x41/0x42 mapping and its error path. Droppingsled_typefromread_bays/poll_baystill leaves the not-an-Oxide-sled check in place.- installinator's
WriteDestinationandInternalDiskDetails.slotboth use the label mapping, so they agree with boot-disk selection.
Extended reasoning...
The change touches M.2 slot and boot-disk identification in sled-hardware, sled-agent's config-reconciler, sled-agent types and installinator. It moves both from PCIe slot numbers to the chassis labels "M.2 East" and "M.2 West". It touches no auth, crypto or data-exposure surface. Since the earlier review, new commits route boot-disk selection through the same label mapping as the reported slot. The change still decides which physical drive a sled treats as its boot disk and where installinator writes, and the PR says it has not been run on real hardware.
Stacked on #11590.
M2Slot::from_chassis_locationmaps "M.2 East" to A and "M.2 West" to B. Sled-agent and installinator now use it everywhere they turn an M.2 into A or B:TryFrom<i64>impl keyed on PCIe slot numbers 0x11/0x12)boot-storage-unitproperty says whether the host booted from A or B, and sled-hardware now matches that against each socket's label instead ofOxideSled::bootdisk_pcie_slots, which is removedThose PCIe numbers are internal to the board and may change across host OS versions. The chassis labels are what the topology promises. On Gimlet and Cosmo the two mappings agree. Using one mapping for both also means the reported boot slot and the boot disk can't disagree. With this change, disk discovery in sled-hardware no longer depends on which board it's running on.
The East→A / West→B pairing is still board knowledge that lives in omicron, though it's now written in terms of labels rather than PCIe numbers.
Synthetic M.2s (
m2_0/m2_1) are labelled East and West, so they now get slots A and B. Before, their offset PCIe slots matched neither. On a4x2 this changes which host phase 2 error is reported; that check still fails.Testing: unit tests for the label mapping and for boot-disk selection (East/West × A/B, and U.2 bays); sled-hardware and installinator build and pass tests on illumos. Tested on london via mupdate (installinator works, sled agent is able to identify boot disk via label).