Fix handling of pci_requests in consume_from_instance.

Properly retrieve requests from pci_requests in consume_from_instance.
Without this the call to numa_fit_instance_to_host will fail because
it expects the request list.
And change the order in which apply_requests and numa_fit_instance_to_host
are called. Calling apply_requests first will remove devices from pools
and  may make numa_fit_instance_to_host fail.

Change-Id: I41cf4e8e5c1dea5f91e5261a8f5e88f46c7994ef
Closes-bug: #1444021
(cherry picked from commit 0913e799e9ce3138235f5ea6f80159f468ad2aaa)

4 files changed, 85 insertions, 10 deletions

diff --git a/nova/objects/instance_pci_requests.py b/nova/objects/instance_pci_requests.py
index 8e4dee10f3..2a07b148ba 100644
--- a/nova/objects/instance_pci_requests.py
+++ b/nova/objects/instance_pci_requests.py
@@ -153,3 +153,9 @@ class InstancePCIRequests(base.NovaObject,
         blob = self.to_json()
         db.instance_extra_update_by_uuid(self._context, self.instance_uuid,
                                          {'pci_requests': blob})
+
+    @classmethod
+    def from_request_spec_instance_props(cls, pci_requests):
+        objs = [InstancePCIRequest(**request)
+            for request in pci_requests['requests']]
+        return cls(requests=objs, instance_uuid=pci_requests['instance_uuid'])
diff --git a/nova/scheduler/host_manager.py b/nova/scheduler/host_manager.py
index 9b2acf7a6f..0dcf23f59e 100644
--- a/nova/scheduler/host_manager.py
+++ b/nova/scheduler/host_manager.py
@@ -256,28 +256,33 @@ class HostState(object):
         # Track number of instances on host
         self.num_instances += 1
 
-        instance_numa_topology = hardware.instance_topology_from_instance(
-            instance)
-        instance_cells = None
-        if instance_numa_topology:
-            instance_cells = instance_numa_topology.cells
-
         pci_requests = instance.get('pci_requests')
         # NOTE(danms): Instance here is still a dict, which is converted from
-        # an object. Thus, it has a .pci_requests field, which gets converted
-        # to a primitive early on, and is thus a dict here. Convert this when
+        # an object. The pci_requests are a dict as well. Convert this when
         # we get an object all the way to this path.
         if pci_requests and pci_requests['requests'] and self.pci_stats:
+            pci_requests = objects.InstancePCIRequests \
+                .from_request_spec_instance_props(pci_requests)
             pci_requests = pci_requests.requests
-            self.pci_stats.apply_requests(pci_requests, instance_cells)
+        else:
+            pci_requests = None
 
         # Calculate the numa usage
         host_numa_topology, _fmt = hardware.host_topology_and_format_from_host(
                                 self)
+        instance_numa_topology = hardware.instance_topology_from_instance(
+            instance)
+
         instance['numa_topology'] = hardware.numa_fit_instance_to_host(
             host_numa_topology, instance_numa_topology,
             limits=self.limits.get('numa_topology'),
             pci_requests=pci_requests, pci_stats=self.pci_stats)
+        if pci_requests:
+            instance_cells = None
+            if instance['numa_topology']:
+                instance_cells = instance['numa_topology'].cells
+            self.pci_stats.apply_requests(pci_requests, instance_cells)
+
         self.numa_topology = hardware.get_host_numa_usage_from_instance(
                 self, instance)
 
diff --git a/nova/tests/unit/objects/test_instance_pci_requests.py b/nova/tests/unit/objects/test_instance_pci_requests.py
index fbccb93563..8eee4c5548 100644
--- a/nova/tests/unit/objects/test_instance_pci_requests.py
+++ b/nova/tests/unit/objects/test_instance_pci_requests.py
@@ -190,6 +190,23 @@ class _TestInstancePCIRequests(object):
         self.assertEqual(2, len(req.requests))
         self.assertEqual('alias_1', req.requests[0].alias_name)
 
+    def test_from_request_spec_instance_props(self):
+        requests = objects.InstancePCIRequests(
+            requests=[objects.InstancePCIRequest(count=1,
+                                                 request_id=FAKE_UUID,
+                                                 spec=[{'vendor_id': '8086',
+                                                        'device_id': '1502'}])
+                      ],
+            instance_uuid=FAKE_UUID)
+        result = jsonutils.to_primitive(requests)
+        result = objects.InstancePCIRequests.from_request_spec_instance_props(
+                                                                        result)
+        self.assertEqual(1, len(result.requests))
+        self.assertEqual(1, result.requests[0].count)
+        self.assertEqual(FAKE_UUID, result.requests[0].request_id)
+        self.assertEqual([{'vendor_id': '8086', 'device_id': '1502'}],
+                          result.requests[0].spec)
+
 
 class TestInstancePCIRequests(test_objects._LocalTest,
                               _TestInstancePCIRequests):
diff --git a/nova/tests/unit/scheduler/test_host_manager.py b/nova/tests/unit/scheduler/test_host_manager.py
index 9217c26cbd..faa783141d 100644
--- a/nova/tests/unit/scheduler/test_host_manager.py
+++ b/nova/tests/unit/scheduler/test_host_manager.py
@@ -29,8 +29,10 @@ from nova.compute import vm_states
 from nova import exception
 from nova import objects
 from nova.objects import base as obj_base
+from nova.pci import stats as pci_stats
 from nova.scheduler import filters
 from nova.scheduler import host_manager
+from nova.scheduler import utils as sched_utils
 from nova import test
 from nova.tests.unit import fake_instance
 from nova.tests.unit import matchers
@@ -880,7 +882,8 @@ class HostStateTestCase(test.NoDBTestCase):
                         project_id='12345', vm_state=vm_states.BUILDING,
                         task_state=task_states.SCHEDULING, os_type='Linux',
                         uuid='fake-uuid',
-                        numa_topology=fake_numa_topology)
+                        numa_topology=fake_numa_topology,
+                        pci_requests={'requests': []})
         host = host_manager.HostState("fakehost", "fakenode")
 
         host.consume_from_instance(instance)
