Merge "PowerMax driver - check cylinder count of source and target volumes"

cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py

@@ -564,6 +564,7 @@ class PowerMaxData(object):
                         'SLO': 'Diamond'}
 
     volume_details = [{'cap_gb': 2,
+                       'cap_cyl': 1092,
                        'num_of_storage_groups': 1,
                        'volumeId': device_id,
                        'volume_identifier': 'OS-%s' % test_volume.id,
@@ -573,6 +574,7 @@ class PowerMaxData(object):
                        'storageGroupId': [defaultstoragegroup_name,
                                           storagegroup_name_f]},
                       {'cap_gb': 1,
+                       'cap_cyl': 546,
                        'num_of_storage_groups': 1,
                        'volumeId': device_id2,
                        'volume_identifier': 'OS-%s' % test_volume.id,
@@ -580,11 +582,13 @@ class PowerMaxData(object):
                        'storageGroupId': [defaultstoragegroup_name,
                                           storagegroup_name_f]},
                       {'cap_gb': 1,
+                       'cap_cyl': 546,
                        'num_of_storage_groups': 0,
                        'volumeId': device_id3,
                        'volume_identifier': '123',
                        'wwn': '600012345'},
                       {'cap_gb': 1,
+                       'cap_cyl': 546,
                        'num_of_storage_groups': 1,
                        'volumeId': device_id4,
                        'volume_identifier': 'random_name',

cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py

@@ -1179,7 +1179,7 @@ class PowerMaxCommonTest(test.TestCase):
                 self.data.extra_specs, snap_name)
             mock_cleanup.assert_called_once_with(
                 array, device_id, device_id, clone_name, snap_name,
-                extra_specs)
+                extra_specs, target_volume=clone_volume)
 
     def test_create_replica_failed_no_target(self):
         array = self.data.array
@@ -1196,6 +1196,27 @@ class PowerMaxCommonTest(test.TestCase):
                     source_device_id, self.data.extra_specs, snap_name)
                 mock_cleanup.assert_not_called()
 
+    @mock.patch.object(
+        utils.PowerMaxUtils,
+        'compare_cylinders',
+        side_effect=exception.VolumeBackendAPIException)
+    def test_create_replica_cylinder_mismatch(self, mock_cyl):
+        array = self.data.array
+        clone_volume = self.data.test_clone_volume
+        source_device_id = self.data.device_id
+        snap_name = self.data.snap_location['snap_name']
+        clone_name = 'OS-' + clone_volume.id
+        with mock.patch.object(
+                self.common, '_cleanup_target') as mock_cleanup:
+            self.assertRaises(
+                Exception, self.common._create_replica, array,
+                clone_volume, source_device_id,
+                self.data.extra_specs, snap_name)  # noqa: ignore=H202
+            mock_cleanup.assert_called_once_with(
+                array, source_device_id, source_device_id,
+                clone_name, snap_name, self.data.extra_specs,
+                target_volume=clone_volume)
+
     @mock.patch.object(
         masking.PowerMaxMasking,
         'remove_and_reset_members')
@@ -1220,7 +1241,8 @@ class PowerMaxCommonTest(test.TestCase):
                     array, target_device_id, source_device_id,
                     snap_name, extra_specs, generation)
 
-    def test_cleanup_target_no_sync(self):
+    @mock.patch.object(masking.PowerMaxMasking, 'remove_volume_from_sg')
+    def test_cleanup_target_no_sync(self, mock_remove):
         array = self.data.array
         clone_volume = self.data.test_clone_volume
         source_device_id = self.data.device_id

cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_utils.py

@@ -523,3 +523,20 @@ class PowerMaxUtilsTest(test.TestCase):
         ret_prop_dict = self.utils.validate_qos_distribution_type(
             sg_value, qos_extra_spec, input_prop_dict)
         self.assertEqual(input_prop_dict, ret_prop_dict)
+
+    def test_compare_cylinders(self):
+        source_cylinders = '12345'
+        target_cylinders = '12345'
+        self.utils.compare_cylinders(source_cylinders, target_cylinders)
+
+    def test_compare_cylinders_target_larger(self):
+        source_cylinders = '12345'
+        target_cylinders = '12346'
+        self.utils.compare_cylinders(source_cylinders, target_cylinders)
+
+    def test_compare_cylinders_source_larger(self):
+        source_cylinders = '12347'
+        target_cylinders = '12346'
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.utils.compare_cylinders, source_cylinders,
+                          target_cylinders)

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

