Merge "Generate request_id for Flavor based InstancePCIRequest"

nova/network/neutron.py

@@ -43,7 +43,6 @@
 from nova.network import model as network_model
 from nova import objects
 from nova.objects import fields as obj_fields
-from nova.pci import manager as pci_manager
 from nova.pci import request as pci_request
 from nova.pci import utils as pci_utils
 from nova.pci import whitelist as pci_whitelist
@@ -1631,8 +1630,7 @@ def _populate_neutron_binding_profile(self, instance, pci_request_id,
             pci_request_id cannot be found on the instance.
         """
         if pci_request_id:
-            pci_devices = pci_manager.get_instance_pci_devs(
-                instance, pci_request_id)
+            pci_devices = instance.get_pci_devices(request_id=pci_request_id)
             if not pci_devices:
                 # The pci_request_id likely won't mean much except for tracing
                 # through the logs since it is generated per request.
@@ -1662,8 +1660,7 @@ def _populate_pci_mac_address(instance, pci_request_id, port_req_body):
         Currently this is done only for PF passthrough.
         """
         if pci_request_id is not None:
-            pci_devs = pci_manager.get_instance_pci_devs(
-                instance, pci_request_id)
+            pci_devs = instance.get_pci_devices(request_id=pci_request_id)
             if len(pci_devs) != 1:
                 # NOTE(ndipanov): We shouldn't ever get here since
                 # InstancePCIRequest instances built from network requests

nova/objects/instance.py

@@ -13,6 +13,7 @@
 #    under the License.
 
 import contextlib
+import typing as ty
 
 from oslo_config import cfg
 from oslo_db import exception as db_exc
@@ -1226,6 +1227,46 @@ def remove_pci_device_and_request(self, pci_device):
             pci_req for pci_req in self.pci_requests.requests
             if pci_req.request_id != pci_device.request_id]
 
+    def get_pci_devices(
+        self,
+        source: ty.Optional[int] = None,
+        request_id: ty.Optional[str] = None,
+    ) -> ty.List["objects.PciDevice"]:
+        """Return the PCI devices allocated to the instance
+
+        :param source: Filter by source. It can be
+            InstancePCIRequest.FLAVOR_ALIAS or InstancePCIRequest.NEUTRON_PORT
+            or None. None means returns devices from both type of requests.
+        :param request_id: Filter by PciDevice.request_id. None means do not
+            filter by request_id.
+        :return: a list of matching PciDevice objects
+        """
+        if not self.pci_devices:
+            # return early to avoid an extra lazy load on self.pci_requests
+            # if there are no devices allocated to be filtered
+            return []
+        else:
+            devs = self.pci_devices.objects
+
+        if request_id is not None:
+            devs = [dev for dev in devs if dev.request_id == request_id]
+
+        if source is not None:
+            # NOTE(gibi): this happens to work for the old requests when the
+            # request has request_id None and therefore the device allocated
+            # due to that request has request_id None too, so they will be
+            # mapped via the None key.
+            req_id_to_req = {
+                req.request_id: req for req in self.pci_requests.requests
+            }
+            devs = [
+                dev
+                for dev in devs
+                if (req_id_to_req[dev.request_id].source == source)
+            ]
+
+        return devs
+
 
 def _make_instance_list(context, inst_list, db_inst_list, expected_attrs):
     get_fault = expected_attrs and 'fault' in expected_attrs

nova/pci/manager.py

@@ -480,24 +480,3 @@ def clean_usage(
                 devs = self.allocations.pop(uuid, [])
                 for dev in devs:
                     self._free_device(dev)
-
-
-def get_instance_pci_devs(
-    inst: 'objects.Instance', request_id: str = None,
-) -> ty.List['objects.PciDevice']:
-    """Get the devices allocated to one or all requests for an instance.
-
-    - For generic PCI request, the request id is None.
-    - For sr-iov networking, the request id is a valid uuid
-    - There are a couple of cases where all the PCI devices allocated to an
-      instance need to be returned. Refer to libvirt driver that handles
-      soft_reboot and hard_boot of 'xen' instances.
-    """
-    pci_devices = inst.pci_devices
-    if pci_devices is None:
-        return []
-
-    return [
-        device for device in pci_devices if
-        device.request_id == request_id or request_id == 'all'
-    ]

nova/pci/request.py

@@ -43,6 +43,7 @@
 import jsonschema
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
+from oslo_utils import uuidutils
 
 import nova.conf
 from nova import context as ctx
@@ -183,7 +184,9 @@ def _translate_alias_to_requests(
             count=int(count),
             spec=spec,
             alias_name=name,
-            numa_policy=policy))
+            numa_policy=policy,
+            request_id=uuidutils.generate_uuid(),
+        ))
     return pci_requests
 
 