@@ -908,6 +911,50 @@ class HostStateTestCase(test.NoDBTestCase):
         self.assertEqual(((host, instance),), numa_usage_mock.call_args)
         self.assertEqual('fake-consumed-twice', host.numa_topology)
 
+    def test_stat_consumption_from_instance_pci(self):
+
+        inst_topology = objects.InstanceNUMATopology(
+                            cells = [objects.InstanceNUMACell(
+                                                      cpuset=set([0]),
+                                                      memory=512, id=0)])
+
+        fake_requests = [{'request_id': 'fake_request1', 'count': 1,
+                          'spec': [{'vendor_id': '8086'}]}]
+        fake_requests_obj = objects.InstancePCIRequests(
+                                requests=[objects.InstancePCIRequest(**r)
+                                          for r in fake_requests],
+                                instance_uuid='fake-uuid')
+        instance = objects.Instance(root_gb=0, ephemeral_gb=0, memory_mb=512,
+                        vcpus=1,
+                        project_id='12345', vm_state=vm_states.BUILDING,
+                        task_state=task_states.SCHEDULING, os_type='Linux',
+                        uuid='fake-uuid',
+                        numa_topology=inst_topology,
+                        pci_requests=fake_requests_obj,
+                        id = 1243)
+        req_spec = sched_utils.build_request_spec(None,
+                                                  None,
+                                                  [instance],
+                                                  objects.Flavor(
+                                                             root_gb=0,
+                                                             ephemeral_gb=0,
+                                                             memory_mb=1024,
+                                                             vcpus=1))
+        host = host_manager.HostState("fakehost", "fakenode")
+        host.pci_stats = pci_stats.PciDeviceStats(
+                                      [objects.PciDevicePool(vendor_id='8086',
+                                                             product_id='15ed',
+                                                             numa_node=1,
+                                                             count=1)])
+        host.numa_topology = fakes.NUMA_TOPOLOGY
+        host.consume_from_instance(req_spec['instance_properties'])
+        self.assertIsInstance(req_spec['instance_properties']['numa_topology'],
+                              objects.InstanceNUMATopology)
+
+        self.assertEqual(512, host.numa_topology.cells[1].memory_usage)
+        self.assertEqual(1, host.numa_topology.cells[1].cpu_usage)
+        self.assertEqual(0, len(host.pci_stats.pools))
+
     def test_resources_consumption_from_compute_node(self):
         metrics = [
             dict(name='res1',