From 702868fd00e6b99d7ae5bda64d3410b2e5f9ddc5 Mon Sep 17 00:00:00 2001 From: xing-yang Date: Sat, 3 Dec 2016 14:06:44 -0500 Subject: [PATCH] Remove cgsnapshot_id before snapshot.save There are a few more places where information is missing for consistencygroup or cgsnapshot due to the CG migration changes. This patch tries to fix them. cgsnapshot_id should be removed from the snapshot object before snapshot.save is called in the exception block because cgsnapshot does not exist in the db. cgsnapshot object is added to the snapshot object when snapshots are passed to the driver in create/delete cgsnapshot. Change-Id: I253a98b8a2058a00c2c577927c0b701c2f88c81b Closes-Bug: #1662684 --- cinder/objects/snapshot.py | 7 +++-- cinder/objects/volume.py | 2 +- .../unit/objects/test_consistencygroup.py | 30 +++++++++++++++++++ cinder/volume/manager.py | 15 ++++++++-- 4 files changed, 49 insertions(+), 5 deletions(-) diff --git a/cinder/objects/snapshot.py b/cinder/objects/snapshot.py index 171649775..94f7daf90 100644 --- a/cinder/objects/snapshot.py +++ b/cinder/objects/snapshot.py @@ -190,8 +190,11 @@ class Snapshot(cleanable.CinderCleanableObject, base.CinderObject, raise exception.ObjectActionError(action='save', reason=_('volume changed')) if 'cgsnapshot' in updates: - raise exception.ObjectActionError( - action='save', reason=_('cgsnapshot changed')) + # NOTE(xyang): Allow this to pass if 'cgsnapshot' is + # set to None. This is to support backward compatibility. + if updates.get('cgsnapshot'): + raise exception.ObjectActionError( + action='save', reason=_('cgsnapshot changed')) if 'group_snapshot' in updates: raise exception.ObjectActionError( action='save', reason=_('group_snapshot changed')) diff --git a/cinder/objects/volume.py b/cinder/objects/volume.py index ea12de04f..867db51ab 100644 --- a/cinder/objects/volume.py +++ b/cinder/objects/volume.py @@ -340,7 +340,7 @@ class Volume(cleanable.CinderCleanableObject, base.CinderObject, updates = self.cinder_obj_get_changes() if updates: if 'consistencygroup' in updates: - # NOTE(xyang): Allow this to pass if 'consistencygroup'is + # NOTE(xyang): Allow this to pass if 'consistencygroup' is # set to None. This is to support backward compatibility. if updates.get('consistencygroup'): raise exception.ObjectActionError( diff --git a/cinder/tests/unit/objects/test_consistencygroup.py b/cinder/tests/unit/objects/test_consistencygroup.py index 6e7e7ff07..e27d66fa6 100644 --- a/cinder/tests/unit/objects/test_consistencygroup.py +++ b/cinder/tests/unit/objects/test_consistencygroup.py @@ -48,6 +48,18 @@ fake_cgsnapshot = { 'consistencygroup_id': fake.CONSISTENCY_GROUP_ID, } +fake_group = { + 'id': fake.GROUP_ID, + 'user_id': fake.USER_ID, + 'project_id': fake.PROJECT_ID, + 'host': 'fake_host', + 'availability_zone': 'fake_az', + 'name': 'fake_name', + 'description': 'fake_description', + 'group_type_id': fake.GROUP_TYPE_ID, + 'status': fields.GroupStatus.CREATING, +} + class TestConsistencyGroup(test_objects.BaseObjectsTestCase): @@ -77,6 +89,24 @@ class TestConsistencyGroup(test_objects.BaseObjectsTestCase): consistencygroup.create() self._compare(self, fake_consistencygroup, consistencygroup) + @mock.patch('cinder.db.group_create', + return_value=fake_group) + def test_create_from_group(self, group_create): + fake_grp = fake_group.copy() + del fake_grp['id'] + group = objects.Group(context=self.context, + **fake_grp) + group.create() + volumes_objs = [objects.Volume(context=self.context, id=i) + for i in [fake.VOLUME_ID, fake.VOLUME2_ID, + fake.VOLUME3_ID]] + volumes = objects.VolumeList(objects=volumes_objs) + group.volumes = volumes + consistencygroup = objects.ConsistencyGroup() + consistencygroup.from_group(group) + self.assertEqual(group.id, consistencygroup.id) + self.assertEqual(group.name, consistencygroup.name) + def test_create_with_id_except_exception(self, ): consistencygroup = objects.ConsistencyGroup( context=self.context, **{'id': fake.CONSISTENCY_GROUP_ID}) diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 257e9c23a..797119257 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -2862,6 +2862,7 @@ class VolumeManager(manager.CleanableManager, resource={'type': 'group', 'id': group.id}) # Update volume status to 'error' as well. + self._remove_consistencygroup_id_from_volumes(volumes) for vol in volumes: vol.status = 'error' vol.save() @@ -3227,6 +3228,7 @@ class VolumeManager(manager.CleanableManager, # Update volume status to 'error' if driver returns # None for volumes_model_update. if not volumes_model_update: + self._remove_consistencygroup_id_from_volumes(volumes) for vol_obj in volumes: vol_obj.status = 'error' vol_obj.save() @@ -3293,6 +3295,7 @@ class VolumeManager(manager.CleanableManager, cg.from_group(group) for vol in volumes: vol.consistencygroup_id = vol.group_id + vol.consistencygroup = cg return cg, volumes @@ -3301,6 +3304,7 @@ class VolumeManager(manager.CleanableManager, return for vol in volumes: vol.consistencygroup_id = None + vol.consistencygroup = None def _convert_group_snapshot_to_cgsnapshot(self, group_snapshot, snapshots, ctxt): @@ -3308,14 +3312,16 @@ class VolumeManager(manager.CleanableManager, return None, None cgsnap = cgsnapshot.CGSnapshot() cgsnap.from_group_snapshot(group_snapshot) - for snap in snapshots: - snap.cgsnapshot_id = snap.group_snapshot_id # Populate consistencygroup object grp = objects.Group.get_by_id(ctxt, group_snapshot.group_id) cg, __ = self._convert_group_to_cg(grp, []) cgsnap.consistencygroup = cg + for snap in snapshots: + snap.cgsnapshot_id = snap.group_snapshot_id + snap.cgsnapshot = cgsnap + return cgsnap, snapshots def _remove_cgsnapshot_id_from_snapshots(self, snapshots): @@ -3323,6 +3329,7 @@ class VolumeManager(manager.CleanableManager, return for snap in snapshots: snap.cgsnapshot_id = None + snap.cgsnapshot = None def _create_group_generic(self, context, group): """Creates a group.""" @@ -3615,6 +3622,8 @@ class VolumeManager(manager.CleanableManager, for add_vol in add_volumes_ref: add_vol.status = 'error' add_vol.save() + self._remove_consistencygroup_id_from_volumes( + remove_volumes_ref) for rem_vol in remove_volumes_ref: rem_vol.status = 'error' rem_vol.save() @@ -3834,6 +3843,7 @@ class VolumeManager(manager.CleanableManager, group_snapshot.save() # Update snapshot status to 'error' if driver returns # None for snapshots_model_update. + self._remove_cgsnapshot_id_from_snapshots(snapshots) if not snapshots_model_update: for snapshot in snapshots: snapshot.status = fields.SnapshotStatus.ERROR @@ -4102,6 +4112,7 @@ class VolumeManager(manager.CleanableManager, # Update snapshot status to 'error' if driver returns # None for snapshots_model_update. if not snapshots_model_update: + self._remove_cgsnapshot_id_from_snapshots(snapshots) for snapshot in snapshots: snapshot.status = fields.SnapshotStatus.ERROR snapshot.save()