diff options
author | Hans Lindgren <hanlind@kth.se> | 2013-03-03 16:19:08 +0100 |
---|---|---|
committer | Hans Lindgren <hanlind@kth.se> | 2013-03-04 13:49:00 +0100 |
commit | 4a2c57684c18f90f23263156f952561154751e09 (patch) | |
tree | afee74d0f91c21896dc48c611ea3505410cc6a64 | |
parent | 690d8f3d4061443b149b13f96116ad72e5dcb06c (diff) | |
download | nova-4a2c57684c18f90f23263156f952561154751e09.tar.gz nova-4a2c57684c18f90f23263156f952561154751e09.tar.xz nova-4a2c57684c18f90f23263156f952561154751e09.zip |
Fix an error in compute api snapshot_volume_backed bdm code
Stumbled upon an error similar to the one found in
https://review.openstack.org/#/c/23354/ where a dbm dict
item is tried to be read as an attribute.
Looked at the next couple of lines and discovered more
cases of this error.
Updates to some api tests that contain faulty fakes that
hide this error adds some coverage for this error. Rebased
this patch on the above mentioned review to be able to
change the tests, since the code path is the same and they
now hit both of these errors.
Resolves bug 1142734.
Change-Id: Ie639cd483334b6222085a139159da47ce6fa3671
-rw-r--r-- | nova/compute/api.py | 20 | ||||
-rw-r--r-- | nova/tests/api/ec2/test_cloud.py | 70 | ||||
-rw-r--r-- | nova/tests/api/openstack/compute/test_server_actions.py | 33 |
3 files changed, 29 insertions, 94 deletions
diff --git a/nova/compute/api.py b/nova/compute/api.py index 53bba667e..75bbe5264 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1595,17 +1595,10 @@ class API(base.Base): mapping = [] for bdm in bdms: - if bdm.no_device: + if bdm['no_device']: continue - m = {} - for attr in ('device_name', 'snapshot_id', 'volume_id', - 'volume_size', 'delete_on_termination', 'no_device', - 'virtual_name'): - val = getattr(bdm, attr) - if val is not None: - m[attr] = val - - volume_id = m.get('volume_id') + + volume_id = bdm.get('volume_id') if volume_id: # create snapshot based on volume_id volume = self.volume_api.get(context, volume_id) @@ -1615,11 +1608,10 @@ class API(base.Base): name = _('snapshot for %s') % image_meta['name'] snapshot = self.volume_api.create_snapshot_force( context, volume, name, volume['display_description']) - m['snapshot_id'] = snapshot['id'] - del m['volume_id'] + bdm['snapshot_id'] = snapshot['id'] + del bdm['volume_id'] - if m: - mapping.append(m) + mapping.append(bdm) for m in block_device.mappings_prepend_dev(properties.get('mappings', [])): diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py index b8a4712c4..2ba8cfc60 100644 --- a/nova/tests/api/ec2/test_cloud.py +++ b/nova/tests/api/ec2/test_cloud.py @@ -1907,34 +1907,15 @@ class CloudTestCase(test.TestCase): self.stubs.Set(fake._FakeImageService, 'show', fake_show) def fake_block_device_mapping_get_all_by_instance(context, inst_id): - class BDM(object): - def __init__(self): - self.no_device = None - self.values = dict(id=1, - snapshot_id=snapshots[0], - volume_id=volumes[0], - virtual_name=None, - volume_size=1, - device_name='sda1', - delete_on_termination=False, - connection_info='{"foo":"bar"}') - - def __getattr__(self, name): - """Properly delegate dotted lookups.""" - if name in self.__dict__['values']: - return self.values.get(name) - try: - return self.__dict__[name] - except KeyError: - raise AttributeError - - def __getitem__(self, key): - return self.values.get(key) - - def iteritems(self): - return self.values.iteritems() - - return [BDM()] + return [dict(id=1, + snapshot_id=snapshots[0], + volume_id=volumes[0], + virtual_name=None, + volume_size=1, + device_name='sda1', + delete_on_termination=False, + no_device=None, + connection_info='{"foo":"bar"}')] self.stubs.Set(db, 'block_device_mapping_get_all_by_instance', fake_block_device_mapping_get_all_by_instance) @@ -1998,32 +1979,13 @@ class CloudTestCase(test.TestCase): ec2_instance_id = self._run_instance(**kwargs) def fake_block_device_mapping_get_all_by_instance(context, inst_id): - class BDM(object): - def __init__(self): - self.no_device = None - self.values = dict(snapshot_id=snapshots[0], - volume_id=volumes[0], - virtual_name=None, - volume_size=1, - device_name='vda', - delete_on_termination=False) - - def __getattr__(self, name): - """Properly delegate dotted lookups.""" - if name in self.__dict__['values']: - return self.values.get(name) - try: - return self.__dict__[name] - except KeyError: - raise AttributeError - - def __getitem__(self, key): - return self.values.get(key) - - def iteritems(self): - return self.values.iteritems() - - return [BDM()] + return [dict(snapshot_id=snapshots[0], + volume_id=volumes[0], + virtual_name=None, + volume_size=1, + device_name='vda', + delete_on_termination=False, + no_device=None)] self.stubs.Set(db, 'block_device_mapping_get_all_by_instance', fake_block_device_mapping_get_all_by_instance) diff --git a/nova/tests/api/openstack/compute/test_server_actions.py b/nova/tests/api/openstack/compute/test_server_actions.py index f94b77280..62a688962 100644 --- a/nova/tests/api/openstack/compute/test_server_actions.py +++ b/nova/tests/api/openstack/compute/test_server_actions.py @@ -784,32 +784,13 @@ class ServerActionsControllerTest(test.TestCase): image_service.create(None, original_image) def fake_block_device_mapping_get_all_by_instance(context, inst_id): - class BDM(object): - def __init__(self): - self.no_device = None - self.values = dict(volume_id=_fake_id('a'), - virtual_name=None, - volume_size=1, - device_name='vda', - snapshot_id=1, - delete_on_termination=False) - - def __getattr__(self, name): - """Properly delegate dotted lookups.""" - if name in self.__dict__['values']: - return self.values.get(name) - try: - return self.__dict__[name] - except KeyError: - raise AttributeError - - def __getitem__(self, key): - return self.values.get(key) - - def iteritems(self): - return self.values.iteritems() - - return [BDM()] + return [dict(volume_id=_fake_id('a'), + virtual_name=None, + volume_size=1, + device_name='vda', + snapshot_id=1, + delete_on_termination=False, + no_device=None)] self.stubs.Set(db, 'block_device_mapping_get_all_by_instance', fake_block_device_mapping_get_all_by_instance) |