From 7fb37c2000a148f37bc8a6c17e913c46865776bd Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Mon, 1 Jun 2020 17:35:48 +0200 Subject: [PATCH] Improve WWN detection To avoid issues with the scsi_id command getting stuck and blocking the attachment we use sysfs to search for the WWN, but it can happen that we fail to detect the WWN even if it's present in sysfs. This happens when the storage array has multiple designators and the multipath daemon detects the multipaths very fast. The flow is: - os-brick attaches volumes using iscsiadm --login - udev generates the symlink with the WWN (this is the one we want) - multipathd detects the volumes and forms the DM - udev replaces the previous symlink to point to the multipath DM - os-brick checks the symlink This patch adds code to get_sysfs_wwn that checks the individual devices belonging to a multipath DM if the symlink points to a DM. Closes-Bug: #1881608 Change-Id: I05f94d31277efec28ad50ae2f3502ab6fccfe37c --- os_brick/initiator/linuxscsi.py | 26 +++++++++-- os_brick/tests/initiator/test_linuxscsi.py | 44 +++++++++++++++++-- ...mprove-get_sysfs_wwn-df38ea88cdcdcc94.yaml | 5 +++ 3 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/improve-get_sysfs_wwn-df38ea88cdcdcc94.yaml diff --git a/os_brick/initiator/linuxscsi.py b/os_brick/initiator/linuxscsi.py index 74eec5002..3c42f0aa7 100644 --- a/os_brick/initiator/linuxscsi.py +++ b/os_brick/initiator/linuxscsi.py @@ -128,16 +128,34 @@ class LinuxSCSI(executor.Executor): if wwid and glob_str + wwid in wwn_paths: return wwid - # If we have multiple designators follow the symlinks + # If we have multiple designators use symlinks to find out the wwn + device_names = set(device_names) for wwn_path in wwn_paths: try: if os.path.islink(wwn_path) and os.stat(wwn_path): path = os.path.realpath(wwn_path) - if path.startswith('/dev/') and path[5:] in device_names: - return wwn_path[len(glob_str):] + if path.startswith('/dev/'): + name = path[5:] + # Symlink may point to the multipath dm if the attach + # was too fast or we took long to check it. Check + # devices belonging to the multipath DM. + if name.startswith('dm-'): + # Get the devices that belong to the DM + slaves_path = '/sys/class/block/%s/slaves' % name + dm_devs = os.listdir(slaves_path) + # This is the right wwn_path if the devices we have + # attached belong to the dm we followed + if device_names.intersection(dm_devs): + break + + # This is the right wwn_path if devices we have + elif name in device_names: + break except OSError: continue - return '' + else: + return '' + return wwn_path[len(glob_str):] def get_sysfs_wwid(self, device_names): """Return the wwid from sysfs in any of devices in udev format.""" diff --git a/os_brick/tests/initiator/test_linuxscsi.py b/os_brick/tests/initiator/test_linuxscsi.py index 1f2c9afc7..13d806aa4 100644 --- a/os_brick/tests/initiator/test_linuxscsi.py +++ b/os_brick/tests/initiator/test_linuxscsi.py @@ -874,25 +874,53 @@ loop0 0""" glob_mock.assert_called_once_with('/dev/disk/by-id/scsi-*') get_wwid_mock.assert_called_once_with(mock.sentinel.device_names) + @mock.patch('os.listdir', return_value=['sda', 'sdd']) @mock.patch('os.path.realpath', side_effect=('/other/path', + '/dev/dm-5', '/dev/sda', '/dev/sdb')) - @mock.patch('os.path.islink', side_effect=(False, True, True, True, True)) - @mock.patch('os.stat', side_effect=(False, True, True, True)) + @mock.patch('os.path.islink', side_effect=(False,) + (True,) * 5) + @mock.patch('os.stat', side_effect=(False,) + (True,) * 4) @mock.patch('glob.glob') @mock.patch.object(linuxscsi.LinuxSCSI, 'get_sysfs_wwid') def test_get_sysfs_wwn_multiple_designators(self, get_wwid_mock, glob_mock, stat_mock, islink_mock, - realpath_mock): + realpath_mock, listdir_mock): glob_mock.return_value = ['/dev/disk/by-id/scsi-fail-link', '/dev/disk/by-id/scsi-fail-stat', '/dev/disk/by-id/scsi-non-dev', + '/dev/disk/by-id/scsi-another-dm', '/dev/disk/by-id/scsi-wwid1', '/dev/disk/by-id/scsi-wwid2'] + get_wwid_mock.return_value = 'pre-wwid' devices = ['sdb', 'sdc'] res = self.linuxscsi.get_sysfs_wwn(devices) self.assertEqual('wwid2', res) glob_mock.assert_called_once_with('/dev/disk/by-id/scsi-*') + listdir_mock.assert_called_once_with('/sys/class/block/dm-5/slaves') + get_wwid_mock.assert_called_once_with(devices) + + @mock.patch('os.listdir', side_effect=[['sda', 'sdb'], ['sdc', 'sdd']]) + @mock.patch('os.path.realpath', side_effect=('/dev/sde', + '/dev/dm-5', + '/dev/dm-6')) + @mock.patch('os.path.islink', mock.Mock()) + @mock.patch('os.stat', mock.Mock()) + @mock.patch('glob.glob') + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_sysfs_wwid', return_value='') + def test_get_sysfs_wwn_dm_link(self, get_wwid_mock, glob_mock, + realpath_mock, listdir_mock): + glob_mock.return_value = ['/dev/disk/by-id/scsi-wwid1', + '/dev/disk/by-id/scsi-another-dm', + '/dev/disk/by-id/scsi-our-dm'] + + devices = ['sdc', 'sdd'] + res = self.linuxscsi.get_sysfs_wwn(devices) + self.assertEqual('our-dm', res) + glob_mock.assert_called_once_with('/dev/disk/by-id/scsi-*') + listdir_mock.assert_has_calls( + [mock.call('/sys/class/block/dm-5/slaves'), + mock.call('/sys/class/block/dm-6/slaves')]) get_wwid_mock.assert_called_once_with(devices) @mock.patch('os.path.realpath', side_effect=('/dev/sda', '/dev/sdb')) @@ -911,6 +939,16 @@ loop0 0""" glob_mock.assert_called_once_with('/dev/disk/by-id/scsi-*') get_wwid_mock.assert_called_once_with(devices) + @mock.patch('glob.glob', return_value=[]) + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_sysfs_wwid') + def test_get_sysfs_wwn_no_links(self, get_wwid_mock, glob_mock): + get_wwid_mock.return_value = '' + devices = ['sdc'] + res = self.linuxscsi.get_sysfs_wwn(devices) + self.assertEqual('', res) + glob_mock.assert_called_once_with('/dev/disk/by-id/scsi-*') + get_wwid_mock.assert_called_once_with(devices) + @ddt.data({'wwn_type': 't10.', 'num_val': '1'}, {'wwn_type': 'eui.', 'num_val': '2'}, {'wwn_type': 'naa.', 'num_val': '3'}) diff --git a/releasenotes/notes/improve-get_sysfs_wwn-df38ea88cdcdcc94.yaml b/releasenotes/notes/improve-get_sysfs_wwn-df38ea88cdcdcc94.yaml new file mode 100644 index 000000000..443c124df --- /dev/null +++ b/releasenotes/notes/improve-get_sysfs_wwn-df38ea88cdcdcc94.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Improve WWN detection for arrays with multiple designators. + (bug 1881608).