Skip to content

Feature slot speeds direct attach support - #60

Open
joshuaboud wants to merge 3 commits into
masterfrom
feature-slot_speeds_direct_attach_support
Open

Feature slot speeds direct attach support#60
joshuaboud wants to merge 3 commits into
masterfrom
feature-slot_speeds_direct_attach_support

Conversation

@joshuaboud

Copy link
Copy Markdown
Member

Add E16 and VM2 support for the slot_speeds command

@joshuaboud
joshuaboud requested review from JordanKeo45 and hansrachit123 and a lite review from Copilot August 20, 2026 20:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds direct-attached (non-UBM) server support to slot_speeds (specifically STORNADO-E16 and PROXINATOR-VM2) and expands the reported slot metadata to include serial and size, while keeping the existing UBM/storcli2 path intact.

Changes:

  • Refactors the storcli2/UBM JSON collection into get_slot_speeds_json_ubm and adds get_slot_speeds_json_direct_attached.
  • Adds STORNADO-E16 and PROXINATOR-VM2 direct-attached slot-name lists and uses them when UBM support is unavailable.
  • Extends output with serial and size fields/columns.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
ubm/slot_name_map.txt Clarifies intended scope of the UBM slot-name map and removes the STORNADO-E16 mapping from the UBM map file.
tools/slot_speeds Adds direct-attached support and extends slot metadata/columns (serial, size) for both UBM and direct-attached modes.
Suppressed comments (2)

tools/slot_speeds:172

  • Same as above: $DEV_PATH should be DEVPATH, otherwise the ATA link index extraction is empty and the sysfs lookup fails.
                sed 's; Gbps;Gb/s;' "/sys/class/ata_link/link$(echo "$DEV_PATH" | grep -oP 'ata\K[0-9]+')/sata_spd_limit"

tools/slot_speeds:150

  • width_max uses printf with %d, but the command substitution can emit non-numeric placeholders (e.g. ?), which will be coerced to 0 and misreport max link width.
            printf '"width_max": "x%d",\n' "$(

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tools/slot_speeds
Comment on lines +118 to +119
local last_slot
last_slot=$(echo "$SLOT_NAMES" | awk '{print $NF}')
Comment thread tools/slot_speeds
Comment on lines +121 to +124
for slot_name in $SLOT_NAMES; do
if ! test -b "/dev/$slot_name"; then
continue
fi
Comment thread tools/slot_speeds
if [[ "$DEVPATH" == *"/nvme/"* ]]; then
sed 's; GT/s PCIe;GT/s;' "/sys/$DEVPATH/../../../current_link_speed"
elif [[ "$DEVPATH" == *"/ata"* || "$ID_VENDOR" == "ATA" ]]; then
sed 's; Gbps;Gb/s;' "/sys/class/ata_link/link$(echo "$DEV_PATH" | grep -oP 'ata\K[0-9]+')/sata_spd"
Comment thread tools/slot_speeds
else
MAP_KEY=$(get_map_key) || exit $?
SLOT_NAMES=$(
grep "^$MAP_KEY\s" <<EOF | cut -d' ' -f2-
Comment thread tools/slot_speeds
echo ?
fi
)"
printf '"width": "x%d",\n' "$(
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants