From f5f3ca0a55deb0a2648e43f222ebbbc121293f69 Mon Sep 17 00:00:00 2001 From: Yusuke Hayashi Date: Thu, 12 May 2016 21:59:53 +0900 Subject: [PATCH] Unexpected function is called in _migrate_volume_generic When calling _migrate_volume_generic with the volume created by create_volume_from_snapsot or create_cloned_volume, create_volume is not called to create dest-volume for migration. Instead of the function, create_volume_from_snapshot or create_cloned_volume is called to create the dest-volume. This is because the source volume for migration has info of 'snapshot_id' or 'source_volid'. So, This patch temporarily removes the keys and sets after migration. Change-Id: Ibeeb119b72c0a686000f175187c1bfa359bbf6a2 Closes-Bug: #1580013 --- cinder/tests/unit/test_volume.py | 74 ++++++++++++++++++++++++++++++++ cinder/volume/manager.py | 10 ++++- 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index 8f5e1796dac..98af82d0fe0 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -70,6 +70,7 @@ import cinder.volume from cinder.volume import api as volume_api from cinder.volume import configuration as conf from cinder.volume import driver +from cinder.volume.flows.manager import create_volume as create_volume_manager from cinder.volume import manager as vol_manager from cinder.volume import rpcapi as volume_rpcapi import cinder.volume.targets.tgt @@ -4698,6 +4699,79 @@ class VolumeMigrationTestCase(BaseVolumeTestCase): update_server_volume.assert_called_with(self.context, fake_uuid, volume['id'], fake_volume_id) + @mock.patch('cinder.objects.volume.Volume.save') + @mock.patch('cinder.volume.rpcapi.VolumeAPI.create_volume') + @mock.patch('cinder.compute.API') + @mock.patch('cinder.volume.manager.VolumeManager.' + 'migrate_volume_completion') + @mock.patch('cinder.db.sqlalchemy.api.volume_get') + def test_migrate_volume_generic_volume_from_snap(self, volume_get, + migrate_volume_completion, + nova_api, create_volume, + save): + def fake_create_volume(*args, **kwargs): + context, volume, host, request_spec, filter_properties = args + fake_db = mock.Mock() + task = create_volume_manager.ExtractVolumeSpecTask(fake_db) + specs = task.execute(context, volume, {}) + self.assertEqual('raw', specs['type']) + + def fake_copy_volume_data_with_chk_param(*args, **kwargs): + context, src, dest = args + self.assertEqual(src['snapshot_id'], dest['snapshot_id']) + + fake_db_new_volume = {'status': 'available', 'id': fake.VOLUME_ID} + fake_new_volume = fake_volume.fake_db_volume(**fake_db_new_volume) + host_obj = {'host': 'newhost', 'capabilities': {}} + volume_get.return_value = fake_new_volume + + volume_from_snap = tests_utils.create_volume(self.context, size=1, + host=CONF.host) + volume_from_snap['snapshot_id'] = fake.SNAPSHOT_ID + create_volume.side_effect = fake_create_volume + + with mock.patch.object(self.volume, '_copy_volume_data') as \ + mock_copy_volume: + mock_copy_volume.side_effect = fake_copy_volume_data_with_chk_param + self.volume._migrate_volume_generic(self.context, volume_from_snap, + host_obj, None) + + @mock.patch('cinder.objects.volume.Volume.save') + @mock.patch('cinder.volume.rpcapi.VolumeAPI.create_volume') + @mock.patch('cinder.compute.API') + @mock.patch('cinder.volume.manager.VolumeManager.' + 'migrate_volume_completion') + @mock.patch('cinder.db.sqlalchemy.api.volume_get') + def test_migrate_volume_generic_for_clone(self, volume_get, + migrate_volume_completion, + nova_api, create_volume, save): + def fake_create_volume(*args, **kwargs): + context, volume, host, request_spec, filter_properties = args + fake_db = mock.Mock() + task = create_volume_manager.ExtractVolumeSpecTask(fake_db) + specs = task.execute(context, volume, {}) + self.assertEqual('raw', specs['type']) + + def fake_copy_volume_data_with_chk_param(*args, **kwargs): + context, src, dest = args + self.assertEqual(src['source_volid'], dest['source_volid']) + + fake_db_new_volume = {'status': 'available', 'id': fake.VOLUME_ID} + fake_new_volume = fake_volume.fake_db_volume(**fake_db_new_volume) + host_obj = {'host': 'newhost', 'capabilities': {}} + volume_get.return_value = fake_new_volume + + clone = tests_utils.create_volume(self.context, size=1, + host=CONF.host) + clone['source_volid'] = fake.VOLUME2_ID + create_volume.side_effect = fake_create_volume + + with mock.patch.object(self.volume, '_copy_volume_data') as \ + mock_copy_volume: + mock_copy_volume.side_effect = fake_copy_volume_data_with_chk_param + self.volume._migrate_volume_generic(self.context, clone, + host_obj, None) + @mock.patch.object(volume_rpcapi.VolumeAPI, 'update_migrated_volume') @mock.patch.object(volume_rpcapi.VolumeAPI, 'delete_volume') @mock.patch.object(volume_rpcapi.VolumeAPI, 'create_volume') diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index a23c522b4e1..e57b31c8e55 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -1606,7 +1606,8 @@ class VolumeManager(manager.SchedulerDependentManager): rpcapi = volume_rpcapi.VolumeAPI() # Create new volume on remote host - skip = self._VOLUME_CLONE_SKIP_PROPERTIES | {'host'} + tmp_skip = {'snapshot_id', 'source_volid'} + skip = self._VOLUME_CLONE_SKIP_PROPERTIES | tmp_skip | {'host'} new_vol_values = {k: volume[k] for k in set(volume.keys()) - skip} if new_type_id: new_vol_values['volume_type_id'] = new_type_id @@ -1649,6 +1650,13 @@ class VolumeManager(manager.SchedulerDependentManager): # available new_volume = objects.Volume.get_by_id(ctxt, new_volume.id) + # Set skipped value to avoid calling + # function except for _create_raw_volume + tmp_skipped_values = {k: volume[k] for k in tmp_skip if volume.get(k)} + if tmp_skipped_values: + new_volume.update(tmp_skipped_values) + new_volume.save() + # Copy the source volume to the destination volume try: attachments = volume.volume_attachment