[SVF] Set volume IOPS based on volume size

[Spectrum Virtualize Family] Adding support to set volume IOPS based on
the value 'iops_per_gb' and volume size and updating in volume metadata
for volume actions like creation, update, resize and retype to avoid
retype of volume for changing the throttling value.

closes bug: #1905988

Change-Id: I07fec25c1790089563f1baf6f43a67f0dacf77a7

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()
+        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,
+    @ddt.unpack
-                                                            add_vdisk_qos):
+    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,6 +5230,7 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase):
         # for create_volume_from_snapshot.
         with mock.patch.object(storwize_svc_iscsi.StorwizeSVCISCSIDriver,
                                '_get_vdisk_params') as get_vdisk_params:
+            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)
@@ -5222,19 +5239,22 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase):
 
             # 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()
+            if empty_qos:
                 fake_opts['qos'] = None
                 get_vdisk_params.return_value = fake_opts
                 self.driver.create_cloned_volume(vol3, vol2)
@@ -5243,13 +5263,15 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase):
                 self.driver.delete_volume(vol3)
 
             # If the qos is not empty, chvdisk should be called
-            # for create_volume_from_snapshot.
+            # for create_cloned_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_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()
+        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)
 
             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)
+
+        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_volume(volume)
 
     @mock.patch.object(storwize_rep.StorwizeSVCReplicationGlobalMirror,
@@ -5696,16 +5749,29 @@ 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'}
+        fake_qos = {'qos:IOThrottling': fake_iothrottling_value,
-        expected_qos = {'IOThrottling': 5000}
+                    '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.
+        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,
+            params = \
+                self.driver._get_vdisk_params(type_id,
                                               volume_type=vol_type_empty_qos,
                                               volume_metadata=None)
             self.assertEqual(fake_opts, params)
@@ -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}]
+        metadata = [{'key': 'qos:IOThrottling', 'value': '4000'},
-        expected_qos_metadata = {'IOThrottling': 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'}
+        fake_qos_associate = {'qos:IOThrottling': '6000',
-        expected_qos_associate = {'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,16 +5920,21 @@ 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]
+            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)
@@ -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,16 +6094,21 @@ 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]
+            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)
@@ -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"
+        if empty_qos:
             opts = {'rsize': 2, 'iogrp': 0, 'qos': None, 'flashcopy_rate': 50}
-        self.driver._helpers.create_flashcopy_to_consistgrp(source,
+            self.driver._helpers.create_flashcopy_to_consistgrp(
-                                                            target, consistgrp,
+                source, target, consistgrp, config,
-                                                            config, opts,
+                opts, full_copy=False, pool=pool)
-                                                            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,

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'

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',
+    svc_qos = {'IOThrottling': {'default': '0',
                                 'param': 'rate',
-                                     'type': int}}
+                                '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
+        if (opts['qos'] is None and config.storwize_svc_allow_tenant_qos and
-                and volume_metadata):
+                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():
+            if key in self.svc_qos and key == "IOThrottling":
-                param = self.svc_qos_keys[key]['param']
+                param = self.svc_qos[key]['param']
-                self.ssh.chvdisk(vdisk, ['-' + param, str(value)])
+                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():
+        iothrottling = 'IOThrottling'
-            param = value['param']
+        if iothrottling in qos:
-            if key in qos.keys():
+            throttling_value = qos[iothrottling]
-                # If the value is set in QoS, take the value from
+            key_unit = self.svc_qos[iothrottling]['unit']
-                # the QoS configuration.
+            throttling_unit = qos[key_unit]
-                v = qos[key]
+
+            # 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:
-                # If not, set the value to default.
+                    # This means that we have to disable mbps limit (bandwidth)
-                v = value['default']
+                    # and enable iops limit
-            self.ssh.chvdisk(vdisk, ['-' + param, str(v)])
+                    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():
+            if key in self.svc_qos and key == 'IOThrottling':
-                param = self.svc_qos_keys[key]['param']
+                # qos of previous volume type is in format:
-                # Take the default value.
+                # qos - {'IOThrottling': 1000, 'IOThrottling_unit': 'iops'}
-                value = self.svc_qos_keys[key]['default']
+                param = self.svc_qos[key]['param']
-                self.ssh.chvdisk(vdisk, ['-' + param, value])
+                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(),
+        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'] = metadata
+        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':
+        model_update[
-                        fields.ReplicationStatus.NOT_CAPABLE}
+            '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':
+            model_update[
-                            fields.ReplicationStatus.ENABLED}
+                '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':
+            model_update['replication_status'] = (
-                            fields.ReplicationStatus.DISABLED,
+                fields.ReplicationStatus.DISABLED)
-                            'replication_driver_data': None,
+            model_update['replication_driver_data'] = None
-                            'replication_extended_status': 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':
+            model_update['replication_status'] = (
-                            fields.ReplicationStatus.ENABLED}
+                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
 

releasenotes/notes/bug-1905988-ibm-svf-fix-volume-iops-throttling-issue-b2b89e31af5973b2.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    IBM Spectrum Virtualize Family `Bug #1905988
+    <https://bugs.launchpad.net/cinder/+bug/1905988>`_:
+    Fixed volume IOPS throttling issue with a new option
+    to set volume IOPS based on volume size.