VMAX driver - Compression, replacing SMI-S with REST

In VMAX driver version 3.0, SMI-S has been replaced with
unisphere REST. This is porting Compression from SMIS to REST.
See original https://review.openstack.org/#/c/400824/ for more
details

Change-Id: I2d7cda0b5856ec5c3c1ca7df4f2ad93936f15b57
Partially-Implements: blueprint vmax-rest

cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py

@@ -60,6 +60,7 @@ class VMAXCommonData(object):
     storagegroup_name_i = 'OS-HostX-SRP_1-Diamond-DSS-OS-iscsi-PG'
     defaultstoragegroup_name = 'OS-SRP_1-Diamond-DSS-SG'
     default_sg_no_slo = 'OS-no_SLO-SG'
+    default_sg_compr_disabled = 'OS-SRP_1-Diamond-DSS-CD-SG'
     failed_resource = 'OS-failed-resource'
     fake_host = 'HostX@Backend#Diamond+DSS+SRP_1+000197800123'
     new_host = 'HostX@Backend#Silver+OLTP+SRP_1+000197800123'
@@ -155,6 +156,9 @@ class VMAXCommonData(object):
 
     # extra-specs
     vol_type_extra_specs = {'pool_name': u'Diamond+DSS+SRP_1+000197800123'}
+    vol_type_extra_specs_compr_disabled = {
+        'pool_name': u'Diamond+DSS+SRP_1+000197800123',
+        'storagetype:disablecompression': "true"}
     extra_specs = {'pool_name': u'Diamond+DSS+SRP_1+000197800123',
                    'slo': slo,
                    'workload': workload,
@@ -162,6 +166,8 @@ class VMAXCommonData(object):
                    'array': array,
                    'interval': 3,
                    'retries': 120}
+    extra_specs_disable_compression = deepcopy(extra_specs)
+    extra_specs_disable_compression[utils.DISABLECOMPRESSION] = "true"
     extra_specs_intervals_set = deepcopy(extra_specs)
     extra_specs_intervals_set['interval'] = 1
     extra_specs_intervals_set['retries'] = 1
@@ -176,6 +182,7 @@ class VMAXCommonData(object):
         'maskingview_name': masking_view_name_f,
         'parent_sg_name': parent_sg_f,
         'srp': srp,
+        'storagetype:disablecompression': False,
         'port_group_name': port_group_name_f,
         'slo': slo,
         'storagegroup_name': storagegroup_name_f,
@@ -190,6 +197,7 @@ class VMAXCommonData(object):
         'initiator_check': False,
         'maskingview_name': masking_view_name_f,
         'srp': srp,
+        'storagetype:disablecompression': False,
         'port_group_name': port_group_name_f,
         'slo': None,
         'parent_sg_name': parent_sg_f,
@@ -197,8 +205,25 @@ class VMAXCommonData(object):
         'volume_name': test_volume.name,
         'workload': None}
 
+    masking_view_dict_compression_disabled = {
+        'array': array,
+        'connector': connector,
+        'device_id': '00001',
+        'init_group_name': initiatorgroup_name_f,
+        'initiator_check': False,
+        'maskingview_name': masking_view_name_f,
+        'srp': srp,
+        'storagetype:disablecompression': True,
+        'port_group_name': port_group_name_f,
+        'slo': slo,
+        'parent_sg_name': parent_sg_f,
+        'storagegroup_name': 'OS-HostX-SRP_1-DiamondDSS-OS-fibre-PG-CD',
+        'volume_name': test_volume['name'],
+        'workload': workload}
+
     # vmax data
     # sloprovisioning
+    compression_info = {"symmetrixId": ["000197800128"]}
     inititiatorgroup = [{"initiator": [wwpn1],
                          "hostId": initiatorgroup_name_f,
                          "maskingview": [masking_view_name_f]},
@@ -452,6 +477,8 @@ class FakeRequestsSession(object):
                 return_object = self.data.srp_details
             elif 'workloadtype' in url:
                 return_object = self.data.workloadtype
+            elif 'compressionCapable' in url:
+                return_object = self.data.compression_info
             else:
                 return_object = self.data.slo_details
 
@@ -857,6 +884,14 @@ class VMAXUtilsTest(test.TestCase):
             srp_name, slo, workload)
         self.assertEqual(self.data.default_sg_no_slo, sg_name)
 
+    def test_get_default_storage_group_name_compr_disabled(self):
+        srp_name = self.data.srp
+        slo = self.data.slo
+        workload = self.data.workload
+        sg_name = self.utils.get_default_storage_group_name(
+            srp_name, slo, workload, True)
+        self.assertEqual(self.data.default_sg_compr_disabled, sg_name)
+
     def test_get_time_delta(self):
         start_time = 1487781721.09
         end_time = 1487781758.16
@@ -910,6 +945,41 @@ class VMAXUtilsTest(test.TestCase):
         pg3 = self.utils.get_pg_short_name(pg_over_12_chars)
         self.assertEqual(pg2, pg3)
 
