openstack/nova
Code Issues Proposed changes

Fix swap_volumes

Browse Source 
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
This commit is contained in:
git-harry 2014-06-23 16:11:37 +01:00
parent 8f49b1ed9f
commit 98739761f1
2 changed files with 3 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
nova/compute/manager.py
View File

@ -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):

31
nova/tests/compute/test_compute_mgr.py
View File

@ -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'