From 9974c39f0355bd0a0c3c3364297688de2eccf467 Mon Sep 17 00:00:00 2001 From: Shunei Shiono Date: Tue, 28 Nov 2017 19:04:38 +0900 Subject: [PATCH] NEC driver: delete an unused configuration parameter. The `ldset_controller_node_name' parameter has been deprecated and is ineffective already. This patch refactors NEC driver by deleting the unused codes. Change-Id: I0fb5dc6404ee4db54a3dfc839b9a56622bff6906 Closes-Bug: #1734633 --- .../unit/volume/drivers/nec/test_volume.py | 96 +++++--- cinder/volume/drivers/nec/cli.py | 3 - cinder/volume/drivers/nec/volume_common.py | 22 +- cinder/volume/drivers/nec/volume_helper.py | 217 +++++------------- ...ete-unused-parameter-367bc9447acbb03e.yaml | 4 + 5 files changed, 127 insertions(+), 215 deletions(-) create mode 100644 releasenotes/notes/nec-delete-unused-parameter-367bc9447acbb03e.yaml diff --git a/cinder/tests/unit/volume/drivers/nec/test_volume.py b/cinder/tests/unit/volume/drivers/nec/test_volume.py index 1bf13c54ca8..bae1da716df 100644 --- a/cinder/tests/unit/volume/drivers/nec/test_volume.py +++ b/cinder/tests/unit/volume/drivers/nec/test_volume.py @@ -303,38 +303,6 @@ xml_out = ''' 1000-0090-FAA0-786A - -
- WN - TNES120250 -
-
- 1000-0090-FA76-9605 -
-
- 1000-0090-FA76-9604 -
- - -
- WN - TNES140098 -
-
- 1000-0090-FA53-302C -
-
- 1000-0090-FA53-302D -
-
- 0000 - 0005 -
-
- 0001 - 0006 -
-
LX @@ -604,6 +572,39 @@ class NominatePoolLDTest(volume_helper.MStorageDSVDriver, test.TestCase): 999999999999) +class GetInformationTest(volume_helper.MStorageDSVDriver, test.TestCase): + + def setUp(self): + super(GetInformationTest, self).setUp() + self._set_config(conf.Configuration(None), 'dummy', 'dummy') + self.do_setup(None) + + @mock.patch('cinder.volume.drivers.nec.volume_common.MStorageVolumeCommon.' + '_create_ismview_dir', new=mock.Mock()) + @mock.patch('cinder.volume.drivers.nec.cli.MStorageISMCLI.' + 'view_all', patch_view_all) + def test_get_ldset(self): + self.xml = self._cli.view_all() + (self.pools, + self.lds, + self.ldsets, + self.used_ldns, + self.hostports, + self.max_ld_count) = self.configs(self.xml) + self._properties['ldset_name'] = '' + ldset = self.get_ldset(self.ldsets) + self.assertIsNone(ldset) + self._properties['ldset_name'] = 'LX:OpenStack1' + ldset = self.get_ldset(self.ldsets) + self.assertEqual('LX:OpenStack1', ldset['ldsetname']) + self._properties['ldset_name'] = 'LX:OpenStackX' + with self.assertRaisesRegexp(exception.NotFound, + 'Logical Disk Set' + ' `LX:OpenStackX`' + ' could not be found.'): + self.get_ldset(self.ldsets) + + class VolumeCreateTest(volume_helper.MStorageDSVDriver, test.TestCase): @mock.patch('cinder.volume.drivers.nec.volume_common.MStorageVolumeCommon.' @@ -799,9 +800,6 @@ class ExportTest(volume_helper.MStorageDSVDriver, test.TestCase): self.used_ldns, self.hostports, self.max_ld_count) = self.configs(self.xml) - mock_getldset = mock.Mock() - self.get_ldset = mock_getldset - self.get_ldset.return_value = self.ldsets["LX:OpenStack0"] @mock.patch('cinder.volume.drivers.nec.cli.MStorageISMCLI._execute', patch_execute) @@ -950,6 +948,34 @@ class ExportTest(volume_helper.MStorageDSVDriver, test.TestCase): self.assertEqual('fibre_channel', info['driver_volume_type']) self.assertEqual({}, info['data']) + @mock.patch('cinder.volume.drivers.nec.cli.MStorageISMCLI._execute', + patch_execute) + @mock.patch('cinder.volume.drivers.nec.cli.MStorageISMCLI.view_all', + patch_view_all) + def test_iscsi_portal_with_controller_node_name(self): + self.vol.id = "46045673-41e7-44a7-9333-02f07feab04b" + self.vol.status = 'downloading' + connector = {'initiator': "iqn.1994-05.com.redhat:d1d8e8f23255"} + self._properties['ldset_controller_node_name'] = 'LX:OpenStack1' + self._properties['portal_number'] = 2 + location = self.iscsi_do_export(None, self.vol, connector) + self.assertEqual('192.168.1.90:3260;192.168.1.91:3260;' + '192.168.2.92:3260;192.168.2.93:3260' + ',1 iqn.2001-03.target0000 0', + location['provider_location']) + + @mock.patch('cinder.volume.drivers.nec.cli.MStorageISMCLI._execute', + patch_execute) + @mock.patch('cinder.volume.drivers.nec.cli.MStorageISMCLI.view_all', + patch_view_all) + def test_fc_do_export_with_controller_node_name(self): + self.vol.id = "46045673-41e7-44a7-9333-02f07feab04b" + self.vol.status = 'downloading' + connector = {'wwpns': ["10000090FAA0786A", "10000090FAA0786B"]} + self._properties['ldset_controller_node_name'] = 'LX:OpenStack0' + location = self.fc_do_export(None, self.vol, connector) + self.assertIsNone(location) + class DeleteDSVVolume_test(volume_helper.MStorageDSVDriver, test.TestCase): diff --git a/cinder/volume/drivers/nec/cli.py b/cinder/volume/drivers/nec/cli.py index c0d1e9d2278..00810401644 100644 --- a/cinder/volume/drivers/nec/cli.py +++ b/cinder/volume/drivers/nec/cli.py @@ -613,9 +613,6 @@ class MStorageISMCLI(object): class UnpairWait(object): - error_updates = {'status': 'error', - 'progress': '100%', - 'migration_status': None} def __init__(self, volume_properties, cli): super(UnpairWait, self).__init__() diff --git a/cinder/volume/drivers/nec/volume_common.py b/cinder/volume/drivers/nec/volume_common.py index ebcf43840bc..15567e26628 100644 --- a/cinder/volume/drivers/nec/volume_common.py +++ b/cinder/volume/drivers/nec/volume_common.py @@ -69,9 +69,6 @@ mstorage_opts = [ cfg.StrOpt('nec_ismview_dir', default='/tmp/nec/cinder', help='Output path of iSMview file.'), - cfg.StrOpt('nec_ldset_for_controller_node', - default='', - help='M-Series Storage LD Set name for Controller Node.'), cfg.IntOpt('nec_ssh_pool_port_number', default=22, help='Port number of ssh pool.'), @@ -241,8 +238,6 @@ class MStorageVolumeCommon(object): 'pool_actual_free_capacity': confobj.safe_get('nec_actual_free_capacity'), 'ldset_name': confobj.safe_get('nec_ldset'), - 'ldset_controller_node_name': - confobj.safe_get('nec_ldset_for_controller_node'), 'ld_name_format': confobj.safe_get('nec_ldname_format'), 'ld_backupname_format': confobj.safe_get('nec_backup_ldname_format'), @@ -316,22 +311,9 @@ class MStorageVolumeCommon(object): return volformat % ldname - def get_ldset(self, ldsets, metadata=None): + def get_ldset(self, ldsets): ldset = None - if metadata is not None and 'ldset' in metadata: - ldset_meta = metadata['ldset'] - LOG.debug('ldset(metadata)=%s.', ldset_meta) - for tldset in ldsets.values(): - if tldset['ldsetname'] == ldset_meta: - ldset = ldsets[ldset_meta] - LOG.debug('ldset information(metadata specified)=%s.', - ldset) - break - if ldset is None: - msg = _('Logical Disk Set could not be found.') - LOG.error(msg) - raise exception.NotFound(msg) - elif self._properties['ldset_name'] == '': + if self._properties['ldset_name'] == '': nldset = len(ldsets) if nldset == 0: msg = _('Logical Disk Set could not be found.') diff --git a/cinder/volume/drivers/nec/volume_helper.py b/cinder/volume/drivers/nec/volume_helper.py index 39b671179a4..ac352b9a761 100644 --- a/cinder/volume/drivers/nec/volume_helper.py +++ b/cinder/volume/drivers/nec/volume_helper.py @@ -206,8 +206,8 @@ class MStorageDriver(volume_common.MStorageVolumeCommon): raise exception.NotFound(msg) return ldname - def _validate_iscsildset_exist(self, ldsets, connector, metadata=None): - ldset = self.get_ldset(ldsets, metadata) + def _validate_iscsildset_exist(self, ldsets, connector): + ldset = self.get_ldset(ldsets) if ldset is None: for tldset in ldsets.values(): if 'initiator_list' not in tldset: @@ -225,8 +225,8 @@ class MStorageDriver(volume_common.MStorageVolumeCommon): raise exception.NotFound(msg) return ldset - def _validate_fcldset_exist(self, ldsets, connector, metadata=None): - ldset = self.get_ldset(ldsets, metadata) + def _validate_fcldset_exist(self, ldsets, connector): + ldset = self.get_ldset(ldsets) if ldset is None: for conect in connector['wwpns']: length = len(conect) @@ -682,64 +682,33 @@ class MStorageDriver(volume_common.MStorageVolumeCommon): LOG.debug('_iscsi_do_export' '(Volume ID = %(id)s, connector = %(connector)s) Start.', {'id': volume.id, 'connector': connector}) - while True: + + xml = self._cli.view_all(self._properties['ismview_path']) + pools, lds, ldsets, used_ldns, hostports, max_ld_count = ( + self.configs(xml)) + + # find LD Set. + ldset = self._validate_iscsildset_exist( + ldsets, connector) + ldname = self.get_ldname( + volume.id, self._properties['ld_name_format']) + + # add LD to LD set. + if ldname not in lds: + msg = _('Logical Disk `%s` could not be found.') % ldname + raise exception.NotFound(msg) + ld = lds[ldname] + + if ld['ldn'] not in ldset['lds']: + # assign the LD to LD Set. + self._cli.addldsetld(ldset['ldsetname'], ldname) + # update local info. xml = self._cli.view_all(self._properties['ismview_path']) pools, lds, ldsets, used_ldns, hostports, max_ld_count = ( self.configs(xml)) - - # find LD Set. - - # get target LD Set name. - metadata = {} - # image to volume or volume to image. - if (volume.status in ['downloading', 'uploading'] and - self._properties['ldset_controller_node_name'] != ''): - metadata['ldset'] = ( - self._properties['ldset_controller_node_name']) - LOG.debug('image to volume or volume to image:%s', - volume.status) - # migrate. - elif (hasattr(volume, 'migration_status') and - volume.migration_status is not None and - self._properties['ldset_controller_node_name'] != ''): - metadata['ldset'] = ( - self._properties['ldset_controller_node_name']) - LOG.debug('migrate:%s', volume.migration_status) - - ldset = self._validate_iscsildset_exist( - ldsets, connector, metadata) - - ldname = self.get_ldname( - volume.id, self._properties['ld_name_format']) - - # add LD to LD set. - if ldname not in lds: - msg = _('Logical Disk `%s` could not be found.') % ldname - raise exception.NotFound(msg) - ld = lds[ldname] - - if ld['ldn'] not in ldset['lds']: - # Check the LD is remaining on ldset_controller_node. - ldset_controller_node_name = ( - self._properties['ldset_controller_node_name']) - if ldset_controller_node_name != '': - if ldset_controller_node_name != ldset['ldsetname']: - ldset_controller = ldsets[ldset_controller_node_name] - if ld['ldn'] in ldset_controller['lds']: - LOG.debug( - 'delete remaining the LD from ' - 'ldset_controller_node. ' - 'Ldset Name=%s.', - ldset_controller_node_name) - self._cli.delldsetld(ldset_controller_node_name, - ldname) - # assign the LD to LD Set. - self._cli.addldsetld(ldset['ldsetname'], ldname) - - LOG.debug('Add LD `%(ld)s` to LD Set `%(ldset)s`.', - {'ld': ldname, 'ldset': ldset['ldsetname']}) - else: - break + ldset = self._validate_iscsildset_exist(ldsets, connector) + LOG.debug('Add LD `%(ld)s` to LD Set `%(ldset)s`.', + {'ld': ldname, 'ldset': ldset['ldsetname']}) # enumerate portals for iscsi multipath. prefered_director = ld['pool_num'] % 2 @@ -762,7 +731,8 @@ class MStorageDriver(volume_common.MStorageVolumeCommon): % {'id': volume.id, 'wwpns': connector['wwpns']}) try: - ret = self._fc_do_export(_ctx, volume, connector, ensure) + ret = self._fc_do_export(_ctx, volume, connector, ensure, + self._properties['diskarray_name']) LOG.info('Created FC Export (%s)', msgparm) return ret except exception.CinderException as e: @@ -771,79 +741,41 @@ class MStorageDriver(volume_common.MStorageVolumeCommon): '(%(msgparm)s) (%(exception)s)', {'msgparm': msgparm, 'exception': e}) - def _fc_do_export(self, _ctx, volume, connector, ensure): + @coordination.synchronized('mstorage_bind_execute_{diskarray_name}') + def _fc_do_export(self, _ctx, volume, connector, ensure, diskarray_name): LOG.debug('_fc_do_export' '(Volume ID = %(id)s, connector = %(connector)s) Start.', {'id': volume.id, 'connector': connector}) - while True: - xml = self._cli.view_all(self._properties['ismview_path']) - pools, lds, ldsets, used_ldns, hostports, max_ld_count = ( - self.configs(xml)) + xml = self._cli.view_all(self._properties['ismview_path']) + pools, lds, ldsets, used_ldns, hostports, max_ld_count = ( + self.configs(xml)) - # find LD Set. + # get target LD Set. + ldset = self._validate_fcldset_exist(ldsets, connector) + ldname = self.get_ldname(volume.id, self._properties['ld_name_format']) - # get target LD Set. - metadata = {} - # image to volume or volume to image. - if (volume.status in ['downloading', 'uploading'] and - self._properties['ldset_controller_node_name'] != ''): - metadata['ldset'] = ( - self._properties['ldset_controller_node_name']) - LOG.debug('image to volume or volume to image:%s', - volume.status) - # migrate. - elif (hasattr(volume, 'migration_status') and - volume.migration_status is not None and - self._properties['ldset_controller_node_name'] != '' - ): - metadata['ldset'] = ( - self._properties['ldset_controller_node_name']) - LOG.debug('migrate:%s', volume.migration_status) - - ldset = self._validate_fcldset_exist(ldsets, connector, metadata) - - # get free lun. - luns = [] - ldsetlds = ldset['lds'] - for ld in ldsetlds.values(): - luns.append(ld['lun']) - - target_lun = 0 - for lun in sorted(luns): - if target_lun < lun: - break - target_lun += 1 - - ldname = self.get_ldname( - volume.id, self._properties['ld_name_format']) - - # add LD to LD set. - if ldname not in lds: - msg = _('Logical Disk `%s` could not be found.') % ldname - raise exception.NotFound(msg) - ld = lds[ldname] - - if ld['ldn'] not in ldset['lds']: - # Check the LD is remaining on ldset_controller_node. - ldset_controller_node_name = ( - self._properties['ldset_controller_node_name']) - if ldset_controller_node_name != '': - if ldset_controller_node_name != ldset['ldsetname']: - ldset_controller = ldsets[ldset_controller_node_name] - if ld['ldn'] in ldset_controller['lds']: - LOG.debug( - 'delete remaining the LD from ' - 'ldset_controller_node. ' - 'Ldset Name=%s.', ldset_controller_node_name) - self._cli.delldsetld(ldset_controller_node_name, - ldname) - # assign the LD to LD Set. - self._cli.addldsetld(ldset['ldsetname'], ldname, target_lun) - - LOG.debug('Add LD `%(ld)s` to LD Set `%(ldset)s`.', - {'ld': ldname, 'ldset': ldset['ldsetname']}) - else: + # get free lun. + luns = [] + ldsetlds = ldset['lds'] + for ld in ldsetlds.values(): + luns.append(ld['lun']) + target_lun = 0 + for lun in sorted(luns): + if target_lun < lun: break + target_lun += 1 + + # add LD to LD set. + if ldname not in lds: + msg = _('Logical Disk `%s` could not be found.') % ldname + raise exception.NotFound(msg) + ld = lds[ldname] + + if ld['ldn'] not in ldset['lds']: + # assign the LD to LD Set. + self._cli.addldsetld(ldset['ldsetname'], ldname, target_lun) + LOG.debug('Add LD `%(ld)s` to LD Set `%(ldset)s`.', + {'ld': ldname, 'ldset': ldset['ldsetname']}) LOG.debug('%(ensure)sexport LD `%(ld)s`.', {'ensure': 'ensure_' if ensure else '', @@ -992,24 +924,7 @@ class MStorageDriver(volume_common.MStorageVolumeCommon): self.configs(xml)) # get target LD Set. - metadata = {} - # image to volume or volume to image. - if (volume.status in ['downloading', 'uploading'] and - self._properties['ldset_controller_node_name'] != ''): - metadata['ldset'] = ( - self._properties['ldset_controller_node_name']) - LOG.debug('image to volume or volume to image:%s', - volume.status) - # migrate. - elif (hasattr(volume, 'migration_status') and - volume.migration_status is not None and - self._properties['ldset_controller_node_name'] != '' - ): - metadata['ldset'] = ( - self._properties['ldset_controller_node_name']) - LOG.debug('migrate:%s', volume.migration_status) - - ldset = self.get_ldset(ldsets, metadata) + ldset = self.get_ldset(ldsets) ldname = self.get_ldname( volume.id, self._properties['ld_name_format']) @@ -1531,18 +1446,6 @@ class MStorageDriver(volume_common.MStorageVolumeCommon): LOG.error(msg) raise exception.VolumeBackendAPIException(data=msg) - # Check the LD is remaining on ldset_controller_node. - ldset_controller_node_name = ( - self._properties['ldset_controller_node_name']) - if ldset_controller_node_name != '': - if ldset_controller_node_name in ldsets: - ldset = ldsets[ldset_controller_node_name] - if ld['ldn'] in ldset['lds']: - LOG.debug('delete LD from ldset_controller_node. ' - 'Ldset Name=%s.', - ldset_controller_node_name) - self._cli.delldsetld(ldset_controller_node_name, ldname) - # unbind LD. self._cli.unbind(ldname) LOG.debug('LD unbound. Name=%s.', ldname) diff --git a/releasenotes/notes/nec-delete-unused-parameter-367bc9447acbb03e.yaml b/releasenotes/notes/nec-delete-unused-parameter-367bc9447acbb03e.yaml new file mode 100644 index 00000000000..8a912d08e51 --- /dev/null +++ b/releasenotes/notes/nec-delete-unused-parameter-367bc9447acbb03e.yaml @@ -0,0 +1,4 @@ +--- +upgrade: + - In NEC driver, the deprecated configuration parameter + `ldset_controller_node_name` was deleted.