+    def test_is_compression_disabled_true(self):
+        extra_specs = self.data.extra_specs_disable_compression
+        do_disable_compression = self.utils.is_compression_disabled(
+            extra_specs)
+        self.assertTrue(do_disable_compression)
+
+    def test_is_compression_disabled_false(self):
+        # Path 1: no compressiion extra spec set
+        extra_specs = self.data.extra_specs
+        do_disable_compression = self.utils.is_compression_disabled(
+            extra_specs)
+        self.assertFalse(do_disable_compression)
+        # Path 2: compression extra spec set to false
+        extra_specs2 = deepcopy(extra_specs)
+        extra_specs2.update({utils.DISABLECOMPRESSION: 'false'})
+        do_disable_compression2 = self.utils.is_compression_disabled(
+            extra_specs)
+        self.assertFalse(do_disable_compression2)
+
+    def test_change_compression_type_true(self):
+        source_compr_disabled_true = 'true'
+        new_type_compr_disabled = {
+            'extra_specs': {utils.DISABLECOMPRESSION: 'no'}}
+        ans = self.utils.change_compression_type(
+            source_compr_disabled_true, new_type_compr_disabled)
+        self.assertTrue(ans)
+
+    def test_change_compression_type_false(self):
+        source_compr_disabled_true = True
+        new_type_compr_disabled = {
+            'extra_specs': {utils.DISABLECOMPRESSION: 'true'}}
+        ans = self.utils.change_compression_type(
+            source_compr_disabled_true, new_type_compr_disabled)
+        self.assertFalse(ans)
+
 
 class VMAXRestTest(test.TestCase):
     def setUp(self):
@@ -1115,6 +1185,17 @@ class VMAXRestTest(test.TestCase):
             self.data.slo, self.data.workload)
         self.assertIsNone(headroom_cap)
 
+    def test_is_compression_capable_true(self):
+        compr_capable = self.rest.is_compression_capable('000197800128')
+        self.assertTrue(compr_capable)
+
+    def test_is_compression_capable_false(self):
+        compr_capable = self.rest.is_compression_capable(self.data.array)
+        self.assertFalse(compr_capable)
+        with mock.patch.object(self.rest, 'request', return_value=(200, {})):
+            compr_capable = self.rest.is_compression_capable(self.data.array)
+            self.assertFalse(compr_capable)
+
     def test_get_storage_group(self):
         ref_details = self.data.sg_details[0]
         sg_details = self.rest.get_storage_group(
@@ -1159,6 +1240,28 @@ class VMAXRestTest(test.TestCase):
             None, None, self.data.extra_specs)
         self.assertEqual(self.data.default_sg_no_slo, sg_name)
 
+    def test_create_storage_group_compression_disabled(self):
+        with mock.patch.object(self.rest, '_create_storagegroup',
+                               return_value=(200, self.data.job_list[0])):
+            self.rest.create_storage_group(
+                self.data.array, self.data.default_sg_compr_disabled,
+                self.data.srp, self.data.slo, self.data.workload,
+                self.data.extra_specs, True)
+            payload = {"srpId": self.data.srp,
+                       "storageGroupId": self.data.default_sg_compr_disabled,
+                       "emulation": "FBA",
+                       "create_empty_storage_group": "true",
+                       "sloBasedStorageGroupParam": [
+                           {"num_of_vols": 0,
+                            "sloId": self.data.slo,
+                            "workloadSelection": self.data.workload,
+                            "volumeAttribute": {
+                                "volume_size": "0",
+                                "capacityUnit": "GB"},
+                            "noCompression": "true"}]}
+            self.rest._create_storagegroup.assert_called_once_with(
+                self.data.array, payload)
+
     def test_modify_storage_group(self):
         array = self.data.array
         storagegroup = self.data.defaultstoragegroup_name
@@ -2651,6 +2754,17 @@ class VMAXCommonTest(test.TestCase):
             volume, connector, extra_specs)
         self.assertEqual(ref_mv_dict, masking_view_dict)
 
+    def test_populate_masking_dict_compr_disabled(self):
+        volume = self.data.test_volume
+        connector = self.data.connector
+        extra_specs = deepcopy(self.data.extra_specs)
+        extra_specs['port_group_name'] = self.data.port_group_name_f
+        extra_specs[utils.DISABLECOMPRESSION] = "true"
+        ref_mv_dict = self.data.masking_view_dict_compression_disabled
+        masking_view_dict = self.common._populate_masking_dict(
+            volume, connector, extra_specs)
+        self.assertEqual(ref_mv_dict, masking_view_dict)
+
     def test_create_cloned_volume(self):
         volume = self.data.test_clone_volume
         source_volume = self.data.test_volume
@@ -2823,6 +2937,27 @@ class VMAXCommonTest(test.TestCase):
         extra_specs = self.common._set_vmax_extra_specs({}, srp_record)
         self.assertEqual('Optimized', extra_specs['slo'])
 
+    def test_set_vmax_extra_specs_compr_disabled(self):
+        with mock.patch.object(self.rest, 'is_compression_capable',
+                               return_value=True):
+            srp_record = self.utils.parse_file_to_get_array_map(
+                self.fake_xml)
+            extra_specs = self.common._set_vmax_extra_specs(
+                self.data.vol_type_extra_specs_compr_disabled, srp_record)
+            ref_extra_specs = deepcopy(self.data.extra_specs_intervals_set)
+            ref_extra_specs['port_group_name'] = self.data.port_group_name_f
+            ref_extra_specs[utils.DISABLECOMPRESSION] = "true"
+            self.assertEqual(ref_extra_specs, extra_specs)
+
+    def test_set_vmax_extra_specs_compr_disabled_not_compr_capable(self):
+        srp_record = self.utils.parse_file_to_get_array_map(
+            self.fake_xml)
+        extra_specs = self.common._set_vmax_extra_specs(
+            self.data.vol_type_extra_specs_compr_disabled, srp_record)
+        ref_extra_specs = deepcopy(self.data.extra_specs_intervals_set)
+        ref_extra_specs['port_group_name'] = self.data.port_group_name_f
+        self.assertEqual(ref_extra_specs, extra_specs)
+
     def test_set_vmax_extra_specs_portgroup_as_spec(self):
         srp_record = self.utils.parse_file_to_get_array_map(
             self.fake_xml)
@@ -3161,41 +3296,44 @@ class VMAXCommonTest(test.TestCase):
         extra_specs = self.data.extra_specs_intervals_set
         extra_specs['port_group_name'] = self.data.port_group_name_f
         volume = self.data.test_volume
+        new_type = {'extra_specs': {}}
         host = {'host': self.data.new_host}
-        self.common.retype(volume, host)
+        self.common.retype(volume, new_type, host)
         mock_migrate.assert_called_once_with(
-            device_id, volume, host, volume_name, extra_specs)
+            device_id, volume, host, volume_name, new_type, extra_specs)
         mock_migrate.reset_mock()
         with mock.patch.object(
                 self.common, '_find_device_on_array', return_value=None):
-            self.common.retype(volume, host)
+            self.common.retype(volume, new_type, host)
             mock_migrate.assert_not_called()
 
     def test_slo_workload_migration_valid(self):
         device_id = self.data.volume_details[0]['volumeId']
         volume_name = self.data.test_volume['name']
         extra_specs = self.data.extra_specs
+        new_type = {'extra_specs': {}}
         volume = self.data.test_volume
         host = {'host': self.data.new_host}
         with mock.patch.object(self.common, '_migrate_volume'):
             self.common._slo_workload_migration(
-                device_id, volume, host, volume_name, extra_specs)
+                device_id, volume, host, volume_name, new_type, extra_specs)
             self.common._migrate_volume.assert_called_once_with(
                 extra_specs[utils.ARRAY], device_id,
                 extra_specs[utils.SRP], 'Silver',
-                'OLTP', volume_name, extra_specs)
+                'OLTP', volume_name, new_type, extra_specs)
 
     def test_slo_workload_migration_not_valid(self):
         device_id = self.data.volume_details[0]['volumeId']
         volume_name = self.data.test_volume['name']
         extra_specs = self.data.extra_specs
         volume = self.data.test_volume
+        new_type = {'extra_specs': {}}
         host = {'host': self.data.new_host}
         with mock.patch.object(self.common,
                                '_is_valid_for_storage_assisted_migration',
                                return_value=(False, 'Silver', 'OLTP')):
             migrate_status = self.common._slo_workload_migration(
-                device_id, volume, host, volume_name, extra_specs)
+                device_id, volume, host, volume_name, new_type, extra_specs)
             self.assertFalse(migrate_status)
 
     def test_slo_workload_migration_same_hosts(self):
@@ -3204,10 +3342,31 @@ class VMAXCommonTest(test.TestCase):
         extra_specs = self.data.extra_specs
         volume = self.data.test_volume
         host = {'host': self.data.fake_host}
+        new_type = {'extra_specs': {}}
         migrate_status = self.common._slo_workload_migration(
-            device_id, volume, host, volume_name, extra_specs)
+            device_id, volume, host, volume_name, new_type, extra_specs)
         self.assertFalse(migrate_status)
 
+    def test_slo_workload_migration_same_host_change_compression(self):
+        device_id = self.data.volume_details[0]['volumeId']
+        volume_name = self.data.test_volume['name']
+        extra_specs = self.data.extra_specs
+        volume = self.data.test_volume
+        host = {'host': self.data.fake_host}
+        new_type = {'extra_specs': {utils.DISABLECOMPRESSION: "true"}}
+        with mock.patch.object(
+                self.common, '_is_valid_for_storage_assisted_migration',
+                return_value=(True, self.data.slo, self.data.workload)):
+            with mock.patch.object(self.common, '_migrate_volume'):
+                migrate_status = self.common._slo_workload_migration(
+                    device_id, volume, host, volume_name, new_type,
+                    extra_specs)
+                self.assertTrue(migrate_status)
+                self.common._migrate_volume.assert_called_once_with(
+                    extra_specs[utils.ARRAY], device_id,
+                    extra_specs[utils.SRP], self.data.slo,
+                    self.data.workload, volume_name, new_type, extra_specs)
+
     @mock.patch.object(masking.VMAXMasking, 'remove_and_reset_members')
     def test_migrate_volume_success(self, mock_remove):
         with mock.patch.object(self.rest, 'is_volume_in_storagegroup',
@@ -3215,9 +3374,10 @@ class VMAXCommonTest(test.TestCase):
             device_id = self.data.volume_details[0]['volumeId']
             volume_name = self.data.test_volume['name']
             extra_specs = self.data.extra_specs
+            new_type = {'extra_specs': {}}
             migrate_status = self.common._migrate_volume(
                 self.data.array, device_id, self.data.srp, self.data.slo,
-                self.data.workload, volume_name, extra_specs)
+                self.data.workload, volume_name, new_type, extra_specs)
             self.assertTrue(migrate_status)
             mock_remove.assert_called_once_with(
                 self.data.array, device_id, None, extra_specs, False)
