Merge "Convert pci_device_pools dict to object before passing to scheduler"

nova/compute/resource_tracker.py

@@ -387,9 +387,9 @@ class ResourceTracker(object):
         # from deleted instances.
         if self.pci_tracker:
             self.pci_tracker.clean_usage(instances, migrations, orphans)
-            resources['pci_stats'] = jsonutils.dumps(self.pci_tracker.stats)
+            resources['pci_device_pools'] = self.pci_tracker.stats
         else:
-            resources['pci_stats'] = jsonutils.dumps([])
+            resources['pci_device_pools'] = []
 
         self._report_final_resource_view(resources)
 
@@ -532,7 +532,7 @@ class ResourceTracker(object):
         else:
             tcpu = 0
             ucpu = 0
-        pci_stats = resources.get('pci_stats')
+        pci_device_pools = resources.get('pci_device_pools')
         LOG.info(_LI("Final resource view: "
                      "name=%(node)s "
                      "phys_ram=%(phys_ram)sMB "
@@ -549,7 +549,7 @@ class ResourceTracker(object):
                   'used_disk': resources['local_gb_used'],
                   'total_vcpus': tcpu,
                   'used_vcpus': ucpu,
-                  'pci_stats': pci_stats})
+                  'pci_stats': pci_device_pools})
 
     def _resource_change(self, resources):
         """Check to see if any resouces have changed."""
@@ -668,10 +668,9 @@ class ResourceTracker(object):
                 self.pci_tracker.update_pci_for_migration(context, instance)
             self._update_usage(context, resources, usage)
             if self.pci_tracker:
-                resources['pci_stats'] = jsonutils.dumps(
-                        self.pci_tracker.stats)
+                resources['pci_device_pools'] = self.pci_tracker.stats
             else:
-                resources['pci_stats'] = jsonutils.dumps([])
+                resources['pci_device_pools'] = []
             self.tracked_migrations[uuid] = (migration, itype)
 
     def _update_usage_from_migrations(self, context, resources, migrations):
@@ -740,9 +739,9 @@ class ResourceTracker(object):
 
         resources['current_workload'] = self.stats.calculate_workload()
         if self.pci_tracker:
-            resources['pci_stats'] = jsonutils.dumps(self.pci_tracker.stats)
+            resources['pci_device_pools'] = self.pci_tracker.stats
         else:
