Merge pull request #590 from tkirkland/fix/rx1217-runtime-alias

Fix expansion unit database selection for DSM runtime aliases

CHANGES.txt

@@ -1,3 +1,10 @@
+Unreleased
+- Bug fix for expansion units that DSM reports with different model names.
+  - Adds aliases from each expansion disk's runtime container, such as mapping
+    RX1217-1 to RX1217 when detection reports RX1217RP.
+  - Uses exact model-family matching so RX1217 does not also edit RX1217RP or
+    RX1217SAS database files.
+
 v3.6.131
 - Fix for DSM 7.4 including "rules" db files in the drive db files folder.
 
@@ -470,4 +477,3 @@ v1.0.1
 
 v1.0.0
 - Initial release.
-

syno_hdd_db.sh

@@ -1312,6 +1312,30 @@ if [[ $m2 != "no" ]]; then
 fi
 
 
+get_eunit_container_aliases(){
+    # DSM can report one expansion unit with different model names.
+    # For example, detection can return RX1217RP while the attached disks'
+    # runtime container is RX1217-1 and DSM reads the rx1217 drive database.
+    local ebox_info="$1"
+    local runtime_root="${2:-/run/synostorage/disks}"
+    local disk
+    local container
+
+    while IFS= read -r disk; do
+        [[ -n $disk ]] || continue
+
+        container=$(cat "$runtime_root/${disk##*/}/container" 2>/dev/null)
+        container=$(printf "%s" "$container" | sed -E 's/-[0-9]+$//')
+
+        if printf "%s\n" "$container" |
+            grep -Eqi '^([FRD]XD?[0-9]{3,4})(rp|ii|sas)?$';
+        then
+            printf "%s\n" "$container"
+        fi
+    done < <(printf "%s\n" "$ebox_info" | awk '/Disk path:/ {print $NF}')
+}
+
+
 # Expansion units
 ebox_conected=$(synodisk --enum -t ebox)
 if [[ $ebox_conected ]]; then
@@ -1334,6 +1358,12 @@ if [[ $ebox_conected ]]; then
         file=$(ls $path | tail -n1)
         eunitlist=($(grep -Eowi "([FRD]XD?[0-9]{3,4})(rp|ii|sas){0,2}" "$path/$file" | uniq))
     fi
+
+    # Include the model name DSM uses for each expansion disk's runtime
+    # container. The existing sort below removes duplicates.
+    while IFS= read -r eunit_alias; do
+        [[ -n $eunit_alias ]] && eunitlist+=("$eunit_alias")
+    done < <(get_eunit_container_aliases "$ebox_conected")
 fi
 
 # Sort eunitlist array into new eunits array to remove duplicates
@@ -1365,11 +1395,27 @@ fi
 readarray -t db1list < <(find "$dbpath" -maxdepth 1 -name "*_host*.db" ! -name "rule_*" | sort)
 readarray -t db2list < <(find "$dbpath" -maxdepth 1 -name "*_host*.db.new" ! -name "rule_*" | sort)
 
+find_eunit_db_files(){
+    # Match an exact expansion-unit model family while allowing Synology's
+    # "_v7" and space-delimited filename suffixes. Do not let RX1217 also
+    # select RX1217RP or RX1217SAS.
+    local db_dir="$1"
+    local extension="$2"
+    local eunit
+    shift 2
+
+    for eunit in "$@"; do
+        find "$db_dir" -maxdepth 1 -type f \
+            \( -name "${eunit,,}${extension}" \
+            -o -name "${eunit,,}_*${extension}" \
+            -o -name "${eunit,,} *${extension}" \)
+    done | sort -u
+}
+
+
 # Expansion Unit db files
-for i in "${eunits[@]}"; do
-    readarray -t -O "${#eunitdb1list[@]}" eunitdb1list < <(find "$dbpath" -maxdepth 1 -name "${i,,}*.db" | sort)
-    readarray -t -O "${#eunitdb2list[@]}" eunitdb2list < <(find "$dbpath" -maxdepth 1 -name "${i,,}*.db.new" | sort)
-done
+readarray -t eunitdb1list < <(find_eunit_db_files "$dbpath" ".db" "${eunits[@]}")
+readarray -t eunitdb2list < <(find_eunit_db_files "$dbpath" ".db.new" "${eunits[@]}")
 
 # M.2 Card db files
 for i in "${!m2cards[@]}"; do