@@ -3227,7 +3387,7 @@ class VMAXCommonTest(test.TestCase):
                     return_value=[]):
                 migrate_status = self.common._migrate_volume(
                     self.data.array, device_id, self.data.srp, self.data.slo,
-                    self.data.workload, volume_name, extra_specs)
+                    self.data.workload, volume_name, new_type, extra_specs)
                 self.assertTrue(migrate_status)
                 mock_remove.assert_not_called()
 
@@ -3236,24 +3396,26 @@ class VMAXCommonTest(test.TestCase):
         device_id = self.data.volume_details[0]['volumeId']
         volume_name = self.data.test_volume['name']
         extra_specs = self.data.extra_specs
+        new_type = {'extra_specs': {}}
         with mock.patch.object(
                 self.masking, 'get_or_create_default_storage_group',
                 side_effect=exception.VolumeBackendAPIException):
             migrate_status = self.common._migrate_volume(
                 self.data.array, device_id, self.data.srp, self.data.slo,
-                self.data.workload, volume_name, extra_specs)
+                self.data.workload, volume_name, new_type, extra_specs)
             self.assertFalse(migrate_status)
 
     def test_migrate_volume_failed_vol_not_added(self):
         device_id = self.data.volume_details[0]['volumeId']
         volume_name = self.data.test_volume['name']
         extra_specs = self.data.extra_specs
+        new_type = {'extra_specs': {}}
         with mock.patch.object(
                 self.rest, 'is_volume_in_storagegroup',
                 return_value=False):
             migrate_status = self.common._migrate_volume(
                 self.data.array, device_id, self.data.srp, self.data.slo,
-                self.data.workload, volume_name, extra_specs)
+                self.data.workload, volume_name, new_type, extra_specs)
             self.assertFalse(migrate_status)
 
     def test_is_valid_for_storage_assisted_migration_true(self):
@@ -3262,13 +3424,15 @@ class VMAXCommonTest(test.TestCase):
         volume_name = self.data.test_volume['name']
         ref_return = (True, 'Silver', 'OLTP')
         return_val = self.common._is_valid_for_storage_assisted_migration(
-            device_id, host, self.data.array, self.data.srp, volume_name)
+            device_id, host, self.data.array,
+            self.data.srp, volume_name, False)
         self.assertEqual(ref_return, return_val)
         # No current sgs found
         with mock.patch.object(self.rest, 'get_storage_groups_from_volume',
                                return_value=None):
             return_val = self.common._is_valid_for_storage_assisted_migration(
-                device_id, host, self.data.array, self.data.srp, volume_name)
+                device_id, host, self.data.array, self.data.srp,
+                volume_name, False)
             self.assertEqual(ref_return, return_val)
 
     def test_is_valid_for_storage_assisted_migration_false(self):
@@ -3278,22 +3442,26 @@ class VMAXCommonTest(test.TestCase):
         # IndexError
         host = {'host': 'HostX@Backend#Silver+SRP_1+000197800123'}
         return_val = self.common._is_valid_for_storage_assisted_migration(
-            device_id, host, self.data.array, self.data.srp, volume_name)
+            device_id, host, self.data.array,
+            self.data.srp, volume_name, False)
         self.assertEqual(ref_return, return_val)
         # Wrong array
         host2 = {'host': 'HostX@Backend#Silver+OLTP+SRP_1+00012345678'}
         return_val = self.common._is_valid_for_storage_assisted_migration(
-            device_id, host2, self.data.array, self.data.srp, volume_name)
+            device_id, host2, self.data.array,
+            self.data.srp, volume_name, False)
         self.assertEqual(ref_return, return_val)
         # Wrong srp
         host3 = {'host': 'HostX@Backend#Silver+OLTP+SRP_2+000197800123'}
         return_val = self.common._is_valid_for_storage_assisted_migration(
-            device_id, host3, self.data.array, self.data.srp, volume_name)
+            device_id, host3, self.data.array,
+            self.data.srp, volume_name, False)
         self.assertEqual(ref_return, return_val)
         # Already in correct sg
         host4 = {'host': self.data.fake_host}
         return_val = self.common._is_valid_for_storage_assisted_migration(
-            device_id, host4, self.data.array, self.data.srp, volume_name)
+            device_id, host4, self.data.array,
+            self.data.srp, volume_name, False)
         self.assertEqual(ref_return, return_val)
 
 
