Skip to content

[ssd_generic]Fix ssd no model information #302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[ssd_generic]Fix ssd no model information #302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
301287e
[ssd_generic]Fix ssd no vendor information
hantienEdgecore Aug 17, 2022
9ba0f70
[ssd_generic_test]Update test case for ssd_generic.py modify
hantienEdgecore Mar 7, 2023
647ca17
Merge branch 'master' into master
hantienEdgecore Mar 7, 2023
f5cafd2
[ssd_generic_test] fix ssd_generic_test.py test fail case
hantienEdgecore Mar 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions sonic_platform_base/sonic_ssd/ssd_generic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,9 @@ def __init__(self, diskdev):
self.fetch_vendor_ssd_info(diskdev, vendor)
self.parse_vendor_ssd_info(vendor)
else:
# No handler registered for this disk model
pass
# unknown model name , use Virtium to get information
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this logic to assume it will be Virtium is a hack. Is there a better way to identify the SSD vendor? We should enhance the existing logic to identify the vendor.

Copy link
Author

Choose a reason for hiding this comment

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

I checked the information from SMARTCTL, INNODISK and VIRTIUM in the same device's SSD.
There is no information about vendor. You can see the result from smartctrl that I posted. We can only know "Device Model: TS32XBTMM1600 Serial Number: F318410050". But it can't be a logical statement because it's just a random model name or SN. and there are many this kind of ssd in my work enviorment. The only way I found to find out the correct ssd health and temperature is to use VIRTIUM cmd to check it.

self.vendor_ssd_info =self._execute_shell(self.vendor_ssd_utility["Virtium"]["utility"].format(diskdev))
self.parse_virtium_info()
else:
# Failed to get disk model
self.model = "Unknown"
Expand Down Expand Up @@ -133,8 +134,9 @@ def parse_innodisk_info(self):
self.temperature = temp_raw.split()[-6]

def parse_virtium_info(self):
if self.vendor_ssd_info:
self.temperature = self._parse_re('Temperature_Celsius\s*\d*\s*(\d+?)\s+', self.vendor_ssd_info)
self.temperature = self._parse_re('Temperature_Celsius\s*\d*\s*(\d+?)\s+', self.vendor_ssd_info)
self.health = self._parse_re('Remaining_Life_Left\s*\d*\s*(\d+?)\s+', self.vendor_ssd_info)
if self.health == NOT_AVAILABLE:
nand_endurance = self._parse_re('NAND_Endurance\s*\d*\s*(\d+?)\s+', self.vendor_ssd_info)
avg_erase_count = self._parse_re('Average_Erase_Count\s*\d*\s*(\d+?)\s+', self.vendor_ssd_info)
if nand_endurance != NOT_AVAILABLE and avg_erase_count != NOT_AVAILABLE:
Expand All @@ -147,12 +149,13 @@ def parse_virtium_info(self):
self.health = float(self._parse_re('Remaining_Life_Left\s*\d*\s*(\d+?)\s+', self.vendor_ssd_info))
except ValueError:
pass

def fetch_vendor_ssd_info(self, diskdev, model):
self.vendor_ssd_info = self._execute_shell(self.vendor_ssd_utility[model]["utility"].format(diskdev))

def parse_vendor_ssd_info(self, model):
self.vendor_ssd_utility[model]["parser"]()
if self.vendor_ssd_info:
self.vendor_ssd_utility[model]["parser"]()

def get_health(self):
"""
Expand Down
83 changes: 76 additions & 7 deletions tests/ssd_generic_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

import sys
if sys.version_info.major == 3:
from unittest import mock
Expand Down Expand Up @@ -71,6 +70,74 @@
Num ErrCount SQId CmdId Status PELoc LBA NSID VS
0 5275 0 0x0001 0x0004 - 0 1 -"""

output_SmartCmd_info = """SmartCmd -m /dev/sda
SMART attributes
ID Attribute High Raw Low Raw Value Worst Threshold
1 Raw_Read_Error_Rate 0 0 100 100 0
5 Reserved_Attribute 0 0 100 100 0
9 Power_On_Hours 0 624 100 100 0
12 Power_Cycle_Count 0 113 100 100 0
160 Uncorrectable_Sector_Count 0 0 100 100 0
161 Valid_Spare_Block 0 45 100 100 0
163 Reserved_Attribute 0 23 100 100 0
164 Reserved_Attribute 0 128531 100 100 0
165 Maximum_Erase_Count 0 160 100 100 0
166 Reserved_Attribute 0 60 100 100 0
167 Average_Erase_Count 0 126 100 100 0
168 NAND_Endurance 0 3000 100 100 0
169 Remaining_Life_Left 0 100 100 100 0
175 Reserved_Attribute 0 0 100 100 0
176 Reserved_Attribute 0 0 100 100 0
177 Reserved_Attribute 0 20 100 100 50
178 Reserved_Attribute 0 0 100 100 0
181 Total_Program_Fail 0 0 100 100 0
182 Total_Erase_Fail 0 0 100 100 0
192 Sudden_Power_Lost_Count 0 47 100 100 0
194 Temperature_Celsius 0 37 100 100 0
195 Hardware_ECC_Recovered 0 1063 100 100 0
196 Reallocated_Event_Count 0 0 100 100 16
197 Current_Pending_Sector_Count 0 0 100 100 0
198 Reserved_Attribute 0 0 100 100 0
199 UDMA_CRC_Error_Count 0 0 100 100 50
232 Reserved_Attribute 0 100 100 100 0
241 Total_LBAs_Written 0 79687 100 100 0
242 Total_LBAs_Read 0 51039 100 100 0
245 Reserved_Attribute 0 128531 100 100 0 """

