From 4a2c57684c18f90f23263156f952561154751e09 Mon Sep 17 00:00:00 2001 From: Hans Lindgren Date: Sun, 3 Mar 2013 16:19:08 +0100 Subject: 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 --- nova/compute/api.py | 20 ++----- nova/tests/api/ec2/test_cloud.py | 70 +++++----------------- .../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) -- cgit