openstack/cinder
Code Issues Proposed changes

Make begin_detaching fail if volume not "in-use"

Browse Source 
Like it's counterpart from Nova's volume-attach functionality standpoint
- reserve_volume, begin_detaching should fail if the volume is not in
  the correct state to be detached. This will prevent nova from
attempting to check and then detach, which is inherently racy.

Change-Id: Ie87eb0c9aea068affc94032ab2e53a91888d272a
Partial-bug: #1302774
This commit is contained in:
Nikola Dipanov 2014-04-25 13:38:31 +02:00
parent f40b4a36b4
commit d955c1da44
2 changed files with 24 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
cinder/tests/test_volume.py
View File

@ -2060,9 +2060,19 @@ class VolumeTestCase(BaseVolumeTestCase):
'name',
'description')
def test_begin_detaching_fails_available(self):
volume_api = cinder.volume.api.API()
volume = tests_utils.create_volume(self.context, **self.volume_params)
self.assertRaises(exception.InvalidVolume, volume_api.begin_detaching,
self.context, volume)
def test_begin_roll_detaching_volume(self):
"""Test begin_detaching and roll_detaching functions."""
instance_uuid = '12345678-1234-5678-1234-567812345678'
volume = tests_utils.create_volume(self.context, **self.volume_params)
volume = db.volume_attached(
self.context, volume['id'], instance_uuid, 'fake-host', 'vdb')
volume_api = cinder.volume.api.API()
volume_api.begin_detaching(self.context, volume)
volume = db.volume_get(self.context, volume['id'])

16
cinder/volume/api.py
View File

@ -382,8 +382,20 @@ class API(base.Base):
# If we are in the middle of a volume migration, we don't want the user
# to see that the volume is 'detaching'. Having 'migration_status' set
# will have the same effect internally.
if not volume['migration_status']:
self.update(context, volume, {"status": "detaching"})
if volume['migration_status']:
return
if (volume['status'] != 'in-use' and
volume['attach_status'] != 'attached'):
msg = (_("Unable to detach volume. Volume status must be 'in-use' "
"and attached_status must be 'attached' to detach. "
"Currently: status: '%(status)s', "
"attach_status: '%(attach_status)s'") %
{'status': volume['status'],
'attach_status': volume['attach_status']})
LOG.error(msg)
raise exception.InvalidVolume(reason=msg)
self.update(context, volume, {"status": "detaching"})
@wrap_check_policy
def roll_detaching(self, context, volume):