@@ -3509,11 +3677,12 @@ class VMAXFCTest(test.TestCase):
 
     def test_retype(self):
         host = {'host': self.data.new_host}
+        new_type = {'extra_specs': {}}
         with mock.patch.object(self.common, 'retype',
                                return_value=True):
-            self.driver.retype({}, self.data.test_volume, '', '', host)
+            self.driver.retype({}, self.data.test_volume, new_type, '', host)
             self.common.retype.assert_called_once_with(
-                self.data.test_volume, host)
+                self.data.test_volume, new_type, host)
 
 
 class VMAXISCSITest(test.TestCase):
@@ -3739,11 +3908,12 @@ class VMAXISCSITest(test.TestCase):
 
     def test_retype(self):
         host = {'host': self.data.new_host}
+        new_type = {'extra_specs': {}}
         with mock.patch.object(self.common, 'retype',
                                return_value=True):
-            self.driver.retype({}, self.data.test_volume, '', '', host)
+            self.driver.retype({}, self.data.test_volume, new_type, '', host)
             self.common.retype.assert_called_once_with(
-                self.data.test_volume, host)
+                self.data.test_volume, new_type, host)
 
 
 class VMAXMaskingTest(test.TestCase):

cinder/volume/drivers/dell_emc/vmax/common.py

@@ -19,6 +19,7 @@ import sys
 
 from oslo_config import cfg
 from oslo_log import log as logging
+from oslo_utils import strutils
 import six
 
 from cinder import exception
@@ -854,6 +855,7 @@ class VMAXCommon(object):
         unique_name = self.utils.truncate_string(extra_specs[utils.SRP], 12)
         protocol = self.utils.get_short_protocol_type(self.protocol)
         short_host_name = self.utils.get_host_short_name(host_name)
+        masking_view_dict[utils.DISABLECOMPRESSION] = False
         slo = extra_specs[utils.SLO]
         workload = extra_specs[utils.WORKLOAD]
         short_pg_name = self.utils.get_pg_short_name(
@@ -877,6 +879,12 @@ class VMAXCommon(object):
                    'srpName': unique_name,
                    'combo': slo_wl_combo,
                    'pg': short_pg_name})
+            do_disable_compression = self.utils.is_compression_disabled(
+                extra_specs)
+            if do_disable_compression:
+                child_sg_name = ("%(child_sg_name)s-CD"
+                                 % {'child_sg_name': child_sg_name})
+                masking_view_dict[utils.DISABLECOMPRESSION] = True
         else:
             child_sg_name = (
                 "OS-%(shortHostName)s-No_SLO-%(pg)s"
@@ -1092,9 +1100,13 @@ class VMAXCommon(object):
                    'array': array,
                    'size': volume_size})
 
