From 2e514655194b5385526dec7bc743cd3500a4dca9 Mon Sep 17 00:00:00 2001 From: Patrick East Date: Thu, 17 Sep 2015 11:41:37 -0700 Subject: [PATCH] Wait for FC multipath devices to become writable Sometimes the multipath devices would be read-only block devices which would end up causing problems when anything tries to write to them. We now will wait and rescan a few times to try and get them to become read-write before moving on with the attach. Change-Id: If3c17348f6a6e766f811fcd446c03c82e5c8a883 Partial-Bug: #1495701 --- os_brick/exception.py | 4 + os_brick/initiator/connector.py | 10 ++ os_brick/initiator/linuxscsi.py | 34 +++++ os_brick/tests/initiator/test_connector.py | 141 +++++++++++++++++- os_brick/tests/initiator/test_linuxscsi.py | 157 +++++++++++++++++++++ 5 files changed, 345 insertions(+), 1 deletion(-) diff --git a/os_brick/exception.py b/os_brick/exception.py index c928773d5..f384d028e 100644 --- a/os_brick/exception.py +++ b/os_brick/exception.py @@ -112,3 +112,7 @@ class TargetPortalNotFound(BrickException): class FailedISCSITargetPortalLogin(BrickException): message = _("Unable to login to iSCSI Target Portal") + + +class BlockDeviceReadOnly(BrickException): + message = _("Block device %(device)s is Read-Only.") diff --git a/os_brick/initiator/connector.py b/os_brick/initiator/connector.py index cbab35036..2b8a3ddb3 100644 --- a/os_brick/initiator/connector.py +++ b/os_brick/initiator/connector.py @@ -1051,6 +1051,16 @@ class FibreChannelConnector(InitiatorConnector): LOG.debug("Unable to find multipath device name for " "volume. Using path %(device)s for volume.", {'device': self.host_device}) + + if connection_properties.get('access_mode', '') != 'ro': + try: + # Sometimes the multipath devices will show up as read only + # initially and need additional time/rescans to get to RW. + self._linuxscsi.wait_for_rw(device_wwn, device_path) + except exception.BlockDeviceReadOnly: + LOG.warning(_LW('Block device %s is still read-only. ' + 'Continuing anyway.'), device_path) + else: device_path = self.host_device diff --git a/os_brick/initiator/linuxscsi.py b/os_brick/initiator/linuxscsi.py index 9b7d22922..8072e3153 100644 --- a/os_brick/initiator/linuxscsi.py +++ b/os_brick/initiator/linuxscsi.py @@ -164,6 +164,40 @@ class LinuxSCSI(executor.Executor): else: LOG.debug("%s has shown up.", volume_path) + @utils.retry(exceptions=exception.BlockDeviceReadOnly, retries=5) + def wait_for_rw(self, wwn, device_path): + """Wait for block device to be Read-Write.""" + LOG.debug("Checking to see if %s is read-only.", + device_path) + out, info = self._execute('lsblk', '-o', 'NAME,RO', '-l', '-n') + LOG.debug("lsblk output: %s", out) + blkdevs = out.splitlines() + for blkdev in blkdevs: + # Entries might look like: + # + # "3624a93709a738ed78583fd120013902b (dm-1) 1" + # + # or + # + # "sdd 0" + # + # We are looking for the first and last part of them. For FC + # multipath devices the name is in the format of ' (dm-)' + blkdev_parts = blkdev.split(' ') + ro = blkdev_parts[-1] + name = blkdev_parts[0] + + # We must validate that all pieces of the dm-# device are rw, + # if some are still ro it can cause problems. + if wwn in name and int(ro) == 1: + LOG.debug("Block device %s is read-only", device_path) + self._execute('multipath', '-r', check_exit_code=[0, 1, 21], + run_as_root=True, root_helper=self._root_helper) + raise exception.BlockDeviceReadOnly( + device=device_path) + else: + LOG.debug("Block device %s is not read-only.", device_path) + def find_multipath_device_path(self, wwn): """Look for the multipath device file for a volume WWN. diff --git a/os_brick/tests/initiator/test_connector.py b/os_brick/tests/initiator/test_connector.py index 82d78533a..b95751154 100644 --- a/os_brick/tests/initiator/test_connector.py +++ b/os_brick/tests/initiator/test_connector.py @@ -1090,6 +1090,7 @@ class FibreChannelConnectorTestCase(ConnectorTestCase): '-fc-0x1234567890123456-lun-1'] self.assertEqual(expected, volume_paths) + @mock.patch.object(linuxscsi.LinuxSCSI, 'wait_for_rw') @mock.patch.object(os.path, 'exists', return_value=True) @mock.patch.object(os.path, 'realpath', return_value='/dev/sdb') @mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas') @@ -1101,7 +1102,10 @@ class FibreChannelConnectorTestCase(ConnectorTestCase): get_scsi_wwn_mock, remove_device_mock, get_fc_hbas_info_mock, - get_fc_hbas_mock, realpath_mock, exists_mock): + get_fc_hbas_mock, + realpath_mock, + exists_mock, + wait_for_rw_mock): get_fc_hbas_mock.side_effect = self.fake_get_fc_hbas get_fc_hbas_info_mock.side_effect = self.fake_get_fc_hbas_info @@ -1149,6 +1153,141 @@ class FibreChannelConnectorTestCase(ConnectorTestCase): self.connector.connect_volume, connection_info['data']) + def _test_connect_volume_multipath(self, get_device_info_mock, + get_scsi_wwn_mock, + remove_device_mock, + get_fc_hbas_info_mock, + get_fc_hbas_mock, + realpath_mock, + exists_mock, + wait_for_rw_mock, + find_mp_dev_mock, + access_mode, + should_wait_for_rw): + self.connector.use_multipath = True + get_fc_hbas_mock.side_effect = self.fake_get_fc_hbas + get_fc_hbas_info_mock.side_effect = self.fake_get_fc_hbas_info + + wwn = '1234567890' + multipath_devname = '/dev/md-1' + devices = {"device": multipath_devname, + "id": wwn, + "devices": [{'device': '/dev/sdb', + 'address': '1:0:0:1', + 'host': 1, 'channel': 0, + 'id': 0, 'lun': 1}]} + get_device_info_mock.return_value = devices['devices'][0] + get_scsi_wwn_mock.return_value = wwn + + location = '10.0.2.15:3260' + name = 'volume-00000001' + vol = {'id': 1, 'name': name} + initiator_wwn = ['1234567890123456', '1234567890123457'] + + find_mp_dev_mock.return_value = '/dev/disk/by-id/dm-uuid-mpath-' + wwn + + connection_info = self.fibrechan_connection(vol, location, + initiator_wwn) + connection_info['data']['access_mode'] = access_mode + + self.connector.connect_volume(connection_info['data']) + + self.assertEqual(should_wait_for_rw, wait_for_rw_mock.called) + + @mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device') + @mock.patch.object(linuxscsi.LinuxSCSI, 'wait_for_rw') + @mock.patch.object(os.path, 'exists', return_value=True) + @mock.patch.object(os.path, 'realpath', return_value='/dev/sdb') + @mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas') + @mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas_info') + @mock.patch.object(linuxscsi.LinuxSCSI, 'remove_scsi_device') + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn') + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_info') + def test_connect_volume_multipath_rw(self, get_device_info_mock, + get_scsi_wwn_mock, + remove_device_mock, + get_fc_hbas_info_mock, + get_fc_hbas_mock, + realpath_mock, + exists_mock, + wait_for_rw_mock, + find_mp_dev_mock): + + self._test_connect_volume_multipath(get_device_info_mock, + get_scsi_wwn_mock, + remove_device_mock, + get_fc_hbas_info_mock, + get_fc_hbas_mock, + realpath_mock, + exists_mock, + wait_for_rw_mock, + find_mp_dev_mock, + 'rw', + True) + + @mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device') + @mock.patch.object(linuxscsi.LinuxSCSI, 'wait_for_rw') + @mock.patch.object(os.path, 'exists', return_value=True) + @mock.patch.object(os.path, 'realpath', return_value='/dev/sdb') + @mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas') + @mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas_info') + @mock.patch.object(linuxscsi.LinuxSCSI, 'remove_scsi_device') + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn') + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_info') + def test_connect_volume_multipath_no_access_mode(self, + get_device_info_mock, + get_scsi_wwn_mock, + remove_device_mock, + get_fc_hbas_info_mock, + get_fc_hbas_mock, + realpath_mock, + exists_mock, + wait_for_rw_mock, + find_mp_dev_mock): + + self._test_connect_volume_multipath(get_device_info_mock, + get_scsi_wwn_mock, + remove_device_mock, + get_fc_hbas_info_mock, + get_fc_hbas_mock, + realpath_mock, + exists_mock, + wait_for_rw_mock, + find_mp_dev_mock, + None, + True) + + @mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device') + @mock.patch.object(linuxscsi.LinuxSCSI, 'wait_for_rw') + @mock.patch.object(os.path, 'exists', return_value=True) + @mock.patch.object(os.path, 'realpath', return_value='/dev/sdb') + @mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas') + @mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas_info') + @mock.patch.object(linuxscsi.LinuxSCSI, 'remove_scsi_device') + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn') + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_info') + def test_connect_volume_multipath_ro(self, get_device_info_mock, + get_scsi_wwn_mock, + remove_device_mock, + get_fc_hbas_info_mock, + get_fc_hbas_mock, + realpath_mock, + exists_mock, + wait_for_rw_mock, + find_mp_dev_mock): + + self._test_connect_volume_multipath(get_device_info_mock, + get_scsi_wwn_mock, + remove_device_mock, + get_fc_hbas_info_mock, + get_fc_hbas_mock, + realpath_mock, + exists_mock, + wait_for_rw_mock, + find_mp_dev_mock, + 'ro', + False) + class FibreChannelConnectorS390XTestCase(ConnectorTestCase): diff --git a/os_brick/tests/initiator/test_linuxscsi.py b/os_brick/tests/initiator/test_linuxscsi.py index 52d97f964..11fdeee78 100644 --- a/os_brick/tests/initiator/test_linuxscsi.py +++ b/os_brick/tests/initiator/test_linuxscsi.py @@ -300,3 +300,160 @@ class LinuxSCSITestCase(base.TestCase): self.assertEqual("1", info['devices'][1]['channel']) self.assertEqual("0", info['devices'][1]['id']) self.assertEqual("3", info['devices'][1]['lun']) + + @mock.patch.object(time, 'sleep') + def test_wait_for_rw(self, mock_sleep): + lsblk_output = """3624a93709a738ed78583fd1200143029 (dm-2) 0 +sdb 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdc 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 0 +sdd 0 +3624a93709a738ed78583fd1200143029 (dm-2) 0 +sde 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdf 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 0 +sdg 0 +3624a93709a738ed78583fd1200143029 (dm-2) 0 +sdh 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdi 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 0 +sdj 0 +3624a93709a738ed78583fd1200143029 (dm-2) 0 +sdk 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdl 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 0 +sdm 0 +vda1 0 +vdb 0 +vdb1 0 +loop0 0""" + + mock_execute = mock.Mock() + mock_execute.return_value = (lsblk_output, None) + self.linuxscsi._execute = mock_execute + + wwn = '3624a93709a738ed78583fd120014a2bb' + path = '/dev/disk/by-id/dm-uuid-mpath-' + wwn + + # Ensure no exception is raised and no sleep is called + self.linuxscsi.wait_for_rw(wwn, path) + self.assertFalse(mock_sleep.called) + + @mock.patch.object(time, 'sleep') + def test_wait_for_rw_needs_retry(self, mock_sleep): + lsblk_ro_output = """3624a93709a738ed78583fd1200143029 (dm-2) 0 +sdb 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdc 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 0 +sdd 0 +3624a93709a738ed78583fd1200143029 (dm-2) 1 +sde 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdf 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 0 +sdg 0 +3624a93709a738ed78583fd1200143029 (dm-2) 1 +sdh 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdi 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 0 +sdj 0 +3624a93709a738ed78583fd1200143029 (dm-2) 1 +sdk 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdl 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 0 +sdm 0 +vda1 0 +vdb 0 +vdb1 0 +loop0 0""" + lsblk_rw_output = """3624a93709a738ed78583fd1200143029 (dm-2) 0 +sdb 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdc 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 0 +sdd 0 +3624a93709a738ed78583fd1200143029 (dm-2) 0 +sde 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdf 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 0 +sdg 0 +3624a93709a738ed78583fd1200143029 (dm-2) 0 +sdh 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdi 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 0 +sdj 0 +3624a93709a738ed78583fd1200143029 (dm-2) 0 +sdk 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdl 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 0 +sdm 0 +vda1 0 +vdb 0 +vdb1 0 +loop0 0""" + mock_execute = mock.Mock() + mock_execute.side_effect = [(lsblk_ro_output, None), + ('', None), # multipath -r output + (lsblk_rw_output, None)] + self.linuxscsi._execute = mock_execute + + wwn = '3624a93709a738ed78583fd1200143029' + path = '/dev/disk/by-id/dm-uuid-mpath-' + wwn + + self.linuxscsi.wait_for_rw(wwn, path) + self.assertEqual(1, mock_sleep.call_count) + + @mock.patch.object(time, 'sleep') + def test_wait_for_rw_always_readonly(self, mock_sleep): + lsblk_output = """3624a93709a738ed78583fd1200143029 (dm-2) 0 +sdb 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdc 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 1 +sdd 0 +3624a93709a738ed78583fd1200143029 (dm-2) 0 +sde 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdf 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 1 +sdg 0 +3624a93709a738ed78583fd1200143029 (dm-2) 0 +sdh 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdi 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 1 +sdj 0 +3624a93709a738ed78583fd1200143029 (dm-2) 0 +sdk 0 +3624a93709a738ed78583fd120014724e (dm-1) 0 +sdl 0 +3624a93709a738ed78583fd120014a2bb (dm-0) 1 +sdm 0 +vda1 0 +vdb 0 +vdb1 0 +loop0 0""" + + mock_execute = mock.Mock() + mock_execute.return_value = (lsblk_output, None) + self.linuxscsi._execute = mock_execute + + wwn = '3624a93709a738ed78583fd120014a2bb' + path = '/dev/disk/by-id/dm-uuid-mpath-' + wwn + + self.assertRaises(exception.BlockDeviceReadOnly, + self.linuxscsi.wait_for_rw, + wwn, + path) + + self.assertEqual(4, mock_sleep.call_count)