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

9 files changed, 44 insertions, 44 deletions

diff --git a/nova/api/openstack/compute/plugins/v3/pci.py b/nova/api/openstack/compute/plugins/v3/pci.py
index 12f1cf67c6..3867f5e4a9 100644
--- a/nova/api/openstack/compute/plugins/v3/pci.py
+++ b/nova/api/openstack/compute/plugins/v3/pci.py
@@ -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):
diff --git a/nova/objects/pci_device.py b/nova/objects/pci_device.py
index 5098b7569a..aafca08d4a 100644
--- a/nova/objects/pci_device.py
+++ b/nova/objects/pci_device.py
@@ -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)
diff --git a/nova/pci/device.py b/nova/pci/device.py
index a7cb201cfd..8f4e8d079b 100644
--- a/nova/pci/device.py
+++ b/nova/pci/device.py
@@ -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,
diff --git a/nova/pci/manager.py b/nova/pci/manager.py
index 04719f4f50..dfb161c2f5 100644
--- a/nova/pci/manager.py
+++ b/nova/pci/manager.py
@@ -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):
diff --git a/nova/pci/stats.py b/nova/pci/stats.py
index 46bdc2a652..d5becef55e 100644
--- a/nova/pci/stats.py
+++ b/nova/pci/stats.py
@@ -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
diff --git a/nova/tests/unit/pci/test_device.py b/nova/tests/unit/pci/test_device.py
index c27d43c4bd..e0f3d5f46a 100644
--- a/nova/tests/unit/pci/test_device.py
+++ b/nova/tests/unit/pci/test_device.py
@@ -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'
diff --git a/nova/tests/unit/pci/test_manager.py b/nova/tests/unit/pci/test_manager.py
index 1e8cd1cdf7..5b517f3a68 100644
--- a/nova/tests/unit/pci/test_manager.py
+++ b/nova/tests/unit/pci/test_manager.py
@@ -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']))
 
 
diff --git a/nova/tests/unit/pci/test_stats.py b/nova/tests/unit/pci/test_stats.py
index 25b138cc0c..8cdeb4be21 100644
--- a/nova/tests/unit/pci/test_stats.py
+++ b/nova/tests/unit/pci/test_stats.py
@@ -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')
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index b7f97e4885..0850806a0c 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -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