+        do_disable_compression = self.utils.is_compression_disabled(
+            extra_specs)
+
         storagegroup_name = self.masking.get_or_create_default_storage_group(
             array, extra_specs[utils.SRP], extra_specs[utils.SLO],
-            extra_specs[utils.WORKLOAD], extra_specs)
+            extra_specs[utils.WORKLOAD], extra_specs,
+            do_disable_compression)
         try:
             volume_dict = self.provision.create_volume_from_sg(
                 array, volume_name, storagegroup_name,
@@ -1187,6 +1199,14 @@ class VMAXCommon(object):
             slo_from_extra_spec = None
         extra_specs[utils.SLO] = slo_from_extra_spec
         extra_specs[utils.WORKLOAD] = workload_from_extra_spec
+        if self.rest.is_compression_capable(extra_specs[utils.ARRAY]):
+            if extra_specs.get(utils.DISABLECOMPRESSION):
+                # If not True remove it.
+                if not strutils.bool_from_string(
+                        extra_specs[utils.DISABLECOMPRESSION]):
+                    extra_specs.pop(utils.DISABLECOMPRESSION, None)
+        else:
+            extra_specs.pop(utils.DISABLECOMPRESSION, None)
 
         LOG.debug("SRP is: %(srp)s "
                   "Array is: %(array)s "
@@ -1536,10 +1556,11 @@ class VMAXCommon(object):
             self.rest.rename_volume(
                 extra_specs[utils.ARRAY], device_id, volume_id)
 
-    def retype(self, volume, host):
+    def retype(self, volume, new_type, host):
         """Migrate volume to another host using retype.
 
         :param volume: the volume object including the volume_type_id
+        :param new_type: the new volume type.
         :param host: The host dict holding the relevant target(destination)
             information
         :returns: boolean -- True if retype succeeded, False if error
@@ -1558,23 +1579,30 @@ class VMAXCommon(object):
             return False
 
         return self._slo_workload_migration(device_id, volume, host,
-                                            volume_name, extra_specs)
+                                            volume_name, new_type, extra_specs)
 
     def _slo_workload_migration(self, device_id, volume, host,
-                                volume_name, extra_specs):
+                                volume_name, new_type, extra_specs):
         """Migrate from SLO/Workload combination to another.
 
         :param device_id: the volume device id
         :param volume: the volume object
         :param host: the host dict
         :param volume_name: the name of the volume
+        :param new_type: the type to migrate to
         :param extra_specs: extra specifications
         :returns: boolean -- True if migration succeeded, False if error.
         """
+        is_compression_disabled = self.utils.is_compression_disabled(
+            extra_specs)
+        # Check if old type and new type have different compression types
+        do_change_compression = (self.utils.change_compression_type(
+            is_compression_disabled, new_type))
         is_valid, target_slo, target_workload = (
             self._is_valid_for_storage_assisted_migration(
                 device_id, host, extra_specs[utils.ARRAY],
-                extra_specs[utils.SRP], volume_name))
+                extra_specs[utils.SRP], volume_name,
+                do_change_compression))
 
         if not is_valid:
             LOG.error(
@@ -1582,23 +1610,24 @@ class VMAXCommon(object):
                 "assisted migration using retype.",
                 {'name': volume_name})
             return False
-        if volume.host != host['host']:
+        if volume.host != host['host'] or do_change_compression:
             LOG.debug(
                 "Retype Volume %(name)s from source host %(sourceHost)s "
-                "to target host %(targetHost)s. ",
+                "to target host %(targetHost)s. Compression change is %(cc)r.",
                 {'name': volume_name,
                  'sourceHost': volume.host,
-                 'targetHost': host['host']})
+                 'targetHost': host['host'],
+                 'cc': do_change_compression})
             return self._migrate_volume(
                 extra_specs[utils.ARRAY], device_id,
                 extra_specs[utils.SRP], target_slo,
-                target_workload, volume_name, extra_specs)
+                target_workload, volume_name, new_type, extra_specs)
 
         return False
 
     def _migrate_volume(
             self, array, device_id, srp, target_slo,
-            target_workload, volume_name, extra_specs):
+            target_workload, volume_name, new_type, extra_specs):
         """Migrate from one slo/workload combination to another.
 
         This requires moving the volume from its current SG to a
@@ -1609,23 +1638,28 @@ class VMAXCommon(object):
         :param target_slo: the target service level
         :param target_workload: the target workload
         :param volume_name: the volume name
+        :param new_type: the volume type to migrate to
         :param extra_specs: the extra specifications
         :return: bool
         """
         storagegroups = self.rest.get_storage_groups_from_volume(
             array, device_id)
         if not storagegroups:
-            LOG.warning(
+            LOG.warning("Volume : %(volume_name)s does not currently "
-                "Volume : %(volume_name)s does not currently "
+                        "belong to any storage groups.",
-                "belong to any storage groups.",
+                        {'volume_name': volume_name})
-                {'volume_name': volume_name})
         else:
             self.masking.remove_and_reset_members(
                 array, device_id, None, extra_specs, False)
 
+        target_extra_specs = new_type['extra_specs']
+        is_compression_disabled = self.utils.is_compression_disabled(
+            target_extra_specs)
+
         try:
             target_sg_name = self.masking.get_or_create_default_storage_group(
-                array, srp, target_slo, target_workload, extra_specs)
+                array, srp, target_slo, target_workload, extra_specs,
+                is_compression_disabled)
         except Exception as e:
             LOG.error("Failed to get or create storage group. "
                       "Exception received was %(e)s.", {'e': e})
@@ -1648,7 +1682,7 @@ class VMAXCommon(object):
 
     def _is_valid_for_storage_assisted_migration(
             self, device_id, host, source_array,
-            source_srp, volume_name):
+            source_srp, volume_name, do_change_compression):
         """Check if volume is suitable for storage assisted (pool) migration.
 
         :param device_id: the volume device id
@@ -1656,6 +1690,7 @@ class VMAXCommon(object):
         :param source_array: the volume's current array serial number
         :param source_srp: the volume's current pool name
         :param volume_name: the name of the volume to be migrated
+        :param do_change_compression: do change compression
         :returns: boolean -- True/False
         :returns: string -- targetSlo
         :returns: string -- targetWorkload
@@ -1711,12 +1746,15 @@ class VMAXCommon(object):
                                       % {'targetSlo': target_slo,
                                          'targetWorkload': target_workload})
                 if target_combination in emc_fast_setting:
-                    LOG.warning(
+                    # Check if migration is from compression to non compression
-                        "No action required. Volume: %(volume_name)s is "
+                    # or vice versa
-                        "already part of slo/workload combination: "
+                    if not do_change_compression:
-                        "%(targetCombination)s.",
+                        LOG.warning(
-                        {'volume_name': volume_name,
+                            "No action required. Volume: %(volume_name)s is "
-                         'targetCombination': target_combination})
+                            "already part of slo/workload combination: "
+                            "%(targetCombination)s.",
+                            {'volume_name': volume_name,
+                             'targetCombination': target_combination})
                     return false_ret
 
         return True, target_slo, target_workload

cinder/volume/drivers/dell_emc/vmax/fc.py

@@ -78,6 +78,7 @@ class VMAXFCDriver(driver.FibreChannelDriver):
         3.0.0 - REST based driver
               - Retype (storage-assisted migration)
               - QoS support
+              - Support for compression on All Flash
     """
 
     VERSION = "3.0.0"
@@ -438,4 +439,4 @@ class VMAXFCDriver(driver.FibreChannelDriver):
             target(destination) information
         :returns: boolean -- True if retype succeeded, False if error
         """
-        return self.common.retype(volume, host)
+        return self.common.retype(volume, new_type, host)

cinder/volume/drivers/dell_emc/vmax/iscsi.py

@@ -83,6 +83,7 @@ class VMAXISCSIDriver(driver.ISCSIDriver):
         3.0.0 - REST based driver
               - Retype (storage-assisted migration)
               - QoS support
+              - Support for compression on All Flash
     """
 
     VERSION = "3.0.0"
@@ -382,4 +383,4 @@ class VMAXISCSIDriver(driver.ISCSIDriver):
             target(destination) information
         :returns: boolean -- True if retype succeeded, False if error
         """
-        return self.common.retype(volume, host)
+        return self.common.retype(volume, new_type, host)

cinder/volume/drivers/dell_emc/vmax/masking.py

@@ -141,7 +141,8 @@ class VMAXMasking(object):
         default_sg_name = self.utils.get_default_storage_group_name(
             masking_view_dict[utils.SRP],
             masking_view_dict[utils.SLO],
-            masking_view_dict[utils.WORKLOAD])
+            masking_view_dict[utils.WORKLOAD],
+            masking_view_dict[utils.DISABLECOMPRESSION])
 
         check_vol = self.rest.is_volume_in_storagegroup(
             serial_number, device_id, default_sg_name)
