From: Vincent Hou Date: Wed, 23 Jul 2014 04:06:38 +0000 (-0400) Subject: Add QoS support to IBM Storwize driver X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=4006b5c065bbd50200fbdf094bb9266338157a13;p=openstack-build%2Fcinder-build.git Add QoS support to IBM Storwize driver IBM Storwize driver can be enabled with QoS support by setting the parameter I/O throttling rate, which caps the amount of I/O. This patch add the QoS configuration to the methods of create_volume, create_volume_from_snapshot, create_cloned_volume and retype. The QoS for IBM Storwize storage can be configured in 3 ways: * Add the key "qos:IOThrottling" into a QoS spec and associate this QoS spec with a volume type. * Add the key "qos:IOThrottling" into the extra spec of a volume type. * Add a metadata with the key "qos:IOThrottling". Change-Id: I7c5c6726a61efad6a8ada9c5dcc31a9382ec79f9 implements-blueprint: cinder-storwize-driver-qos --- diff --git a/cinder/tests/test_storwize_svc.py b/cinder/tests/test_storwize_svc.py index 972739c9d..b0221ca28 100644 --- a/cinder/tests/test_storwize_svc.py +++ b/cinder/tests/test_storwize_svc.py @@ -1645,6 +1645,44 @@ class StorwizeSVCDriverTestCase(test.TestCase): volume_types.destroy(ctxt, type_ref['id']) return attrs + def _get_default_opts(self): + opt = {'rsize': 2, + 'warning': 0, + 'autoexpand': True, + 'grainsize': 256, + 'compression': False, + 'easytier': True, + 'protocol': 'iSCSI', + 'multipath': False, + 'iogrp': 0, + 'qos': None} + return opt + + @mock.patch.object(helpers.StorwizeHelpers, 'add_vdisk_qos') + @mock.patch.object(storwize_svc.StorwizeSVCDriver, '_get_vdisk_params') + def test_storwize_svc_create_volume_with_qos(self, get_vdisk_params, + add_vdisk_qos): + vol = testutils.create_volume(self.ctxt) + 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 + self.driver.create_volume(vol) + 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'] = {'rate': 5000} + 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']) + + self.driver.delete_volume(vol) + self._assert_vol_exists(vol['name'], False) + def test_storwize_svc_snapshots(self): vol1 = self._create_volume() snap1 = self._generate_vol_info(vol1['name'], vol1['id']) @@ -1748,6 +1786,71 @@ class StorwizeSVCDriverTestCase(test.TestCase): self.driver.delete_volume(vol1) self._assert_vol_exists(vol1['name'], False) + @mock.patch.object(helpers.StorwizeHelpers, 'add_vdisk_qos') + def test_storwize_svc_create_volfromsnap_clone_with_qos(self, + add_vdisk_qos): + vol1 = self._create_volume() + snap1 = self._generate_vol_info(vol1['name'], vol1['id']) + self.driver.create_snapshot(snap1) + vol2 = self._generate_vol_info(None, None) + vol3 = self._generate_vol_info(None, None) + fake_opts = self._get_default_opts() + + # Succeed + 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. + with mock.patch.object(storwize_svc.StorwizeSVCDriver, + '_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 the qos is not empty, chvdisk should be called + # for create_volume_from_snapshot. + fake_opts['qos'] = {'rate': 5000} + 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']) + + 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. + 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 the qos is not empty, chvdisk should be called + # for create_volume_from_snapshot. + fake_opts['qos'] = {'rate': 5000} + 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']) + + # Delete in the 'opposite' order to make sure it works + self.driver.delete_volume(vol3) + self._assert_vol_exists(vol3['name'], False) + self.driver.delete_volume(vol2) + self._assert_vol_exists(vol2['name'], False) + self.driver.delete_snapshot(snap1) + self._assert_vol_exists(snap1['name'], False) + self.driver.delete_volume(vol1) + self._assert_vol_exists(vol1['name'], False) + def test_storwize_svc_volumes(self): # Create a first volume volume = self._generate_vol_info(None, None) @@ -2297,7 +2400,8 @@ class StorwizeSVCDriverTestCase(test.TestCase): self.driver.migrate_volume(ctxt, volume, host) self._delete_volume(volume) - def test_storwize_svc_retype_no_copy(self): + @mock.patch.object(helpers.StorwizeHelpers, 'add_vdisk_qos') + def test_storwize_svc_retype_no_copy(self, add_vdisk_qos): self.driver.do_setup(None) loc = ('StorwizeSVCDriver:' + self.driver._state['system_id'] + ':openstack') @@ -2328,6 +2432,27 @@ class StorwizeSVCDriverTestCase(test.TestCase): self.assertEqual('off', attrs['autoexpand'], 'Volume retype failed') self.driver.delete_volume(volume) + fake_opts = self._get_default_opts() + self.driver.create_volume(volume) + with mock.patch.object(storwize_svc.StorwizeSVCDriver, + '_get_vdisk_params') as get_vdisk_params: + # If qos is empty, chvdisk will not be called for retype. + get_vdisk_params.return_value = fake_opts + self.driver.retype(ctxt, volume, new_type, diff, host) + self.assertFalse(add_vdisk_qos.called) + self.driver.delete_volume(volume) + + self.driver.create_volume(volume) + add_vdisk_qos.reset_mock() + with mock.patch.object(storwize_svc.StorwizeSVCDriver, + '_get_vdisk_params') as get_vdisk_params: + # If qos is not empty, chvdisk will be called for retype. + fake_opts['qos'] = {'rate': 5000} + get_vdisk_params.return_value = fake_opts + self.driver.retype(ctxt, volume, new_type, diff, host) + add_vdisk_qos.assert_called_with(volume['name'], fake_opts['qos']) + self.driver.delete_volume(volume) + def test_storwize_svc_retype_only_change_iogrp(self): self.driver.do_setup(None) loc = ('StorwizeSVCDriver:' + self.driver._state['system_id'] + @@ -2358,7 +2483,8 @@ class StorwizeSVCDriverTestCase(test.TestCase): 'failed') self.driver.delete_volume(volume) - def test_storwize_svc_retype_need_copy(self): + @mock.patch.object(helpers.StorwizeHelpers, 'add_vdisk_qos') + def test_storwize_svc_retype_need_copy(self, add_vdisk_qos): self.driver.do_setup(None) loc = ('StorwizeSVCDriver:' + self.driver._state['system_id'] + ':openstack') @@ -2389,6 +2515,27 @@ class StorwizeSVCDriverTestCase(test.TestCase): 'failed') self.driver.delete_volume(volume) + fake_opts = self._get_default_opts() + self.driver.create_volume(volume) + with mock.patch.object(storwize_svc.StorwizeSVCDriver, + '_get_vdisk_params') as get_vdisk_params: + # If qos is empty, chvdisk will not be called for retype. + get_vdisk_params.return_value = fake_opts + self.driver.retype(ctxt, volume, new_type, diff, host) + self.assertFalse(add_vdisk_qos.called) + self.driver.delete_volume(volume) + + add_vdisk_qos.reset_mock() + self.driver.create_volume(volume) + with mock.patch.object(storwize_svc.StorwizeSVCDriver, + '_get_vdisk_params') as get_vdisk_params: + # If qos is not empty, chvdisk will be called for retype. + fake_opts['qos'] = {'rate': 5000} + get_vdisk_params.return_value = fake_opts + self.driver.retype(ctxt, volume, new_type, diff, host) + add_vdisk_qos.assert_called_with(volume['name'], fake_opts['qos']) + self.driver.delete_volume(volume) + def test_set_storage_code_level_success(self): res = self.driver._helpers.get_system_info() if self.USESIM: @@ -2891,3 +3038,6 @@ class StorwizeHelpersTestCase(test.TestCase): with mock.patch.object(ssh.StorwizeSSH, 'lslicense') as lslicense: lslicense.return_value = fake_license self.assertTrue(self.helpers.compression_enabled()) + + def test_get_vdisk_params(self): + pass diff --git a/cinder/volume/drivers/ibm/storwize_svc/__init__.py b/cinder/volume/drivers/ibm/storwize_svc/__init__.py index f63856ebf..463e10824 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/__init__.py +++ b/cinder/volume/drivers/ibm/storwize_svc/__init__.py @@ -107,6 +107,9 @@ storwize_svc_opts = [ 'setup. If it is compatible, it will allow no wwpns ' 'being returned on get_conn_fc_wwpns during ' 'initialize_connection'), + cfg.BoolOpt('storwize_svc_allow_tenant_qos', + default=False, + help='Allow tenants to specify QOS on create'), ] CONF = cfg.CONF @@ -128,9 +131,10 @@ class StorwizeSVCDriver(san.SanDriver): WWPNs by comparing lower case 1.2.4 - Fix bug #1278035 (async migration/retype) 1.2.5 - Added support for manage_existing (unmanage is inherited) + 1.2.6 - Added QoS support in terms of I/O throttling rate """ - VERSION = "1.2.5" + VERSION = "1.2.6" VDISKCOPYOPS_INTERVAL = 600 def __init__(self, *args, **kwargs): @@ -298,9 +302,11 @@ class StorwizeSVCDriver(san.SanDriver): LOG.error(msg) raise exception.VolumeDriverException(message=msg) - def _get_vdisk_params(self, type_id, volume_type=None): + def _get_vdisk_params(self, type_id, volume_type=None, + volume_metadata=None): return self._helpers.get_vdisk_params(self.configuration, self._state, - type_id, volume_type=volume_type) + type_id, volume_type=volume_type, + volume_metadata=volume_metadata) @fczm_utils.AddFCZone @utils.synchronized('storwize-host', external=True) @@ -539,10 +545,15 @@ class StorwizeSVCDriver(san.SanDriver): return info def create_volume(self, volume): - opts = self._get_vdisk_params(volume['volume_type_id']) + opts = self._get_vdisk_params(volume['volume_type_id'], + volume_metadata= + volume.get('volume_metadata')) pool = self.configuration.storwize_svc_volpool_name - return self._helpers.create_vdisk(volume['name'], str(volume['size']), + data = self._helpers.create_vdisk(volume['name'], str(volume['size']), 'gb', pool, opts) + if opts['qos']: + self._helpers.add_vdisk_qos(volume['name'], opts['qos']) + return data def delete_volume(self, volume): self._helpers.delete_vdisk(volume['name'], False) @@ -577,10 +588,14 @@ class StorwizeSVCDriver(san.SanDriver): LOG.error(msg) raise exception.InvalidInput(message=msg) - opts = self._get_vdisk_params(volume['volume_type_id']) + opts = self._get_vdisk_params(volume['volume_type_id'], + volume_metadata= + volume.get('volume_metadata')) self._helpers.create_copy(snapshot['name'], volume['name'], snapshot['id'], self.configuration, opts, True) + if opts['qos']: + self._helpers.add_vdisk_qos(volume['name'], opts['qos']) def create_cloned_volume(self, tgt_volume, src_volume): if src_volume['size'] != tgt_volume['size']: @@ -589,10 +604,14 @@ class StorwizeSVCDriver(san.SanDriver): LOG.error(msg) raise exception.InvalidInput(message=msg) - opts = self._get_vdisk_params(tgt_volume['volume_type_id']) + opts = self._get_vdisk_params(tgt_volume['volume_type_id'], + volume_metadata= + tgt_volume.get('volume_metadata')) self._helpers.create_copy(src_volume['name'], tgt_volume['name'], src_volume['id'], self.configuration, opts, True) + if opts['qos']: + self._helpers.add_vdisk_qos(tgt_volume['name'], opts['qos']) def extend_volume(self, volume, new_size): LOG.debug('enter: extend_volume: volume %s' % volume['id']) @@ -782,7 +801,9 @@ class StorwizeSVCDriver(san.SanDriver): no_copy_keys = ['warning', 'autoexpand', 'easytier'] copy_keys = ['rsize', 'grainsize', 'compression'] all_keys = ignore_keys + no_copy_keys + copy_keys - old_opts = self._get_vdisk_params(volume['volume_type_id']) + old_opts = self._get_vdisk_params(volume['volume_type_id'], + volume_metadata= + volume.get('volume_matadata')) new_opts = self._get_vdisk_params(new_type['id'], volume_type=new_type) @@ -826,6 +847,9 @@ class StorwizeSVCDriver(san.SanDriver): self._helpers.change_vdisk_options(volume['name'], vdisk_changes, new_opts, self._state) + qos = new_opts['qos'] or old_opts['qos'] + if qos: + self._helpers.add_vdisk_qos(volume['name'], qos) LOG.debug('exit: retype: ild=%(id)s, new_type=%(new_type)s,' 'diff=%(diff)s, host=%(host)s' % {'id': volume['id'], 'new_type': new_type, @@ -909,7 +933,7 @@ class StorwizeSVCDriver(san.SanDriver): data['total_capacity_gb'] = 0 # To be overwritten data['free_capacity_gb'] = 0 # To be overwritten data['reserved_percentage'] = self.configuration.reserved_percentage - data['QoS_support'] = False + data['QoS_support'] = True pool = self.configuration.storwize_svc_volpool_name backend_name = self.configuration.safe_get('volume_backend_name') diff --git a/cinder/volume/drivers/ibm/storwize_svc/helpers.py b/cinder/volume/drivers/ibm/storwize_svc/helpers.py index 99694c988..3e2007029 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/helpers.py +++ b/cinder/volume/drivers/ibm/storwize_svc/helpers.py @@ -29,6 +29,7 @@ from cinder.openstack.common import log as logging from cinder.openstack.common import loopingcall from cinder.openstack.common import strutils from cinder.volume.drivers.ibm.storwize_svc import ssh as storwize_ssh +from cinder.volume import qos_specs from cinder.volume import utils from cinder.volume import volume_types @@ -36,6 +37,10 @@ LOG = logging.getLogger(__name__) class StorwizeHelpers(object): + + svc_qos_keys = {'IOThrottling': int} + svc_qos_param_dict = {'IOThrottling': 'rate'} + def __init__(self, run_ssh): self.ssh = storwize_ssh.StorwizeSSH(run_ssh) self.check_fcmapping_interval = 3 @@ -381,7 +386,8 @@ class StorwizeHelpers(object): 'easytier': config.storwize_svc_vol_easytier, 'protocol': protocol, 'multipath': config.storwize_svc_multipath_enabled, - 'iogrp': config.storwize_svc_vol_iogrp} + 'iogrp': config.storwize_svc_vol_iogrp, + 'qos': None} return opt @staticmethod @@ -434,7 +440,83 @@ class StorwizeHelpers(object): % {'iogrp': opts['iogrp'], 'avail': avail_grps}) - def get_vdisk_params(self, config, state, type_id, volume_type=None): + def _get_opts_from_specs(self, opts, specs): + qos = {} + for k, value in specs.iteritems(): + # Get the scope, if using scope format + key_split = k.split(':') + if len(key_split) == 1: + scope = None + key = key_split[0] + else: + scope = key_split[0] + key = key_split[1] + + # We generally do not look at capabilities in the driver, but + # protocol is a special case where the user asks for a given + # protocol and we want both the scheduler and the driver to act + # on the value. + if ((not scope or scope == 'capabilities') and + key == 'storage_protocol'): + scope = None + key = 'protocol' + words = value.split() + if not (words and len(words) == 2 and words[0] == ''): + LOG.error(_('Protocol must be specified as ' + '\' iSCSI\' or \' FC\'.')) + del words[0] + value = words[0] + + # Add the QoS. + if scope and scope == 'qos': + type_fn = self.svc_qos_keys[key] + try: + value = type_fn(value) + qos[self.svc_qos_param_dict[key]] = value + except ValueError: + continue + + # Any keys that the driver should look at should have the + # 'drivers' scope. + if scope and scope != 'drivers': + continue + if key in opts: + this_type = type(opts[key]).__name__ + if this_type == 'int': + value = int(value) + elif this_type == 'bool': + value = strutils.bool_from_string(value) + opts[key] = value + if len(qos) != 0: + opts['qos'] = qos + return opts + + def _get_qos_from_volume_metadata(self, volume_metadata): + """Return the QoS information from the volume metadata.""" + qos = {} + for i in volume_metadata: + k = i.get('key', None) + value = i.get('value', None) + key_split = k.split(':') + if len(key_split) == 1: + scope = None + key = key_split[0] + else: + scope = key_split[0] + key = key_split[1] + # Add the QoS. + if scope and scope == 'qos': + if key in self.svc_qos_keys.keys(): + type_fn = self.svc_qos_keys[key] + try: + value = type_fn(value) + qos[self.svc_qos_param_dict[key]] = value + except ValueError: + continue + return qos + + def get_vdisk_params(self, config, state, type_id, volume_type=None, + volume_metadata=None): """Return the parameters for creating the vdisk. Takes volume type and defaults from config options into account. @@ -444,45 +526,23 @@ class StorwizeHelpers(object): ctxt = context.get_admin_context() volume_type = volume_types.get_volume_type(ctxt, type_id) if volume_type: + qos_specs_id = volume_type.get('qos_specs_id') specs = dict(volume_type).get('extra_specs') - for k, value in specs.iteritems(): - # Get the scope, if using scope format - key_split = k.split(':') - if len(key_split) == 1: - scope = None - key = key_split[0] - else: - scope = key_split[0] - key = key_split[1] - - # We generally do not look at capabilities in the driver, but - # protocol is a special case where the user asks for a given - # protocol and we want both the scheduler and the driver to act - # on the value. - if ((not scope or scope == 'capabilities') and - key == 'storage_protocol'): - scope = None - key = 'protocol' - words = value.split() - if not (words and len(words) == 2 and words[0] == ''): - LOG.error(_('Protocol must be specified as ' - '\' iSCSI\' or \' FC\'.')) - del words[0] - value = words[0] - - # Any keys that the driver should look at should have the - # 'drivers' scope. - if scope and scope != 'drivers': - continue - - if key in opts: - this_type = type(opts[key]).__name__ - if this_type == 'int': - value = int(value) - elif this_type == 'bool': - value = strutils.bool_from_string(value) - opts[key] = value - + # NOTE(vhou): We prefer the qos_specs association + # and over-ride any existing + # extra-specs settings if present + if qos_specs_id is not None: + kvs = qos_specs.get_qos_specs(ctxt, qos_specs_id)['specs'] + # Merge the qos_specs into extra_specs and qos_specs has higher + # priority than extra_specs if they have different values for + # 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): + qos = self._get_qos_from_volume_metadata(volume_metadata) + if len(qos) != 0: + opts['qos'] = qos self.check_vdisk_opts(state, opts) return opts @@ -736,6 +796,10 @@ class StorwizeHelpers(object): return None return dest_pool + def add_vdisk_qos(self, vdisk, qos): + for key, value in qos.iteritems(): + self.ssh.chvdisk(vdisk, ['-' + key, str(value)]) + def change_vdisk_options(self, vdisk, changes, opts, state): if 'warning' in opts: opts['warning'] = '%s%%' % str(opts['warning']) diff --git a/etc/cinder/cinder.conf.sample b/etc/cinder/cinder.conf.sample index 9c94166e6..000bd4ac3 100644 --- a/etc/cinder/cinder.conf.sample +++ b/etc/cinder/cinder.conf.sample @@ -1472,6 +1472,9 @@ # value) #storwize_svc_npiv_compatibility_mode=false +# Allow tenants to specify QOS on create (boolean value) +#storwize_svc_allow_tenant_qos=false + # # Options defined in cinder.volume.drivers.ibm.xiv_ds8k