From: Kurt Martin Date: Thu, 18 Jul 2013 17:56:21 +0000 (-0700) Subject: 3PAR Driver modifications to support QOS X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=a048cd66d4af66f2d6095762e5293ae9d81d03a2;p=openstack-build%2Fcinder-build.git 3PAR Driver modifications to support QOS Currently, the OpenStack HP 3PAR Fibre Channel (FC) and iSCSI Drivers do not support Quality of Service (QoS) extra specs. The QoS settings added in this patch include; • maximum MB/second (maxBWS) • maximum IO/second (maxIOPS) These new extra specs will be scoped keys, the scoping will be qos:maxBWS and qos:maxIOPS. A new key hp3par:vvs was also added to allow the admin to predefine QOS settings on a 3PAR virtual volume set and any volume created would be added to that predefined volume set. The 3PAR storage arrays set these values on virtual volume sets, not the actual volume. So the change includes creating a virtual volume set with these settings and then adding the volume to the volume set. 1. Max IO/S & Max MB/S are not QoS guarantees 2. These are per volume maximums which the 3PAR is guaranteed not to exceed. 3. Settings these values does not guarantee these performance rates will be achievable DocImpact Implements blueprint 3par-qos-support Change-Id: I69031c6d3febe11dd5f9ff17095b86f3fe72a2a4 --- diff --git a/cinder/tests/test_hp3par.py b/cinder/tests/test_hp3par.py index 547b4e2c4..f81654f1e 100644 --- a/cinder/tests/test_hp3par.py +++ b/cinder/tests/test_hp3par.py @@ -305,6 +305,7 @@ class HP3PARBaseDriver(): VOLUME_ID_SNAP = '761fc5e5-5191-4ec7-aeba-33e36de44156' FAKE_DESC = 'test description name' FAKE_FC_PORTS = ['0987654321234', '123456789000987'] + QOS = "{'qos:maxIOPS': 1000, 'qos:maxBWS': 50}" FAKE_ISCSI_PORTS = {'1.1.1.2': {'nsp': '8:1:1', 'iqn': ('iqn.2000-05.com.3pardata:' '21810002ac00383d'), @@ -318,6 +319,14 @@ class HP3PARBaseDriver(): 'volume_type': None, 'volume_type_id': None} + volume_qos = {'name': VOLUME_NAME, + 'id': VOLUME_ID, + 'display_name': 'Foo Volume', + 'size': 2, + 'host': FAKE_HOST, + 'volume_type': None, + 'volume_type_id': 'gold'} + snapshot = {'name': SNAPSHOT_NAME, 'id': SNAPSHOT_ID, 'user_id': USER_ID, @@ -336,6 +345,14 @@ class HP3PARBaseDriver(): 'wwnns': ["223456789012345", "223456789054321"], 'host': 'fakehost'} + volume_type = {'name': 'gold', + 'deleted': False, + 'updated_at': None, + 'extra_specs': {'qos:maxBWS': '50', + 'qos:maxIOPS': '1000'}, + 'deleted_at': None, + 'id': 'gold'} + def setup_configuration(self): configuration = mox.MockObject(conf.Configuration) configuration.hp3par_debug = False @@ -401,12 +418,47 @@ class HP3PARBaseDriver(): def fake_get_ports(self): return {'FC': self.FAKE_FC_PORTS, 'iSCSI': self.FAKE_ISCSI_PORTS} + def fake_get_volume_type(self, type_id): + return self.volume_type + + def fake_get_qos_by_volume_type(self, volume_type): + return self.QOS + + def fake_add_volume_to_volume_set(self, volume, volume_name, + cpg, vvs_name, qos): + return volume + def fake_copy_volume(self, src_name, dest_name): pass def fake_get_volume_state(self, vol_name): return "normal" + def test_create_volume(self): + self.flags(lock_path=self.tempdir) + model_update = self.driver.create_volume(self.volume) + metadata = model_update['metadata'] + self.assertFalse(metadata['3ParName'] is None) + self.assertEqual(metadata['CPG'], HP3PAR_CPG) + self.assertEqual(metadata['snapCPG'], HP3PAR_CPG_SNAP) + + def test_create_volume_qos(self): + self.flags(lock_path=self.tempdir) + self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_get_volume_type", + self.fake_get_volume_type) + self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, + "_get_qos_by_volume_type", + self.fake_get_qos_by_volume_type) + self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, + "_add_volume_to_volume_set", + self.fake_add_volume_to_volume_set) + model_update = self.driver.create_volume(self.volume_qos) + metadata = model_update['metadata'] + self.assertFalse(metadata['3ParName'] is None) + self.assertEqual(metadata['CPG'], HP3PAR_CPG) + self.assertEqual(metadata['snapCPG'], HP3PAR_CPG_SNAP) + self.assertEqual(metadata['qos'], True) + def test_delete_volume(self): self.flags(lock_path=self.tempdir) self.driver.delete_volume(self.volume) @@ -449,6 +501,29 @@ class HP3PARBaseDriver(): self.driver.create_volume_from_snapshot, volume, self.snapshot) + def test_create_volume_from_snapshot_qos(self): + self.flags(lock_path=self.tempdir) + self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_get_volume_type", + self.fake_get_volume_type) + self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, + "_get_qos_by_volume_type", + self.fake_get_qos_by_volume_type) + self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, + "_add_volume_to_volume_set", + self.fake_add_volume_to_volume_set) + model_update = self.driver.create_volume_from_snapshot(self.volume_qos, + self.snapshot) + metadata = model_update['metadata'] + self.assertEqual(metadata['qos'], True) + snap_vol = self.driver.common.client.getVolume(self.VOLUME_3PAR_NAME) + self.assertEqual(snap_vol['name'], self.VOLUME_3PAR_NAME) + + volume = self.volume.copy() + volume['size'] = 1 + self.assertRaises(exception.InvalidInput, + self.driver.create_volume_from_snapshot, + volume, self.snapshot) + def test_terminate_connection(self): self.flags(lock_path=self.tempdir) #setup the connections @@ -521,14 +596,6 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): 'driver_volume_type': 'fibre_channel'} return hostname - def test_create_volume(self): - self.flags(lock_path=self.tempdir) - model_update = self.driver.create_volume(self.volume) - metadata = model_update['metadata'] - self.assertFalse(metadata['3ParName'] is None) - self.assertEqual(metadata['CPG'], HP3PAR_CPG) - self.assertEqual(metadata['snapCPG'], HP3PAR_CPG_SNAP) - def test_initialize_connection(self): self.flags(lock_path=self.tempdir) result = self.driver.initialize_connection(self.volume, self.connector) @@ -729,14 +796,6 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase): self._hosts[hostname] = host return hostname - def test_create_volume(self): - self.flags(lock_path=self.tempdir) - model_update = self.driver.create_volume(self.volume) - metadata = model_update['metadata'] - self.assertFalse(metadata['3ParName'] is None) - self.assertEqual(metadata['CPG'], HP3PAR_CPG) - self.assertEqual(metadata['snapCPG'], HP3PAR_CPG_SNAP) - def test_initialize_connection(self): self.flags(lock_path=self.tempdir) result = self.driver.initialize_connection(self.volume, self.connector) diff --git a/cinder/volume/drivers/san/hp/hp_3par_common.py b/cinder/volume/drivers/san/hp/hp_3par_common.py index f21feefac..5fa12ea02 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_common.py +++ b/cinder/volume/drivers/san/hp/hp_3par_common.py @@ -122,7 +122,8 @@ class HP3PARCommon(object): '9 - EGENERA', '10 - ONTAP-legacy', '11 - VMware'] - hp3par_valid_keys = ['cpg', 'snap_cpg', 'provisioning', 'persona'] + hp_qos_keys = ['maxIOPS', 'maxBWS'] + hp3par_valid_keys = ['cpg', 'snap_cpg', 'provisioning', 'persona', 'vvs'] def __init__(self, config): self.sshpool = None @@ -182,7 +183,8 @@ class HP3PARCommon(object): try: cpg = self.client.getCPG(cpg_name) except hpexceptions.HTTPNotFound: - err = (_("CPG (%s) doesn't exist on array.") % cpg_name) + err = (_("Failed to get domain because CPG (%s) doesn't " + "exist on array.") % cpg_name) LOG.error(err) raise exception.InvalidInput(reason=err) @@ -214,6 +216,10 @@ class HP3PARCommon(object): snapshot_name = self._encode_name(snapshot_id) return "oss-%s" % snapshot_name + def _get_3par_vvs_name(self, volume_id): + vvs_name = self._encode_name(volume_id) + return "vvs-%s" % vvs_name + def _encode_name(self, name): uuid_str = name.replace("-", "") vol_uuid = uuid.UUID('urn:uuid:%s' % uuid_str) @@ -521,6 +527,7 @@ exit 'reserved_percentage': 0, 'storage_protocol': None, 'total_capacity_gb': 'unknown', + 'QoS_support': True, 'vendor_name': 'Hewlett-Packard', 'volume_backend_name': None} @@ -569,6 +576,23 @@ exit else: return default + def _get_qos_value(self, qos, key, default=None): + if key in qos: + return qos[key] + else: + return default + + def _get_qos_by_volume_type(self, volume_type): + qos = {} + specs = volume_type.get('extra_specs') + for key, value in specs.iteritems(): + if 'qos:' in key: + fields = key.split(':') + key = fields[1] + if key in self.hp_qos_keys: + qos[key] = int(value) + return qos + def _get_keys_by_volume_type(self, volume_type): hp3par_keys = {} specs = volume_type.get('extra_specs') @@ -580,6 +604,40 @@ exit hp3par_keys[key] = value return hp3par_keys + def _set_qos_rule(self, qos, vvs_name): + max_io = self._get_qos_value(qos, 'maxIOPS') + max_bw = self._get_qos_value(qos, 'maxBWS') + cli_qos_string = "" + if max_io is not None: + cli_qos_string += ('-io %s ' % max_io) + if max_bw is not None: + cli_qos_string += ('-bw %sM ' % max_bw) + self._cli_run('setqos %svvset:%s' % + (cli_qos_string, vvs_name), None) + + def _add_volume_to_volume_set(self, volume, volume_name, + cpg, vvs_name, qos): + if vvs_name is not None: + # Admin has set a volume set name to add the volume to + self._cli_run('createvvset -add %s %s' % (vvs_name, + volume_name), None) + else: + vvs_name = self._get_3par_vvs_name(volume['id']) + domain = self.get_domain(cpg) + self._cli_run('createvvset -domain %s %s' % (domain, + vvs_name), None) + self._set_qos_rule(qos, vvs_name) + self._cli_run('createvvset -add %s %s' % (vvs_name, + volume_name), None) + + def _remove_volume_set(self, vvs_name): + # Must first clear the QoS rules before removing the volume set + self._cli_run('setqos -clear vvset:%s' % (vvs_name), None) + self._cli_run('removevvset -f %s' % (vvs_name), None) + + def _remove_volume_from_volume_set(self, volume_name, vvs_name): + self._cli_run('removevvset -f %s %s' % (vvs_name, volume_name), None) + def get_persona_type(self, volume, hp3par_keys=None): default_persona = self.valid_persona_values[0] type_id = volume.get('volume_type_id', None) @@ -616,11 +674,19 @@ exit # get the options supported by volume types volume_type = None + vvs_name = None hp3par_keys = {} + qos = {} + qos_on_volume = False type_id = volume.get('volume_type_id', None) if type_id is not None: volume_type = self._get_volume_type(type_id) hp3par_keys = self._get_keys_by_volume_type(volume_type) + vvs_name = self._get_key_value(hp3par_keys, 'vvs') + if vvs_name is None: + qos = self._get_qos_by_volume_type(volume_type) + if qos: + qos_on_volume = True cpg = self._get_key_value(hp3par_keys, 'cpg', self.config.hp3par_cpg) @@ -665,6 +731,10 @@ exit if type_id is not None: comments['volume_type_name'] = volume_type.get('name') comments['volume_type_id'] = type_id + if vvs_name is not None: + comments['vvs'] = vvs_name + else: + comments['qos'] = qos extras = {'comment': json.dumps(comments), 'snapCPG': snap_cpg, @@ -673,7 +743,15 @@ exit capacity = self._capacity_from_size(volume['size']) volume_name = self._get_3par_vol_name(volume['id']) self.client.createVolume(volume_name, cpg, capacity, extras) - + if qos or vvs_name is not None: + try: + self._add_volume_to_volume_set(volume, volume_name, + cpg, vvs_name, qos) + except Exception as ex: + # Delete the volume if unable to add it to the volume set + self.client.deleteVolume(volume_name) + LOG.error(str(ex)) + raise exception.CinderException(ex.get_description()) except hpexceptions.HTTPConflict: raise exception.Duplicate(_("Volume (%s) already exists on array") % volume_name) @@ -688,7 +766,8 @@ exit raise exception.CinderException(ex.get_description()) metadata = {'3ParName': volume_name, 'CPG': cpg, - 'snapCPG': extras['snapCPG']} + 'snapCPG': extras['snapCPG'], 'qos': qos_on_volume, + 'vvs': vvs_name} return metadata def _copy_volume(self, src_name, dest_name): @@ -767,6 +846,12 @@ exit def delete_volume(self, volume): try: volume_name = self._get_3par_vol_name(volume['id']) + qos = self.get_volume_metadata_value(volume, 'qos') + vvs_name = self.get_volume_metadata_value(volume, 'vvs') + if vvs_name is not None: + self._remove_volume_from_volume_set(volume_name, vvs_name) + elif qos: + self._remove_volume_set(self._get_3par_vvs_name(volume['id'])) self.client.deleteVolume(volume_name) except hpexceptions.HTTPNotFound as ex: # We'll let this act as if it worked @@ -797,10 +882,26 @@ exit try: snap_name = self._get_3par_snap_name(snapshot['id']) - vol_name = self._get_3par_vol_name(volume['id']) + volume_name = self._get_3par_vol_name(volume['id']) extra = {'volume_id': volume['id'], 'snapshot_id': snapshot['id']} + + volume_type = None + type_id = volume.get('volume_type_id', None) + vvs_name = None + qos = {} + qos_on_volume = False + hp3par_keys = {} + if type_id is not None: + volume_type = self._get_volume_type(type_id) + hp3par_keys = self._get_keys_by_volume_type(volume_type) + vvs_name = self._get_key_value(hp3par_keys, 'vvs') + if vvs_name is None: + qos = self._get_qos_by_volume_type(volume_type) + if qos: + qos_on_volume = True + name = snapshot.get('display_name', None) if name: extra['name'] = name @@ -812,11 +913,29 @@ exit optional = {'comment': json.dumps(extra), 'readOnly': False} - self.client.createSnapshot(vol_name, snap_name, optional) + self.client.createSnapshot(volume_name, snap_name, optional) + if qos or vvs_name is not None: + cpg = self._get_key_value(hp3par_keys, 'cpg', + self.config.hp3par_cpg) + try: + self._add_volume_to_volume_set(volume, volume_name, + cpg, vvs_name, qos) + except Exception as ex: + # Delete the volume if unable to add it to the volume set + self.client.deleteVolume(volume_name) + LOG.error(str(ex)) + raise exception.CinderException(ex.get_description()) except hpexceptions.HTTPForbidden: raise exception.NotAuthorized() except hpexceptions.HTTPNotFound: raise exception.NotFound() + except Exception as ex: + LOG.error(str(ex)) + raise exception.CinderException(ex.get_description()) + + metadata = {'3ParName': volume_name, 'qos': qos_on_volume, + 'vvs': vvs_name} + return metadata def create_snapshot(self, snapshot): LOG.debug("Create Snapshot\n%s" % pprint.pformat(snapshot)) diff --git a/cinder/volume/drivers/san/hp/hp_3par_fc.py b/cinder/volume/drivers/san/hp/hp_3par_fc.py index 5b93c54c3..66046a9f3 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_fc.py +++ b/cinder/volume/drivers/san/hp/hp_3par_fc.py @@ -115,8 +115,9 @@ class HP3PARFCDriver(cinder.volume.driver.FibreChannelDriver): TODO: support using the size from the user. """ self.common.client_login() - self.common.create_volume_from_snapshot(volume, snapshot) + metadata = self.common.create_volume_from_snapshot(volume, snapshot) self.common.client_logout() + return {'metadata': metadata} @utils.synchronized('3par', external=True) def create_snapshot(self, snapshot): diff --git a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py index 3fb90849b..92799df42 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py +++ b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py @@ -178,8 +178,9 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver): TODO: support using the size from the user. """ self.common.client_login() - self.common.create_volume_from_snapshot(volume, snapshot) + metadata = self.common.create_volume_from_snapshot(volume, snapshot) self.common.client_logout() + return {'metadata': metadata} @utils.synchronized('3par', external=True) def create_snapshot(self, snapshot):