@@ -352,12 +353,14 @@ class VMAXMasking(object):
             slo = None
         else:
             slo = extra_specs[utils.SLO]
+        do_disable_compression = (
+            masking_view_dict[utils.DISABLECOMPRESSION])
         storagegroup = self.rest.get_storage_group(
             serial_number, storagegroup_name)
         if storagegroup is None:
             storagegroup = self.provision.create_storage_group(
                 serial_number, storagegroup_name, srp, slo, workload,
-                extra_specs)
+                extra_specs, do_disable_compression)
 
         if storagegroup is None:
             msg = ("Cannot get or create a storage group: "
@@ -1210,16 +1213,19 @@ class VMAXMasking(object):
         :param volume_name: the volume name
         :param extra_specs: the extra specifications
         """
+        do_disable_compression = self.utils.is_compression_disabled(
+            extra_specs)
         storagegroup_name = self.get_or_create_default_storage_group(
             serial_number, extra_specs[utils.SRP], extra_specs[utils.SLO],
-            extra_specs[utils.WORKLOAD], extra_specs)
+            extra_specs[utils.WORKLOAD], extra_specs, do_disable_compression)
 
         self._check_adding_volume_to_storage_group(
             serial_number, device_id, storagegroup_name, volume_name,
             extra_specs)
 
     def get_or_create_default_storage_group(
-            self, serial_number, srp, slo, workload, extra_specs):
+            self, serial_number, srp, slo, workload, extra_specs,
+            do_disable_compression=False):
         """Get or create a default storage group.
 
         :param serial_number: the array serial number
@@ -1227,12 +1233,13 @@ class VMAXMasking(object):
         :param slo: the SLO
         :param workload: the workload
         :param extra_specs: extra specifications
+        :param do_disable_compression: flag for compression
         :returns: storagegroup_name
         :raises: VolumeBackendAPIException
         """
         storagegroup, storagegroup_name = (
             self.rest.get_vmax_default_storage_group(
-                serial_number, srp, slo, workload))
+                serial_number, srp, slo, workload, do_disable_compression))
         if storagegroup is None:
             self.provision.create_storage_group(
                 serial_number, storagegroup_name, srp, slo, workload,

cinder/volume/drivers/dell_emc/vmax/provision.py

@@ -35,7 +35,8 @@ class VMAXProvision(object):
         self.rest = rest
 
     def create_storage_group(
-            self, array, storagegroup_name, srp, slo, workload, extra_specs):
+            self, array, storagegroup_name, srp, slo, workload,
+            extra_specs, do_disable_compression=False):
         """Create a new storage group.
 
         :param array: the array serial number
@@ -44,6 +45,7 @@ class VMAXProvision(object):
         :param slo: the SLO (String)
         :param workload: the workload (String)
         :param extra_specs: additional info
+        :param do_disable_compression: disable compression flag
         :returns: storagegroup - storage group object
         """
         start_time = time.time()
@@ -51,7 +53,8 @@ class VMAXProvision(object):
         @coordination.synchronized("emc-sg-{storage_group}")
         def do_create_storage_group(storage_group):
             storagegroup = self.rest.create_storage_group(
-                array, storage_group, srp, slo, workload, extra_specs)
+                array, storage_group, srp, slo, workload, extra_specs,
+                do_disable_compression)
 
             LOG.debug("Create storage group took: %(delta)s H:MM:SS.",
                       {'delta': self.utils.get_time_delta(start_time,

cinder/volume/drivers/dell_emc/vmax/rest.py

@@ -463,6 +463,22 @@ class VMAXRest(object):
             remaining_capacity = None
         return remaining_capacity
 
+    def is_compression_capable(self, array):
+        """Check if array is compression capable.
+
+        :param array: array serial number
+        :returns: bool
+        """
+        is_compression_capable = False
+        target_uri = "/84/sloprovisioning/symmetrix?compressionCapable=true"
+        status_code, message = self.request(target_uri, GET)
+        self.check_status_code_success(
+            "Check if compression enabled", status_code, message)
+        if message.get('symmetrixId'):
+            if array in message['symmetrixId']:
+                is_compression_capable = True
+        return is_compression_capable
+
     def get_storage_group(self, array, storage_group_name):
         """Given a name, return storage group details.
 
@@ -565,7 +581,8 @@ class VMAXRest(object):
             array, SLOPROVISIONING, 'storagegroup', payload)
 
     def create_storage_group(self, array, storagegroup_name,
-                             srp, slo, workload, extra_specs):
+                             srp, slo, workload, extra_specs,
+                             do_disable_compression=False):
         """Create the volume in the specified storage group.
 
         :param array: the array serial number
@@ -573,6 +590,7 @@ class VMAXRest(object):
         :param srp: the SRP (String)
         :param slo: the SLO (String)
         :param workload: the workload (String)
+        :param do_disable_compression: flag for disabling compression
         :param extra_specs: additional info
         :returns: storagegroup_name - string
         """
