From 98739761f17b5e0b32abd8cd262f5beda030f886 Mon Sep 17 00:00:00 2001 From: git-harry Date: Mon, 23 Jun 2014 16:11:37 +0100 Subject: [PATCH] Fix swap_volumes The attach and detach calls to cinder are better handled by cinder this patch removes them. There is another patch against this bug to modify cinder. The attach call is being made against the new volume after the migrate_volume_completion request. This means it may be done after the new volume ID in the db has been deleted. The detach call is being run at the end on the old ID and so may end up detaching the migrated volume from cinder's perspective. Change-Id: I3f2d98e9e473905a3de2f02e00c92ae3065e6ae7 Partial-Bug: 1316079 --- nova/compute/manager.py | 7 ------ nova/tests/compute/test_compute_mgr.py | 31 +++----------------------- 2 files changed, 3 insertions(+), 35 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 9f0ec90447..9212dafa2c 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -4534,7 +4534,6 @@ class ComputeManager(manager.Manager): new_volume_id) save_volume_id = comp_ret['save_volume_id'] - mountpoint = bdm.device_name # Update bdm values = { @@ -4548,12 +4547,6 @@ class ComputeManager(manager.Manager): 'no_device': None} bdm.update(values) bdm.save() - self.volume_api.attach(context, - new_volume_id, - instance.uuid, - mountpoint) - # Remove old connection - self.volume_api.detach(context.elevated(), old_volume_id) @wrap_exception() def remove_volume_connection(self, context, volume_id, instance): diff --git a/nova/tests/compute/test_compute_mgr.py b/nova/tests/compute/test_compute_mgr.py index 370e4efcfc..5c40566b2c 100644 --- a/nova/tests/compute/test_compute_mgr.py +++ b/nova/tests/compute/test_compute_mgr.py @@ -1016,10 +1016,6 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): 'status': 'available', 'size': 2} - def fake_vol_api_begin_detaching(context, volume_id): - self.assertTrue(uuidutils.is_uuid_like(volume_id)) - volumes[volume_id]['status'] = 'detaching' - def fake_vol_api_roll_detaching(context, volume_id): self.assertTrue(uuidutils.is_uuid_like(volume_id)) if volumes[volume_id]['status'] == 'detaching': @@ -1038,30 +1034,16 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): self.assertTrue(uuidutils.is_uuid_like(volume_id)) return volumes[volume_id] - def fake_vol_attach(context, volume_id, instance_uuid, connector): - self.assertTrue(uuidutils.is_uuid_like(volume_id)) - self.assertIn(volumes[volume_id]['status'], - ['available', 'attaching']) - volumes[volume_id]['status'] = 'in-use' - - def fake_vol_api_reserve(context, volume_id): - self.assertTrue(uuidutils.is_uuid_like(volume_id)) - self.assertEqual(volumes[volume_id]['status'], 'available') - volumes[volume_id]['status'] = 'attaching' - def fake_vol_unreserve(context, volume_id): self.assertTrue(uuidutils.is_uuid_like(volume_id)) if volumes[volume_id]['status'] == 'attaching': volumes[volume_id]['status'] = 'available' - def fake_vol_detach(context, volume_id): - self.assertTrue(uuidutils.is_uuid_like(volume_id)) - volumes[volume_id]['status'] = 'available' - def fake_vol_migrate_volume_completion(context, old_volume_id, new_volume_id, error=False): self.assertTrue(uuidutils.is_uuid_like(old_volume_id)) - self.assertTrue(uuidutils.is_uuid_like(old_volume_id)) + self.assertTrue(uuidutils.is_uuid_like(new_volume_id)) + volumes[old_volume_id]['status'] = 'in-use' return {'save_volume_id': new_volume_id} def fake_func_exc(*args, **kwargs): @@ -1071,21 +1053,15 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): instance, mountpoint, resize_to): self.assertEqual(resize_to, 2) - self.stubs.Set(self.compute.volume_api, 'begin_detaching', - fake_vol_api_begin_detaching) self.stubs.Set(self.compute.volume_api, 'roll_detaching', fake_vol_api_roll_detaching) self.stubs.Set(self.compute.volume_api, 'get', fake_vol_get) self.stubs.Set(self.compute.volume_api, 'initialize_connection', fake_vol_api_func) - self.stubs.Set(self.compute.volume_api, 'attach', fake_vol_attach) - self.stubs.Set(self.compute.volume_api, 'reserve_volume', - fake_vol_api_reserve) self.stubs.Set(self.compute.volume_api, 'unreserve_volume', fake_vol_unreserve) self.stubs.Set(self.compute.volume_api, 'terminate_connection', fake_vol_api_func) - self.stubs.Set(self.compute.volume_api, 'detach', fake_vol_detach) self.stubs.Set(db, 'block_device_mapping_get_by_volume_id', lambda x, y, z: fake_bdm) self.stubs.Set(self.compute.driver, 'get_volume_connector', @@ -1105,8 +1081,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): self.compute.swap_volume(self.context, old_volume_id, new_volume_id, fake_instance.fake_instance_obj( self.context, **{'uuid': 'fake'})) - self.assertEqual(volumes[old_volume_id]['status'], 'available') - self.assertEqual(volumes[new_volume_id]['status'], 'in-use') + self.assertEqual(volumes[old_volume_id]['status'], 'in-use') # Error paths volumes[old_volume_id]['status'] = 'detaching'