openstack
/
nova
Code Issues Proposed changes

cleanup NovaObjectDictCompat subclassing from pci_device 

cleanup subclassing on NovaObjectDictCompat and fix subsequent
Unit tests and code associated with nova/objects/pci_device.py

Due to the exhaustive nature of changes, the cleanup is done
one object at a time.

This is the first patch of the series.
There shall be more patches to follow for other objects.

Related to blueprint liberty-objects

Change-Id: Idb764632e9a4f926bc1f159333de6a8366abcfab
This commit is contained in:
Sudipta Biswas 2015-07-11 00:12:13 +05:30
parent 4442979e71
commit e464267085
9 changed files with 44 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
nova/api/openstack/compute/plugins/v3/pci.py
View File

@ -36,7 +36,7 @@ class PciServerController(wsgi.Controller):
def _extend_server(self, server, instance):
dev_id = []
for dev in instance.pci_devices:
dev_id.append({'id': dev['id']})
dev_id.append({'id': dev.id})
server['%s:pci_devices' % Pci.alias] = dev_id
@wsgi.extends
@ -88,13 +88,13 @@ class PciController(wsgi.Controller):
def _view_pcidevice(self, device, detail=False):
dev_dict = {}
for key in PCI_ADMIN_KEYS:
dev_dict[key] = device[key]
dev_dict[key] = getattr(device, key)
if detail:
for field in PCI_DETAIL_KEYS:
if field == 'instance_uuid':
dev_dict['server_uuid'] = device[field]
dev_dict['server_uuid'] = getattr(device, field)
else:
dev_dict[field] = device[field]
dev_dict[field] = getattr(device, field)
return dev_dict
def _get_all_nodes_pci_devices(self, req, detail, action):

8
nova/objects/pci_device.py
View File

@ -41,10 +41,8 @@ def compare_pci_device_attributes(obj_a, obj_b):
return True
# TODO(berrange): Remove NovaObjectDictCompat
@base.NovaObjectRegistry.register
class PciDevice(base.NovaPersistentObject, base.NovaObject,
base.NovaObjectDictCompat):
class PciDevice(base.NovaPersistentObject, base.NovaObject):
"""Object to represent a PCI device on a compute node.
@ -130,7 +128,7 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject,
for k, v in dev_dict.items():
if k in self.fields.keys():
self[k] = v
setattr(self, k, v)
else:
# Note (yjiang5) extra_info.update does not update
# obj_what_changed, set it explicitely
@ -153,7 +151,7 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject,
def _from_db_object(context, pci_device, db_dev):
for key in pci_device.fields:
if key != 'extra_info':
pci_device[key] = db_dev[key]
setattr(pci_device, key, db_dev[key])
else:
extra_info = db_dev.get("extra_info")
pci_device.extra_info = jsonutils.loads(extra_info)

2
nova/pci/device.py
View File

@ -28,7 +28,7 @@ def check_device_status(dev_status=None):
def outer(f):
@functools.wraps(f)
def inner(devobj, instance=None):
if devobj['status'] not in dev_status:
if devobj.status not in dev_status:
raise exception.PciDeviceInvalidStatus(
compute_node_id=devobj.compute_node_id,
address=devobj.address, status=devobj.status,

24
nova/pci/manager.py
View File

@ -65,12 +65,12 @@ class PciDevTracker(object):
self.allocations = collections.defaultdict(list)
self.claims = collections.defaultdict(list)
for dev in self.pci_devs:
uuid = dev['instance_uuid']
if dev['status'] == 'claimed':
uuid = dev.instance_uuid
if dev.status == 'claimed':
self.claims[uuid].append(dev)
elif dev['status'] == 'allocated':
elif dev.status == 'allocated':
self.allocations[uuid].append(dev)
elif dev['status'] == 'available':
elif dev.status == 'available':
self.stats.add_device(dev)
@property
@ -84,7 +84,7 @@ class PciDevTracker(object):
dev.save()
self.pci_devs = [dev for dev in self.pci_devs
if dev['status'] != 'deleted']
if dev.status != 'deleted']
@property
def pci_stats(self):
@ -103,11 +103,11 @@ class PciDevTracker(object):
or removed while assigned.
"""
exist_addrs = set([dev['address'] for dev in self.pci_devs])
exist_addrs = set([dev.address for dev in self.pci_devs])
new_addrs = set([dev['address'] for dev in devices])
for existed in self.pci_devs:
if existed['address'] in exist_addrs - new_addrs:
if existed.address in exist_addrs - new_addrs:
try:
device.remove(existed)
except exception.PciDeviceInvalidStatus as e:
@ -126,9 +126,9 @@ class PciDevTracker(object):
self.stats.remove_device(existed)
else:
new_value = next((dev for dev in devices if
dev['address'] == existed['address']))
dev['address'] == existed.address))
new_value['compute_node_id'] = self.node_id
if existed['status'] in ('claimed', 'allocated'):
if existed.status in ('claimed', 'allocated'):
# Pci properties may change while assigned because of
# hotplug or config changes. Although normally this should
# not happen.
@ -183,7 +183,7 @@ class PciDevTracker(object):
def _free_device(self, dev, instance=None):
device.free(dev, instance)
stale = self.stale.pop(dev['address'], None)
stale = self.stale.pop(dev.address, None)
if stale:
dev.update_device(stale)
self.stats.add_device(dev)
@ -195,8 +195,8 @@ class PciDevTracker(object):
# information, not the claimed one. So we can't use
# instance['pci_devices'] to check the devices to be freed.
for dev in self.pci_devs:
if (dev['status'] in ('claimed', 'allocated') and
dev['instance_uuid'] == instance['uuid']):
if (dev.status in ('claimed', 'allocated') and
dev.instance_uuid == instance['uuid']):
self._free_device(dev)
def update_pci_for_instance(self, context, instance):

