From b03a23618122ee8abf596a471da326bfcb9e1710 Mon Sep 17 00:00:00 2001 From: yixuanzhang Date: Fri, 3 Nov 2017 15:17:26 +0800 Subject: [PATCH] Storwize: disable creating volume with non cg_snapshot group_id Currently, creating a volume with replication group_id, the volume creates successfully, but it isn't added to storwize replication group. Creating a volume with hyperswap group_id has the same problem. Since adding a volume to a replication group immediately follow the volume creation may be failed on storwize, this patch disables creating volume with non cg_snapshot group_id. Change-Id: Ibf71696740129c254f0c065679fe6422bfa2c633 Closes-Bug: 1729782 --- .../volume/drivers/ibm/test_storwize_svc.py | 81 ++++++++++++++++--- .../ibm/storwize_svc/storwize_svc_common.py | 19 ++++- ...ith-non-cgsnap-group-6cba8073e3d6cadd.yaml | 4 + 3 files changed, 93 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/storwize-disable-create-volume-with-non-cgsnap-group-6cba8073e3d6cadd.yaml diff --git a/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py b/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py index f60b9b4dbe1..ef036c8a08d 100644 --- a/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py +++ b/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py @@ -5473,7 +5473,7 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): '_delete_replication_grp') def test_storwize_delete_group(self, _del_rep_grp, is_grp_a_cg_rep_type, is_grp_a_cg_snapshot_type): - is_grp_a_cg_snapshot_type.side_effect = [False, True] + is_grp_a_cg_snapshot_type.side_effect = [True, True, False, True] is_grp_a_cg_rep_type.side_effect = [False, False] type_ref = volume_types.create(self.ctxt, 'testtype', None) group = testutils.create_group(self.ctxt, @@ -5522,7 +5522,7 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): new=testutils.ZeroIntervalLoopingCall) @mock.patch('cinder.volume.utils.is_group_a_cg_snapshot_type') def test_storwize_create_group_snapshot(self, is_grp_a_cg_snapshot_type): - is_grp_a_cg_snapshot_type.side_effect = [False, True] + is_grp_a_cg_snapshot_type.side_effect = [True, True, False, True] type_ref = volume_types.create(self.ctxt, 'testtype', None) group = testutils.create_group(self.ctxt, group_type_id=fake.GROUP_TYPE_ID, @@ -5551,7 +5551,7 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): new=testutils.ZeroIntervalLoopingCall) @mock.patch('cinder.volume.utils.is_group_a_cg_snapshot_type') def test_storwize_delete_group_snapshot(self, is_grp_a_cg_snapshot_type): - is_grp_a_cg_snapshot_type.side_effect = [True, False, True] + is_grp_a_cg_snapshot_type.side_effect = [True, True, True, False, True] type_ref = volume_types.create(self.ctxt, 'testtype', None) group = testutils.create_group(self.ctxt, group_type_id=fake.GROUP_TYPE_ID, @@ -6375,6 +6375,60 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): True,) startfcmap.assert_called_once_with('1', True) + def test_storwize_create_volume_with_group_id(self): + """Tests creating volume with gorup_id.""" + + type_ref = volume_types.create(self.ctxt, 'testtype', None) + cg_spec = {'consistent_group_snapshot_enabled': ' True'} + rccg_spec = {'consistent_group_replication_enabled': ' True'} + cg_type_ref = group_types.create(self.ctxt, 'cg_type_1', cg_spec) + rccg_type_ref = group_types.create(self.ctxt, 'rccg_type_2', rccg_spec) + + group1 = self._create_group_in_db(volume_type_ids=[type_ref.id], + group_type_id=rccg_type_ref.id) + + group2 = self._create_group_in_db(volume_type_ids=[type_ref.id], + group_type_id=cg_type_ref.id) + + # Create volume with replication group id will be failed + vol1 = testutils.create_volume(self.ctxt, volume_type_id=type_ref.id, + group_id=group1.id) + self.assertRaises(exception.VolumeDriverException, + self.driver.create_volume, + vol1) + # Create volume with cg_snapshot group id will success. + vol2 = testutils.create_volume(self.ctxt, volume_type_id=type_ref.id, + group_id=group2.id) + self.driver.create_volume(vol2) + + # Create cloned volume with replication group id will be failed + vol3 = testutils.create_volume(self.ctxt, volume_type_id=type_ref.id, + group_id=group1.id, + source_volid=vol2.id) + self.assertRaises(exception.VolumeDriverException, + self.driver.create_cloned_volume, + vol3, vol2) + # Create cloned volume with cg_snapshot group id will success. + vol4 = testutils.create_volume(self.ctxt, volume_type_id=type_ref.id, + group_id=group2.id, + source_volid=vol2.id) + self.driver.create_cloned_volume(vol4, vol2) + + snapshot = self._generate_snap_info(vol2.id) + self.driver.create_snapshot(snapshot) + # Create volume from snapshot with replication group id will be failed + vol5 = testutils.create_volume(self.ctxt, volume_type_id=type_ref.id, + group_id=group1.id, + snapshot_id=snapshot.id) + self.assertRaises(exception.VolumeDriverException, + self.driver.create_volume_from_snapshot, + vol5, snapshot) + # Create volume from snapshot with cg_snapshot group id will success. + vol6 = testutils.create_volume(self.ctxt, volume_type_id=type_ref.id, + group_id=group2.id, + snapshot_id=snapshot.id) + self.driver.create_volume_from_snapshot(vol6, snapshot) + class CLIResponseTestCase(test.TestCase): def test_empty(self): @@ -7827,12 +7881,15 @@ class StorwizeSVCReplicationTestCase(test.TestCase): group1 = self._create_test_rccg(self.rccg_type, [self.mm_type.id]) group2 = self._create_test_rccg(self.rccg_type, [self.gm_type.id]) mm_vol1, model_update = self._create_test_volume( - self.mm_type, group_id=group1.id, status='available') + self.mm_type, status='available') mm_vol2, model_update = self._create_test_volume( - self.mm_type, group_id=group1.id, status='in-use') + self.mm_type, status='in-use') gm_vol3, model_update = self._create_test_volume( - self.gm_type, group_id=group2.id, - status='available', previous_status='in-use') + self.gm_type, status='available', previous_status='in-use') + ctxt = context.get_admin_context() + self.db.volume_update(ctxt, mm_vol1['id'], {'group_id': group1.id}) + self.db.volume_update(ctxt, mm_vol2['id'], {'group_id': group1.id}) + self.db.volume_update(ctxt, gm_vol3['id'], {'group_id': group2.id}) vols1 = [mm_vol1, mm_vol2] self.driver.update_group(self.ctxt, group1, vols1, []) mm_vol1.group = group1 @@ -8086,12 +8143,16 @@ class StorwizeSVCReplicationTestCase(test.TestCase): group1 = self._create_test_rccg(self.rccg_type, [self.mm_type.id]) group2 = self._create_test_rccg(self.rccg_type, [self.gm_type.id]) mm_vol1, model_update = self._create_test_volume( - self.mm_type, group_id=group1.id, status='available') + self.mm_type, status='available') mm_vol2, model_update = self._create_test_volume( - self.mm_type, group_id=group1.id, status='in-use') + self.mm_type, status='in-use') gm_vol3, model_update = self._create_test_volume( - self.gm_type, group_id=group2.id, + self.gm_type, status='available', previous_status='in-use') + ctxt = context.get_admin_context() + self.db.volume_update(ctxt, mm_vol1['id'], {'group_id': group1.id}) + self.db.volume_update(ctxt, mm_vol2['id'], {'group_id': group1.id}) + self.db.volume_update(ctxt, gm_vol3['id'], {'group_id': group2.id}) vols1 = [mm_vol1, mm_vol2] self.driver.update_group(self.ctxt, group1, vols1, []) mm_vol1.group = group1 diff --git a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py index 96f74fc3728..cdd570a6248 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py +++ b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py @@ -2604,8 +2604,20 @@ class StorwizeSVCCommonDriver(san.SanDriver, volume_type=volume_type, volume_metadata=volume_metadata) + def _check_if_group_type_cg_snapshot(self, volume): + if (volume.group_id and + not utils.is_group_a_cg_snapshot_type(volume.group)): + msg = _('Create volume with a replication or hyperswap ' + 'group_id is not supported. Please add volume to ' + 'group after volume creation.') + LOG.error(msg) + raise exception.VolumeDriverException(reason=msg) + def create_volume(self, volume): LOG.debug('enter: create_volume: volume %s', volume['name']) + # Create a replication or hyperswap volume with group_id is not + # allowed. + self._check_if_group_type_cg_snapshot(volume) opts = self._get_vdisk_params(volume['volume_type_id'], volume_metadata= volume.get('volume_metadata')) @@ -2707,6 +2719,9 @@ class StorwizeSVCCommonDriver(san.SanDriver, self._helpers.delete_vdisk(snapshot['name'], False) def create_volume_from_snapshot(self, volume, snapshot): + # Create volume from snapshot with a replication or hyperswap group_id + # is not allowed. + self._check_if_group_type_cg_snapshot(volume) opts = self._get_vdisk_params(volume['volume_type_id'], volume_metadata= volume.get('volume_metadata')) @@ -2743,7 +2758,9 @@ class StorwizeSVCCommonDriver(san.SanDriver, def create_cloned_volume(self, tgt_volume, src_volume): """Creates a clone of the specified volume.""" - + # Create a cloned volume with a replication or hyperswap group_id is + # not allowed. + self._check_if_group_type_cg_snapshot(tgt_volume) opts = self._get_vdisk_params(tgt_volume['volume_type_id'], volume_metadata= tgt_volume.get('volume_metadata')) diff --git a/releasenotes/notes/storwize-disable-create-volume-with-non-cgsnap-group-6cba8073e3d6cadd.yaml b/releasenotes/notes/storwize-disable-create-volume-with-non-cgsnap-group-6cba8073e3d6cadd.yaml new file mode 100644 index 00000000000..5e521ed44b2 --- /dev/null +++ b/releasenotes/notes/storwize-disable-create-volume-with-non-cgsnap-group-6cba8073e3d6cadd.yaml @@ -0,0 +1,4 @@ +--- +features: + - Disable creating volume with non cg_snapshot group_id in + Storwize/SVC driver.