openstack/cinder
Code Issues Proposed changes

Merge "PowerMax Driver - Compression Change Bug Fix"

Browse Source
This commit is contained in:
Zuul
2020-04-19 21:39:12 +00:00
committed by Gerrit Code Review
parent c79268c63f 9fabc40ccf
commit bedcb240e6
4 changed files with 98 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py
View File

@@ -355,6 +355,12 @@ class PowerMaxData(object):
# extra-specs
vol_type_extra_specs = {'pool_name': u'Diamond+DSS+SRP_1+000197800123'}
vol_type_extra_specs_none_pool = {
'pool_name': u'None+NONE+SRP_1+000197800123'}
vol_type_extra_specs_optimised_pool = {
'pool_name': u'Optimized+NONE+SRP_1+000197800123'}
vol_type_extra_specs_next_gen_pool = {
'pool_name': u'Optimized+SRP_1+000197800123'}
vol_type_extra_specs_compr_disabled = {
'pool_name': u'Diamond+DSS+SRP_1+000197800123',
'storagetype:disablecompression': 'true'}

4
cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py
View File

@@ -1798,7 +1798,7 @@ class PowerMaxCommonTest(test.TestCase):
device_id = self.data.device_id
volume_name = self.data.test_volume.name
extra_specs = self.data.extra_specs
new_type = {'extra_specs': {}}
new_type = {'extra_specs': self.data.vol_type_extra_specs}
volume = self.data.test_volume
host = {'host': self.data.new_host}
with mock.patch.object(self.common, '_migrate_volume') as mock_migrate:
@@ -1814,7 +1814,7 @@ class PowerMaxCommonTest(test.TestCase):
volume_name = self.data.test_volume.name
extra_specs = self.data.extra_specs
volume = self.data.test_volume
new_type = {'extra_specs': {}}
new_type = {'extra_specs': self.data.vol_type_extra_specs}
host = {'host': self.data.new_host}
with mock.patch.object(
self.common, '_is_valid_for_storage_assisted_migration',

49
cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_utils.py
View File

@@ -223,11 +223,9 @@ class PowerMaxUtilsTest(test.TestCase):
extra_specs)
self.assertTrue(do_disable_compression)
# Compression disabled by no SL/WL combination
extra_specs2 = deepcopy(self.data.extra_specs)
extra_specs2[utils.SLO] = None
do_disable_compression2 = self.utils.is_compression_disabled(
extra_specs2)
self.assertTrue(do_disable_compression2)
extra_specs = deepcopy(self.data.vol_type_extra_specs_none_pool)
self.assertTrue(self.utils.is_compression_disabled(
extra_specs))
def test_is_compression_disabled_false(self):
# Path 1: no compression extra spec set
@@ -243,19 +241,19 @@ class PowerMaxUtilsTest(test.TestCase):
self.assertFalse(do_disable_compression2)
def test_change_compression_type_true(self):
source_compr_disabled_true = 'true'
source_compr_disabled = True
new_type_compr_disabled = {
'extra_specs': {utils.DISABLECOMPRESSION: 'no'}}
'extra_specs': {utils.DISABLECOMPRESSION: 'false'}}
ans = self.utils.change_compression_type(
source_compr_disabled_true, new_type_compr_disabled)
source_compr_disabled, new_type_compr_disabled)
self.assertTrue(ans)
def test_change_compression_type_false(self):
source_compr_disabled_true = True
source_compr_disabled = True
new_type_compr_disabled = {
'extra_specs': {utils.DISABLECOMPRESSION: 'true'}}
ans = self.utils.change_compression_type(
source_compr_disabled_true, new_type_compr_disabled)
source_compr_disabled, new_type_compr_disabled)
self.assertFalse(ans)
def test_is_replication_enabled(self):
@@ -1592,3 +1590,34 @@ class PowerMaxUtilsTest(test.TestCase):
minimum_version = '9.1.0.5'
self.assertTrue(
self.utils.version_meet_req(version, minimum_version))
def test_parse_specs_from_pool_name_workload_included(self):
pool_name = self.data.vol_type_extra_specs.get('pool_name')
array_id, srp, service_level, workload = (
self.utils.parse_specs_from_pool_name(pool_name))
pool_details = pool_name.split('+')
self.assertEqual(array_id, pool_details[3])
self.assertEqual(srp, pool_details[2])
self.assertEqual(workload, pool_details[1])
self.assertEqual(service_level, pool_details[0])
def test_parse_specs_from_pool_name_workload_not_included(self):
pool_name = (
self.data.vol_type_extra_specs_next_gen_pool.get('pool_name'))
array_id, srp, service_level, workload = (
self.utils.parse_specs_from_pool_name(pool_name))
pool_details = pool_name.split('+')
self.assertEqual(array_id, pool_details[2])
self.assertEqual(srp, pool_details[1])
self.assertEqual(service_level, pool_details[0])
self.assertEqual(workload, str())
def test_parse_specs_from_pool_name_invalid_pool(self):
pool_name = 'This+Is+An+Invalid+Pool'
self.assertRaises(exception.VolumeBackendAPIException,
self.utils.parse_specs_from_pool_name, pool_name)
def test_parse_specs_from_pool_name_no_pool(self):
self.assertRaises(exception.VolumeBackendAPIException,
self.utils.parse_specs_from_pool_name, '')