nova/tests/unit/network/test_neutron.py

@@ -42,7 +42,6 @@
 from nova.objects import fields as obj_fields
 from nova.objects import network_request as net_req_obj
 from nova.objects import virtual_interface as obj_vif
-from nova.pci import manager as pci_manager
 from nova.pci import request as pci_request
 from nova.pci import utils as pci_utils
 from nova.pci import whitelist as pci_whitelist
@@ -7738,11 +7737,11 @@ def test_populate_neutron_extension_values_binding(self, mock_get_client):
             'vf_num': 1,
         }))
     @mock.patch.object(pci_whitelist.Whitelist, 'get_devspec')
-    @mock.patch.object(pci_manager, 'get_instance_pci_devs')
+    @mock.patch('nova.objects.Instance.get_pci_devices')
     def test_populate_neutron_extension_values_binding_sriov(
         self, mock_get_instance_pci_devs, mock_get_pci_device_devspec):
         host_id = 'my_host_id'
-        instance = {'host': host_id}
+        instance = objects.Instance(host=host_id)
         port_req_body = {'port': {}}
         pci_req_id = 'my_req_id'
         pci_dev = {'vendor_id': '1377',
@@ -7783,11 +7782,11 @@ def test_populate_neutron_extension_values_binding_sriov(
         })
     )
     @mock.patch.object(pci_whitelist.Whitelist, 'get_devspec')
-    @mock.patch.object(pci_manager, 'get_instance_pci_devs')
+    @mock.patch('nova.objects.Instance.get_pci_devices')
     def test_populate_neutron_extension_values_binding_sriov_card_serial(
         self, mock_get_instance_pci_devs, mock_get_pci_device_devspec):
         host_id = 'my_host_id'
-        instance = {'host': host_id}
+        instance = objects.Instance(host=host_id)
         port_req_body = {'port': {}}
         pci_req_id = 'my_req_id'
         pci_dev = {'vendor_id': 'a2d6',
@@ -7867,11 +7866,11 @@ def test_populate_neutron_extension_values_with_arq(self,
         })
     )
     @mock.patch.object(pci_whitelist.Whitelist, 'get_devspec')
-    @mock.patch.object(pci_manager, 'get_instance_pci_devs')
+    @mock.patch('nova.objects.Instance.get_pci_devices')
     def test_populate_neutron_extension_values_binding_sriov_with_cap(
         self, mock_get_instance_pci_devs, mock_get_pci_device_devspec):
         host_id = 'my_host_id'
-        instance = {'host': host_id}
+        instance = objects.Instance(host=host_id)
         port_req_body = {'port': {
                              constants.BINDING_PROFILE: {
                                 'capabilities': ['switchdev']}}}
@@ -7907,12 +7906,12 @@ def test_populate_neutron_extension_values_binding_sriov_with_cap(
                              constants.BINDING_PROFILE])
 
     @mock.patch.object(pci_whitelist.Whitelist, 'get_devspec')
-    @mock.patch.object(pci_manager, 'get_instance_pci_devs')
+    @mock.patch('nova.objects.Instance.get_pci_devices')
     def test_populate_neutron_extension_values_binding_sriov_pf(
         self, mock_get_instance_pci_devs, mock_get_devspec
     ):
         host_id = 'my_host_id'
-        instance = {'host': host_id}
+        instance = objects.Instance(host=host_id)
         port_req_body = {'port': {}}
 
         pci_dev = objects.PciDevice(
@@ -8041,11 +8040,11 @@ def test__get_pci_device_profile_pf(self, mock_get_pci_device_devspec):
         )
 
     @mock.patch.object(pci_whitelist.Whitelist, 'get_devspec')
-    @mock.patch.object(pci_manager, 'get_instance_pci_devs')
+    @mock.patch('nova.objects.Instance.get_pci_devices')
     def test_populate_neutron_extension_values_binding_sriov_fail(
         self, mock_get_instance_pci_devs, mock_get_pci_device_devspec):
         host_id = 'my_host_id'
