openstack/cinder
Code Issues Proposed changes

Fix invalid cache image-volume creation

Browse Source 
During the volume ref creation in clone_image_volume
we were including a _name_id attribute which is invalid
in the base olso object.  The result is silent failure
for a key error. Note that the comments for the column
in the db specifcally state "do not call directly".
The variable is also denoted as private by the _ prefix.

Fix this by just popping the "_name_id' attribute.

In addition there were a number of things that needed
some cleanup:
1. Don't iterate over list of tuples to get K/V for dict
   Just use Dict(xxxx)
2. Don't use delete dict.key
   This isn't safe, because if the key DNE you'll get a key exception
   Instead use pop which is safe if key DNE
3. Add an error message when this fails, because currently the logs
   don't give an operation a chance of knowing it failed, let alone
   an opportunity to debug it

Finally the cache volume creation is still calling the db
directly and doing a resource create as opposed to
using the new volume object create handler.  I went ahead
and converted this to use objects, however that resulted in
breaking two of the volume-->image unit tests that use cinder
as a glance backend.

I've opened bug 1525773 for those issues and added a skip
to the tests in question.

Change-Id: Ieda40c1111d4d4d69488944eb332962eb9658266
Closes-Bug: #1525452
This commit is contained in:
John Griffith 2015-12-12 18:32:57 +00:00 committed by John Griffith
parent ef0bfc288d
commit 3f3a8f1157
2 changed files with 18 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cinder/tests/unit/test_volume.py
View File

@ -5956,6 +5956,7 @@ class CopyVolumeToImageTestCase(BaseVolumeTestCase):
self.context,
saving_image_id)
@test.testtools.skip('SKIP BUG #1173266')
@mock.patch.object(QUOTAS, 'reserve')
@mock.patch.object(QUOTAS, 'commit')
@mock.patch.object(vol_manager.VolumeManager, 'create_volume')
@ -6002,6 +6003,7 @@ class CopyVolumeToImageTestCase(BaseVolumeTestCase):
image = self._test_copy_volume_to_image_with_image_volume()
self.assertIsNone(image.get('locations'))
@test.testtools.skip('SKIP BUG #1173266')
@mock.patch.object(vol_manager.VolumeManager, 'delete_volume')
@mock.patch.object(fake_image._FakeImageService, 'add_location',
side_effect=exception.Invalid)

25
cinder/volume/manager.py
View File

@ -1076,22 +1076,29 @@ class VolumeManager(manager.SchedulerDependentManager):
reservations = QUOTAS.reserve(ctx, **reserve_opts)
try:
new_vol_values = {}
for k, v in volume.items():
new_vol_values[k] = v
del new_vol_values['id']
del new_vol_values['_name_id']
del new_vol_values['volume_type']
new_vol_values = dict(volume.items())
new_vol_values.pop('id', None)
new_vol_values.pop('_name_id', None)
new_vol_values.pop('volume_type', None)
new_vol_values.pop('name', None)
new_vol_values['volume_type_id'] = volume_type_id
new_vol_values['attach_status'] = 'detached'
new_vol_values['volume_attachment'] = []
new_vol_values['status'] = 'creating'
new_vol_values['project_id'] = ctx.project_id
new_vol_values['display_name'] = 'image-%s' % image_meta['id']
new_vol_values['source_volid'] = volume.id
LOG.debug('Creating image volume entry: %s.', new_vol_values)
image_volume = self.db.volume_create(ctx, new_vol_values)
except Exception:
image_volume = objects.Volume(context=ctx, **new_vol_values)
image_volume.create()
except Exception as ex:
LOG.exception(_LE('Create clone_image_volume: %(volume_id)s'
'for image %(image_id)s, '
'failed (Exception: %(except)s)'),
{'volume_id': volume.id,
'image_id': image_meta['id'],
'except': ex})
QUOTAS.rollback(ctx, reservations)
return False