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 50f9055e9dc..5a747188964 100644 --- a/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py +++ b/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py @@ -4968,27 +4968,38 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): } return opt + @ddt.data(('5000', 'iops', True), + ('500', 'iops_per_gb', False), + ('3000', 'mbps', False)) @mock.patch.object(storwize_svc_common.StorwizeHelpers, 'add_vdisk_qos') @mock.patch.object(storwize_svc_common.StorwizeSVCCommonDriver, '_get_vdisk_params') - def test_storwize_svc_create_volume_with_qos(self, get_vdisk_params, + @ddt.unpack + def test_storwize_svc_create_volume_with_qos(self, + fake_iothrottling_value, + fake_iothrottling_unit, + empty_qos, + get_vdisk_params, add_vdisk_qos): fake_opts = self._get_default_opts() # If the qos is empty, chvdisk should not be called # for create_volume. get_vdisk_params.return_value = fake_opts vol = self._create_volume() - self._assert_vol_exists(vol['name'], True) - self.assertFalse(add_vdisk_qos.called) + if empty_qos: + self._assert_vol_exists(vol['name'], True) + self.assertFalse(add_vdisk_qos.called) self.driver.delete_volume(vol) # If the qos is not empty, chvdisk should be called # for create_volume. - fake_opts['qos'] = {'IOThrottling': 5000} + fake_opts['qos'] = {'IOThrottling': fake_iothrottling_value, + 'IOThrottling_unit': fake_iothrottling_unit} get_vdisk_params.return_value = fake_opts self.driver.create_volume(vol) self._assert_vol_exists(vol['name'], True) - add_vdisk_qos.assert_called_once_with(vol['name'], fake_opts['qos']) + add_vdisk_qos.assert_called_once_with(vol['name'], fake_opts['qos'], + vol['size']) self.driver.delete_volume(vol) self._assert_vol_exists(vol['name'], False) @@ -5196,9 +5207,14 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): self.driver.delete_volume(vol1) self._assert_vol_exists(vol1['name'], False) + @ddt.data(('5000', 'iops', True), + ('500', 'iops_per_gb', False), + ('3000', 'mbps', False)) @mock.patch.object(storwize_svc_common.StorwizeHelpers, 'add_vdisk_qos') - def test_storwize_svc_create_volfromsnap_clone_with_qos(self, - add_vdisk_qos): + @ddt.unpack + def test_storwize_svc_create_volfromsnap_clone_with_qos( + self, fake_iothrottling_value, fake_iothrottling_unit, + empty_qos, add_vdisk_qos): vol1 = self._create_volume() snap1 = self._generate_snap_info(vol1.id) self.driver.create_snapshot(snap1) @@ -5214,42 +5230,48 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): # for create_volume_from_snapshot. with mock.patch.object(storwize_svc_iscsi.StorwizeSVCISCSIDriver, '_get_vdisk_params') as get_vdisk_params: - get_vdisk_params.return_value = fake_opts - self.driver.create_volume_from_snapshot(vol2, snap1) - self._assert_vol_exists(vol2['name'], True) - self.assertFalse(add_vdisk_qos.called) - self.driver.delete_volume(vol2) + if empty_qos: + get_vdisk_params.return_value = fake_opts + self.driver.create_volume_from_snapshot(vol2, snap1) + self._assert_vol_exists(vol2['name'], True) + self.assertFalse(add_vdisk_qos.called) + self.driver.delete_volume(vol2) # If the qos is not empty, chvdisk should be called # for create_volume_from_snapshot. - fake_opts['qos'] = {'IOThrottling': 5000} + fake_opts['qos'] = {'IOThrottling': fake_iothrottling_value, + 'IOThrottling_unit': fake_iothrottling_unit} get_vdisk_params.return_value = fake_opts self.driver.create_volume_from_snapshot(vol2, snap1) self._assert_vol_exists(vol2['name'], True) add_vdisk_qos.assert_called_once_with(vol2['name'], - fake_opts['qos']) + fake_opts['qos'], + vol2['size']) if self.USESIM: self.sim.error_injection('lsfcmap', 'speed_up') # If the qos is empty, chvdisk should not be called - # for create_volume_from_snapshot. + # for create_cloned_volume. add_vdisk_qos.reset_mock() - fake_opts['qos'] = None - get_vdisk_params.return_value = fake_opts - self.driver.create_cloned_volume(vol3, vol2) - self._assert_vol_exists(vol3['name'], True) - self.assertFalse(add_vdisk_qos.called) - self.driver.delete_volume(vol3) + if empty_qos: + fake_opts['qos'] = None + get_vdisk_params.return_value = fake_opts + self.driver.create_cloned_volume(vol3, vol2) + self._assert_vol_exists(vol3['name'], True) + self.assertFalse(add_vdisk_qos.called) + self.driver.delete_volume(vol3) # If the qos is not empty, chvdisk should be called - # for create_volume_from_snapshot. - fake_opts['qos'] = {'IOThrottling': 5000} + # for create_cloned_volume. + fake_opts['qos'] = {'IOThrottling': fake_iothrottling_value, + 'IOThrottling_unit': fake_iothrottling_unit} get_vdisk_params.return_value = fake_opts self.driver.create_cloned_volume(vol3, vol2) self._assert_vol_exists(vol3['name'], True) add_vdisk_qos.assert_called_once_with(vol3['name'], - fake_opts['qos']) + fake_opts['qos'], + vol3['size']) # Delete in the 'opposite' order to make sure it works self.driver.delete_volume(vol3) @@ -5598,21 +5620,52 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): self.driver.delete_volume(volume) volume_types.destroy(ctxt, type_ref['id']) - def test_storwize_svc_extend_volume(self): + @ddt.data(('100', 'iops', '100', True), + ('100', 'iops_per_gb', '1500', False), + ('200', 'mbps', '200', False)) + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'update_vdisk_qos') + @ddt.unpack + def test_storwize_svc_extend_volume(self, old_iothrottling_value, + iothrottling_unit, + new_iothrottling_value, + empty_qos, + update_vdisk_qos): volume = self._create_volume() - self.driver.extend_volume(volume, '13') - attrs = self.driver._helpers.get_vdisk_attributes(volume['name']) - vol_size = int(attrs['capacity']) / units.Gi + if empty_qos: + self.driver.extend_volume(volume, '13') + attrs = self.driver._helpers.get_vdisk_attributes(volume['name']) + vol_size = int(attrs['capacity']) / units.Gi + self.assertAlmostEqual(vol_size, 13) + self.assertFalse(update_vdisk_qos.called) - self.assertAlmostEqual(vol_size, 13) + snap = self._generate_snap_info(volume.id) + self.driver.create_snapshot(snap) + self._assert_vol_exists(snap['name'], True) + self.assertRaises(exception.VolumeDriverException, + self.driver.extend_volume, volume, '16') + self.driver.delete_snapshot(snap) - snap = self._generate_snap_info(volume.id) - self.driver.create_snapshot(snap) - self._assert_vol_exists(snap['name'], True) - self.assertRaises(exception.VolumeDriverException, - self.driver.extend_volume, volume, '16') + with mock.patch.object(storwize_svc_iscsi.StorwizeSVCISCSIDriver, + '_get_vdisk_params') as get_vdisk_params: + # If qos is specified for source volume with 'iops_per_gb' as + # IOThrottling_unit, update_vdisk_qos will be called for + # extend_volume. + fake_opts_qos = self._get_default_opts() + fake_opts_qos['qos'] = {'IOThrottling': new_iothrottling_value, + 'IOThrottling_unit': iothrottling_unit} + get_vdisk_params.return_value = fake_opts_qos + + self.driver.extend_volume(volume, 15) + attrs = self.driver._helpers.get_vdisk_attributes(volume['name']) + vol_size = int(attrs['capacity']) / units.Gi + if fake_opts_qos['qos']['IOThrottling_unit'] == 'iops_per_gb': + update_vdisk_qos.assert_called_with(volume['name'], + fake_opts_qos['qos'], + vol_size) + else: + self.assertFalse(update_vdisk_qos.called) - self.driver.delete_snapshot(snap) self.driver.delete_volume(volume) @mock.patch.object(storwize_rep.StorwizeSVCReplicationGlobalMirror, @@ -5696,20 +5749,33 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): self.driver.migrate_volume(ctxt, volume, host) self._delete_volume(volume) - def test_storwize_svc_get_vdisk_params(self): + @ddt.data(('5000', 'iops', 5000, 'iops', True), + ('500', 'iops_per_gb', 500, 'iops_per_gb', False), + ('2000', 'mbps', 2000, 'mbps', False)) + @ddt.unpack + def test_storwize_svc_get_vdisk_params(self, + fake_iothrottling_value, + fake_iothrottling_unit, + expected_iothrottling_value, + expected_iothrottling_unit, + empty_qos): self.driver.do_setup(None) - fake_qos = {'qos:IOThrottling': '5000'} - expected_qos = {'IOThrottling': 5000} + fake_qos = {'qos:IOThrottling': fake_iothrottling_value, + 'qos:IOThrottling_unit': fake_iothrottling_unit} + expected_qos = {'IOThrottling': float(expected_iothrottling_value), + 'IOThrottling_unit': expected_iothrottling_unit} fake_opts = self._get_default_opts() # The parameters retured should be the same to the default options, # if the QoS is empty. - vol_type_empty_qos = self._create_volume_type_qos(True, None) - type_id = vol_type_empty_qos['id'] - params = self.driver._get_vdisk_params(type_id, - volume_type=vol_type_empty_qos, - volume_metadata=None) - self.assertEqual(fake_opts, params) - volume_types.destroy(self.ctxt, type_id) + if empty_qos: + vol_type_empty_qos = self._create_volume_type_qos(True, None) + type_id = vol_type_empty_qos['id'] + params = \ + self.driver._get_vdisk_params(type_id, + volume_type=vol_type_empty_qos, + volume_metadata=None) + self.assertEqual(fake_opts, params) + volume_types.destroy(self.ctxt, type_id) # If the QoS is set via the qos association with the volume type, # qos value should be set in the retured parameters. @@ -5767,8 +5833,12 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): # If the QoS is set in the volume metadata, # qos value should be set in the retured parameters. - metadata = [{'key': 'qos:IOThrottling', 'value': 4000}] - expected_qos_metadata = {'IOThrottling': 4000} + metadata = [{'key': 'qos:IOThrottling', 'value': '4000'}, + {'key': 'qos:IOThrottling_unit', + 'value': fake_iothrottling_unit}] + expected_qos_metadata = { + 'IOThrottling': 4000.0, + 'IOThrottling_unit': expected_iothrottling_unit} params = self.driver._get_vdisk_params(None, volume_type=None, volume_metadata=metadata) self.assertEqual(expected_qos_metadata, params['qos']) @@ -5784,8 +5854,11 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): # If the QoS is set both via the qos association and the # extra specs, the one from the qos association will take effect. - fake_qos_associate = {'qos:IOThrottling': '6000'} - expected_qos_associate = {'IOThrottling': 6000} + fake_qos_associate = {'qos:IOThrottling': '6000', + 'qos:IOThrottling_unit': fake_iothrottling_unit} + expected_qos_associate = { + 'IOThrottling': 6000.0, + 'IOThrottling_unit': expected_iothrottling_unit} vol_type_qos = self._create_volume_type_qos_both(fake_qos, fake_qos_associate) type_id = vol_type_qos['id'] @@ -5796,11 +5869,23 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): volume_types.destroy(self.ctxt, type_id) qos_specs.delete(self.ctxt, qos_spec['qos_specs']['id']) + @ddt.data(('5000', 'iops', 500, 'iops_per_gb', True), + ('5000', 'iops', '2000', 'mbps', False), + ('500', 'iops_per_gb', '5000', 'iops', False), + ('500', 'iops_per_gb', '2000', 'mbps', False), + ('2000', 'mbps', '5000', 'iops', False), + ('2000', 'mbps', '500', 'iops_per_gb', False)) @mock.patch.object(storwize_svc_common.StorwizeHelpers, 'disable_vdisk_qos') @mock.patch.object(storwize_svc_common.StorwizeHelpers, 'update_vdisk_qos') - def test_storwize_svc_retype_no_copy(self, update_vdisk_qos, + @ddt.unpack + def test_storwize_svc_retype_no_copy(self, old_iothrottling_value, + old_iothrottling_unit, + new_iothrottling_value, + new_iothrottling_unit, + empty_qos, + update_vdisk_qos, disable_vdisk_qos): self.driver.do_setup(None) loc = ('StorwizeSVCDriver:' + self.driver._state['system_id'] + @@ -5835,19 +5920,24 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): fake_opts = self._get_default_opts() fake_opts_old = self._get_default_opts() - fake_opts_old['qos'] = {'IOThrottling': 4000} + fake_opts_old['qos'] = {'IOThrottling': old_iothrottling_value, + 'IOThrottling_unit': old_iothrottling_unit} fake_opts_qos = self._get_default_opts() - fake_opts_qos['qos'] = {'IOThrottling': 5000} + fake_opts_qos['qos'] = {'IOThrottling': new_iothrottling_value, + 'IOThrottling_unit': new_iothrottling_unit} self.driver.create_volume(volume) with mock.patch.object(storwize_svc_iscsi.StorwizeSVCISCSIDriver, '_get_vdisk_params') as get_vdisk_params: # If qos is empty for both the source and target volumes, - # add_vdisk_qos and disable_vdisk_qos will not be called for + # update_vdisk_qos and disable_vdisk_qos will not be called for # retype. - get_vdisk_params.side_effect = [fake_opts, fake_opts, fake_opts] - self.driver.retype(ctxt, volume, new_type, diff, host) - self.assertFalse(update_vdisk_qos.called) - self.assertFalse(disable_vdisk_qos.called) + if empty_qos: + get_vdisk_params.side_effect = [fake_opts, + fake_opts, + fake_opts] + self.driver.retype(ctxt, volume, new_type, diff, host) + self.assertFalse(update_vdisk_qos.called) + self.assertFalse(disable_vdisk_qos.called) self.driver.delete_volume(volume) self.driver.create_volume(volume) @@ -5855,13 +5945,14 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): with mock.patch.object(storwize_svc_iscsi.StorwizeSVCISCSIDriver, '_get_vdisk_params') as get_vdisk_params: # If qos is specified for both source and target volumes, - # add_vdisk_qos will be called for retype, and disable_vdisk_qos + # update_vdisk_qos will be called for retype, and disable_vdisk_qos # will not be called. get_vdisk_params.side_effect = [fake_opts_old, fake_opts_qos, fake_opts_old] self.driver.retype(ctxt, volume, new_type, diff, host) update_vdisk_qos.assert_called_with(volume['name'], - fake_opts_qos['qos']) + fake_opts_qos['qos'], + volume['size']) self.assertFalse(disable_vdisk_qos.called) self.driver.delete_volume(volume) @@ -5870,13 +5961,14 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): with mock.patch.object(storwize_svc_iscsi.StorwizeSVCISCSIDriver, '_get_vdisk_params') as get_vdisk_params: # If qos is empty for source and speficied for target volume, - # add_vdisk_qos will be called for retype, and disable_vdisk_qos + # update_vdisk_qos will be called for retype, and disable_vdisk_qos # will not be called. get_vdisk_params.side_effect = [fake_opts, fake_opts_qos, fake_opts] self.driver.retype(ctxt, volume, new_type, diff, host) update_vdisk_qos.assert_called_with(volume['name'], - fake_opts_qos['qos']) + fake_opts_qos['qos'], + volume['size']) self.assertFalse(disable_vdisk_qos.called) self.driver.delete_volume(volume) @@ -5885,7 +5977,7 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): with mock.patch.object(storwize_svc_iscsi.StorwizeSVCISCSIDriver, '_get_vdisk_params') as get_vdisk_params: # If qos is empty for target volume and specified for source - # volume, add_vdisk_qos will not be called for retype, and + # volume, update_vdisk_qos will not be called for retype, and # disable_vdisk_qos will be called. get_vdisk_params.side_effect = [fake_opts_qos, fake_opts, fake_opts] @@ -5944,11 +6036,23 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): self.driver.retype, ctxt, volume, new_type, diff, host) + @ddt.data(('5000', 'iops', 500, 'iops_per_gb', True), + ('5000', 'iops', '2000', 'mbps', False), + ('500', 'iops_per_gb', '5000', 'iops', False), + ('500', 'iops_per_gb', '2000', 'mbps', False), + ('2000', 'mbps', '5000', 'iops', False), + ('2000', 'mbps', '500', 'iops_per_gb', False)) @mock.patch.object(storwize_svc_common.StorwizeHelpers, 'disable_vdisk_qos') @mock.patch.object(storwize_svc_common.StorwizeHelpers, 'update_vdisk_qos') - def test_storwize_svc_retype_need_copy(self, update_vdisk_qos, + @ddt.unpack + def test_storwize_svc_retype_need_copy(self, old_iothrottling_value, + old_iothrottling_unit, + new_iothrottling_value, + new_iothrottling_unit, + empty_qos, + update_vdisk_qos, disable_vdisk_qos): with mock.patch.object(storwize_svc_common.StorwizeHelpers, 'get_system_info') as get_system_info: @@ -5990,19 +6094,24 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): fake_opts = self._get_default_opts() fake_opts_old = self._get_default_opts() - fake_opts_old['qos'] = {'IOThrottling': 4000} + fake_opts_old['qos'] = {'IOThrottling': old_iothrottling_value, + 'IOThrottling_unit': old_iothrottling_unit} fake_opts_qos = self._get_default_opts() - fake_opts_qos['qos'] = {'IOThrottling': 5000} + fake_opts_qos['qos'] = {'IOThrottling': new_iothrottling_value, + 'IOThrottling_unit': new_iothrottling_unit} self.driver.create_volume(volume) with mock.patch.object(storwize_svc_iscsi.StorwizeSVCISCSIDriver, '_get_vdisk_params') as get_vdisk_params: # If qos is empty for both the source and target volumes, - # add_vdisk_qos and disable_vdisk_qos will not be called for + # update_vdisk_qos and disable_vdisk_qos will not be called for # retype. - get_vdisk_params.side_effect = [fake_opts, fake_opts, fake_opts] - self.driver.retype(ctxt, volume, new_type, diff, host) - self.assertFalse(update_vdisk_qos.called) - self.assertFalse(disable_vdisk_qos.called) + if empty_qos: + get_vdisk_params.side_effect = [fake_opts, + fake_opts, + fake_opts] + self.driver.retype(ctxt, volume, new_type, diff, host) + self.assertFalse(update_vdisk_qos.called) + self.assertFalse(disable_vdisk_qos.called) self.driver.delete_volume(volume) self.driver.create_volume(volume) @@ -6010,13 +6119,14 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): with mock.patch.object(storwize_svc_iscsi.StorwizeSVCISCSIDriver, '_get_vdisk_params') as get_vdisk_params: # If qos is specified for both source and target volumes, - # add_vdisk_qos will be called for retype, and disable_vdisk_qos + # update_vdisk_qos will be called for retype, and disable_vdisk_qos # will not be called. get_vdisk_params.side_effect = [fake_opts_old, fake_opts_qos, fake_opts_qos] self.driver.retype(ctxt, volume, new_type, diff, host) update_vdisk_qos.assert_called_with(volume['name'], - fake_opts_qos['qos']) + fake_opts_qos['qos'], + volume['size']) self.assertFalse(disable_vdisk_qos.called) self.driver.delete_volume(volume) @@ -6025,13 +6135,14 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): with mock.patch.object(storwize_svc_iscsi.StorwizeSVCISCSIDriver, '_get_vdisk_params') as get_vdisk_params: # If qos is empty for source and speficied for target volume, - # add_vdisk_qos will be called for retype, and disable_vdisk_qos + # update_vdisk_qos will be called for retype, and disable_vdisk_qos # will not be called. get_vdisk_params.side_effect = [fake_opts, fake_opts_qos, fake_opts] self.driver.retype(ctxt, volume, new_type, diff, host) update_vdisk_qos.assert_called_with(volume['name'], - fake_opts_qos['qos']) + fake_opts_qos['qos'], + volume['size']) self.assertFalse(disable_vdisk_qos.called) self.driver.delete_volume(volume) @@ -6040,7 +6151,7 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): with mock.patch.object(storwize_svc_iscsi.StorwizeSVCISCSIDriver, '_get_vdisk_params') as get_vdisk_params: # If qos is empty for target volume and specified for source - # volume, add_vdisk_qos will not be called for retype, and + # volume, update_vdisk_qos will not be called for retype, and # disable_vdisk_qos will be called. get_vdisk_params.side_effect = [fake_opts_qos, fake_opts, fake_opts] @@ -6473,6 +6584,9 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): for volume in model_update[1]: self.assertEqual('deleted', volume['status']) + @ddt.data(('5000', 'iops', True), + ('500', 'iops_per_gb', False), + ('3000', 'mbps', False)) @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', new=testutils.ZeroIntervalLoopingCall) @mock.patch.object(storwize_svc_common.StorwizeHelpers, @@ -6485,7 +6599,12 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): '_get_pool') @mock.patch.object(storwize_svc_common.StorwizeHelpers, 'add_vdisk_qos') - def test_storwize_create_flashcopy_to_consistgrp(self, add_vdisk_qos, + @ddt.unpack + def test_storwize_create_flashcopy_to_consistgrp(self, + fake_iothrottling_value, + fake_iothrottling_unit, + empty_qos, + add_vdisk_qos, _get_pool, mkfcmap, create_vdisk, @@ -6493,24 +6612,26 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): source = "volume-36cd5a6f-a13c-456c-8129-c3e8874fb15c" target = "volume-55eb6c7e-a13c-456c-8129-c3e8874kl34f" consistgrp = "cg_snap-9021b016-ce1e-4145-a1f0-0bd4007a3a78" + fake_target_size = 0 config = self.driver.configuration pool = "openstack2" - opts = {'rsize': 2, 'iogrp': 0, 'qos': None, 'flashcopy_rate': 50} - self.driver._helpers.create_flashcopy_to_consistgrp(source, - target, consistgrp, - config, opts, - full_copy=False, - pool=pool) - _get_pool.assert_not_called() - add_vdisk_qos.assert_not_called() + if empty_qos: + opts = {'rsize': 2, 'iogrp': 0, 'qos': None, 'flashcopy_rate': 50} + self.driver._helpers.create_flashcopy_to_consistgrp( + source, target, consistgrp, config, + opts, full_copy=False, pool=pool) + _get_pool.assert_not_called() + add_vdisk_qos.assert_not_called() - opts = {'rsize': 2, 'iogrp': 0, 'qos': 'abc', 'flashcopy_rate': 50} + qos = {'IOThrottling': fake_iothrottling_value, + 'IOThrottling_unit': fake_iothrottling_unit} + opts = {'rsize': 2, 'iogrp': 0, 'qos': qos, 'flashcopy_rate': 50} self.driver._helpers.create_flashcopy_to_consistgrp(source, target, consistgrp, config, opts, full_copy=False, pool=pool) - add_vdisk_qos.assert_called_with(target, opts['qos']) + add_vdisk_qos.assert_called_with(target, opts['qos'], fake_target_size) pool = None self.driver._helpers.create_flashcopy_to_consistgrp(source, target, consistgrp, diff --git a/cinder/volume/drivers/ibm/storwize_svc/storwize_const.py b/cinder/volume/drivers/ibm/storwize_svc/storwize_const.py index e891b031a0e..ddfe0786562 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/storwize_const.py +++ b/cinder/volume/drivers/ibm/storwize_svc/storwize_const.py @@ -57,3 +57,8 @@ REP_SYNC = 'synchronized' REP_IDL = 'idling' REP_IDL_DISC = 'idling_disconnected' REP_STATUS_ON_LINE = 'online' + +# IOThrottling types +MBPS = 'mbps' +IOPS = 'iops' +IOPS_PER_GB = 'iops_per_gb' 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 b10df882bb8..df927c2bc5b 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py +++ b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py @@ -743,14 +743,22 @@ class StorwizeSSH(object): class StorwizeHelpers(object): # All the supported QoS key are saved in this dict. When a new - # key is going to add, three values MUST be set: + # key is going to add, four values MUST be set: # 'default': to indicate the value, when the parameter is disabled. # 'param': to indicate the corresponding parameter in the command. # 'type': to indicate the type of this value. + # 'unit': to indicate the string, a supported QoS parameter. WAIT_TIME = 5 - svc_qos_keys = {'IOThrottling': {'default': '0', - 'param': 'rate', - 'type': int}} + svc_qos = {'IOThrottling': {'default': '0', + 'param': 'rate', + 'type': float, + 'unit': 'IOThrottling_unit'}, + 'IOThrottling_unit': {'default': 'iops', + 'enum': ['iops', 'mbps', 'iops_per_gb'], + 'type': str, + 'mbps': 'unitmb', + 'iops': 'rate', + 'iops_per_gb': 'rate'}} def __init__(self, run_ssh): self.ssh = StorwizeSSH(run_ssh) @@ -1394,7 +1402,6 @@ class StorwizeHelpers(object): else: scope = key_split[0] key = key_split[1] - # 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 @@ -1412,9 +1419,9 @@ class StorwizeHelpers(object): # Add the QoS. if scope and scope == 'qos': - if key in self.svc_qos_keys.keys(): + if key in self.svc_qos: try: - type_fn = self.svc_qos_keys[key]['type'] + type_fn = self.svc_qos[key]['type'] value = type_fn(value) qos[key] = value except ValueError: @@ -1432,8 +1439,43 @@ class StorwizeHelpers(object): elif this_type == 'bool': value = strutils.bool_from_string(value) opts[key] = value - if len(qos) != 0: + if len(qos): opts['qos'] = qos + opts = self._validate_qos_opts(opts) + return opts + + def _validate_qos_opts(self, opts): + """Override to add IOThrottling_unit to qos from extra_specs""" + qos = {} + for key, value in opts['qos'].items(): + # Validate IOThrottle rate value + if key in self.svc_qos and key == "IOThrottling": + if int(value) >= 0: + qos[key] = value + else: + msg = (_("I/O Throttle rate cannot be negative or Zero. " + "So skipping setting of I/O Throttle rate on " + "volumes.")) + LOG.warning(msg) + continue + + # Validate IOThrottle Unit + if key in self.svc_qos and key == 'IOThrottling_unit': + if value: + enum_values = self.svc_qos[key]['enum'] + if value in enum_values: + qos[key] = value + else: + msg = (_("An invalid '%(actual)s' unit was configured " + "for IOThrottling_unit on Storage Template. " + "It should be one of the values: " + "%(expected)s. So skipping setting of I/O " + "Throttle rate on volumes.") % + dict(actual=value, expected=enum_values)) + LOG.warning(msg) + continue + if len(qos) != 2: + opts['qos'] = {} return opts def _get_qos_from_volume_metadata(self, volume_metadata): @@ -1451,9 +1493,9 @@ class StorwizeHelpers(object): key = key_split[1] # Add the QoS. if scope and scope == 'qos': - if key in self.svc_qos_keys.keys(): + if key in self.svc_qos: try: - type_fn = self.svc_qos_keys[key]['type'] + type_fn = self.svc_qos[key]['type'] value = type_fn(value) qos[key] = value except ValueError: @@ -1520,8 +1562,8 @@ class StorwizeHelpers(object): # the same key. specs.update(kvs) opts = self._get_opts_from_specs(opts, specs) - if (opts['qos'] is None and config.storwize_svc_allow_tenant_qos - and volume_metadata): + if (opts['qos'] is None and config.storwize_svc_allow_tenant_qos and + volume_metadata): qos = self._get_qos_from_volume_metadata(volume_metadata) if len(qos) != 0: opts['qos'] = qos @@ -2056,7 +2098,8 @@ class StorwizeHelpers(object): opts['iogrp'] = src_attrs['IO_group_id'] self.create_vdisk(target, src_size, 'b', pool, opts) if opts['qos']: - self.add_vdisk_qos(target, opts['qos']) + vdisk_size = int(float(src_size) / (1 << 30)) + self.add_vdisk_qos(target, opts['qos'], vdisk_size) self.check_flashcopy_rate(opts['flashcopy_rate']) self.ssh.mkfcmap(source, target, full_copy, opts['flashcopy_rate'], @@ -2523,40 +2566,91 @@ class StorwizeHelpers(object): return None return dest_pool - def add_vdisk_qos(self, vdisk, qos): + def add_vdisk_qos(self, vdisk, qos, vdisk_size): """Add the QoS configuration to the volume.""" for key, value in qos.items(): - if key in self.svc_qos_keys.keys(): - param = self.svc_qos_keys[key]['param'] - self.ssh.chvdisk(vdisk, ['-' + param, str(value)]) + if key in self.svc_qos and key == "IOThrottling": + param = self.svc_qos[key]['param'] + if storwize_const.IOPS_PER_GB in qos.values(): + value = value * vdisk_size + if not int(value): + value = 1 + vdisk_params = ['-' + param, str(int(value))] + # Add -unitmb param to the chvdisk if qos:IOThrottling_unit + # is added in extra specs + key_unit = self.svc_qos[key].get('unit', None) + if key_unit in qos: + key_unit_param = qos.get(key_unit) + if (key_unit_param and + key_unit_param == storwize_const.MBPS): + t_val = '-' + self.svc_qos[key_unit][key_unit_param] + vdisk_params.append(t_val) + self.ssh.chvdisk(vdisk, vdisk_params) - def update_vdisk_qos(self, vdisk, qos): + def update_vdisk_qos(self, vdisk, qos, vdisk_size): """Update all the QoS in terms of a key and value. - svc_qos_keys saves all the supported QoS parameters. Going through + svc_qos saves all the supported QoS parameters. Going through this dict, we set the new values to all the parameters. If QoS is available in the QoS configuration, the value is taken from it; if not, the value will be set to default. """ - for key, value in self.svc_qos_keys.items(): - param = value['param'] - if key in qos.keys(): - # If the value is set in QoS, take the value from - # the QoS configuration. - v = qos[key] - else: - # If not, set the value to default. - v = value['default'] - self.ssh.chvdisk(vdisk, ['-' + param, str(v)]) + iothrottling = 'IOThrottling' + if iothrottling in qos: + throttling_value = qos[iothrottling] + key_unit = self.svc_qos[iothrottling]['unit'] + throttling_unit = qos[key_unit] + + # check if throttling unit specified is in allowed units + # if not allowed - we will go with default unit - iops + param = self.svc_qos[iothrottling]['param'] + unit_param = self.svc_qos[key_unit][storwize_const.MBPS] + default_throttling_value = self.svc_qos[iothrottling]['default'] + if throttling_unit in self.svc_qos[key_unit]: + # check if specified throttling unit is not the default unit + # if not default unit - specify the parameter for the + # special unit + if throttling_unit == storwize_const.MBPS: + # Uppdating vdisk_params to disable iops limit and + # enable only bandwidth limit - in mbps + # disable iops + disable_vdisk_params = ['-' + param, + default_throttling_value] + # enable mbps + enable_vdisk_params = ['-' + param, + str(int(throttling_value)), + '-' + unit_param] + else: + # This means that we have to disable mbps limit (bandwidth) + # and enable iops limit + if throttling_unit == storwize_const.IOPS_PER_GB: + throttling_value = throttling_value * vdisk_size + # disable mbps + disable_vdisk_params = ['-' + param, + default_throttling_value, + '-' + unit_param] + # enable iops + enable_vdisk_params = ['-' + param, + str(int(throttling_value))] + # Disable conditional vdisk_params + self.ssh.chvdisk(vdisk, disable_vdisk_params) + # Enable conditional vdisk_params + self.ssh.chvdisk(vdisk, enable_vdisk_params) def disable_vdisk_qos(self, vdisk, qos): """Disable the QoS.""" for key, value in qos.items(): - if key in self.svc_qos_keys.keys(): - param = self.svc_qos_keys[key]['param'] - # Take the default value. - value = self.svc_qos_keys[key]['default'] - self.ssh.chvdisk(vdisk, ['-' + param, value]) + if key in self.svc_qos and key == 'IOThrottling': + # qos of previous volume type is in format: + # qos - {'IOThrottling': 1000, 'IOThrottling_unit': 'iops'} + param = self.svc_qos[key]['param'] + vdisk_params = ['-' + param, self.svc_qos[key]['default']] + # clear out iops limit + self.ssh.chvdisk(vdisk, vdisk_params) + vdisk_params.append( + '-' + self.svc_qos['IOThrottling_unit']['mbps']) + # clear out mbps limit + self.ssh.chvdisk(vdisk, vdisk_params) def change_vdisk_options(self, vdisk, changes, opts, state): change_value = {'warning': '', 'easytier': '', 'autoexpand': ''} @@ -3197,9 +3291,17 @@ class StorwizeSVCCommonDriver(san.SanDriver, raise exception.VolumeDriverException(reason=msg) def _update_replication_properties(self, ctxt, volume, model_update): - metadata = self.db.volume_admin_metadata_get(ctxt.elevated(), - volume['id']) - model_update['metadata'] = metadata + model_update = model_update or dict() + vol_metadata = model_update.get('metadata', {}) + + db_metadata = self.db.volume_metadata_get(ctxt.elevated(), + volume['id']) + model_update['metadata'] = db_metadata if db_metadata else dict() + if (('IOThrottle_rate' not in vol_metadata) and + ('IOThrottle_rate' in model_update['metadata'])): + del model_update['metadata']['IOThrottle_rate'] + model_update['metadata'].update(vol_metadata) + rel_info = self._helpers.get_relationship_info(volume.name) rep_properties = { 'Id': 'id', @@ -3231,7 +3333,7 @@ class StorwizeSVCCommonDriver(san.SanDriver, } # Update model for replication if not rel_info: - for key in rep_properties.keys(): + for key in rep_properties: if key in model_update['metadata']: del model_update['metadata'][key] else: @@ -3286,7 +3388,9 @@ class StorwizeSVCCommonDriver(san.SanDriver, self._helpers.create_vdisk(volume['name'], str(volume['size']), 'gb', pool, opts) if opts['qos']: - self._helpers.add_vdisk_qos(volume['name'], opts['qos']) + self._helpers.add_vdisk_qos(volume['name'], opts['qos'], + volume['size']) + model_update = self._qos_model_update(model_update, volume) model_update[ 'replication_status'] = fields.ReplicationStatus.NOT_CAPABLE @@ -3410,6 +3514,7 @@ class StorwizeSVCCommonDriver(san.SanDriver, def create_volume_from_snapshot(self, volume, snapshot): # Create volume from snapshot with a replication or hyperswap group_id # is not allowed. + model_update = dict() self._check_if_group_type_cg_snapshot(volume) opts = self._get_vdisk_params(volume['volume_type_id'], volume_metadata= @@ -3430,19 +3535,21 @@ class StorwizeSVCCommonDriver(san.SanDriver, self._extend_volume_op(volume, volume['size'], snapshot['volume_size']) if opts['qos']: - self._helpers.add_vdisk_qos(volume['name'], opts['qos']) + self._helpers.add_vdisk_qos(volume['name'], opts['qos'], + volume['size']) + model_update = self._qos_model_update(model_update, volume) ctxt = context.get_admin_context() - model_update = {'replication_status': - fields.ReplicationStatus.NOT_CAPABLE} + model_update[ + 'replication_status'] = fields.ReplicationStatus.NOT_CAPABLE rep_type = self._get_volume_replicated_type(ctxt, volume) if rep_type: self._validate_replication_enabled() replica_obj = self._get_replica_obj(rep_type) replica_obj.volume_replication_setup(ctxt, volume) - model_update = {'replication_status': - fields.ReplicationStatus.ENABLED} + model_update[ + 'replication_status'] = fields.ReplicationStatus.ENABLED # Updating replication properties for a volume with replication # enabled. model_update = self._update_replication_properties(ctxt, volume, @@ -3451,9 +3558,9 @@ class StorwizeSVCCommonDriver(san.SanDriver, def create_cloned_volume(self, tgt_volume, src_volume): """Creates a clone of the specified volume.""" - model_update = dict() # Create a cloned volume with a replication or hyperswap group_id is # not allowed. + model_update = dict() self._check_if_group_type_cg_snapshot(tgt_volume) opts = self._get_vdisk_params(tgt_volume['volume_type_id'], volume_metadata= @@ -3477,7 +3584,9 @@ class StorwizeSVCCommonDriver(san.SanDriver, src_volume['size']) if opts['qos']: - self._helpers.add_vdisk_qos(tgt_volume['name'], opts['qos']) + self._helpers.add_vdisk_qos(tgt_volume['name'], opts['qos'], + tgt_volume['size']) + model_update = self._qos_model_update(model_update, tgt_volume) if opts['volume_topology'] == 'hyperswap': LOG.debug('The source volume %s to be cloned is a hyperswap ' @@ -3498,6 +3607,7 @@ class StorwizeSVCCommonDriver(san.SanDriver, model_update[ 'replication_status'] = fields.ReplicationStatus.NOT_CAPABLE + ctxt = context.get_admin_context() rep_type = self._get_volume_replicated_type(ctxt, tgt_volume) if rep_type: @@ -3589,6 +3699,53 @@ class StorwizeSVCCommonDriver(san.SanDriver, self._helpers.extend_vdisk(volume_name, extend_amt) LOG.debug('leave: _extend_volume_op: volume %s', volume.id) + # Update the QoS IOThrottling value to the volume properties + opts = self._get_vdisk_params(volume['volume_type_id'], + volume_metadata= + volume.get('volume_matadata')) + if opts['qos'] and opts['qos']['IOThrottling_unit']: + unit = opts['qos']['IOThrottling_unit'] + if storwize_const.IOPS_PER_GB in unit: + self._helpers.update_vdisk_qos(volume_name, + opts['qos'], + new_size) + # Add the QoS IOThrottling value to Volume Metadata + model_update = self._qos_model_update(dict(), volume) + # Update the Volume Metadata in the DB + self.db.volume_metadata_update( + context.get_admin_context(), + volume['id'], model_update['metadata'], False) + + def _qos_model_update(self, model_update, volume): + """add volume wwn and IOThrottle_rate to the metadata of the volume""" + model_update = model_update or dict() + vol_metadata = model_update.get('metadata', {}) + + db_meta = self.db.volume_metadata_get(context.get_admin_context(), + volume['id']) + model_update['metadata'] = db_meta if db_meta else dict() + model_update['metadata'].update(vol_metadata) + + attrs = self._helpers.get_vdisk_attributes(volume['name']) + model_update['metadata']['volume_wwn'] = attrs['vdisk_UID'] + iops_limit = attrs.get('IOPs_limit') + bw_limit_mbps = attrs.get('bandwidth_limit_MB') + if iops_limit: + model_update['metadata']['IOThrottle_rate'] = ( + "%s IOps" % iops_limit) + elif bw_limit_mbps: + model_update['metadata']['IOThrottle_rate'] = ( + "%s MBps" % bw_limit_mbps) + else: + # there is no IOThrottle_rate defined - remove it from metadata + # This case is seen during retype from a storage template + # with qos to storage template without qos (the qos rate + # was leftover in the volume details on UI). + if 'IOThrottle_rate' in model_update['metadata']: + del model_update['metadata']['IOThrottle_rate'] + model_update['host'] = volume['host'] + return(model_update) + def add_vdisk_copy(self, volume, dest_pool, vol_type, auto_delete=False): return self._helpers.add_vdisk_copy(volume, dest_pool, vol_type, self._state, @@ -5034,10 +5191,13 @@ class StorwizeSVCCommonDriver(san.SanDriver, if new_opts['qos']: # Add the new QoS setting to the volume. If the volume has an # old QoS setting, it will be overwritten. - self._helpers.update_vdisk_qos(volume['name'], new_opts['qos']) + self._helpers.update_vdisk_qos(volume['name'], new_opts['qos'], + volume['size']) + model_update = self._qos_model_update(model_update, volume) elif old_opts['qos']: # If the old_opts contain QoS keys, disable them. self._helpers.disable_vdisk_qos(volume['name'], old_opts['qos']) + model_update = self._qos_model_update(model_update, volume) if new_opts['flashcopy_rate'] != old_opts['flashcopy_rate']: self._helpers.update_flashcopy_rate(volume.name, @@ -5060,10 +5220,10 @@ class StorwizeSVCCommonDriver(san.SanDriver, self._helpers.delete_vdisk( storwize_const.REPLICA_CHG_VOL_PREFIX + volume['name'], force_unmap=force_unmap, force_delete=False) - model_update = {'replication_status': - fields.ReplicationStatus.DISABLED, - 'replication_driver_data': None, - 'replication_extended_status': None} + model_update['replication_status'] = ( + fields.ReplicationStatus.DISABLED) + model_update['replication_driver_data'] = None + model_update['replication_extended_status'] = None # Updating replication properties for a volume with replication # enabled. model_update = self._update_replication_properties(ctxt, volume, @@ -5077,8 +5237,8 @@ class StorwizeSVCCommonDriver(san.SanDriver, self._helpers.change_relationship_cycleperiod( volume['name'], new_opts.get('cycle_period_seconds')) - model_update = {'replication_status': - fields.ReplicationStatus.ENABLED} + model_update['replication_status'] = ( + fields.ReplicationStatus.ENABLED) # Updating replication properties for a volume with replication # enabled. model_update = self._update_replication_properties(ctxt, volume, @@ -5649,6 +5809,17 @@ class StorwizeSVCCommonDriver(san.SanDriver, self._update_replication_properties( context, vol, volumes_model[volumes.index(vol)])) + opts = self._get_vdisk_params(vol['volume_type_id'], + volume_metadata= + vol.get('volume_metadata')) + if opts['qos']: + # Updating QoS properties for a volume + self._helpers.add_vdisk_qos(vol['name'], opts['qos'], + vol['size']) + volumes_model[volumes.index(vol)] = ( + self._qos_model_update( + volumes_model[volumes.index(vol)], vol)) + LOG.debug("Leave: create_group_from_src.") return model_update, volumes_model diff --git a/releasenotes/notes/bug-1905988-ibm-svf-fix-volume-iops-throttling-issue-b2b89e31af5973b2.yaml b/releasenotes/notes/bug-1905988-ibm-svf-fix-volume-iops-throttling-issue-b2b89e31af5973b2.yaml new file mode 100644 index 00000000000..a87e5e59fa0 --- /dev/null +++ b/releasenotes/notes/bug-1905988-ibm-svf-fix-volume-iops-throttling-issue-b2b89e31af5973b2.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + IBM Spectrum Virtualize Family `Bug #1905988 + `_: + Fixed volume IOPS throttling issue with a new option + to set volume IOPS based on volume size.