58
cinder/volume/drivers/dell_emc/powermax/utils.py
View File

@@ -429,18 +429,28 @@ class PowerMaxUtils(object):
message=exception_message)
return array, device_id.upper()
@staticmethod
def is_compression_disabled(extra_specs):
def is_compression_disabled(self, extra_specs):
"""Check is compression is to be disabled.
:param extra_specs: extra specifications
:returns: boolean
"""
do_disable_compression = False
if (DISABLECOMPRESSION in extra_specs and strutils.bool_from_string(
extra_specs[DISABLECOMPRESSION])) or not extra_specs.get(SLO):
do_disable_compression = True
return do_disable_compression
compression_disabled = False
if extra_specs.get(DISABLECOMPRESSION, False):
if strutils.bool_from_string(extra_specs.get(DISABLECOMPRESSION)):
compression_disabled = True
else:
if extra_specs.get(SLO):
service_level = extra_specs.get(SLO)
else:
__, __, service_level, __ = self.parse_specs_from_pool_name(
extra_specs.get('pool_name'))
if not service_level:
compression_disabled = True
return compression_disabled
def change_compression_type(self, is_source_compr_disabled, new_type):
"""Check if volume type have different compression types
@@ -1919,3 +1929,37 @@ class PowerMaxUtils(object):
"""
from pkg_resources import parse_version
return parse_version(version) >= parse_version(minimum_version)
@staticmethod
def parse_specs_from_pool_name(pool_name):
"""Parse basic volume type specs from pool_name.
:param pool_name: the pool name -- str
:returns: array_id, srp, service_level, workload -- str, str, str, str
"""
array_id, srp, service_level, workload = str(), str(), str(), str()
pool_details = pool_name.split('+')
if len(pool_details) == 4:
array_id = pool_details[3]
srp = pool_details[2]
service_level = pool_details[0]
if not pool_details[1].lower() == 'none':
workload = pool_details[1]
elif len(pool_details) == 3:
service_level = pool_details[0]
srp = pool_details[1]
array_id = pool_details[2]
else:
if not pool_name:
msg = (_('No pool_name specified in volume-type.'))
else:
msg = (_("There has been a problem parsing the pool "
"information from pool_name '%(pool)s'." % {
'pool': pool_name}))
raise exception.VolumeBackendAPIException(msg)
if service_level.lower() == 'none':
service_level = str()
return array_id, srp, service_level, workload