2
nova/pci/stats.py
View File

@ -88,7 +88,7 @@ class PciDeviceStats(object):
if not devspec:
return
tags = devspec.get_tags()
pool = {k: dev.get(k) for k in self.pool_keys}
pool = {k: getattr(dev, k) for k in self.pool_keys}
if tags:
pool.update(tags)
return pool

6
nova/tests/unit/pci/test_device.py
View File

@ -71,8 +71,10 @@ class PciDeviceTestCase(test.NoDBTestCase):
self.assertEqual(self.devobj.status, 'allocated')
self.assertEqual(self.devobj.instance_uuid, 'fake-inst-uuid')
self.assertEqual(len(self.inst.pci_devices), 1)
self.assertEqual(self.inst.pci_devices[0]['vendor_id'], 'v')
self.assertEqual(self.inst.pci_devices[0]['status'], 'allocated')
self.assertEqual(self.inst.pci_devices[0].vendor_id,
'v')
self.assertEqual(self.inst.pci_devices[0].status,
'allocated')
def test_allocacte_device_fail_status(self):
self.devobj.status = 'removed'

28
nova/tests/unit/pci/test_manager.py
View File

@ -145,11 +145,11 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
copy.deepcopy(fake_pci_2), copy.deepcopy(fake_pci_3)]
self.tracker.set_hvdevs(fake_pci_devs)
self.assertEqual(len(self.tracker.pci_devs), 4)
self.assertEqual(set([dev['address'] for
self.assertEqual(set([dev.address for
dev in self.tracker.pci_devs]),
set(['0000:00:00.1', '0000:00:00.2',
'0000:00:00.3', '0000:00:00.4']))
self.assertEqual(set([dev['vendor_id'] for
self.assertEqual(set([dev.vendor_id for
dev in self.tracker.pci_devs]),
set(['v', 'v1', 'v2']))
@ -158,14 +158,14 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
fake_pci_devs = [copy.deepcopy(fake_pci), copy.deepcopy(fake_pci_2),
copy.deepcopy(fake_pci_v2)]
self.tracker.set_hvdevs(fake_pci_devs)
self.assertEqual(set([dev['vendor_id'] for
self.assertEqual(set([dev.vendor_id for
dev in self.tracker.pci_devs]),
set(['v', 'v1']))
def test_set_hvdev_remove(self):
self.tracker.set_hvdevs([fake_pci])
self.assertEqual(len([dev for dev in self.tracker.pci_devs
if dev['status'] == 'removed']),
if dev.status == 'removed']),
2)
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance')
@ -186,7 +186,7 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
self.tracker.update_pci_for_instance(None, self.inst)
free_devs = self.tracker.pci_stats.get_free_devs()
self.assertEqual(len(free_devs), 1)
self.assertEqual(free_devs[0]['vendor_id'], 'v')
self.assertEqual(free_devs[0].vendor_id, 'v')
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance')
def test_update_pci_for_instance_fail(self, mock_get):
@ -214,8 +214,8 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
self.tracker.update_pci_for_instance(None, self.inst)
free_devs = self.tracker.pci_stats.get_free_devs()
self.assertEqual(2, len(free_devs))
self.assertEqual('v1', free_devs[0]['vendor_id'])
self.assertEqual('v1', free_devs[1]['vendor_id'])
self.assertEqual('v1', free_devs[0].vendor_id)
self.assertEqual('v1', free_devs[1].vendor_id)
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance')
def test_update_pci_for_instance_with_numa_fail(self, mock_get):
@ -238,7 +238,7 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
self.tracker.update_pci_for_instance(None, self.inst)
free_devs = self.tracker.pci_stats.get_free_devs()
self.assertEqual(len(free_devs), 3)
self.assertEqual(set([dev['vendor_id'] for
self.assertEqual(set([dev.vendor_id for
dev in self.tracker.pci_devs]),
set(['v', 'v1']))
@ -272,7 +272,7 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
self.tracker.update_pci_for_migration(None, self.inst)
free_devs = self.tracker.pci_stats.get_free_devs()
self.assertEqual(len(free_devs), 1)
self.assertEqual(free_devs[0]['vendor_id'], 'v')
self.assertEqual(free_devs[0].vendor_id, 'v')
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance')
def test_update_pci_for_migration_out(self, mock_get):
@ -281,7 +281,7 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
self.tracker.update_pci_for_migration(None, self.inst, sign=-1)
free_devs = self.tracker.pci_stats.get_free_devs()
self.assertEqual(len(free_devs), 3)
self.assertEqual(set([dev['vendor_id'] for
self.assertEqual(set([dev.vendor_id for
dev in self.tracker.pci_devs]),
set(['v', 'v1']))
@ -322,13 +322,13 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
self.tracker.update_pci_for_instance(None, inst_2)
free_devs = self.tracker.pci_stats.get_free_devs()
self.assertEqual(len(free_devs), 1)
self.assertEqual(free_devs[0]['vendor_id'], 'v')
self.assertEqual(free_devs[0].vendor_id, 'v')
self.tracker.clean_usage([self.inst], [migr], [orph])
free_devs = self.tracker.pci_stats.get_free_devs()
self.assertEqual(len(free_devs), 2)
self.assertEqual(
set([dev['vendor_id'] for dev in free_devs]),
set([dev.vendor_id for dev in free_devs]),
set(['v', 'v1']))
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance')
@ -350,7 +350,7 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
free_devs = self.tracker.pci_stats.get_free_devs()
self.assertEqual(len(free_devs), 2)
self.assertEqual(
set([dev['vendor_id'] for dev in free_devs]),
set([dev.vendor_id for dev in free_devs]),
set(['v', 'v1']))
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance')
@ -366,7 +366,7 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
free_devs = self.tracker.pci_stats.get_free_devs()
self.assertEqual(3, len(free_devs))
self.assertEqual(
set([dev['address'] for dev in free_devs]),
set([dev.address for dev in free_devs]),
set(['0000:00:00.1', '0000:00:00.2', '0000:00:00.3']))

8
nova/tests/unit/pci/test_stats.py
View File

@ -152,7 +152,7 @@ class PciDeviceStatsTestCase(test.NoDBTestCase):
devs = self.pci_stats.consume_requests(pci_requests)
self.assertEqual(2, len(devs))
self.assertEqual(set(['v1', 'v2']),
set([dev['vendor_id'] for dev in devs]))
set([dev.vendor_id for dev in devs]))
def test_consume_requests_empty(self):
devs = self.pci_stats.consume_requests([])
@ -187,7 +187,7 @@ class PciDeviceStatsTestCase(test.NoDBTestCase):
devs = self.pci_stats.consume_requests(pci_requests, cells)
self.assertEqual(2, len(devs))
self.assertEqual(set(['v1', 'v2']),
set([dev['vendor_id'] for dev in devs]))
set([dev.vendor_id for dev in devs]))
def test_consume_requests_numa_failed(self):
cells = [objects.NUMACell(id=0, cpuset=set(), memory=0)]
@ -202,7 +202,7 @@ class PciDeviceStatsTestCase(test.NoDBTestCase):
devs = self.pci_stats.consume_requests(pci_request, cells)
self.assertEqual(1, len(devs))
self.assertEqual(set(['v3']),
set([dev['vendor_id'] for dev in devs]))
set([dev.vendor_id for dev in devs]))
@mock.patch.object(whitelist, 'get_pci_devices_filter')
@ -286,7 +286,7 @@ class PciDeviceStatsWithTagsTestCase(test.NoDBTestCase):
devs = self.pci_stats.consume_requests(pci_requests)
self.assertEqual(2, len(devs))
self.assertEqual(set(['0071', '0072']),
set([dev['product_id'] for dev in devs]))
set([dev.product_id for dev in devs]))
self._assertPoolContent(self.pci_stats.pools[0], '1137', '0072', 2)
self._assertPoolContent(self.pci_stats.pools[1], '1137', '0071', 3,
physical_network='physnet1')

2
nova/virt/libvirt/driver.py
View File

@ -3346,7 +3346,7 @@ class LibvirtDriver(driver.ComputeDriver):
def _get_guest_pci_device(self, pci_device):
dbsf = pci_utils.parse_address(pci_device['address'])
dbsf = pci_utils.parse_address(pci_device.address)
dev = vconfig.LibvirtConfigGuestHostdevPCI()
dev.domain, dev.bus, dev.slot, dev.function = dbsf