From 0dc4daa2140bd9d96c17269ceeef2758c678f7fd Mon Sep 17 00:00:00 2001 From: GirishChilukuri Date: Wed, 27 Jan 2021 04:43:16 +0000 Subject: [PATCH] [SVF]:Fix multiple lshost calls during attach. [Spectrum Virtualize Family] During attach or detach operation there is multiple lshost calls from "get_host_from_connector", which is causing some delay during attach or detach operations. This patch fixes the issue by caching the host information during initialization and searching for host from the cached information. Closes-Bug: #1913363 Change-Id: I73e58f8b0f4908e9fb097f7ce151a9178cf6b96f --- .../volume/drivers/ibm/test_storwize_svc.py | 59 ++++++++----- .../ibm/storwize_svc/storwize_svc_common.py | 84 +++++++++++++++---- .../ibm/storwize_svc/storwize_svc_fc.py | 24 ++++-- .../ibm/storwize_svc/storwize_svc_iscsi.py | 26 ++++-- ..._calls_during_attach-528f92b44a0ff6b8.yaml | 8 ++ 5 files changed, 152 insertions(+), 49 deletions(-) create mode 100644 releasenotes/notes/bug-1913363-ibm-svf_Fix_multiple_lshost_calls_during_attach-528f92b44a0ff6b8.yaml diff --git a/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py b/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py index a51b4b20bcb..b447985d9f9 100644 --- a/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py +++ b/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py @@ -176,6 +176,9 @@ class StorwizeSVCManagementSimulator(object): 'CMMVC8783E': ('', 'CMMVC8783E The volume copy was not deleted ' 'because the volume is part of a consistency ' 'group.'), + 'CMMVC6578E': ('', 'CMMVC6578E The command has failed because ' + 'the iSCSI name is already assigned or is ' + 'not valid.'), } self._fc_transitions = {'begin': {'make': 'idle_or_copied'}, 'idle_or_copied': {'prepare': 'preparing', @@ -1207,7 +1210,10 @@ port_speed!N/A continue for port in v[added_key]: if port == added_val: - return self._errors['CMMVC6581E'] + error = 'CMMVC6035E' + if 'iscsiname' in kwargs: + error = 'CMMVC6578E' + return self._errors[error] return ('', '') # Make a host @@ -3262,7 +3268,10 @@ class StorwizeSVCISCSIDriverTestCase(test.TestCase): snapshot, connector) - def test_storwize_initialize_iscsi_connection_with_host_site(self): + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'initialize_host_info') + def test_storwize_initialize_iscsi_connection_with_host_site(self, + init_host): connector = {'host': 'storwize-svc-host', 'wwnns': ['20000090fa17311e', '20000090fa17311f'], 'wwpns': ['ff00000000000000', 'ff00000000000001'], @@ -3277,6 +3286,8 @@ class StorwizeSVCISCSIDriverTestCase(test.TestCase): volume_iSCSI_2 = self._create_volume() volume_iSCSI_2['volume_type_id'] = vol_type_iSCSI['id'] self.iscsi_driver.initialize_connection(volume_iSCSI, connector) + init_host.assert_called() + self.assertEqual(1, init_host.call_count) host_name = self.iscsi_driver._helpers.get_host_from_connector( connector, iscsi=True) host_info = self.iscsi_driver._helpers.ssh.lshost(host=host_name) @@ -3335,7 +3346,10 @@ class StorwizeSVCISCSIDriverTestCase(test.TestCase): volume_iSCSI, connector) term_conn.assert_called_once_with(volume_iSCSI, connector) - def test_storwize_terminate_iscsi_connection_multi_attach(self): + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'initialize_host_info') + def test_storwize_terminate_iscsi_connection_multi_attach(self, + init_host_info): # create an iSCSI volume volume_iSCSI = self._create_volume() extra_spec = {'capabilities:storage_protocol': ' iSCSI'} @@ -3353,6 +3367,7 @@ class StorwizeSVCISCSIDriverTestCase(test.TestCase): # map and unmap the volume to two hosts normal case self.iscsi_driver.initialize_connection(volume_iSCSI, connector) + init_host_info.assert_called() self.iscsi_driver.initialize_connection(volume_iSCSI, connector2) for conn in [connector, connector2]: host = self.iscsi_driver._helpers.get_host_from_connector( @@ -3380,6 +3395,7 @@ class StorwizeSVCISCSIDriverTestCase(test.TestCase): rmmap.side_effect = Exception('boom') self.iscsi_driver.terminate_connection(volume_iSCSI, connector2) + init_host_info.assert_called() host_name = self.iscsi_driver._helpers.get_host_from_connector( connector2, iscsi=True) self.assertIsNone(host_name) @@ -4399,12 +4415,6 @@ class StorwizeSVCFcDriverTestCase(test.TestCase): # Check bad output from lsfabric for the 2nd volume if protocol == 'FC' and self.USESIM: - for error in ['remove_field', 'header_mismatch']: - self.sim.error_injection('lsfabric', error) - self.assertRaises(exception.VolumeBackendAPIException, - self.fc_driver.initialize_connection, - volume2, self._connector) - with mock.patch.object(storwize_svc_common.StorwizeHelpers, 'get_conn_fc_wwpns') as conn_fc_wwpns: conn_fc_wwpns.return_value = [] @@ -9714,17 +9724,17 @@ class StorwizeHelpersTestCase(test.TestCase): access=access) startrcrelationship.assert_called_once_with(opts['RC_name'], None) - @mock.patch.object(storwize_svc_common.StorwizeSSH, 'lsfabric') + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'get_host_from_host_info') @mock.patch.object(storwize_svc_common.StorwizeSSH, 'lshost') @mock.patch.object(storwize_svc_common.StorwizeSSH, 'lsvdiskhostmap') def test_get_host_from_connector_with_vol(self, lsvdishosmap, lshost, - lsfabric): + get_host_from_host_info): vol = 'testvol' connector = {"wwpns": ["10000090fa3870d7", "C050760825191B00"]} - lsfabric.return_value = [{"remote_wwpn": "10000090fa3870d7", - "name": "test_host"}] + get_host_from_host_info.return_value = "test_host", [] raw = "id!name!host_id!host_name!vdisk_UID\n2594!testvol!315!"\ "test_host!60050768028110A4700000000001168E" ssh_cmd = ['svcinfo', 'lsvdiskhostmap', '-delim', '!', '"%s"' % vol] @@ -9735,23 +9745,23 @@ class StorwizeHelpersTestCase(test.TestCase): host = self.storwize_svc_common.get_host_from_connector(connector, vol) self.assertEqual(host, "test_host") - lsfabric.assert_called_with(wwpn='10000090fa3870d7') - self.assertEqual(1, lsfabric.call_count) + get_host_from_host_info.assert_called_with(connector, False) + self.assertEqual(1, get_host_from_host_info.call_count) lsvdishosmap.assert_called_with(vol) self.assertEqual(1, lsvdishosmap.call_count) lshost.assert_not_called() - @mock.patch.object(storwize_svc_common.StorwizeSSH, 'lsfabric') + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'get_host_from_host_info') @mock.patch.object(storwize_svc_common.StorwizeSSH, 'lshost') @mock.patch.object(storwize_svc_common.StorwizeSSH, 'lsvdiskhostmap') def test_get_host_from_connector_wo_vol(self, lsvdishosmap, lshost, - lsfabric): + get_host_from_host_info): vol = 'testvol' connector = {"wwpns": ["10000090fa3870d7", "C050760825191B00"]} - lsfabric.return_value = [{"remote_wwpn": "10000090fa3870d7", - "name": "test_host"}] + get_host_from_host_info.return_value = "test_host", [] raw = "id!name!host_id!host_name!vdisk_UID\n2594!testvol!315!"\ "test_host!60050768028110A4700000000001168E" ssh_cmd = ['svcinfo', 'lsvdiskhostmap', '-delim', '!', '"%s"' % vol] @@ -9761,8 +9771,8 @@ class StorwizeHelpersTestCase(test.TestCase): True) host = self.storwize_svc_common.get_host_from_connector(connector) self.assertEqual(host, "test_host") - lsfabric.assert_called_with(wwpn='10000090fa3870d7') - self.assertEqual(1, lsfabric.call_count) + get_host_from_host_info.assert_called_with(connector, False) + self.assertEqual(1, get_host_from_host_info.call_count) lsvdishosmap.assert_not_called() self.assertEqual(0, lsvdishosmap.call_count) lshost.assert_not_called() @@ -12553,11 +12563,14 @@ class StorwizeSVCReplicationTestCase(test.TestCase): self.driver._sync_with_aux_grp(self.ctxt, rccg_name) startrccg.assert_called_with(rccg_name, 'aux') + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'initialize_host_info') @mock.patch.object(storwize_svc_common.StorwizeHelpers, 'get_host_from_connector') @mock.patch.object(storwize_svc_common.StorwizeHelpers, 'check_vol_mapped_to_host') - def test_get_map_info_from_connector(self, is_mapped, get_host_from_conn): + def test_get_map_info_from_connector(self, is_mapped, get_host_from_conn, + initialize_host_info): self.driver.configuration.set_override('replication_device', [self.rep_target]) self.driver.do_setup(self.ctxt) @@ -12584,6 +12597,8 @@ class StorwizeSVCReplicationTestCase(test.TestCase): backend_helper) self.assertEqual(self.driver._master_state, node_state) + initialize_host_info.assert_called() + self.assertEqual(1, initialize_host_info.call_count) connector = {'host': 'storwize-svc-host', 'wwnns': ['20000090fa17311e', '20000090fa17311f'], diff --git a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py index fc19f1afd5d..d4de2fd02da 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py +++ b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py @@ -788,6 +788,7 @@ class StorwizeHelpers(object): self.check_fcmapping_interval = 3 self.code_level = None self.stats = {} + self.Host_connector_info = {"FC": {}, "ISCSI": {}} @staticmethod def handle_keyerror(cmd, out): @@ -1121,28 +1122,48 @@ class StorwizeHelpers(object): wwpns.add(wwpn) return list(wwpns) + def initialize_host_info(self): + """Get the host,wwpn,iscsi and store in Host_connector_info.""" + if (not self.Host_connector_info['FC'] and + not self.Host_connector_info['ISCSI']): + hosts_info = self.ssh.lshost() + host_list = list(hosts_info.select('name')) + for eachhost in host_list: + resp = self.ssh.lshost(host=eachhost) + if list(resp.select("WWPN")) != [None]: + for wwpn in resp.select('WWPN'): + if wwpn not in self.Host_connector_info['FC'].keys(): + self.Host_connector_info['FC'][wwpn] = eachhost + elif list(resp.select('iscsi_name')) != [None]: + for iscsi_name in resp.select('iscsi_name'): + if (iscsi_name not in + self.Host_connector_info['ISCSI'].keys()): + self.Host_connector_info['ISCSI'][iscsi_name] = ( + eachhost) + + def get_host_from_host_info(self, connector, iscsi=False): + host_name = None + new_wwpn = [] + if iscsi and 'initiator' in connector: + if connector['initiator'] in self.Host_connector_info['ISCSI']: + iqn = connector['initiator'] + host_name = self.Host_connector_info['ISCSI'][iqn] + elif 'wwpns' in connector: + for wwpn in connector['wwpns']: + if wwpn.upper() in self.Host_connector_info['FC']: + host_name = self.Host_connector_info['FC'][wwpn.upper()] + else: + new_wwpn.append(['wwpn', '%s' % wwpn]) + + return host_name, new_wwpn + def get_host_from_connector(self, connector, volume_name=None, iscsi=False): """Return the Storwize host described by the connector.""" LOG.debug('Enter: get_host_from_connector: %s.', connector) # If we have FC information, we have a faster lookup option - host_name = None - if 'wwpns' in connector and not iscsi: - for wwpn in connector['wwpns']: - resp = self.ssh.lsfabric(wwpn=wwpn) - for wwpn_info in resp: - try: - if (wwpn_info['remote_wwpn'] and - wwpn_info['name'] and - wwpn_info['remote_wwpn'].lower() == - wwpn.lower()): - host_name = wwpn_info['name'] - break - except KeyError: - self.handle_keyerror('lsfabric', wwpn_info) - if host_name: - break + host_name, new_wwpn = self.get_host_from_host_info(connector, iscsi) if host_name and volume_name: hosts_map_info = self.ssh.lsvdiskhostmap(volume_name) @@ -1158,6 +1179,11 @@ class StorwizeHelpers(object): host_name = None if host_name: + for port in new_wwpn: + LOG.debug('update wwpn %(wwpn)s to host %(host)s.', + {'wwpn': port, 'host': host_name}) + self.ssh.addhostport(host_name, port[0], port[1]) + LOG.debug('Leave: get_host_from_connector: host %s.', host_name) return host_name @@ -1288,6 +1314,13 @@ class StorwizeHelpers(object): for port in ports: self.ssh.addhostport(host_name, port[0], port[1]) + if iscsi and 'initiator' in connector: + iqn = connector['initiator'] + self.Host_connector_info['ISCSI'][iqn] = host_name + elif 'wwpns' in connector: + for wwpn in connector['wwpns']: + self.Host_connector_info['FC'][wwpn.upper()] = host_name + LOG.debug('Leave: create_host: host %(host)s - %(host_name)s.', {'host': connector['host'], 'host_name': host_name}) return host_name @@ -1298,6 +1331,23 @@ class StorwizeHelpers(object): def delete_host(self, host_name): self.ssh.rmhost(host_name) + if host_name in self.Host_connector_info['ISCSI'].values(): + host_iqn = None + for iqn, host in self.Host_connector_info['ISCSI'].items(): + if host == host_name: + host_iqn = iqn + break + if host_iqn: + self.Host_connector_info['ISCSI'].pop(host_iqn) + elif host_name in self.Host_connector_info['FC'].values(): + host_wwpn = [] + for wwpn, host in self.Host_connector_info['FC'].items(): + if host == host_name: + host_wwpn.append(wwpn) + + for wwpn in host_wwpn: + self.Host_connector_info['FC'].pop(wwpn) + def _get_unused_lun_id(self, host_name): luns_used = [] result_lun = '-1' @@ -4852,6 +4902,8 @@ class StorwizeSVCCommonDriver(san.SanDriver, vol_name, backend_helper, node_state = self._get_vol_sys_info( volume) + backend_helper.initialize_host_info() + info = {} if 'host' in connector: # get host according to FC protocol diff --git a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_fc.py b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_fc.py index a626144d00b..de42d168e14 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_fc.py +++ b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_fc.py @@ -172,6 +172,8 @@ class StorwizeSVCFCDriver(storwize_common.StorwizeSVCCommonDriver): volume) host_site = None + backend_helper.initialize_host_info() + is_hyper_volume = self.is_volume_hyperswap(volume) if is_hyper_volume: host_site = self._get_volume_host_site_from_conf(volume, @@ -183,12 +185,24 @@ class StorwizeSVCFCDriver(storwize_common.StorwizeSVCCommonDriver): LOG.error(msg) raise exception.VolumeDriverException(message=msg) - # Check if a host object is defined for this host name - host_name = backend_helper.get_host_from_connector(connector) - if host_name is None: - # Host does not exist - add a new host to Storwize/SVC + # Try creating the host, if host creation is successfull continue + # with intialization flow, else search for host object defined for + # this connector info. + host_name = None + try: host_name = backend_helper.create_host(connector, site=host_site) - elif is_hyper_volume: + except exception.VolumeBackendAPIException as excp: + if "CMMVC6035E" in excp.msg: + msg = (_('Host already exists for connector ' + '%(conn)s'), {'conn': connector}) + LOG.info(msg) + host_name = backend_helper.get_host_from_connector(connector) + else: + msg = (_('Error creating host %(ex)s'), {'ex': excp.msg}) + LOG.error(msg) + raise exception.VolumeDriverException(message=msg) + + if is_hyper_volume: self._update_host_site_for_hyperswap_volume(host_name, host_site) volume_attributes = backend_helper.get_vdisk_attributes(volume_name) diff --git a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_iscsi.py b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_iscsi.py index eece51d71b6..26eb8c98ec0 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_iscsi.py +++ b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_iscsi.py @@ -166,6 +166,8 @@ class StorwizeSVCISCSIDriver(storwize_common.StorwizeSVCCommonDriver): volume_name, backend_helper, node_state = self._get_vol_sys_info( volume) + backend_helper.initialize_host_info() + host_site = self._get_volume_host_site_from_conf(volume, connector, iscsi=True) @@ -176,14 +178,26 @@ class StorwizeSVCISCSIDriver(storwize_common.StorwizeSVCCommonDriver): LOG.error(msg) raise exception.VolumeDriverException(message=msg) - # Check if a host object is defined for this host name - host_name = backend_helper.get_host_from_connector(connector, - iscsi=True) - if host_name is None: - # Host does not exist - add a new host to Storwize/SVC + # Try creating the host, if host creation is successfull continue + # with intialization flow, else search for host object defined for + # this connector info. + host_name = None + try: host_name = backend_helper.create_host(connector, iscsi=True, site=host_site) - elif is_hyper_volume: + except exception.VolumeBackendAPIException as excp: + if "CMMVC6578E" in excp.msg: + msg = (_('Host already exists for connector ' + '%(conn)s'), {'conn': connector}) + LOG.info(msg) + host_name = backend_helper.get_host_from_connector(connector, + iscsi=True) + else: + msg = (_('Error creating host %(ex)s'), {'ex': excp.msg}) + LOG.error(msg) + raise exception.VolumeDriverException(message=msg) + + if is_hyper_volume: self._update_host_site_for_hyperswap_volume(host_name, host_site) chap_secret = backend_helper.get_chap_secret_for_host(host_name) diff --git a/releasenotes/notes/bug-1913363-ibm-svf_Fix_multiple_lshost_calls_during_attach-528f92b44a0ff6b8.yaml b/releasenotes/notes/bug-1913363-ibm-svf_Fix_multiple_lshost_calls_during_attach-528f92b44a0ff6b8.yaml new file mode 100644 index 00000000000..a1c897b8508 --- /dev/null +++ b/releasenotes/notes/bug-1913363-ibm-svf_Fix_multiple_lshost_calls_during_attach-528f92b44a0ff6b8.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + IBM Spectrum Virtualize Family driver `Bug #1913363 + `_: Fixed + issue in get_host_from_connector by caching the host + information during attach or detach operations and using + host details from cached information.