From 6e13ecd613c3beede1d91270918982a1dd5a0108 Mon Sep 17 00:00:00 2001 From: desbma Date: Fri, 24 Feb 2017 22:05:54 +0100 Subject: [PATCH] Improve hdparm error handling (should fix #6) --- hddfancontrol/__init__.py | 20 ++++++++++++++------ tests/__init__.py | 34 ++++++++++++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/hddfancontrol/__init__.py b/hddfancontrol/__init__.py index 9ff36c3..0c87970 100755 --- a/hddfancontrol/__init__.py +++ b/hddfancontrol/__init__.py @@ -63,6 +63,7 @@ class Drive: DriveState = enum.Enum("DriveState", ("UNKNOWN", "ACTIVE_IDLE", "STANDBY", "SLEEPING")) HDPARM_GET_TEMP_HITACHI_REGEX = re.compile("drive temperature \(celsius\) is:\s*([0-9]*)") + HDPARM_GET_TEMP_HITACHI_ERROR_REGEX = re.compile("^SG_IO: .* sense data", re.MULTILINE) HDPARM_GET_MODEL_REGEX = re.compile("Model Number:\s*(.*)") def __init__(self, device_filepath, hddtemp_daemon_port): @@ -91,16 +92,23 @@ class Drive: def supportsHitachiTempQuery(self): # test if drive supports hdparm -H + supported = True cmd = ("hdparm", "-H", self.device_filepath) try: - subprocess.check_call(cmd, - stdin=subprocess.DEVNULL, - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL) + output = subprocess.check_output(cmd, + stdin=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + universal_newlines=True) except subprocess.CalledProcessError: + supported = False + else: + # catch non fatal errors + # see: https://github.com/Distrotech/hdparm/blob/4517550db29a91420fb2b020349523b1b4512df2/sgio.c#L308-L315 + if __class__.HDPARM_GET_TEMP_HITACHI_ERROR_REGEX.search(output) is not None: + supported = False + if not supported: self.logger.warning("Drive does not allow querying temperature without going out of low power mode.") - return False - return True + return supported def getState(self): """ Get drive power state, as a DriveState enum. """ diff --git a/tests/__init__.py b/tests/__init__.py index d1398e7..c567f52 100755 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -36,11 +36,11 @@ class TestDrive(unittest.TestCase): def setUp(self): with unittest.mock.patch("hddfancontrol.os.stat") as os_stat_mock, \ unittest.mock.patch("hddfancontrol.stat") as stat_mock, \ - unittest.mock.patch("hddfancontrol.subprocess") as subprocess_check_call_mock, \ + unittest.mock.patch("hddfancontrol.subprocess.check_output") as subprocess_check_output_mock, \ unittest.mock.patch("hddfancontrol.Drive.getPrettyName") as drive_getPrettyName: os_stat_mock.return_value = os.stat_result stat_mock.stat.S_IFBLK.return_value = True - subprocess_check_call_mock.side_effect = subprocess.CalledProcessError(0, "") + subprocess_check_output_mock.side_effect = subprocess.CalledProcessError(0, "") drive_getPrettyName.return_value = "drive_name" self.drive = hddfancontrol.Drive("/dev/sdz", None) self.hddtemp_daemon = None @@ -60,6 +60,36 @@ class TestDrive(unittest.TestCase): stderr=subprocess.DEVNULL, universal_newlines=True) + def test_supportsHitachiTempQuery(self): + with unittest.mock.patch("hddfancontrol.subprocess.check_output") as subprocess_check_output_mock: + subprocess_check_output_mock.return_value = "\n/dev/sdz:\n drive temperature (celsius) is: 30\n drive temperature in range: yes" + self.assertEqual(self.drive.supportsHitachiTempQuery(), True) + subprocess_check_output_mock.assert_called_once_with(("hdparm", "-H", "/dev/sdz"), + stdin=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + universal_newlines=True) + with unittest.mock.patch("hddfancontrol.subprocess.check_output") as subprocess_check_output_mock: + subprocess_check_output_mock.return_value = "\n/dev/sdz:\nSG_IO: questionable sense data, results may be incorrect\n drive temperature (celsius) is: -18\n drive temperature in range: yes" + self.assertEqual(self.drive.supportsHitachiTempQuery(), False) + subprocess_check_output_mock.assert_called_once_with(("hdparm", "-H", "/dev/sdz"), + stdin=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + universal_newlines=True) + with unittest.mock.patch("hddfancontrol.subprocess.check_output") as subprocess_check_output_mock: + subprocess_check_output_mock.return_value = "\n/dev/sdz:\nSG_IO: missing sense data, results may be incorrect\n drive temperature (celsius) is: -18\n drive temperature in range: yes" + self.assertEqual(self.drive.supportsHitachiTempQuery(), False) + subprocess_check_output_mock.assert_called_once_with(("hdparm", "-H", "/dev/sdz"), + stdin=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + universal_newlines=True) + with unittest.mock.patch("hddfancontrol.subprocess.check_output") as subprocess_check_output_mock: + subprocess_check_output_mock.return_value = "\n/dev/sdz:\nSG_IO: bad/missing sense data, sb[]: 70 00 05 00 00 00 00 0a 04 51 40 00 21 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00\n drive temperature (celsius) is: -18\n drive temperature in range: yes" + self.assertEqual(self.drive.supportsHitachiTempQuery(), False) + subprocess_check_output_mock.assert_called_once_with(("hdparm", "-H", "/dev/sdz"), + stdin=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + universal_newlines=True) + def test_getState(self): with unittest.mock.patch("hddfancontrol.subprocess.check_output") as subprocess_check_output_mock: subprocess_check_output_mock.return_value = "\n/dev/sdz:\n drive state is: active/idle\n"