From ba220f0b03f6658333532d93707f1b33775333a3 Mon Sep 17 00:00:00 2001 From: Ashwin Srinivasan Date: Tue, 2 Jul 2024 08:15:22 +0000 Subject: [PATCH 01/11] Added function to check if disk is SATA --- ssdutil/main.py | 46 +++++++++++++++++++++++++++++++---- tests/mocked_libs/__init__.py | 0 tests/mocked_libs/blkinfo.py | 26 ++++++++++++++++++++ tests/mocked_libs/psutil.py | 5 ++++ tests/ssdutil_test.py | 38 +++++++++++++++++++++++++++++ 5 files changed, 110 insertions(+), 5 deletions(-) create mode 100644 tests/mocked_libs/__init__.py create mode 100644 tests/mocked_libs/blkinfo.py create mode 100644 tests/mocked_libs/psutil.py diff --git a/ssdutil/main.py b/ssdutil/main.py index 7b6f2c1ca1..06753fda3b 100755 --- a/ssdutil/main.py +++ b/ssdutil/main.py @@ -6,9 +6,12 @@ # try: - import argparse import os import sys + import argparse + import json + import psutil + from blkinfo import BlkDiskInfo from sonic_py_common import device_info, logger except ImportError as e: @@ -16,10 +19,36 @@ DEFAULT_DEVICE="/dev/sda" SYSLOG_IDENTIFIER = "ssdutil" +DISK_TYPE_SSD = "sata" # Global logger instance log = logger.Logger(SYSLOG_IDENTIFIER) +def get_default_disk(): + """Check default disk""" + default_device = DEFAULT_DEVICE + host_mnt = '/host' + host_partition = None + partitions = psutil.disk_partitions() + + for parts in partitions: + if parts.mountpoint == "/host": + host_partition = parts + break + + disk_major = os.major(os.stat(host_partition.device).st_rdev) + filters = { + 'maj:min': '{}:0'.format(disk_major) + } + + myblkd = BlkDiskInfo() + my_filtered_disks = myblkd.get_disks(filters) + json_output = eval(my_filtered_disks)[0] + blkdev = json_output['name'] + disk_type = json_output['tran'] + default_device = os.path.join("/dev/", blkdev) + + return default_device, disk_type def import_ssd_api(diskdev): """ @@ -39,10 +68,13 @@ def import_ssd_api(diskdev): except ImportError as e: log.log_warning("Platform specific SsdUtil module not found. Falling down to the generic implementation") try: - from sonic_platform_base.sonic_storage.ssd import SsdUtil + from sonic_platform_base.sonic_ssd.ssd_generic import SsdUtil except ImportError as e: - log.log_error("Failed to import default SsdUtil. Error: {}".format(str(e)), True) - raise e + try: + from sonic_platform_base.sonic_storage.ssd import SsdUtil + except ImportError as e: + log.log_error("Failed to import default SsdUtil. Error: {}".format(str(e)), True) + raise e return SsdUtil(diskdev) @@ -60,11 +92,15 @@ def ssdutil(): sys.exit(1) parser = argparse.ArgumentParser() - parser.add_argument("-d", "--device", help="Device name to show health info", default=DEFAULT_DEVICE) + (default_device , disk_type) = get_default_disk() + parser.add_argument("-d", "--device", help="Device name to show health info", default=default_device) parser.add_argument("-v", "--verbose", action="store_true", default=False, help="Show verbose output (some additional parameters)") parser.add_argument("-e", "--vendor", action="store_true", default=False, help="Show vendor output (extended output if provided by platform vendor)") args = parser.parse_args() + if DISK_TYPE_SSD not in disk_type: + print("Disk type is not SSD") + ssd = import_ssd_api(args.device) print("Device Model : {}".format(ssd.get_model())) diff --git a/tests/mocked_libs/__init__.py b/tests/mocked_libs/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/mocked_libs/blkinfo.py b/tests/mocked_libs/blkinfo.py new file mode 100644 index 0000000000..3c81b4cc2c --- /dev/null +++ b/tests/mocked_libs/blkinfo.py @@ -0,0 +1,26 @@ +mock_json_op = """ +[{'name': 'sda', 'kname': 'sda', 'fstype': '', 'label': '', 'mountpoint': '', 'size': '3965714432', \ +'maj:min': '8:0', 'rm': '0', 'model': 'SMART EUSB', 'vendor': 'SMART EUSB', 'serial': 'SPG200807J1', \ +'hctl': '2:0:0:0', 'tran': 'usb', 'rota': '1', 'type': 'disk', 'ro': '0', 'owner': '', 'group': '', \ +'mode': 'brw-rw----', 'children': [{'name': 'sda1', 'kname': 'sda1', 'fstype': 'ext4', 'label': '', \ +'mountpoint': '/host', 'size': '3964665856', 'maj:min': '8:1', 'rm': '0', 'model': ' ', 'vendor': ' ', \ +'serial': '', 'hctl': '', 'tran': '', 'rota': '1', 'type': 'part', 'ro': '0', 'owner': '', 'group': '', \ +'mode': 'brw-rw----', 'children': [], 'parents': ['sda'], 'statistics': {'major': '8', 'minor': '1', \ +'kname': 'sda1', 'reads_completed': '22104', 'reads_merged': '5299', 'sectors_read': '1091502', \ +'time_spent_reading_ms': '51711', 'writes_completed': '11283', 'writes_merged': '13401', \ +'sectors_written': '443784', 'time_spent_ writing': '133398', 'ios_in_progress': '0', \ +'time_spent_doing_ios_ms': '112040', 'weighted_time_ios_ms': '112040'}}], 'parents': [], \ +'statistics': {'major': '8', 'minor': '0', 'kname': 'sda', 'reads_completed': '22151', \ +'reads_merged': '5299', 'sectors_read': '1093606', 'time_spent_reading_ms': '52005', \ +'writes_completed': '11283', 'writes_merged': '13401', 'sectors_written': '443784', \ +'time_spent_ writing': '133398', 'ios_in_progress': '0', 'time_spent_doing_ios_ms': '112220', \ +'weighted_time_ios_ms': '112220'}}] +""" + +class BlkDiskInfo: + def __init__(self): + return + + def get_disks(self, filters): + return mock_json_op + diff --git a/tests/mocked_libs/psutil.py b/tests/mocked_libs/psutil.py new file mode 100644 index 0000000000..0706861d75 --- /dev/null +++ b/tests/mocked_libs/psutil.py @@ -0,0 +1,5 @@ +from collections import namedtuple + +def disk_partitions(): + sdiskpart = namedtuple('sdiskpart', ['mountpoint', 'device']) + return [sdiskpart(mountpoint="/host", device="/dev/sda1")] \ No newline at end of file diff --git a/tests/ssdutil_test.py b/tests/ssdutil_test.py index bd57b0cbe7..a2e6680272 100644 --- a/tests/ssdutil_test.py +++ b/tests/ssdutil_test.py @@ -1,8 +1,22 @@ +import os import sys import argparse +import pytest +from collections import namedtuple from unittest.mock import patch, MagicMock import sonic_platform_base # noqa: F401 +tests_path = os.path.dirname(os.path.abspath(__file__)) + +# Add mocked_libs path so that the file under test can load mocked modules from there +mocked_libs_path = os.path.join(tests_path, "mocked_libs") +sys.path.insert(0, mocked_libs_path) + +from .mocked_libs import psutil +from .mocked_libs.blkinfo import BlkDiskInfo + +sys.modules['os.stat'] = MagicMock() +sys.modules['os.major'] = MagicMock(return_value=8) sys.modules['sonic_platform'] = MagicMock() sys.modules['sonic_platform_base.sonic_ssd.ssd_generic'] = MagicMock() @@ -32,6 +46,30 @@ def get_vendor_output(self): class TestSsdutil: + def test_get_default_disk(self): + + class mock_os: + def __init__(self): + pass + + def major(self, arg): + return 8 + + class stat(): + def __init__(self): + self.st_rdev = 2049 + + (default_device , disk_type) = ssdutil.get_default_disk() + + assert default_device == "/dev/sda" + assert disk_type == 'usb' + + + def test_is_number_valueerror(self): + outcome = ssdutil.is_number("nope") + assert outcome == False + + @patch('sonic_py_common.device_info.get_paths_to_platform_and_hwsku_dirs', MagicMock(return_value=("test_path", ""))) # noqa: E501 @patch('os.geteuid', MagicMock(return_value=0)) def test_sonic_storage_path(self): From 04445f0a6ae86479edb76f4551f3187e904c77c0 Mon Sep 17 00:00:00 2001 From: Ashwin Srinivasan Date: Thu, 4 Jul 2024 00:02:04 +0000 Subject: [PATCH 02/11] Fixed eval bug unconvered by UT --- ssdutil/main.py | 2 +- tests/mocked_libs/blkinfo.py | 101 ++++++++++++++++++++++++++++------- 2 files changed, 84 insertions(+), 19 deletions(-) diff --git a/ssdutil/main.py b/ssdutil/main.py index 06753fda3b..772f484634 100755 --- a/ssdutil/main.py +++ b/ssdutil/main.py @@ -43,7 +43,7 @@ def get_default_disk(): myblkd = BlkDiskInfo() my_filtered_disks = myblkd.get_disks(filters) - json_output = eval(my_filtered_disks)[0] + json_output = my_filtered_disks[0] blkdev = json_output['name'] disk_type = json_output['tran'] default_device = os.path.join("/dev/", blkdev) diff --git a/tests/mocked_libs/blkinfo.py b/tests/mocked_libs/blkinfo.py index 3c81b4cc2c..4c536b70c9 100644 --- a/tests/mocked_libs/blkinfo.py +++ b/tests/mocked_libs/blkinfo.py @@ -1,21 +1,86 @@ -mock_json_op = """ -[{'name': 'sda', 'kname': 'sda', 'fstype': '', 'label': '', 'mountpoint': '', 'size': '3965714432', \ -'maj:min': '8:0', 'rm': '0', 'model': 'SMART EUSB', 'vendor': 'SMART EUSB', 'serial': 'SPG200807J1', \ -'hctl': '2:0:0:0', 'tran': 'usb', 'rota': '1', 'type': 'disk', 'ro': '0', 'owner': '', 'group': '', \ -'mode': 'brw-rw----', 'children': [{'name': 'sda1', 'kname': 'sda1', 'fstype': 'ext4', 'label': '', \ -'mountpoint': '/host', 'size': '3964665856', 'maj:min': '8:1', 'rm': '0', 'model': ' ', 'vendor': ' ', \ -'serial': '', 'hctl': '', 'tran': '', 'rota': '1', 'type': 'part', 'ro': '0', 'owner': '', 'group': '', \ -'mode': 'brw-rw----', 'children': [], 'parents': ['sda'], 'statistics': {'major': '8', 'minor': '1', \ -'kname': 'sda1', 'reads_completed': '22104', 'reads_merged': '5299', 'sectors_read': '1091502', \ -'time_spent_reading_ms': '51711', 'writes_completed': '11283', 'writes_merged': '13401', \ -'sectors_written': '443784', 'time_spent_ writing': '133398', 'ios_in_progress': '0', \ -'time_spent_doing_ios_ms': '112040', 'weighted_time_ios_ms': '112040'}}], 'parents': [], \ -'statistics': {'major': '8', 'minor': '0', 'kname': 'sda', 'reads_completed': '22151', \ -'reads_merged': '5299', 'sectors_read': '1093606', 'time_spent_reading_ms': '52005', \ -'writes_completed': '11283', 'writes_merged': '13401', 'sectors_written': '443784', \ -'time_spent_ writing': '133398', 'ios_in_progress': '0', 'time_spent_doing_ios_ms': '112220', \ -'weighted_time_ios_ms': '112220'}}] -""" +mock_json_op = \ +[ + { + "name": "sda", + "kname": "sda", + "fstype": "", + "label": "", + "mountpoint": "", + "size": "3965714432", + "maj:min": "8:0", + "rm": "0", + "model": "SMART EUSB", + "vendor": "SMART EUSB", + "serial": "SPG200807J1", + "hctl": "2:0:0:0", + "tran": "usb", + "rota": "1", + "type": "disk", + "ro": "0", + "owner": "", + "group": "", + "mode": "brw-rw----", + "children": [ + { + "name": "sda1", + "kname": "sda1", + "fstype": "ext4", + "label": "", + "mountpoint": "/host", + "size": "3964665856", + "maj:min": "8:1", + "rm": "0", + "model": " ", + "vendor": " ", + "serial": "", + "hctl": "", + "tran": "", + "rota": "1", + "type": "part", + "ro": "0", + "owner": "", + "group": "", + "mode": "brw-rw----", + "children": [], + "parents": ["sda"], + "statistics": { + "major": "8", + "minor": "1", + "kname": "sda1", + "reads_completed": "22104", + "reads_merged": "5299", + "sectors_read": "1091502", + "time_spent_reading_ms": "51711", + "writes_completed": "11283", + "writes_merged": "13401", + "sectors_written": "443784", + "time_spent_ writing": "133398", + "ios_in_progress": "0", + "time_spent_doing_ios_ms": "112040", + "weighted_time_ios_ms": "112040", + }, + } + ], + "parents": [], + "statistics": { + "major": "8", + "minor": "0", + "kname": "sda", + "reads_completed": "22151", + "reads_merged": "5299", + "sectors_read": "1093606", + "time_spent_reading_ms": "52005", + "writes_completed": "11283", + "writes_merged": "13401", + "sectors_written": "443784", + "time_spent_ writing": "133398", + "ios_in_progress": "0", + "time_spent_doing_ios_ms": "112220", + "weighted_time_ios_ms": "112220", + }, + } +] + class BlkDiskInfo: def __init__(self): From 1fa0cddda51a4bac478a78373b13ea6a492c38e4 Mon Sep 17 00:00:00 2001 From: Ashwin Srinivasan Date: Mon, 8 Jul 2024 18:48:01 +0000 Subject: [PATCH 03/11] Changed logger to syslogger; fixed UT failures --- ssdutil/main.py | 4 ++-- tests/mocked_libs/blkinfo.py | 14 +++++++------- tests/mocked_libs/psutil.py | 2 +- tests/ssdutil_test.py | 36 +++++++++++++----------------------- 4 files changed, 23 insertions(+), 33 deletions(-) diff --git a/ssdutil/main.py b/ssdutil/main.py index 772f484634..2b9d3ee1f0 100755 --- a/ssdutil/main.py +++ b/ssdutil/main.py @@ -13,7 +13,7 @@ import psutil from blkinfo import BlkDiskInfo - from sonic_py_common import device_info, logger + from sonic_py_common import device_info, syslogger except ImportError as e: raise ImportError("%s - required module not found" % str(e)) @@ -22,7 +22,7 @@ DISK_TYPE_SSD = "sata" # Global logger instance -log = logger.Logger(SYSLOG_IDENTIFIER) +log = syslogger.SysLogger(SYSLOG_IDENTIFIER) def get_default_disk(): """Check default disk""" diff --git a/tests/mocked_libs/blkinfo.py b/tests/mocked_libs/blkinfo.py index 4c536b70c9..23e10d594f 100644 --- a/tests/mocked_libs/blkinfo.py +++ b/tests/mocked_libs/blkinfo.py @@ -1,8 +1,8 @@ mock_json_op = \ [ { - "name": "sda", - "kname": "sda", + "name": "sdx", + "kname": "sdx", "fstype": "", "label": "", "mountpoint": "", @@ -22,8 +22,8 @@ "mode": "brw-rw----", "children": [ { - "name": "sda1", - "kname": "sda1", + "name": "sdx1", + "kname": "sdx1", "fstype": "ext4", "label": "", "mountpoint": "/host", @@ -42,11 +42,11 @@ "group": "", "mode": "brw-rw----", "children": [], - "parents": ["sda"], + "parents": ["sdx"], "statistics": { "major": "8", "minor": "1", - "kname": "sda1", + "kname": "sdx1", "reads_completed": "22104", "reads_merged": "5299", "sectors_read": "1091502", @@ -65,7 +65,7 @@ "statistics": { "major": "8", "minor": "0", - "kname": "sda", + "kname": "sdx", "reads_completed": "22151", "reads_merged": "5299", "sectors_read": "1093606", diff --git a/tests/mocked_libs/psutil.py b/tests/mocked_libs/psutil.py index 0706861d75..63eec0444b 100644 --- a/tests/mocked_libs/psutil.py +++ b/tests/mocked_libs/psutil.py @@ -2,4 +2,4 @@ def disk_partitions(): sdiskpart = namedtuple('sdiskpart', ['mountpoint', 'device']) - return [sdiskpart(mountpoint="/host", device="/dev/sda1")] \ No newline at end of file + return [sdiskpart(mountpoint="/host", device="/dev/sdx1")] \ No newline at end of file diff --git a/tests/ssdutil_test.py b/tests/ssdutil_test.py index a2e6680272..0f7a8b11c0 100644 --- a/tests/ssdutil_test.py +++ b/tests/ssdutil_test.py @@ -1,19 +1,18 @@ import os import sys import argparse -import pytest -from collections import namedtuple from unittest.mock import patch, MagicMock import sonic_platform_base # noqa: F401 tests_path = os.path.dirname(os.path.abspath(__file__)) -# Add mocked_libs path so that the file under test can load mocked modules from there -mocked_libs_path = os.path.join(tests_path, "mocked_libs") +# Add mocked_libs path so that the file under test +# can load mocked modules from there +mocked_libs_path = os.path.join(tests_path, "mocked_libs") # noqa: E402,F401 sys.path.insert(0, mocked_libs_path) -from .mocked_libs import psutil -from .mocked_libs.blkinfo import BlkDiskInfo +from .mocked_libs import psutil # noqa: E402,F401 +from .mocked_libs.blkinfo import BlkDiskInfo # noqa: E402,F401 sys.modules['os.stat'] = MagicMock() sys.modules['os.major'] = MagicMock(return_value=8) @@ -46,32 +45,23 @@ def get_vendor_output(self): class TestSsdutil: + @patch('os.geteuid', MagicMock(return_value=0)) + @patch('os.stat', MagicMock(st_rdev=2049)) + @patch('os.major', MagicMock(return_value=8)) def test_get_default_disk(self): + (default_device, disk_type) = ssdutil.get_default_disk() - class mock_os: - def __init__(self): - pass - - def major(self, arg): - return 8 - - class stat(): - def __init__(self): - self.st_rdev = 2049 - - (default_device , disk_type) = ssdutil.get_default_disk() - - assert default_device == "/dev/sda" + assert default_device == "/dev/sdx" assert disk_type == 'usb' - def test_is_number_valueerror(self): outcome = ssdutil.is_number("nope") - assert outcome == False - + assert outcome is False @patch('sonic_py_common.device_info.get_paths_to_platform_and_hwsku_dirs', MagicMock(return_value=("test_path", ""))) # noqa: E501 @patch('os.geteuid', MagicMock(return_value=0)) + @patch('os.stat', MagicMock(st_rdev=2049)) + @patch('os.major', MagicMock(return_value=8)) def test_sonic_storage_path(self): with patch('argparse.ArgumentParser.parse_args', MagicMock()) as mock_args: # noqa: E501 From 7a4bf4de5dc6194fc52ba821675f64ee7c6d2865 Mon Sep 17 00:00:00 2001 From: Ashwin Srinivasan Date: Mon, 8 Jul 2024 21:12:55 +0000 Subject: [PATCH 04/11] Added checks for partitions and filtered_disks; fixed static analysis issues --- ssdutil/main.py | 33 ++++--- tests/mocked_libs/blkinfo.py | 163 +++++++++++++++++------------------ tests/mocked_libs/psutil.py | 3 +- tests/ssdutil_test.py | 11 +++ 4 files changed, 116 insertions(+), 94 deletions(-) diff --git a/ssdutil/main.py b/ssdutil/main.py index 2b9d3ee1f0..4a6e2f1d63 100755 --- a/ssdutil/main.py +++ b/ssdutil/main.py @@ -9,7 +9,6 @@ import os import sys import argparse - import json import psutil from blkinfo import BlkDiskInfo @@ -17,13 +16,14 @@ except ImportError as e: raise ImportError("%s - required module not found" % str(e)) -DEFAULT_DEVICE="/dev/sda" +DEFAULT_DEVICE = "/dev/sda" SYSLOG_IDENTIFIER = "ssdutil" DISK_TYPE_SSD = "sata" # Global logger instance log = syslogger.SysLogger(SYSLOG_IDENTIFIER) + def get_default_disk(): """Check default disk""" default_device = DEFAULT_DEVICE @@ -31,8 +31,11 @@ def get_default_disk(): host_partition = None partitions = psutil.disk_partitions() + if partitions is None: + return (default_device, None) + for parts in partitions: - if parts.mountpoint == "/host": + if parts.mountpoint == host_mnt: host_partition = parts break @@ -43,6 +46,10 @@ def get_default_disk(): myblkd = BlkDiskInfo() my_filtered_disks = myblkd.get_disks(filters) + + if my_filtered_disks is None: + return (default_device, None) + json_output = my_filtered_disks[0] blkdev = json_output['name'] disk_type = json_output['tran'] @@ -50,6 +57,7 @@ def get_default_disk(): return default_device, disk_type + def import_ssd_api(diskdev): """ Loads platform specific or generic ssd_util module from source @@ -66,18 +74,19 @@ def import_ssd_api(diskdev): sys.path.append(os.path.abspath(platform_plugins_path)) from ssd_util import SsdUtil except ImportError as e: - log.log_warning("Platform specific SsdUtil module not found. Falling down to the generic implementation") + log.log_warning("Platform specific SsdUtil module not found. Falling down to the generic implementation") # noqa: E501 try: from sonic_platform_base.sonic_ssd.ssd_generic import SsdUtil except ImportError as e: try: from sonic_platform_base.sonic_storage.ssd import SsdUtil except ImportError as e: - log.log_error("Failed to import default SsdUtil. Error: {}".format(str(e)), True) + log.log_error("Failed to import default SsdUtil. Error: {}".format(str(e)), True) # noqa: E501 raise e return SsdUtil(diskdev) + def is_number(s): try: float(s) @@ -85,6 +94,7 @@ def is_number(s): except ValueError: return False + # ==================== Entry point ==================== def ssdutil(): if os.geteuid() != 0: @@ -92,10 +102,10 @@ def ssdutil(): sys.exit(1) parser = argparse.ArgumentParser() - (default_device , disk_type) = get_default_disk() - parser.add_argument("-d", "--device", help="Device name to show health info", default=default_device) - parser.add_argument("-v", "--verbose", action="store_true", default=False, help="Show verbose output (some additional parameters)") - parser.add_argument("-e", "--vendor", action="store_true", default=False, help="Show vendor output (extended output if provided by platform vendor)") + (default_device, disk_type) = get_default_disk() + parser.add_argument("-d", "--device", help="Device name to show health info", default=default_device) # noqa: E501 + parser.add_argument("-v", "--verbose", action="store_true", default=False, help="Show verbose output (some additional parameters)") # noqa: E501 + parser.add_argument("-e", "--vendor", action="store_true", default=False, help="Show vendor output (extended output if provided by platform vendor)") # noqa: E501 args = parser.parse_args() if DISK_TYPE_SSD not in disk_type: @@ -107,10 +117,11 @@ def ssdutil(): if args.verbose: print("Firmware : {}".format(ssd.get_firmware())) print("Serial : {}".format(ssd.get_serial())) - print("Health : {}{}".format(ssd.get_health(), "%" if is_number(ssd.get_health()) else "")) - print("Temperature : {}{}".format(ssd.get_temperature(), "C" if is_number(ssd.get_temperature()) else "")) + print("Health : {}{}".format(ssd.get_health(), "%" if is_number(ssd.get_health()) else "")) # noqa: E501 + print("Temperature : {}{}".format(ssd.get_temperature(), "C" if is_number(ssd.get_temperature()) else "")) # noqa: E501 if args.vendor: print(ssd.get_vendor_output()) + if __name__ == '__main__': ssdutil() diff --git a/tests/mocked_libs/blkinfo.py b/tests/mocked_libs/blkinfo.py index 23e10d594f..6d5d809837 100644 --- a/tests/mocked_libs/blkinfo.py +++ b/tests/mocked_libs/blkinfo.py @@ -1,91 +1,90 @@ mock_json_op = \ -[ - { - "name": "sdx", - "kname": "sdx", - "fstype": "", - "label": "", - "mountpoint": "", - "size": "3965714432", - "maj:min": "8:0", - "rm": "0", - "model": "SMART EUSB", - "vendor": "SMART EUSB", - "serial": "SPG200807J1", - "hctl": "2:0:0:0", - "tran": "usb", - "rota": "1", - "type": "disk", - "ro": "0", - "owner": "", - "group": "", - "mode": "brw-rw----", - "children": [ - { - "name": "sdx1", - "kname": "sdx1", - "fstype": "ext4", - "label": "", - "mountpoint": "/host", - "size": "3964665856", - "maj:min": "8:1", - "rm": "0", - "model": " ", - "vendor": " ", - "serial": "", - "hctl": "", - "tran": "", - "rota": "1", - "type": "part", - "ro": "0", - "owner": "", - "group": "", - "mode": "brw-rw----", - "children": [], - "parents": ["sdx"], - "statistics": { - "major": "8", - "minor": "1", - "kname": "sdx1", - "reads_completed": "22104", - "reads_merged": "5299", - "sectors_read": "1091502", - "time_spent_reading_ms": "51711", - "writes_completed": "11283", - "writes_merged": "13401", - "sectors_written": "443784", - "time_spent_ writing": "133398", - "ios_in_progress": "0", - "time_spent_doing_ios_ms": "112040", - "weighted_time_ios_ms": "112040", - }, - } - ], - "parents": [], - "statistics": { - "major": "8", - "minor": "0", + [ + { + "name": "sdx", "kname": "sdx", - "reads_completed": "22151", - "reads_merged": "5299", - "sectors_read": "1093606", - "time_spent_reading_ms": "52005", - "writes_completed": "11283", - "writes_merged": "13401", - "sectors_written": "443784", - "time_spent_ writing": "133398", - "ios_in_progress": "0", - "time_spent_doing_ios_ms": "112220", - "weighted_time_ios_ms": "112220", - }, - } -] + "fstype": "", + "label": "", + "mountpoint": "", + "size": "3965714432", + "maj:min": "8:0", + "rm": "0", + "model": "SMART EUSB", + "vendor": "SMART EUSB", + "serial": "SPG200807J1", + "hctl": "2:0:0:0", + "tran": "usb", + "rota": "1", + "type": "disk", + "ro": "0", + "owner": "", + "group": "", + "mode": "brw-rw----", + "children": [ + { + "name": "sdx1", + "kname": "sdx1", + "fstype": "ext4", + "label": "", + "mountpoint": "/host", + "size": "3964665856", + "maj:min": "8:1", + "rm": "0", + "model": " ", + "vendor": " ", + "serial": "", + "hctl": "", + "tran": "", + "rota": "1", + "type": "part", + "ro": "0", + "owner": "", + "group": "", + "mode": "brw-rw----", + "children": [], + "parents": ["sdx"], + "statistics": { + "major": "8", + "minor": "1", + "kname": "sdx1", + "reads_completed": "22104", + "reads_merged": "5299", + "sectors_read": "1091502", + "time_spent_reading_ms": "51711", + "writes_completed": "11283", + "writes_merged": "13401", + "sectors_written": "443784", + "time_spent_ writing": "133398", + "ios_in_progress": "0", + "time_spent_doing_ios_ms": "112040", + "weighted_time_ios_ms": "112040", + }, + } + ], + "parents": [], + "statistics": { + "major": "8", + "minor": "0", + "kname": "sdx", + "reads_completed": "22151", + "reads_merged": "5299", + "sectors_read": "1093606", + "time_spent_reading_ms": "52005", + "writes_completed": "11283", + "writes_merged": "13401", + "sectors_written": "443784", + "time_spent_ writing": "133398", + "ios_in_progress": "0", + "time_spent_doing_ios_ms": "112220", + "weighted_time_ios_ms": "112220", + }, + } + ] class BlkDiskInfo: def __init__(self): return - + def get_disks(self, filters): return mock_json_op - diff --git a/tests/mocked_libs/psutil.py b/tests/mocked_libs/psutil.py index 63eec0444b..f43f024d1c 100644 --- a/tests/mocked_libs/psutil.py +++ b/tests/mocked_libs/psutil.py @@ -1,5 +1,6 @@ from collections import namedtuple + def disk_partitions(): sdiskpart = namedtuple('sdiskpart', ['mountpoint', 'device']) - return [sdiskpart(mountpoint="/host", device="/dev/sdx1")] \ No newline at end of file + return [sdiskpart(mountpoint="/host", device="/dev/sdx1")] diff --git a/tests/ssdutil_test.py b/tests/ssdutil_test.py index 0f7a8b11c0..d7de9eea47 100644 --- a/tests/ssdutil_test.py +++ b/tests/ssdutil_test.py @@ -54,6 +54,17 @@ def test_get_default_disk(self): assert default_device == "/dev/sdx" assert disk_type == 'usb' + + @patch('os.geteuid', MagicMock(return_value=0)) + @patch('os.stat', MagicMock(st_rdev=2049)) + @patch('os.major', MagicMock(return_value=8)) + @patch('psutil.disk_partitions', MagicMock(return_value=None)) + def test_get_default_disk_none_partitions(self): + (default_device, disk_type) = ssdutil.get_default_disk() + + assert default_device == "/dev/sda" + assert disk_type == None + def test_is_number_valueerror(self): outcome = ssdutil.is_number("nope") assert outcome is False From 584d11e870c84d4cc171c25da2e7aab65967b517 Mon Sep 17 00:00:00 2001 From: Ashwin Srinivasan Date: Mon, 8 Jul 2024 21:21:20 +0000 Subject: [PATCH 05/11] Fix static analysis errors E303, E711 --- tests/ssdutil_test.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/ssdutil_test.py b/tests/ssdutil_test.py index d7de9eea47..dc27526ea7 100644 --- a/tests/ssdutil_test.py +++ b/tests/ssdutil_test.py @@ -54,7 +54,6 @@ def test_get_default_disk(self): assert default_device == "/dev/sdx" assert disk_type == 'usb' - @patch('os.geteuid', MagicMock(return_value=0)) @patch('os.stat', MagicMock(st_rdev=2049)) @patch('os.major', MagicMock(return_value=8)) @@ -63,7 +62,7 @@ def test_get_default_disk_none_partitions(self): (default_device, disk_type) = ssdutil.get_default_disk() assert default_device == "/dev/sda" - assert disk_type == None + assert disk_type is None def test_is_number_valueerror(self): outcome = ssdutil.is_number("nope") From 5ce53b318d5412dfe56df86d3f580f2ea81c8c37 Mon Sep 17 00:00:00 2001 From: Ashwin Srinivasan Date: Thu, 1 Aug 2024 06:08:39 +0000 Subject: [PATCH 06/11] Changed information delivery semantics per review comment --- ssdutil/main.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ssdutil/main.py b/ssdutil/main.py index 4a6e2f1d63..46c6972c2d 100755 --- a/ssdutil/main.py +++ b/ssdutil/main.py @@ -108,9 +108,7 @@ def ssdutil(): parser.add_argument("-e", "--vendor", action="store_true", default=False, help="Show vendor output (extended output if provided by platform vendor)") # noqa: E501 args = parser.parse_args() - if DISK_TYPE_SSD not in disk_type: - print("Disk type is not SSD") - + print("Disk type: {0}".format(disk_type.upper())) ssd = import_ssd_api(args.device) print("Device Model : {}".format(ssd.get_model())) From 53aac463a7b68d7bbf11ecf9dd0d2b86c37f5ebc Mon Sep 17 00:00:00 2001 From: Ashwin Srinivasan Date: Sat, 10 Aug 2024 21:22:16 +0000 Subject: [PATCH 07/11] Reverted syslogger to logger to maintain backward compatibility The syslogger-to-logger change is unrelated to the sonic-utilities change and breaks backwards compatibility. It should be a separate commit once SysLogger is in all the older versions. --- ssdutil/main.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ssdutil/main.py b/ssdutil/main.py index 46c6972c2d..399b89fc9c 100755 --- a/ssdutil/main.py +++ b/ssdutil/main.py @@ -12,7 +12,7 @@ import psutil from blkinfo import BlkDiskInfo - from sonic_py_common import device_info, syslogger + from sonic_py_common import device_info, logger except ImportError as e: raise ImportError("%s - required module not found" % str(e)) @@ -21,7 +21,7 @@ DISK_TYPE_SSD = "sata" # Global logger instance -log = syslogger.SysLogger(SYSLOG_IDENTIFIER) +log = logger.Logger(SYSLOG_IDENTIFIER) def get_default_disk(): From a9f24901dc298ec34cd708ce82a5835e0c6dfdb1 Mon Sep 17 00:00:00 2001 From: Ashwin Srinivasan Date: Sat, 10 Aug 2024 21:40:48 +0000 Subject: [PATCH 08/11] Changed Disk 'type' --> 'Type' for uniformity --- ssdutil/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ssdutil/main.py b/ssdutil/main.py index 399b89fc9c..3a9619c62d 100755 --- a/ssdutil/main.py +++ b/ssdutil/main.py @@ -108,7 +108,7 @@ def ssdutil(): parser.add_argument("-e", "--vendor", action="store_true", default=False, help="Show vendor output (extended output if provided by platform vendor)") # noqa: E501 args = parser.parse_args() - print("Disk type: {0}".format(disk_type.upper())) + print("Disk Type: {0}".format(disk_type.upper())) ssd = import_ssd_api(args.device) print("Device Model : {}".format(ssd.get_model())) From 75ec32c4cb8cb8c70706b021d3fd448237ffb70f Mon Sep 17 00:00:00 2001 From: Ashwin Srinivasan Date: Sat, 10 Aug 2024 21:53:14 +0000 Subject: [PATCH 09/11] Made the fields look uniform --- ssdutil/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ssdutil/main.py b/ssdutil/main.py index 3a9619c62d..7e917132a5 100755 --- a/ssdutil/main.py +++ b/ssdutil/main.py @@ -108,7 +108,7 @@ def ssdutil(): parser.add_argument("-e", "--vendor", action="store_true", default=False, help="Show vendor output (extended output if provided by platform vendor)") # noqa: E501 args = parser.parse_args() - print("Disk Type: {0}".format(disk_type.upper())) + print("Disk Type : {0}".format(disk_type.upper())) ssd = import_ssd_api(args.device) print("Device Model : {}".format(ssd.get_model())) From e1bf142a5de1bfdcf9344ccd8ff4505c0bae26ee Mon Sep 17 00:00:00 2001 From: Ashwin Srinivasan Date: Sat, 10 Aug 2024 22:13:18 +0000 Subject: [PATCH 10/11] Disk Type Support for eMMC devices --- ssdutil/main.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ssdutil/main.py b/ssdutil/main.py index 7e917132a5..dec8069b09 100755 --- a/ssdutil/main.py +++ b/ssdutil/main.py @@ -55,6 +55,9 @@ def get_default_disk(): disk_type = json_output['tran'] default_device = os.path.join("/dev/", blkdev) + # Disk Type Support for eMMC devices + disk_type = 'eMMC' if len(disk_type) == 0 and 'mmcblk' in host_partition.device else disk_type # noqa: E501 + return default_device, disk_type From cbbd2835ac56caf863300877ecf846a0bd2afee6 Mon Sep 17 00:00:00 2001 From: Ashwin Srinivasan Date: Thu, 3 Oct 2024 07:46:51 +0000 Subject: [PATCH 11/11] Removed old sonic_ssd import --- ssdutil/main.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/ssdutil/main.py b/ssdutil/main.py index dec8069b09..460c7f769a 100755 --- a/ssdutil/main.py +++ b/ssdutil/main.py @@ -79,13 +79,10 @@ def import_ssd_api(diskdev): except ImportError as e: log.log_warning("Platform specific SsdUtil module not found. Falling down to the generic implementation") # noqa: E501 try: - from sonic_platform_base.sonic_ssd.ssd_generic import SsdUtil + from sonic_platform_base.sonic_storage.ssd import SsdUtil except ImportError as e: - try: - from sonic_platform_base.sonic_storage.ssd import SsdUtil - except ImportError as e: - log.log_error("Failed to import default SsdUtil. Error: {}".format(str(e)), True) # noqa: E501 - raise e + log.log_error("Failed to import default SsdUtil. Error: {}".format(str(e)), True) # noqa: E501 + raise e return SsdUtil(diskdev)