@@ -2734,4 +2780,3 @@ elif [[ $dsm -eq "6" || $rebootmsg == "yes" ]]; then
 fi
 
 exit
-

tests/test_expansion_unit_db_selection.sh

@@ -0,0 +1,92 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+repo_root=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)
+script="$repo_root/syno_hdd_db.sh"
+tmpdir=$(mktemp -d)
+trap 'rm -rf "$tmpdir"' EXIT
+
+extract_function() {
+    local function_name="$1"
+    sed -n "/^${function_name}(){/,/^}/p" "$script"
+}
+
+eval "$(extract_function get_eunit_container_aliases)"
+eval "$(extract_function find_eunit_db_files)"
+
+mkdir -p "$tmpdir/runtime/sata13" "$tmpdir/db"
+printf '%s\n' "RX1217-1" > "$tmpdir/runtime/sata13/container"
+
+ebox_info=$(cat <<'EOF'
+************ Disk Info ***************
+>> Disk id: 1
+>> Slot id: -1
+>> Disk path: /dev/sata13
+>> Disk model: ST4000VN006-3CW104
+EOF
+)
+
+mapfile -t runtime_aliases < <(
+    get_eunit_container_aliases "$ebox_info" "$tmpdir/runtime"
+)
+
+if [[ ${runtime_aliases[*]} != "RX1217" ]]; then
+    echo "Expected runtime alias RX1217, got: ${runtime_aliases[*]-<none>}" >&2
+    exit 1
+fi
+
+eunitlist=("RX1217rp" "${runtime_aliases[@]}")
+mapfile -t eunits < <(printf '%s\n' "${eunitlist[@]}" | sort -u)
+
+touch \
+    "$tmpdir/db/rx1217.db" \
+    "$tmpdir/db/rx1217 module_v7.db" \
+    "$tmpdir/db/rx1217_v7.db" \
+    "$tmpdir/db/rx1217rp.db" \
+    "$tmpdir/db/rx1217rp_v7.db" \
+    "$tmpdir/db/rx1217sas_v7.db" \
+    "$tmpdir/db/rx1217.db.new" \
+    "$tmpdir/db/rx1217rp.db.new" \
+    "$tmpdir/db/rx1217sas_v7.db.new"
+
+mapfile -t selected < <(
+    find_eunit_db_files "$tmpdir/db" ".db" "${eunits[@]}" |
+        while IFS= read -r file; do basename "$file"; done |
+        sort
+)
+
+expected=(
+    "rx1217 module_v7.db"
+    "rx1217.db"
+    "rx1217_v7.db"
+    "rx1217rp.db"
+    "rx1217rp_v7.db"
+)
+
+if [[ ${selected[*]} != "${expected[*]}" ]]; then
+    echo "Unexpected expansion-unit database selection" >&2
+    printf 'Expected: %s\n' "${expected[*]}" >&2
+    printf 'Actual:   %s\n' "${selected[*]-<none>}" >&2
+    exit 1
+fi
+
+mapfile -t selected_new < <(
+    find_eunit_db_files "$tmpdir/db" ".db.new" "${eunits[@]}" |
+        while IFS= read -r file; do basename "$file"; done |
+        sort
+)
+
+expected_new=(
+    "rx1217.db.new"
+    "rx1217rp.db.new"
+)
+
+if [[ ${selected_new[*]} != "${expected_new[*]}" ]]; then
+    echo "Unexpected expansion-unit .db.new selection" >&2
+    printf 'Expected: %s\n' "${expected_new[*]}" >&2
+    printf 'Actual:   %s\n' "${selected_new[*]-<none>}" >&2
+    exit 1
+fi
+
+echo "PASS: runtime aliases select RX1217 and RX1217RP databases without RX1217SAS"