diff --git a/cinder/tests/unit/volume/drivers/nec/test_volume.py b/cinder/tests/unit/volume/drivers/nec/test_volume.py index 164160381a5..adb3d853fa1 100644 --- a/cinder/tests/unit/volume/drivers/nec/test_volume.py +++ b/cinder/tests/unit/volume/drivers/nec/test_volume.py @@ -1703,3 +1703,64 @@ class SetQosSpec_test(volume_helper.MStorageDSVDriver, volume_type_id = '33cd6136-0465-4ee0-82fa-b5f3a9138249' ret = self._set_qos_spec(ldname, volume_type_id) self.assertIsNone(ret) + + def test_get_qos_parameters(self): + specs = {} + qos_params = self.get_qos_parameters(specs, True) + self.assertEqual(0, qos_params['upperlimit']) + self.assertEqual(0, qos_params['lowerlimit']) + self.assertEqual('off', qos_params['upperreport']) + + specs = {} + qos_params = self.get_qos_parameters(specs, False) + self.assertIsNone(qos_params['upperlimit']) + self.assertIsNone(qos_params['lowerlimit']) + self.assertIsNone(qos_params['upperreport']) + + specs = {u'upperlimit': u'1000', + u'lowerlimit': u'500', + u'upperreport': u'off'} + qos_params = self.get_qos_parameters(specs, False) + self.assertEqual(1000, qos_params['upperlimit']) + self.assertEqual(500, qos_params['lowerlimit']) + self.assertEqual('off', qos_params['upperreport']) + + specs = {u'upperreport': u'on'} + qos_params = self.get_qos_parameters(specs, False) + self.assertIsNone(qos_params['upperlimit']) + self.assertIsNone(qos_params['lowerlimit']) + self.assertEqual('on', qos_params['upperreport']) + + specs = {u'upperreport': u'aaa'} + qos_params = self.get_qos_parameters(specs, False) + self.assertIsNone(qos_params['upperlimit']) + self.assertIsNone(qos_params['lowerlimit']) + self.assertIsNone(qos_params['upperreport']) + + specs = {u'upperlimit': u'1000001', + u'lowerlimit': u'500'} + with self.assertRaisesRegex(exception.InvalidConfigurationValue, + 'Value "1000001" is not valid for ' + 'configuration option "upperlimit"'): + self.get_qos_parameters(specs, False) + + specs = {u'upperlimit': u'aaa', + u'lowerlimit': u'500'} + with self.assertRaisesRegex(exception.InvalidConfigurationValue, + 'Value "aaa" is not valid for ' + 'configuration option "upperlimit"'): + self.get_qos_parameters(specs, False) + + specs = {u'upperlimit': u'1000', + u'lowerlimit': u'aaa'} + with self.assertRaisesRegex(exception.InvalidConfigurationValue, + 'Value "aaa" is not valid for ' + 'configuration option "lowerlimit"'): + self.get_qos_parameters(specs, False) + + specs = {u'upperlimit': u'1000', + u'lowerlimit': u'1'} + with self.assertRaisesRegex(exception.InvalidConfigurationValue, + 'Value "1" is not valid for ' + 'configuration option "lowerlimit"'): + self.get_qos_parameters(specs, False) diff --git a/cinder/volume/drivers/nec/cli.py b/cinder/volume/drivers/nec/cli.py index 4bd7e202149..400f1fb3102 100644 --- a/cinder/volume/drivers/nec/cli.py +++ b/cinder/volume/drivers/nec/cli.py @@ -653,18 +653,10 @@ class MStorageISMCLI(object): query_status = out[15:39].strip() return query_status - def set_io_limit(self, ldname, specs, force_delete=True): - if specs['upperlimit'] is not None: - upper = int(specs['upperlimit'], 10) - else: - upper = None - - if specs['lowerlimit'] is not None: - lower = int(specs['lowerlimit'], 10) - else: - lower = None - - report = specs['upperreport'] + def set_io_limit(self, ldname, qos_params, force_delete=True): + upper = qos_params['upperlimit'] + lower = qos_params['lowerlimit'] + report = qos_params['upperreport'] if upper is None and lower is None and report is None: return cmd = 'iSMioc setlimit -ldname %s' % ldname diff --git a/cinder/volume/drivers/nec/volume.py b/cinder/volume/drivers/nec/volume.py index 397181bdf16..8fa50f2fa76 100644 --- a/cinder/volume/drivers/nec/volume.py +++ b/cinder/volume/drivers/nec/volume.py @@ -47,7 +47,7 @@ class MStorageISCSIDriver(volume_helper.MStorageDSVDriver, Fixed bug #1777385: driver removed access permission from the destination node after live-migraion. Fixed bug #1778669: LUNs of detached volumes are never reused. - 1.11.1 - Add support pytyon 3. + 1.11.1 - Add support python 3. Add support for multi-attach. Add support of more than 4 iSCSI portals for a node. Add support to revert a volume to a snapshot. @@ -115,7 +115,7 @@ class MStorageFCDriver(volume_helper.MStorageDSVDriver, Fixed bug #1777385: driver removed access permission from the destination node after live-migraion. Fixed bug #1778669: LUNs of detached volumes are never reused. - 1.11.1 - Add support pytyon 3. + 1.11.1 - Add support python 3. Add support for multi-attach. Add support of more than 4 iSCSI portals for a node. Add support to revert a volume to a snapshot. diff --git a/cinder/volume/drivers/nec/volume_common.py b/cinder/volume/drivers/nec/volume_common.py index ab1bc12cdc6..bd2186e736c 100644 --- a/cinder/volume/drivers/nec/volume_common.py +++ b/cinder/volume/drivers/nec/volume_common.py @@ -824,9 +824,8 @@ class MStorageVolumeCommon(object): specs = {} ctxt = context.get_admin_context() - type_id = volume_type_id - if type_id is not None: - volume_type = volume_types.get_volume_type(ctxt, type_id) + if volume_type_id is not None: + volume_type = volume_types.get_volume_type(ctxt, volume_type_id) qos_specs_id = volume_type.get('qos_specs_id') if qos_specs_id is not None: @@ -841,7 +840,8 @@ class MStorageVolumeCommon(object): 'specs': specs}) return specs - def correct_qos_parameter(self, specs, reset): + def get_qos_parameters(self, specs, reset): + qos_params = {} if 'upperlimit' in specs and specs['upperlimit'] is not None: if self.validates_number(specs['upperlimit']) is True: upper_limit = int(specs['upperlimit'], 10) @@ -849,13 +849,16 @@ class MStorageVolumeCommon(object): ((upper_limit < 10) or (upper_limit > 1000000))): raise exception.InvalidConfigurationValue( value=upper_limit, option='upperlimit') + qos_params['upperlimit'] = upper_limit else: raise exception.InvalidConfigurationValue( value=specs['upperlimit'], option='upperlimit') else: # 0: Set to no limit.(default) + # On the QoS function in NEC Storage, 0 means there is no + # limit. # None: Keep current value. - specs['upperlimit'] = '0' if reset else None + qos_params['upperlimit'] = 0 if reset else None if 'lowerlimit' in specs and specs['lowerlimit'] is not None: if self.validates_number(specs['lowerlimit']) is True: @@ -864,24 +867,30 @@ class MStorageVolumeCommon(object): lower_limit > 1000000)): raise exception.InvalidConfigurationValue( value=lower_limit, option='lowerlimit') + qos_params['lowerlimit'] = lower_limit else: raise exception.InvalidConfigurationValue( value=specs['lowerlimit'], option='lowerlimit') else: # 0: Set to no limit.(default) + # On the QoS function in NEC Storage, 0 means there is no + # limit. # None: Keep current value. - specs['lowerlimit'] = '0' if reset else None + qos_params['lowerlimit'] = 0 if reset else None if 'upperreport' in specs: if specs['upperreport'] not in ['on', 'off']: LOG.debug('Illegal arguments. ' 'upperreport is not on or off.' 'upperreport=%s', specs['upperreport']) - specs['upperreport'] = 'off' if reset else None + qos_params['upperreport'] = 'off' if reset else None + else: + qos_params['upperreport'] = specs['upperreport'] else: # off: Set to no report.(default) # None: Keep current value. - specs['upperreport'] = 'off' if reset else None + qos_params['upperreport'] = 'off' if reset else None + return qos_params def check_accesscontrol(self, ldsets, ld): """Check Logical disk is in-use or not.""" diff --git a/cinder/volume/drivers/nec/volume_helper.py b/cinder/volume/drivers/nec/volume_helper.py index a6b73dcaa43..38fcb02972e 100644 --- a/cinder/volume/drivers/nec/volume_helper.py +++ b/cinder/volume/drivers/nec/volume_helper.py @@ -318,13 +318,12 @@ class MStorageDriver(volume_common.MStorageVolumeCommon): LOG.debug('_create_volume Start.') # select ld number and LD bind. - (ldname, - ldn, - selected_pool) = self._bind_ld(volume, - volume.size, - None, - self._convert_id2name, - self._select_leastused_poolnumber) + ldname, ldn, selected_pool = self._bind_ld( + volume, + volume.size, + None, + self._convert_id2name, + self._select_leastused_poolnumber) self._set_qos_spec(ldname, volume.volume_type_id) @@ -478,13 +477,12 @@ class MStorageDriver(volume_common.MStorageVolumeCommon): raise exception.VolumeBackendAPIException(data=msg) # Creating Cloned Volume. - (volume_name, - ldn, - selected_pool) = self._bind_ld(volume, - src_vref.size, - None, - self._convert_id2name, - self._select_leastused_poolnumber) + volume_name, ldn, selected_pool = self._bind_ld( + volume, + src_vref.size, + None, + self._convert_id2name, + self._select_leastused_poolnumber) self._set_qos_spec(volume_name, volume.volume_type_id) @@ -524,11 +522,11 @@ class MStorageDriver(volume_common.MStorageVolumeCommon): def _set_qos_spec(self, ldname, volume_type_id, reset=False): # check io limit. specs = self.get_volume_type_qos_specs(volume_type_id) - self.correct_qos_parameter(specs, reset) + qos_params = self.get_qos_parameters(specs, reset) # set io limit. - self._cli.set_io_limit(ldname, specs) - LOG.debug('_set_qos_spec(Specs = %s) End.', specs) + self._cli.set_io_limit(ldname, qos_params) + LOG.debug('_set_qos_spec(Specs = %s) End.', qos_params) return @@ -721,14 +719,13 @@ class MStorageDriver(volume_common.MStorageVolumeCommon): def _migrate(self, volume, host, volume_type_id, validator, pool_selecter): # bind LD. - (rvname, - ldn, - selected_pool) = self._bind_ld(volume, - volume.size, - validator, - self._convert_id2migratename, - pool_selecter, - host) + rvname, __, selected_pool = self._bind_ld( + volume, + volume.size, + validator, + self._convert_id2migratename, + pool_selecter, + host) if selected_pool >= 0: self._set_qos_spec(rvname, volume_type_id) @@ -1908,14 +1905,13 @@ class MStorageDSVDriver(MStorageDriver): mv_capacity = rv['ld_capacity'] rv_capacity = volume.size - (new_rvname, - rvnumber, - selected_pool) = self._bind_ld(volume, - mv_capacity, - None, - self._convert_id2name, - self._select_volddr_poolnumber, - mv_capacity) + new_rvname, rvnumber, selected_pool = self._bind_ld( + volume, + mv_capacity, + None, + self._convert_id2name, + self._select_volddr_poolnumber, + mv_capacity) self._set_qos_spec(new_rvname, volume.volume_type_id)