@@ -2162,13 +2162,21 @@ class PowerMaxCommon(object):
         clone_id = clone_volume.id
         clone_name = self.utils.get_volume_element_name(clone_id)
         create_snap = False
+
+        volume_dict = self.rest.get_volume(array, source_device_id)
         # PowerMax/VMAX supports using a target volume that is bigger than
         # the source volume, so we create the target volume the desired
         # size at this point to avoid having to extend later
         try:
             clone_dict = self._create_volume(
                 clone_name, clone_volume.size, extra_specs)
+
             target_device_id = clone_dict['device_id']
+            if target_device_id:
+                clone_volume_dict = self.rest.get_volume(
+                    array, target_device_id)
+                self.utils.compare_cylinders(
+                    volume_dict['cap_cyl'], clone_volume_dict['cap_cyl'])
             LOG.info("The target device id is: %(device_id)s.",
                      {'device_id': target_device_id})
             if not snap_name:
@@ -2185,7 +2193,8 @@ class PowerMaxCommon(object):
                             {'cloneName': clone_name, 'e': e})
                 self._cleanup_target(
                     array, target_device_id, source_device_id,
-                    clone_name, snap_name, extra_specs)
+                    clone_name, snap_name, extra_specs,
+                    target_volume=clone_volume)
                 # Re-throw the exception.
             raise
         # add source id and snap_name to the clone dict
@@ -2195,7 +2204,8 @@ class PowerMaxCommon(object):
 
     def _cleanup_target(
             self, array, target_device_id, source_device_id,
-            clone_name, snap_name, extra_specs, generation=0):
+            clone_name, snap_name, extra_specs, generation=0,
+            target_volume=None):
         """Cleanup target volume on failed clone/ snapshot creation.
 
         :param array: the array serial number
@@ -2204,6 +2214,7 @@ class PowerMaxCommon(object):
         :param clone_name: the name of the clone volume
         :param extra_specs: the extra specifications
         :param generation: the generation number of the snapshot
+        :param target_volume: the target volume object
         """
         snap_session = self.rest.get_sync_session(
             array, source_device_id, snap_name, target_device_id, generation)
@@ -2211,6 +2222,8 @@ class PowerMaxCommon(object):
             self.provision.break_replication_relationship(
                 array, target_device_id, source_device_id,
                 snap_name, extra_specs, generation)
+        self._remove_vol_and_cleanup_replication(
+            array, target_device_id, clone_name, extra_specs, target_volume)
         self._delete_from_srp(
             array, target_device_id, clone_name, extra_specs)
 

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

@@ -1880,7 +1880,7 @@ class PowerMaxRest(object):
         snapshot = None
         snap_info = self.get_volume_snap_info(array, device_id)
         if snap_info:
-            if (snap_info.get('snapshotSrcs') and
+            if (snap_info.get('snapshotSrcs', None) and
                     bool(snap_info['snapshotSrcs'])):
                 for snap in snap_info['snapshotSrcs']:
                     if snap['snapshotName'] == snap_name:
@@ -1899,7 +1899,8 @@ class PowerMaxRest(object):
         snapshot_list = []
         snap_info = self.get_volume_snap_info(array, source_device_id)
         if snap_info:
-            if bool(snap_info['snapshotSrcs']):
+            if (snap_info.get('snapshotSrcs', None) and
+                    bool(snap_info['snapshotSrcs'])):
                 snapshot_list = snap_info['snapshotSrcs']
         return snapshot_list
 

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

@@ -949,3 +949,21 @@ class PowerMaxUtils(object):
             raise exception.VolumeBackendAPIException(
                 message=exception_message)
         return property_dict
+
+    @staticmethod
+    def compare_cylinders(cylinders_source, cylinder_target):
+        """Compare number of cylinders of source and target.
+
+        :param cylinders_source: number of cylinders on source
+        :param cylinders_target: number of cylinders on target
+        """
+        if float(cylinders_source) > float(cylinder_target):
+            exception_message = (
+                _("The number of source cylinders %(cylinders_source)s "
+                  "cannot be greater than the number of target cylinders "
+                  "%(cylinder_target)s. Please extend your source volume by "
+                  "at least 1GiB.") % {
+                    'cylinders_source': cylinders_source,
+                    'cylinder_target': cylinder_target})
+            raise exception.VolumeBackendAPIException(
+                message=exception_message)