storage_info: Change smartctl -x blacklist rule to cover more version
Some Sandisk SSD hang when we read PHY statistic from the SSD using
"smartctl -x" command. This change updates the smartctl -x blacklist
rule to cover more SSD model/firmware combination that contain this bug.
Also add unit test to test for these rules.
BUG=chromium:328587
TEST=Boot normally in chromebook with defect SSD and unit test pass
Change-Id: Ie17d96071d32db04eecfd70f4d39682d4bb84da1
Reviewed-on: https://chromium-review.googlesource.com/182502
Commit-Queue: Puthikorn Voravootivat <puthik@chromium.org>
Tested-by: Puthikorn Voravootivat <puthik@chromium.org>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
diff --git a/scripts/storage_info b/scripts/storage_info
index 93010dd..d8664f3 100755
--- a/scripts/storage_info
+++ b/scripts/storage_info
@@ -1,6 +1,6 @@
#! /bin/sh
-# Copyright (c) 2013 The Chromium OS Authors. All rights reserved.
+# Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
#
@@ -12,12 +12,15 @@
STORAGE_INFO_FILE="/var/log/storage_info.txt"
SSD_CMD_0="hdparm -I"
-SSD_CMD_1_NORMAL="smartctl -a"
+SSD_CMD_1_NORMAL="smartctl -x"
SSD_CMD_1_ALTERNATE="smartctl -a"
SSD_CMD_MAX=1
-# This match SanDisk SSD U100 with any size with version 10.52.*
-SSD_BLACKLIST="Device_Model:_*SanDisk_SSD_U100.*Firmware_Version:_*10\.52\..*"
+# This match SanDisk SSD U100/i100 with any size with version *.xx.* when x < 54
+# Seen Error: U100 10.52.01 / i100 CS.51.00 / U100 10.01.04
+MODEL_BLACKLIST_0="SanDisk_SSD_[iU]100.*"
+VERSION_BLACKLIST_0="(CS|10)\.([01234].|5[0123])\..*"
+BLACKLIST_MAX=0
MMC_NAME_0="cid"
MMC_NAME_1="csd"
@@ -45,7 +48,7 @@
[ $(cat /sys/block/$1/removable) -eq 1 ]
}
-# We need to trim white space because vendor is padding with white space
+# We need to trim white space because vendor field is padding with white space.
is_vendor_ata() {
[ "$(cat /sys/block/$1/device/vendor | tr -d ' ')" = "ATA" ]
}
@@ -54,15 +57,38 @@
is_blockdev $1 && ! is_removable $1 && is_vendor_ata $1
}
-# replace space and new line with "_"
-get_ssd_model_and_version() {
- smartctl -i /dev/$1 \
- | grep -E "Device Model|Firmware Version" \
- | sed -r "N; s/\n| /_/g"
+# Example of hdparm result
+# Model=SanDisk SSD i100 32GB, FwRev=11.56.00, SerialNo=123456789012
+# This command extracts the model string and replaces space with an underscore.
+get_ssd_model() {
+ echo "$1" | sed -e "s/^.*Model=//g" -e "s/,.*//g" -e "s/ /_/g"
+}
+
+get_ssd_version() {
+ echo "$1" | sed -e "s/^.*FwRev=//g" -e "s/,.*//g" -e "s/ /_/g"
+}
+
+is_blacklist() {
+ echo "$1" | grep -Eq $2
}
is_ssd_blacklist() {
- echo "$(get_ssd_model_and_version $1)" | grep -Eq $SSD_BLACKLIST
+ local model=$1
+ local version=$2
+ local model_blacklist
+ local version_blacklist
+ local i
+
+ for i in $(seq 0 ${BLACKLIST_MAX}); do
+ eval model_blacklist=\${MODEL_BLACKLIST_${i}}
+ if is_blacklist ${model} ${model_blacklist}; then
+ eval version_blacklist=\${VERSION_BLACKLIST_${i}}
+ if is_blacklist ${version} ${version_blacklist}; then
+ return 0
+ fi
+ fi
+ done
+ return 1
}
is_type_mmc() {
@@ -74,54 +100,77 @@
}
print_ssd_info() {
- # BUG: On stout smartctl -x causes SSD error (crbug.com/328587)
+ # BUG: On some machines, smartctl -x causes SSD error (crbug.com/328587)
# We need to check model and firmware version of the SSD to avoid this bug.
- if is_ssd_blacklist $1; then
- SSD_CMD_1=$SSD_CMD_1_ALTERNATE
+
+ # SSD model and firmware version is on the same line in hdparm result
+ local hdparm_result="$(hdparm -i /dev/$1 | grep "Model=")"
+ local model="$(get_ssd_model "${hdparm_result}")"
+ local version="$(get_ssd_version "${hdparm_result}")"
+ local ssd_cmd
+ local i
+
+ if is_ssd_blacklist ${model} ${version}; then
+ SSD_CMD_1=${SSD_CMD_1_ALTERNATE}
else
- SSD_CMD_1=$SSD_CMD_1_NORMAL
+ SSD_CMD_1=${SSD_CMD_1_NORMAL}
fi
- for i in $(seq 0 $SSD_CMD_MAX); do
+ for i in $(seq 0 ${SSD_CMD_MAX}); do
# use eval for variable indirection
- eval SSD_CMD=\$SSD_CMD_$i
- echo "$ $SSD_CMD /dev/$1"
- $SSD_CMD /dev/$1
+ eval ssd_cmd=\${SSD_CMD_${i}}
+ echo "$ ${ssd_cmd} /dev/$1"
+ ${ssd_cmd} /dev/$1
echo ""
done
}
print_mmc_info() {
- for i in $(seq 0 $MMC_NAME_MAX); do
- eval MMC_NAME=\$MMC_NAME_$i
- MMC_PATH=/sys/block/$1/device/$MMC_NAME
- MMC_RESULT=$(cat $MMC_PATH 2>/dev/null)
- printf "%-20s | %s\n" "$MMC_NAME" "$MMC_RESULT"
+ local mmc_name
+ local mmc_path
+ local mmc_result
+ local i
+
+ for i in $(seq 0 ${MMC_NAME_MAX}); do
+ eval mmc_name=\${MMC_NAME_${i}}
+ mmc_path=/sys/block/$1/device/${mmc_name}
+ mmc_result=$(cat ${mmc_path} 2>/dev/null)
+ printf "%-20s | %s\n" "${mmc_name}" "${mmc_result}"
done
}
-rm -f $STORAGE_INFO_FILE
-touch $STORAGE_INFO_FILE
-chmod 644 $STORAGE_INFO_FILE
-exec > $STORAGE_INFO_FILE
+main() {
+ local dev
-# We need to check both sda and sdb for internal disk because:
-# 1) If we use rootdev command to determine the disk to check, it will return
-# the usb stick, not the internal drive, when we boot from usb.
-# 2) We also can not assume that sda is an internal disk because in some
-# systems (e.g., parrot), the usb stick might be labeled as sda and the
-# internal disk as sdb.
-for dev in sda sdb; do
- if is_internal_ssd $dev; then
- print_ssd_info $dev
- fi
-done
+ rm -f ${STORAGE_INFO_FILE}
+ touch ${STORAGE_INFO_FILE}
+ chmod 644 ${STORAGE_INFO_FILE}
+ exec > ${STORAGE_INFO_FILE}
-# For devices with eMMC, the internal disk will always be labeled as mmcblk0
-# so we can check only mmcblk0, but to be safe we will check both mmcblk0 and
-# mmcblk1.
-for dev in mmcblk0 mmcblk1; do
- if is_internal_mmc $dev; then
- print_mmc_info $dev
- fi
-done
+ # We need to check both sda and sdb for internal disk because:
+ # 1) If we use rootdev command to determine the disk to check, it will return
+ # the usb stick, not the internal drive, when we boot from usb.
+ # 2) We also can not assume that sda is an internal disk because in some
+ # systems (e.g., parrot), the usb stick might be labeled as sda and the
+ # internal disk as sdb.
+ for dev in sda sdb; do
+ if is_internal_ssd ${dev}; then
+ print_ssd_info ${dev}
+ fi
+ done
+
+ # For devices with eMMC, the internal disk will always be labeled as mmcblk0
+ # so we can check only mmcblk0, but to be safe we will check both mmcblk0 and
+ # mmcblk1.
+ for dev in mmcblk0 mmcblk1; do
+ if is_internal_mmc ${dev}; then
+ print_mmc_info ${dev}
+ fi
+ done
+}
+
+# don't run script in test mode
+if [ ! "$1" = "--test" ]; then
+ main
+fi
+
diff --git a/test/storage_info_unit_test b/test/storage_info_unit_test
new file mode 100755
index 0000000..774c447
--- /dev/null
+++ b/test/storage_info_unit_test
@@ -0,0 +1,66 @@
+#! /bin/sh
+
+# Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+# unit test for storage_info
+
+. scripts/storage_info --test
+
+TEST_COUNT=7
+
+# stout - old fw crbug.com/328587#c11
+MODEL_1="SanDisk_SSD_U100_16GB"
+VERSION_1="10.52.01"
+RESULT_1="MATCH"
+
+# stout - new fw crbug.com/328587#c13
+MODEL_2="SanDisk_SSD_U100_16GB"
+VERSION_2="CS.56.00"
+RESULT_2="NOT_MATCH"
+
+# stout - new fw 2 crbug.com/328587#c34
+MODEL_3="SanDisk_SSD_i100_32GB"
+VERSION_3="10.54.01"
+RESULT_3="NOT_MATCH"
+
+# link - old fw crbug.com/328587#c37
+MODEL_4="SanDisk_SSD_i100_32GB"
+VERSION_4="CS.51.00"
+RESULT_4="MATCH"
+
+# link - new fw
+MODEL_5="SanDisk_SSD_i100_32GB"
+VERSION_5="11.56.00"
+RESULT_5="NOT_MATCH"
+
+# crbug.com/328587#c39
+MODEL_6="SanDisk_SSD_U100_32GB"
+VERSION_6="10.01.04"
+RESULT_6="MATCH"
+
+# falco
+MODEL_7="LITEONIT_LSS-32L6G-HP"
+VERSION_7="DS51704"
+RESULT_7="NOT_MATCH"
+
+get_is_ssd_blacklist_result() {
+ if is_ssd_blacklist $1 $2; then
+ echo "MATCH"
+ else
+ echo "NOT_MATCH"
+ fi
+}
+
+
+for i in $(seq 1 $TEST_COUNT); do
+ # use eval for variable indirection
+ eval MODEL=\$MODEL_$i
+ eval VERSION=\$VERSION_$i
+ eval RESULT=\$RESULT_$i
+ if [ ! "$(get_is_ssd_blacklist_result $MODEL $VERSION)" = "$RESULT" ]; then
+ echo "Test Fail: result for $MODEL $VERSION is not $RESULT"
+ exit 1
+ fi
+done