output_SmartCmd_info_2 = """SmartCmd -m /dev/sda
SMART attributes
ID Attribute High Raw Low Raw Value Worst Threshold
1 Raw_Read_Error_Rate 0 0 100 100 0
5 Reserved_Attribute 0 0 100 100 0
9 Power_On_Hours 0 624 100 100 0
12 Power_Cycle_Count 0 113 100 100 0
160 Uncorrectable_Sector_Count 0 0 100 100 0
161 Valid_Spare_Block 0 45 100 100 0
163 Reserved_Attribute 0 23 100 100 0
164 Reserved_Attribute 0 128531 100 100 0
165 Maximum_Erase_Count 0 160 100 100 0
166 Reserved_Attribute 0 60 100 100 0
167 Average_Erase_Count 0 126 100 100 0
168 NAND_Endurance 0 3000 100 100 0
169 Remaining_Life_Left 0 95 100 100 0
175 Reserved_Attribute 0 0 100 100 0
176 Reserved_Attribute 0 0 100 100 0
177 Reserved_Attribute 0 20 100 100 50
178 Reserved_Attribute 0 0 100 100 0
181 Total_Program_Fail 0 0 100 100 0
182 Total_Erase_Fail 0 0 100 100 0
192 Sudden_Power_Lost_Count 0 47 100 100 0
194 Temperature_Celsius 0 30 100 100 0
195 Hardware_ECC_Recovered 0 1063 100 100 0
196 Reallocated_Event_Count 0 0 100 100 16
197 Current_Pending_Sector_Count 0 0 100 100 0
198 Reserved_Attribute 0 0 100 100 0
199 UDMA_CRC_Error_Count 0 0 100 100 50
232 Reserved_Attribute 0 100 100 100 0
241 Total_LBAs_Written 0 79687 100 100 0
242 Total_LBAs_Read 0 51039 100 100 0
245 Reserved_Attribute 0 128531 100 100 0 """

output_ssd = """smartctl 6.6 2017-11-05 r4594 [x86_64-linux-5.10.0-8-2-amd64] (local build)
Copyright (C) 2002-17, Bruce Allen, Christian Franke, www.smartmontools.org

Expand Down Expand Up @@ -533,14 +600,14 @@
"""

class TestSsdGeneric:
@mock.patch('sonic_platform_base.sonic_ssd.ssd_generic.SsdUtil._execute_shell', mock.MagicMock(return_value=output_nvme_ssd))
@mock.patch('sonic_platform_base.sonic_ssd.ssd_generic.SsdUtil._execute_shell', mock.MagicMock(side_effect=[output_nvme_ssd,output_SmartCmd_info]))
def test_nvme_ssd(self):
# Test parsing nvme ssd info
nvme_ssd = SsdUtil('/dev/nvme0n1')
assert(nvme_ssd.get_health() == 100.0)
assert(nvme_ssd.get_health() == '100')
assert(nvme_ssd.get_model() == 'SFPC020GM1EC2TO-I-5E-11P-STD')
assert(nvme_ssd.get_firmware() == "COT6OQ")
assert(nvme_ssd.get_temperature() == 37)
assert(nvme_ssd.get_temperature() == '37')
assert(nvme_ssd.get_serial() == "A0221030722410000027")

@mock.patch('sonic_platform_base.sonic_ssd.ssd_generic.SsdUtil._execute_shell', mock.MagicMock(return_value=output_lack_info_ssd))
Expand All @@ -553,7 +620,7 @@ def test_nvme_ssd_with_na_path(self):
assert(nvme_ssd.get_temperature() == "N/A")
assert(nvme_ssd.get_serial() == "N/A")

@mock.patch('sonic_platform_base.sonic_ssd.ssd_generic.SsdUtil._execute_shell', mock.MagicMock(return_value=output_ssd))
@mock.patch('sonic_platform_base.sonic_ssd.ssd_generic.SsdUtil._execute_shell', mock.MagicMock(side_effect=[output_ssd,output_SmartCmd_info_2]))
def test_ssd(self):
# Test parsing a normal ssd info
ssd = SsdUtil('/dev/sda')
Expand Down Expand Up @@ -604,15 +671,15 @@ def test_Innodisk_missing_names_ssd(self):
def test_virtium_ssd(self, mock_exec):
mock_exec.side_effect = [output_virtium_generic, output_virtium_vendor]
virtium_ssd = SsdUtil('/dev/sda')
assert virtium_ssd.get_health() == 1
assert virtium_ssd.get_health() == '1'
assert virtium_ssd.get_model() == 'VSFDM8XC240G-V11-T'
assert virtium_ssd.get_firmware() == "0913-000"
assert virtium_ssd.get_temperature() == '34'
assert virtium_ssd.get_serial() == "60237-0037"

mock_exec.side_effect = [output_virtium_generic, output_virtium_no_remain_life]
virtium_ssd = SsdUtil('/dev/sda')
assert virtium_ssd.get_health() == 99.42
assert virtium_ssd.get_health() == '100'

mock_exec.side_effect = [output_virtium_generic, output_virtium_invalid_nand_endurance]
virtium_ssd = SsdUtil('/dev/sda')
Expand All @@ -621,3 +688,5 @@ def test_virtium_ssd(self, mock_exec):
mock_exec.side_effect = [output_virtium_generic, output_virtium_invalid_remain_life]
virtium_ssd = SsdUtil('/dev/sda')
assert virtium_ssd.get_health() == "N/A"