From 1872b734bd11ddc87b7d3909124445d0d01f895a Mon Sep 17 00:00:00 2001 From: Dmitry Bogun Date: Fri, 10 Feb 2017 14:57:04 +0200 Subject: [PATCH] Certain subset of hardware may lead to failue The old way to detect SCSI address of block devices rely on incorrect assumption. It rely that folder /sys/class/scsi_device/{scsi_address}/device/block exist for any SCSI device. And because this assumption is incorrect, if fails with unhandled exception, on such hardware. Rewrite core responsible for detection SCSI address of block devices. Change-Id: I3c9938e145fc8c6d3036fdb769fd20f8b8f63678 --- bareon/tests/test_block_device.py | 14 +++++++--- bareon/tests/test_hardware_utils.py | 24 ++++++++++++++++- bareon/utils/block_device.py | 12 ++++++--- bareon/utils/hardware.py | 40 ++++++++++++++++------------- 4 files changed, 64 insertions(+), 26 deletions(-) diff --git a/bareon/tests/test_block_device.py b/bareon/tests/test_block_device.py index 2dbe9b4..c16803c 100644 --- a/bareon/tests/test_block_device.py +++ b/bareon/tests/test_block_device.py @@ -1,5 +1,5 @@ # -# Copyright 2015 Cray Inc. All Rights Reserved. +# Copyright 2017 Cray Inc. All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -63,29 +63,37 @@ class TestDeviceFinder(unittest2.TestCase): self.block_device_list = mock.Mock() self.device_info = mock.Mock() + self.dev_to_scsi_map = mock.Mock() for path, m in ( ('bareon.utils.hardware.' 'get_block_data_from_udev', self.block_device_list), ('bareon.utils.hardware.' - 'get_device_info', self.device_info)): + 'get_device_info', self.device_info), + ('bareon.utils.hardware.' + 'dev_to_scsi_map', self.dev_to_scsi_map)): patch = mock.patch(path, m) patch.start() self.addCleanup(patch.stop) + self.dev_to_scsi_map.return_value = {} + def test(self): self.block_device_list.side_effect = [ ['/dev/sda'], []] self.device_info.side_effect = [ self._device_info['sample0']] + self.dev_to_scsi_map.return_value = { + '/dev/sda': '0:0:0:0'} finder = block_device.DeviceFinder() for kind, needle in ( ('name', 'sda'), ('name', '/dev/sda'), ('path', 'disk/by-id/wwn-0x5000c50060fce0bf'), - ('path', '/dev/disk/by-id/wwn-0x5000c50060fce0bf')): + ('path', '/dev/disk/by-id/wwn-0x5000c50060fce0bf'), + ('scsi', '0:0:0:0')): dev = finder(kind, needle) self.assertEqual(dev['uspec']['DEVNAME'], '/dev/sda') diff --git a/bareon/tests/test_hardware_utils.py b/bareon/tests/test_hardware_utils.py index 22da813..a20b753 100644 --- a/bareon/tests/test_hardware_utils.py +++ b/bareon/tests/test_hardware_utils.py @@ -1,4 +1,5 @@ -# Copyright 2014 Mirantis, Inc. +# +# Copyright 2017 Cray Inc. All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -12,6 +13,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import errno + import six import unittest2 @@ -165,6 +168,25 @@ supports-register-dump: yes '--name=/dev/fake', check_exit_code=[0]) + @mock.patch('os.listdir') + def test_dev_to_scsi_map(self, mock_listdir): + mock_listdir.side_effect = [ + ('0:0:0:0', '8:0:0:0'), + ('sda', ), + OSError( + errno.ENOENT, + 'No such file or directory', + '/sys/class/scsi_device/8:0:0:0/device/block')] + + expected = {'/dev/sda': '0:0:0:0'} + actual = hu.dev_to_scsi_map() + self.assertEqual(expected, actual) + self.assertEqual( + mock_listdir.call_args_list, [ + mock.call('/sys/class/scsi_device'), + mock.call('/sys/class/scsi_device/0:0:0:0/device/block'), + mock.call('/sys/class/scsi_device/8:0:0:0/device/block')]) + def test_multipath_true(self): uspec = { 'DEVLINKS': ['/dev/disk/by-id/fakeid1', diff --git a/bareon/utils/block_device.py b/bareon/utils/block_device.py index ae1be28..d3a0a06 100644 --- a/bareon/utils/block_device.py +++ b/bareon/utils/block_device.py @@ -1,5 +1,5 @@ # -# Copyright 2015 Cray Inc. All Rights Reserved. +# Copyright 2017 Cray Inc. All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -71,6 +71,8 @@ class DeviceFinder(object): self.dev_by_scsi = {} self.dev_by_path = {} + self._dev_to_scsi = hardware.dev_to_scsi_map() + disks = hardware.get_block_data_from_udev('disk') partitions = hardware.get_block_data_from_udev('partition') for dev in itertools.chain(disks, partitions): @@ -81,7 +83,6 @@ class DeviceFinder(object): def _parse(self, record): dev = record['uspec']['DEVNAME'] - record['scsi'] = hardware.scsi_address(dev) self.dev_list.append(record) @@ -89,9 +90,12 @@ class DeviceFinder(object): self.dev_by_name[self._cut_prefix(dev, '/dev/')] = record self.dev_by_path[dev] = record - scsi_addr = record['scsi'] - if scsi_addr: + try: + scsi_addr = self._dev_to_scsi[dev] self.dev_by_scsi[scsi_addr] = record + record['scsi'] = scsi_addr + except KeyError: + record['scsi'] = None for p in record['uspec'].get('DEVLINKS', ()): match_uuid = re.search( diff --git a/bareon/utils/hardware.py b/bareon/utils/hardware.py index 8837cdc..6c63de6 100644 --- a/bareon/utils/hardware.py +++ b/bareon/utils/hardware.py @@ -1,4 +1,5 @@ -# Copyright 2014 Mirantis, Inc. +# +# Copyright 2017 Cray Inc. All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -12,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import errno import os import re import stat @@ -255,29 +257,31 @@ def extrareport(dev): return spec +def dev_to_scsi_map(): + dev_to_scsi = {} + + sys_path = '/sys/class/scsi_device' + block_device_mapping = 'device/block' + + for scsi_addr in os.listdir(sys_path): + mapping_path = os.path.join(sys_path, scsi_addr, block_device_mapping) + try: + for block_devices in os.listdir(mapping_path): + block_devices = os.path.join('/dev', block_devices) + dev_to_scsi[block_devices] = scsi_addr + except OSError as e: + if e.errno != errno.ENOENT: + raise + + return dev_to_scsi + + def is_block_device(filepath): """Check whether `filepath` is a block device.""" mode = os.stat(filepath).st_mode return stat.S_ISBLK(mode) -def scsi_address_list(): - scsi_sg_path = '/proc/scsi/sg/' - try: - scsi_devices = open(scsi_sg_path + 'devices').read().splitlines() - except IOError: - return [] - else: - return [':'.join(dev.split()[:4]) for dev in scsi_devices] - - -def scsi_address(dev): - for address in scsi_address_list(): - scsi_path = '/sys/class/scsi_device/%s/device/block/' % address - if dev == os.path.join('/dev', os.listdir(scsi_path)[0]): - return address - - def is_multipath_device(device, uspec=None): """Check whether block device with given uspec is multipath device""" if uspec is None: