From ef98a0b1bc1ab13656e9dffec33753dfc0838752 Mon Sep 17 00:00:00 2001 From: zengyingzhe Date: Thu, 1 Dec 2016 17:19:06 +0800 Subject: [PATCH] Swap volume type for migration started by retype Currently, while finishing migration, volume type is skipped while swapping source and destination fields, but for the circumstance of retype, this'll make the migrated volume doesn't have the correct volume type info. This patch fixes this bug. Change-Id: I8906e2e7438617fb29c7632d993cede44e952036 Closes-Bug: #1597247 --- cinder/objects/volume.py | 10 ++++++++-- cinder/tests/unit/objects/test_volume.py | 17 ++++++++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/cinder/objects/volume.py b/cinder/objects/volume.py index 78d31d6489b..4dbb53c1e87 100644 --- a/cinder/objects/volume.py +++ b/cinder/objects/volume.py @@ -456,8 +456,14 @@ class Volume(cleanable.CinderCleanableObject, base.CinderObject, # We swap fields between source (i.e. self) and destination at the # end of migration because we want to keep the original volume id # in the DB but now pointing to the migrated volume. - skip = ({'id', 'provider_location', 'glance_metadata', - 'volume_type'} | set(self.obj_extra_fields)) + skip = {'id', 'provider_location', 'glance_metadata' + } | set(self.obj_extra_fields) + + # For the migration started by retype, we should swap the + # volume type, other circumstances still skip volume type. + if self.status != 'retyping': + skip.add('volume_type') + for key in set(dest_volume.fields.keys()) - skip: # Only swap attributes that are already set. We do not want to # unexpectedly trigger a lazy-load. diff --git a/cinder/tests/unit/objects/test_volume.py b/cinder/tests/unit/objects/test_volume.py index 4f00ad9c776..db4b68fda15 100644 --- a/cinder/tests/unit/objects/test_volume.py +++ b/cinder/tests/unit/objects/test_volume.py @@ -353,15 +353,22 @@ class TestVolume(test_objects.BaseObjectsTestCase): @ddt.data({'src_vol_type_id': fake.VOLUME_TYPE_ID, 'dest_vol_type_id': fake.VOLUME_TYPE2_ID}, {'src_vol_type_id': None, - 'dest_vol_type_id': fake.VOLUME_TYPE2_ID}) + 'dest_vol_type_id': fake.VOLUME_TYPE2_ID}, + {'src_vol_type_id': fake.VOLUME_TYPE_ID, + 'dest_vol_type_id': fake.VOLUME_TYPE2_ID, + 'src_vol_status': 'retyping'},) @ddt.unpack def test_finish_volume_migration(self, volume_update, metadata_update, - src_vol_type_id, dest_vol_type_id): + src_vol_type_id, dest_vol_type_id, + src_vol_status=None): src_volume_db = fake_volume.fake_db_volume( **{'id': fake.VOLUME_ID, 'volume_type_id': src_vol_type_id}) if src_vol_type_id: src_volume_db['volume_type'] = fake_volume.fake_db_volume_type( id=src_vol_type_id) + if src_vol_status: + src_volume_db['status'] = src_vol_status + dest_volume_db = fake_volume.fake_db_volume( **{'id': fake.VOLUME2_ID, 'volume_type_id': dest_vol_type_id}) if dest_vol_type_id: @@ -385,7 +392,11 @@ class TestVolume(test_objects.BaseObjectsTestCase): mock.call(self.context, src_volume.id, mock.ANY), mock.call(self.context, dest_volume.id, mock.ANY)]) ctxt, vol_id, updates = volume_update.call_args[0] - self.assertNotIn('volume_type', updates) + + if src_vol_status and src_vol_status == 'retyping': + self.assertIn('volume_type', updates) + else: + self.assertNotIn('volume_type', updates) # Ensure that the destination volume type has not been overwritten self.assertEqual(dest_vol_type_id,