diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py index 9e407658f5e..200493bbdd0 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py +++ b/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', diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py index 4701dc7bee3..a7fc2c2028e 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py +++ b/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 diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_utils.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_utils.py index 0cfe7037073..1bfef219557 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_utils.py +++ b/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) diff --git a/cinder/volume/drivers/dell_emc/powermax/common.py b/cinder/volume/drivers/dell_emc/powermax/common.py index a6c9ce3a740..5a57ea8914d 100644 --- a/cinder/volume/drivers/dell_emc/powermax/common.py +++ b/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) diff --git a/cinder/volume/drivers/dell_emc/powermax/rest.py b/cinder/volume/drivers/dell_emc/powermax/rest.py index 7b4438f60f0..09aff693fc5 100644 --- a/cinder/volume/drivers/dell_emc/powermax/rest.py +++ b/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 diff --git a/cinder/volume/drivers/dell_emc/powermax/utils.py b/cinder/volume/drivers/dell_emc/powermax/utils.py index ec4d2c5cbe9..a77f7e4adf6 100644 --- a/cinder/volume/drivers/dell_emc/powermax/utils.py +++ b/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)