-            resources['pci_stats'] = jsonutils.dumps([])
+            resources['pci_device_pools'] = []
 
     def _update_usage_from_instances(self, context, resources, instances):
         """Calculate resource usage based on instance utilization.  This is

nova/scheduler/client/report.py

@@ -53,7 +53,21 @@ class SchedulerReportClient(object):
                                            id=compute_node_id)
         compute_node.obj_reset_changes()
         for k, v in updates.items():
-            setattr(compute_node, k, v)
+            if k == 'pci_device_pools':
+                # NOTE(danms): Since the updates are actually the result of
+                # a obj_to_primitive() on some real objects, we need to convert
+                # back to a real object (not from_dict() or _from_db_object(),
+                # which expect a db-formatted object) but just an attr-based
+                # reconstruction. When we start getting a ComputeNode from
+                # scheduler this "bandage" can go away.
+                if v:
+                    devpools = [objects.PciDevicePool.from_dict(x) for x in v]
+                else:
+                    devpools = []
+                compute_node.pci_device_pools = objects.PciDevicePoolList(
+                    objects=devpools)
+            else:
+                setattr(compute_node, k, v)
         compute_node.save()
 
         LOG.info(_LI('Compute_service record updated for '

nova/tests/unit/compute/test_resource_tracker.py

@@ -721,7 +721,7 @@ class TrackerTestCase(BaseTrackerTestCase):
         self.assertFalse(self.tracker.disabled)
         self.assertEqual(0, self.tracker.compute_node['current_workload'])
         self.assertEqual(driver.pci_stats,
-            jsonutils.loads(self.tracker.compute_node['pci_stats']))
+            self.tracker.compute_node['pci_device_pools'])
 
 
 class SchedulerClientTrackerTestCase(BaseTrackerTestCase):
@@ -778,8 +778,12 @@ class TrackerPciStatsTestCase(BaseTrackerTestCase):
         self._assert(FAKE_VIRT_LOCAL_GB, 'free_disk_gb')
         self.assertFalse(self.tracker.disabled)
         self.assertEqual(0, self.tracker.compute_node['current_workload'])
-        self.assertEqual(driver.pci_stats,
-            jsonutils.loads(self.tracker.compute_node['pci_stats']))
+
+        # NOTE(danms): PciDeviceStats only supports iteration, so we have to
+        # listify it before we can examine the contents by index.
+        pools = list(self.tracker.compute_node['pci_device_pools'])
+        self.assertEqual(driver.pci_stats[0]['product_id'],
+                         pools[0]['product_id'])
 
     def _driver(self):
         return FakeVirtDriver(pci_support=True)

nova/tests/unit/compute/test_tracker.py

@@ -395,7 +395,7 @@ class TestUpdateAvailableResources(BaseTestCase):
             'local_gb': 6,
             'free_ram_mb': 512,
             'memory_mb_used': 0,
-            'pci_stats': '[]',
+            'pci_device_pools': [],
             'vcpus_used': 0,
             'hypervisor_type': 'fake',
             'local_gb_used': 0,
@@ -431,7 +431,7 @@ class TestUpdateAvailableResources(BaseTestCase):
             'local_gb': 6,
             'free_ram_mb': 0,  # 512MB avail - 512MB reserved
             'memory_mb_used': 512,  # 0MB used + 512MB reserved
-            'pci_stats': '[]',
+            'pci_device_pools': [],
             'vcpus_used': 0,
             'hypervisor_type': 'fake',
             'local_gb_used': 1,  # 0GB used + 1 GB reserved
@@ -466,7 +466,7 @@ class TestUpdateAvailableResources(BaseTestCase):
             'local_gb': 6,
             'free_ram_mb': 384,  # 512 - 128 used
             'memory_mb_used': 128,
-            'pci_stats': '[]',
+            'pci_device_pools': [],
             # NOTE(jaypipes): Due to the design of the ERT, which now is used
             #                 track VCPUs, the actual used VCPUs isn't
             #                 "written" to the resources dictionary that is
@@ -532,7 +532,7 @@ class TestUpdateAvailableResources(BaseTestCase):
             'local_gb': 6,
             'free_ram_mb': 448,  # 512 - 64 orphaned usage
             'memory_mb_used': 64,
-            'pci_stats': '[]',
+            'pci_device_pools': [],
             'vcpus_used': 0,
             'hypervisor_type': 'fake',
             'local_gb_used': 0,
@@ -586,7 +586,7 @@ class TestUpdateAvailableResources(BaseTestCase):
             'local_gb': 6,
             'free_ram_mb': 384,  # 512 total - 128 for possible revert of orig
             'memory_mb_used': 128,  # 128 possible revert amount
-            'pci_stats': '[]',
+            'pci_device_pools': [],
             'vcpus_used': 0,
             'hypervisor_type': 'fake',
             'local_gb_used': 1,
@@ -635,7 +635,7 @@ class TestUpdateAvailableResources(BaseTestCase):
             'local_gb': 6,
             'free_ram_mb': 256,  # 512 total - 256 for possible confirm of new
             'memory_mb_used': 256,  # 256 possible confirmed amount
-            'pci_stats': '[]',
+            'pci_device_pools': [],
             'vcpus_used': 0,  # See NOTE(jaypipes) above about why this is 0
             'hypervisor_type': 'fake',
             'local_gb_used': 5,
@@ -688,7 +688,7 @@ class TestUpdateAvailableResources(BaseTestCase):
             # 512 total - 128 existing - 256 new flav - 128 old flav
             'free_ram_mb': 0,
             'memory_mb_used': 512,  # 128 exist + 256 new flav + 128 old flav
-            'pci_stats': '[]',
+            'pci_device_pools': [],
             # See NOTE(jaypipes) above for reason why this isn't accurate until
             # _sync_compute_node() is called.
             'vcpus_used': 0,
@@ -743,7 +743,7 @@ class TestSyncComputeNode(BaseTestCase):
             'local_gb': 6,
             'free_ram_mb': 512,
             'memory_mb_used': 0,
-            'pci_stats': '[]',
+            'pci_device_pools': [],
             'vcpus_used': 0,
             'hypervisor_type': 'fake',
             'local_gb_used': 0,
@@ -813,7 +813,7 @@ class TestSyncComputeNode(BaseTestCase):
             'local_gb': 6,
             'free_ram_mb': 512,
             'memory_mb_used': 0,
-            'pci_stats': '[]',
+            'pci_device_pools': [],
             'vcpus_used': 0,
             'hypervisor_type': 'fake',
             'local_gb_used': 0,
@@ -862,7 +862,7 @@ class TestSyncComputeNode(BaseTestCase):
             'local_gb': 6,
             'free_ram_mb': 384,
             'memory_mb_used': 128,
-            'pci_stats': '[]',
+            'pci_device_pools': [],
             'vcpus_used': 2,
             'hypervisor_type': 'fake',
             'local_gb_used': 4,
@@ -960,7 +960,7 @@ class TestInstanceClaim(BaseTestCase):
             "free_ram_mb": expected['memory_mb'] - self.instance.memory_mb,
             'running_vms': 1,
             # 'vcpus_used': 0,  # vcpus are not claimed
-            'pci_stats': '[]',
+            'pci_device_pools': [],
         })
         with mock.patch.object(self.rt, '_update') as update_mock:
             self.rt.instance_claim(self.ctx, self.instance, None)

nova/tests/unit/scheduler/test_client.py

@@ -39,7 +39,11 @@ class SchedulerReportClientTestCase(test.NoDBTestCase):
 
     @mock.patch.object(objects.ComputeNode, '__new__')
     def test_update_compute_node_works(self, mock_cn):
-        stats = {"id": 1, "foo": "bar"}
+        stats = {"id": 1, "foo": "bar",
+                 "pci_device_pools": [{"vendor_id": "foo",
+                                       "product_id": "foo",
+                                       "count": 1,
+                                       "a": "b"}]}
         self.client.update_resource_stats(self.context,
                                           ('fakehost', 'fakenode'),
                                           stats)
@@ -48,6 +52,7 @@ class SchedulerReportClientTestCase(test.NoDBTestCase):
                                         id=1)
         cn = mock_cn()
         cn.obj_reset_changes.assert_called_once_with()
+        self.assertEqual("b", cn.pci_device_pools[0].tags["a"])
         cn.save.assert_called_once_with()
         self.assertEqual('bar', cn.foo)
 
@@ -57,6 +62,14 @@ class SchedulerReportClientTestCase(test.NoDBTestCase):
                           self.client.update_resource_stats,
                           self.context, ('fakehost', 'fakenode'), stats)
 
+    @mock.patch('nova.objects.ComputeNode.save')
+    def test_update_resource_stats_pci_device_pools_none(self, mock_save):
+        stats = {"id": 1, "foo": "bar",
+                 "pci_device_pools": None}
+        self.client.update_resource_stats(self.context,
+                                          ('fakehost', 'fakenode'),
+                                          stats)
+
 
 class SchedulerQueryClientTestCase(test.NoDBTestCase):