-        instance = {'host': host_id}
+        instance = objects.Instance(host=host_id)
         port_req_body = {'port': {}}
         pci_req_id = 'my_req_id'
         pci_objs = [objects.PciDevice(vendor_id='1377',
@@ -8062,7 +8061,7 @@ def test_populate_neutron_extension_values_binding_sriov_fail(
             self.api._populate_neutron_binding_profile,
             instance, pci_req_id, port_req_body, None)
 
-    @mock.patch.object(pci_manager, 'get_instance_pci_devs', return_value=[])
+    @mock.patch('nova.objects.Instance.get_pci_devices', return_value=[])
     def test_populate_neutron_binding_profile_pci_dev_not_found(
             self, mock_get_instance_pci_devs):
         api = neutronapi.API()
@@ -8073,7 +8072,7 @@ def test_populate_neutron_binding_profile_pci_dev_not_found(
                           api._populate_neutron_binding_profile,
                           instance, pci_req_id, port_req_body, None)
         mock_get_instance_pci_devs.assert_called_once_with(
-            instance, pci_req_id)
+            request_id=pci_req_id)
 
     @mock.patch.object(
         pci_utils, 'is_physical_function',
@@ -8089,7 +8088,7 @@ def test_populate_neutron_binding_profile_pci_dev_not_found(
         new=mock.MagicMock(side_effect=(lambda vf_a: {
             '0000:0a:00.0': '52:54:00:1e:59:c6'}.get(vf_a)))
     )
-    @mock.patch.object(pci_manager, 'get_instance_pci_devs')
+    @mock.patch('nova.objects.Instance.get_pci_devices')
     def test_pci_parse_whitelist_called_once(
         self, mock_get_instance_pci_devs
     ):
@@ -8108,7 +8107,7 @@ def test_pci_parse_whitelist_called_once(
         # after the 'device_spec' is set in this test case.
         api = neutronapi.API()
         host_id = 'my_host_id'
-        instance = {'host': host_id}
+        instance = objects.Instance(host=host_id)
         pci_req_id = 'my_req_id'
         port_req_body = {'port': {}}
         pci_dev = {'vendor_id': '1377',
@@ -8144,7 +8143,7 @@ def _populate_pci_mac_address_fakes(self):
         vf.update_device(pci_dev)
         return instance, pf, vf
 
-    @mock.patch.object(pci_manager, 'get_instance_pci_devs')
+    @mock.patch('nova.objects.Instance.get_pci_devices')
     @mock.patch.object(pci_utils, 'get_mac_by_pci_address')
     def test_populate_pci_mac_address_pf(self, mock_get_mac_by_pci_address,
                                          mock_get_instance_pci_devs):
@@ -8158,7 +8157,7 @@ def test_populate_pci_mac_address_pf(self, mock_get_mac_by_pci_address,
         self.api._populate_pci_mac_address(instance, 0, req)
         self.assertEqual(expected_port_req_body, req)
 
-    @mock.patch.object(pci_manager, 'get_instance_pci_devs')
+    @mock.patch('nova.objects.Instance.get_pci_devices')
     @mock.patch.object(pci_utils, 'get_mac_by_pci_address')
     def test_populate_pci_mac_address_vf(self, mock_get_mac_by_pci_address,
                                          mock_get_instance_pci_devs):
@@ -8170,7 +8169,7 @@ def test_populate_pci_mac_address_vf(self, mock_get_mac_by_pci_address,
         self.api._populate_pci_mac_address(instance, 42, port_req_body)
         self.assertEqual(port_req_body, req)
 
-    @mock.patch.object(pci_manager, 'get_instance_pci_devs')
+    @mock.patch('nova.objects.Instance.get_pci_devices')
     @mock.patch.object(pci_utils, 'get_mac_by_pci_address')
     def test_populate_pci_mac_address_vf_fail(self,
                                               mock_get_mac_by_pci_address,
@@ -8185,7 +8184,7 @@ def test_populate_pci_mac_address_vf_fail(self,
         self.api._populate_pci_mac_address(instance, 42, port_req_body)
         self.assertEqual(port_req_body, req)
 
-    @mock.patch.object(pci_manager, 'get_instance_pci_devs')
+    @mock.patch('nova.objects.Instance.get_pci_devices')
     @mock.patch('nova.network.neutron.LOG.error')
     def test_populate_pci_mac_address_no_device(self, mock_log_error,
                                                 mock_get_instance_pci_devs):