@@ -589,6 +607,11 @@ class VMAXRest(object):
                          "volumeAttribute": {
                              "volume_size": "0",
                              "capacityUnit": "GB"}}
+            if do_disable_compression:
+                slo_param.update({"noCompression": "true"})
+            elif self.is_compression_capable(array):
+                slo_param.update({"noCompression": "false"})
+
             payload.update({"sloBasedStorageGroupParam": [slo_param]})
 
         status_code, job = self._create_storagegroup(array, payload)
@@ -783,13 +806,15 @@ class VMAXRest(object):
                 return_value = False
         return return_value
 
-    def get_vmax_default_storage_group(self, array, srp, slo, workload):
+    def get_vmax_default_storage_group(self, array, srp, slo, workload,
+                                       do_disable_compression=False):
         """Get the default storage group.
 
         :param array: the array serial number
         :param srp: the pool name
         :param slo: the SLO
         :param workload: the workload
+        :param do_disable_compression: flag for disabling compression
         :returns: the storage group dict (or None), the storage group name
         """
         storagegroup_name = self.utils.get_default_storage_group_name(

cinder/volume/drivers/dell_emc/vmax/utils.py

@@ -20,6 +20,7 @@ import re
 from xml.dom import minidom
 
 from oslo_log import log as logging
+from oslo_utils import strutils
 import six
 
 from cinder import exception
@@ -52,6 +53,7 @@ PARENT_SG_NAME = 'parent_sg_name'
 CONNECTOR = 'connector'
 VOL_NAME = 'volume_name'
 EXTRA_SPECS = 'extra_specs'
+DISABLECOMPRESSION = 'storagetype:disablecompression'
 
 
 class VMAXUtils(object):
@@ -144,18 +146,24 @@ class VMAXUtils(object):
         return six.text_type(datetime.timedelta(seconds=int(delta)))
 
     @staticmethod
-    def get_default_storage_group_name(srp_name, slo, workload):
+    def get_default_storage_group_name(
+            srp_name, slo, workload, is_compression_disabled=False):
         """Determine default storage group from extra_specs.
 
         :param srp_name: the name of the srp on the array
         :param slo: the service level string e.g Bronze
         :param workload: the workload string e.g DSS
+        :param is_compression_disabled:  flag for disabling compression
         :returns: storage_group_name
         """
         if slo and workload:
             prefix = ("OS-%(srpName)s-%(slo)s-%(workload)s"
                       % {'srpName': srp_name, 'slo': slo,
                          'workload': workload})
+
+            if is_compression_disabled:
+                prefix += "-CD"
+
         else:
             prefix = "OS-no_SLO"
 
@@ -399,3 +407,30 @@ class VMAXUtils(object):
             raise exception.VolumeBackendAPIException(
                 data=exception_message)
         return array, device_id
+
+    @staticmethod
+    def is_compression_disabled(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:
+            if strutils.bool_from_string(extra_specs[DISABLECOMPRESSION]):
+                do_disable_compression = True
+        return do_disable_compression
+
+    def change_compression_type(self, is_source_compr_disabled, new_type):
+        """Check if volume type have different compression types
+
+        :param is_source_compr_disabled: from source
+        :param new_type: from target
+        :returns: boolean
+        """
+        extra_specs = new_type['extra_specs']
+        is_target_compr_disabled = self.is_compression_disabled(extra_specs)
+        if is_target_compr_disabled == is_source_compr_disabled:
+            return False
+        else:
+            return True

releasenotes/notes/vmax-rest-compression-10c2590052a9465e.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - |
+    Adding compression functionality to VMAX driver version 3.0.