From: Ryan McNair Date: Tue, 19 Jan 2016 16:39:56 +0000 (+0000) Subject: Rework Storwize/SVC protocol to fix add_vdisk_copy X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=dde6ad78968a1df6e45a630168d274bab34dcaa3;p=openstack-build%2Fcinder-build.git Rework Storwize/SVC protocol to fix add_vdisk_copy Reworking some of the Storwize helper logic so that "protocol" no longer needs to be passed around to helper functions. The protocol being used is validated against the storage device's supported protocols just once now, during check_for_setup_error. Also, now that the drivers have been split, we don't need to look at the QoS' protocol key because this is handled by the scheduler to see which drivers support given protocols. Change-Id: Ib331fd32b00a28115c44e7c3e4e3e998c5d9b80b Closes-Bug: #1535921 --- diff --git a/cinder/tests/unit/test_storwize_svc.py b/cinder/tests/unit/test_storwize_svc.py index 9135a9c33..1e63f1335 100644 --- a/cinder/tests/unit/test_storwize_svc.py +++ b/cinder/tests/unit/test_storwize_svc.py @@ -2089,6 +2089,13 @@ class StorwizeSVCISCSIDriverTestCase(test.TestCase): self.iscsi_driver.terminate_connection(volume, conn2) self.iscsi_driver.terminate_connection(volume, self._connector) + def test_add_vdisk_copy_iscsi(self): + # Ensure only iSCSI is available + self.iscsi_driver._state['enabled_protocols'] = set(['iSCSI']) + volume = self._generate_vol_info(None, None) + self.iscsi_driver.create_volume(volume) + self.iscsi_driver.add_vdisk_copy(volume['name'], 'fake-pool', None) + class StorwizeSVCFcDriverTestCase(test.TestCase): @mock.patch.object(time, 'sleep') @@ -2495,6 +2502,13 @@ class StorwizeSVCFcDriverTestCase(test.TestCase): self.fc_driver.terminate_connection(volume, conn2) self.fc_driver.terminate_connection(volume, self._connector) + def test_add_vdisk_copy_fc(self): + # Ensure only FC is available + self.fc_driver._state['enabled_protocols'] = set(['FC']) + volume = self._generate_vol_info(None, None) + self.fc_driver.create_volume(volume) + self.fc_driver.add_vdisk_copy(volume['name'], 'fake-pool', None) + class StorwizeSVCCommonDriverTestCase(test.TestCase): @mock.patch.object(time, 'sleep') @@ -2717,7 +2731,6 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): 'grainsize': 256, 'compression': False, 'easytier': True, - 'protocol': 'iSCSI', 'iogrp': 0, 'qos': None, 'replication': False, 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 f1c6eca69..57380f4cd 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py +++ b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py @@ -811,7 +811,7 @@ class StorwizeHelpers(object): return self.ssh.lshostvdiskmap(host_name) @staticmethod - def build_default_opts(config, protocol): + def build_default_opts(config): # Ignore capitalization cluster_partner = config.storwize_svc_stretched_cluster_partner @@ -821,7 +821,6 @@ class StorwizeHelpers(object): 'grainsize': config.storwize_svc_vol_grainsize, 'compression': config.storwize_svc_vol_compression, 'easytier': config.storwize_svc_vol_easytier, - 'protocol': protocol, 'iogrp': config.storwize_svc_vol_iogrp, 'qos': None, 'stretched_cluster': cluster_partner, @@ -849,14 +848,6 @@ class StorwizeHelpers(object): reason=_('If compression is set to True, rsize must ' 'also be set (not equal to -1).')) - # Check that the requested protocol is enabled - if opts['protocol'] not in state['enabled_protocols']: - raise exception.InvalidInput( - reason=_('The storage device does not support %(prot)s. ' - 'Please configure the device to support %(prot)s or ' - 'switch to a driver using a different protocol.') - % {'prot': opts['protocol']}) - if opts['iogrp'] not in state['available_iogrps']: avail_grps = ''.join(str(e) for e in state['available_iogrps']) raise exception.InvalidInput( @@ -882,21 +873,6 @@ class StorwizeHelpers(object): scope = key_split[0] key = key_split[1] - # We generally do not look at capabilities in the driver, but - # protocol is a special case where the user asks for a given - # protocol and we want both the scheduler and the driver to act - # on the value. - if ((not scope or scope == 'capabilities') and - key == 'storage_protocol'): - scope = None - key = 'protocol' - words = value.split() - if not (words and len(words) == 2 and words[0] == ''): - LOG.error(_LE('Protocol must be specified as ' - '\' iSCSI\' or \' FC\'.')) - del words[0] - value = words[0] - # We generally do not look at capabilities in the driver, but # replication is a special case where the user asks for # a volume to be replicated, and we want both the scheduler and @@ -989,13 +965,13 @@ class StorwizeHelpers(object): timer = loopingcall.FixedIntervalLoopingCall(_inner) timer.start(interval=interval).wait() - def get_vdisk_params(self, config, state, type_id, protocol = 'iSCSI', + def get_vdisk_params(self, config, state, type_id, volume_type=None, volume_metadata=None): """Return the parameters for creating the vdisk. Takes volume type and defaults from config options into account. """ - opts = self.build_default_opts(config, protocol) + opts = self.build_default_opts(config) ctxt = context.get_admin_context() if volume_type is None and type_id is not None: volume_type = volume_types.get_volume_type(ctxt, type_id) @@ -1717,7 +1693,7 @@ class StorwizeSVCCommonDriver(san.SanDriver, self._helpers = StorwizeHelpers(self._run_ssh) self._vdiskcopyops = {} self._vdiskcopyops_loop = None - self.protocol = '' + self.protocol = None self.replication = None self._state = {'storage_nodes': {}, 'enabled_protocols': set(), @@ -1760,6 +1736,26 @@ class StorwizeSVCCommonDriver(san.SanDriver, # Get the iSCSI and FC names of the Storwize/SVC nodes self._state['storage_nodes'] = self._helpers.get_node_info() + # Add the iSCSI IP addresses and WWPNs to the storage node info + self._helpers.add_iscsi_ip_addrs(self._state['storage_nodes']) + self._helpers.add_fc_wwpns(self._state['storage_nodes']) + + # For each node, check what connection modes it supports. Delete any + # nodes that do not support any types (may be partially configured). + to_delete = [] + for k, node in self._state['storage_nodes'].items(): + if ((len(node['ipv4']) or len(node['ipv6'])) + and len(node['iscsi_name'])): + node['enabled_protocols'].append('iSCSI') + self._state['enabled_protocols'].add('iSCSI') + if len(node['WWPN']): + node['enabled_protocols'].append('FC') + self._state['enabled_protocols'].add('FC') + if not len(node['enabled_protocols']): + to_delete.append(k) + for delkey in to_delete: + del self._state['storage_nodes'][delkey] + # Build the list of in-progress vdisk copy operations if ctxt is None: admin_context = context.get_admin_context() @@ -1780,6 +1776,7 @@ class StorwizeSVCCommonDriver(san.SanDriver, self._vdiskcopyops_loop = loopingcall.FixedIntervalLoopingCall( self._check_volume_copy_ops) self._vdiskcopyops_loop.start(interval=self.VDISKCOPYOPS_INTERVAL) + LOG.debug('leave: do_setup') def check_for_setup_error(self): """Ensure that the flags are set properly.""" @@ -1793,6 +1790,21 @@ class StorwizeSVCCommonDriver(san.SanDriver, exception_msg = (_('Unable to determine system id.')) raise exception.VolumeBackendAPIException(data=exception_msg) + # Make sure we have at least one node configured + if not len(self._state['storage_nodes']): + msg = _('do_setup: No configured nodes.') + LOG.error(msg) + raise exception.VolumeDriverException(message=msg) + + if self.protocol not in self._state['enabled_protocols']: + # TODO(mc_nair): improve this error message by looking at + # self._state['enabled_protocols'] to tell user what driver to use + raise exception.InvalidInput( + reason=_('The storage device does not support %(prot)s. ' + 'Please configure the device to support %(prot)s or ' + 'switch to a driver using a different protocol.') + % {'prot': self.protocol}) + required_flags = ['san_ip', 'san_ssh_port', 'san_login', 'storwize_svc_volpool_name'] for flag in required_flags: @@ -1807,8 +1819,7 @@ class StorwizeSVCCommonDriver(san.SanDriver, 'authentication: set either san_password or ' 'san_private_key option.')) - opts = self._helpers.build_default_opts(self.configuration, - self.protocol) + opts = self._helpers.build_default_opts(self.configuration) self._helpers.check_vdisk_opts(self._state, opts) LOG.debug('leave: check_for_setup_error') @@ -1835,7 +1846,6 @@ class StorwizeSVCCommonDriver(san.SanDriver, volume_metadata=None): return self._helpers.get_vdisk_params(self.configuration, self._state, type_id, - self.protocol, volume_type=volume_type, volume_metadata=volume_metadata) @@ -2130,10 +2140,9 @@ class StorwizeSVCCommonDriver(san.SanDriver, 'diff': diff, 'host': host}) - ignore_keys = ['protocol'] no_copy_keys = ['warning', 'autoexpand', 'easytier'] copy_keys = ['rsize', 'grainsize', 'compression'] - all_keys = ignore_keys + no_copy_keys + copy_keys + all_keys = no_copy_keys + copy_keys old_opts = self._get_vdisk_params(volume['volume_type_id'], volume_metadata= volume.get('volume_matadata')) 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 d9f163eeb..86fdff204 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_fc.py +++ b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_fc.py @@ -85,42 +85,10 @@ class StorwizeSVCFCDriver(storwize_common.StorwizeSVCCommonDriver): def __init__(self, *args, **kwargs): super(StorwizeSVCFCDriver, self).__init__(*args, **kwargs) + self.protocol = 'FC' self.configuration.append_config_values( storwize_svc_fc_opts) - def do_setup(self, ctxt): - # Set protocol - self.protocol = 'FC' - - # Setup common functionality between FC - super(StorwizeSVCFCDriver, self).do_setup(ctxt) - - # Add WWPNs to the storage node info - self._helpers.add_fc_wwpns(self._state['storage_nodes']) - - # For each node, check what connection modes it supports. Delete any - # nodes that do not support any types (may be partially configured). - to_delete = [] - for k, node in self._state['storage_nodes'].items(): - if len(node['WWPN']): - node['enabled_protocols'].append('FC') - self._state['enabled_protocols'].add('FC') - if not len(node['enabled_protocols']): - LOG.info(_LI("%(node)s will be removed since " - "it is not supported by the" - " FC driver."), {'node': node['name']}) - to_delete.append(k) - for delkey in to_delete: - del self._state['storage_nodes'][delkey] - - # Make sure we have at least one node configured - if not len(self._state['storage_nodes']): - msg = _('do_setup: No configured nodes.') - LOG.error(msg) - raise exception.VolumeDriverException(message=msg) - - LOG.debug('leave: do_setup') - def validate_connector(self, connector): """Check connector for at least one enabled FC protocol.""" if 'wwpns' not in connector: 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 56efbab74..95ffcd745 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_iscsi.py +++ b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_iscsi.py @@ -39,7 +39,7 @@ from oslo_log import log as logging from oslo_utils import excutils from cinder import exception -from cinder.i18n import _, _LE, _LI, _LW +from cinder.i18n import _, _LE, _LW from cinder import utils from cinder.volume.drivers.ibm.storwize_svc import ( @@ -85,46 +85,10 @@ class StorwizeSVCISCSIDriver(storwize_common.StorwizeSVCCommonDriver): def __init__(self, *args, **kwargs): super(StorwizeSVCISCSIDriver, self).__init__(*args, **kwargs) + self.protocol = 'iSCSI' self.configuration.append_config_values( storwize_svc_iscsi_opts) - def do_setup(self, ctxt): - # Set protocol - self.protocol = 'iSCSI' - - # Setup common functionality between FC and iSCSI - super(StorwizeSVCISCSIDriver, self).do_setup(ctxt) - - # Get the iSCSI names of the Storwize/SVC nodes - self._state['storage_nodes'] = self._helpers.get_node_info() - - # Add the iSCSI IP addresses to the storage node info - self._helpers.add_iscsi_ip_addrs(self._state['storage_nodes']) - - # For each node, check what connection modes it supports. Delete any - # nodes that do not support any types (may be partially configured). - to_delete = [] - for k, node in self._state['storage_nodes'].items(): - if ((len(node['ipv4']) or len(node['ipv6'])) - and len(node['iscsi_name'])): - node['enabled_protocols'].append('iSCSI') - self._state['enabled_protocols'].add('iSCSI') - if not len(node['enabled_protocols']): - LOG.info(_LI("%(node)s will be removed since " - "it is not supported by the " - "iSCSI driver."), {'node': node['name']}) - to_delete.append(k) - for delkey in to_delete: - del self._state['storage_nodes'][delkey] - - # Make sure we have at least one node configured - if not len(self._state['storage_nodes']): - msg = _('do_setup: No configured nodes.') - LOG.error(msg) - raise exception.VolumeDriverException(message=msg) - - LOG.debug('leave: do_setup') - def validate_connector(self, connector): """Check connector for at least one enabled iSCSI protocol.""" if 'initiator' not in connector: @@ -148,7 +112,6 @@ class StorwizeSVCISCSIDriver(storwize_common.StorwizeSVCCommonDriver): LOG.debug('enter: initialize_connection: volume %(vol)s with connector' ' %(conn)s', {'vol': volume['id'], 'conn': connector}) - vol_opts = self._get_vdisk_params(volume['volume_type_id']) volume_name = volume['name'] # Check if a host object is defined for this host name @@ -190,7 +153,7 @@ class StorwizeSVCISCSIDriver(storwize_common.StorwizeSVCCommonDriver): preferred_node_entry = None io_group_nodes = [] for node in self._state['storage_nodes'].values(): - if vol_opts['protocol'] not in node['enabled_protocols']: + if self.protocol not in node['enabled_protocols']: continue if node['id'] == preferred_node: preferred_node_entry = node