Update pci stat pools based on PCI device changes

At start up of nova-compute service, the PCI stat pools are
populated based on information in pci_devices table in Nova
database. The pools are updated only when new device is added
or removed but not on any device changes like device type.

If an existing device is configured as SRIOV and nova-compute
is restarted, the pci_devices table gets updated but the device
is still listed under the old pool in pci_tracker.stats.pool
(in-memory object).

This patch looks for device type updates in existing devices
and updates the pools accordingly.

Conflicts:
      nova/tests/functional/libvirt/test_pci_sriov_servers.py
      nova/tests/unit/virt/libvirt/fakelibvirt.py

To avoid the conflicts and make the new functional test execute,
following changes are performed
- Modified the Neutron fixture to use LibvirtNeutron fixture.
- nova/tests/functional/libvirt/base.py is modified to include
tenant_id in the port body.

Change-Id: Id4ebb06e634a612c8be4be6c678d8265e0b99730
Closes-Bug: #1892361
(cherry picked from commit b8695de)
(cherry picked from commit d8b8a81)

nova/pci/manager.py

@@ -225,6 +225,7 @@ def _set_hvdevs(self, devices):
                     self.stale[new_value['address']] = new_value
                 else:
                     existed.update_device(new_value)
+                    self.stats.update_device(existed)
 
         # Track newly discovered devices.
         for dev in [dev for dev in devices if

nova/pci/stats.py

@@ -105,6 +105,32 @@ def _create_pool_keys_from_dev(self, dev):
             pool['parent_ifname'] = dev.extra_info['parent_ifname']
         return pool
 
+    def _get_pool_with_device_type_mismatch(self, dev):
+        """Check for device type mismatch in the pools for a given device.
+
+        Return (pool, device) if device type does not match or a single None
+        if the device type matches.
+        """
+        for pool in self.pools:
+            for device in pool['devices']:
+                if device.address == dev.address:
+                    if dev.dev_type != pool["dev_type"]:
+                        return pool, device
+                    return None
+
+        return None
+
+    def update_device(self, dev):
+        """Update a device to its matching pool."""
+        pool_device_info = self._get_pool_with_device_type_mismatch(dev)
+        if pool_device_info is None:
+            return
+
+        pool, device = pool_device_info
+        pool['devices'].remove(device)
+        self._decrease_pool_count(self.pools, pool)
+        self.add_device(dev)
+
     def add_device(self, dev):
         """Add a device to its matching pool."""
         dev_pool = self._create_pool_keys_from_dev(dev)

nova/tests/functional/libvirt/base.py

@@ -342,6 +342,15 @@ def create_port(self, body=None):
         # network_2_port_1 below at the update call
         port = copy.deepcopy(port)
         port.update(body['port'])
+
+        # the tenant ID is normally extracted from credentials in the request
+        # and is not present in the body
+        if 'tenant_id' not in port:
+            port['tenant_id'] = nova_fixtures.NeutronFixture.tenant_id
+
+        # similarly, these attributes are set by neutron itself
+        port['admin_state_up'] = True
+
         self._ports[port['id']] = port
         # this copy is here as nova sometimes modifies the returned port
         # locally and we want to avoid that nova modifies the fixture internals

nova/tests/functional/libvirt/test_pci_sriov_servers.py

@@ -13,13 +13,16 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import copy
 import ddt
 import fixtures
 import mock
 
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
 
+from nova import context
+from nova import objects
 from nova.objects import fields
 from nova.tests.functional.libvirt import base
 from nova.tests.unit.virt.libvirt import fakelibvirt
@@ -94,10 +97,12 @@ class SRIOVServersTest(_PCIServersTestBase):
         {
             'vendor_id': fakelibvirt.PCI_VEND_ID,
             'product_id': fakelibvirt.PF_PROD_ID,
+            'physical_network': 'physnet4',
         },
         {
             'vendor_id': fakelibvirt.PCI_VEND_ID,
             'product_id': fakelibvirt.VF_PROD_ID,
+            'physical_network': 'physnet4',
         },
     )]
     # PFs will be removed from pools unless they are specifically
@@ -117,6 +122,29 @@ class SRIOVServersTest(_PCIServersTestBase):
         },
     )]
 
+    def setUp(self):
+        super().setUp()
+
+        # The ultimate base class _IntegratedTestBase uses NeutronFixture but
+        # we need a bit more intelligent neutron for these tests. Applying the
+        # new fixture here means that we re-stub what the previous neutron
+        # fixture already stubbed.
+        self.neutron = self.useFixture(base.LibvirtNeutronFixture(self))
+
+    def _disable_sriov_in_pf(self, pci_info):
+        # Check for PF and change the capability from virt_functions
+        # Delete all the VFs
+        vfs_to_delete = []
+
+        for device_name, device in pci_info.devices.items():
+            if 'virt_functions' in device.pci_device:
+                device.generate_xml(skip_capability=True)
+            elif 'phys_function' in device.pci_device:
+                vfs_to_delete.append(device_name)
+
+        for device in vfs_to_delete:
+            del pci_info.devices[device]
+
     def test_create_server_with_VF(self):
 
         host_info = fakelibvirt.HostInfo(cpu_nodes=2, cpu_sockets=1,
@@ -187,6 +215,74 @@ def test_create_server_with_VF_no_PF(self):
         self._run_build_test(flavor_id_vfs)
         self._run_build_test(flavor_id_pfs, end_status='ERROR')
 
+    def test_create_server_after_change_in_nonsriov_pf_to_sriov_pf(self):
+        # Starts a compute with PF not configured with SRIOV capabilities
+        # Updates the PF with SRIOV capability and restart the compute service
+        # Then starts a VM with the sriov port. The VM should be in active
+        # state with sriov port attached.
+
+        # To emulate the device type changing, we first create a
+        # HostPCIDevicesInfo object with PFs and VFs. Then we make a copy
+        # and remove the VFs and the virt_function capability. This is
+        # done to ensure the physical function product id is same in both
+        # the versions.
+        host_info = fakelibvirt.HostInfo(cpu_nodes=2, cpu_sockets=1,
+                                         cpu_cores=2, cpu_threads=2,
+                                         kB_mem=15740000)
+        pci_info = fakelibvirt.HostPCIDevicesInfo(num_pfs=1, num_vfs=1)
+        pci_info_no_sriov = copy.deepcopy(pci_info)
+
+        # Disable SRIOV capabilties in PF and delete the VFs
+        self._disable_sriov_in_pf(pci_info_no_sriov)
+
+        fake_connection = self._get_connection(host_info,
+                                               pci_info=pci_info_no_sriov,
+                                               hostname='test_compute0')
+        self.mock_conn.return_value = fake_connection
+
+        self.compute = self.start_service('compute', host='test_compute0')
+
+        ctxt = context.get_admin_context()
+        pci_devices = objects.PciDeviceList.get_by_compute_node(
+            ctxt,
+            objects.ComputeNode.get_by_nodename(
+                ctxt, 'test_compute0',
+            ).id,
+        )
+        self.assertEqual(1, len(pci_devices))
+        self.assertEqual('type-PCI', pci_devices[0].dev_type)
+
+        # Update connection with original pci info with sriov PFs
+        fake_connection = self._get_connection(host_info,
+                                               pci_info=pci_info,
+                                               hostname='test_compute0')
+        self.mock_conn.return_value = fake_connection
+
+        # Restart the compute service
+        self.restart_compute_service(self.compute)
+
+        # Verify if PCI devices are of type type-PF or type-VF
+        pci_devices = objects.PciDeviceList.get_by_compute_node(
+            ctxt,
+            objects.ComputeNode.get_by_nodename(
+                ctxt, 'test_compute0',
+            ).id,
+        )
+        for pci_device in pci_devices:
+            self.assertIn(pci_device.dev_type, ['type-PF', 'type-VF'])
+
+        # create the port
+        self.neutron.create_port({'port': self.neutron.network_4_port_1})
+
+        # create a server using the VF via neutron
+        flavor_id = self._create_flavor()
+        self._create_server(
+            flavor_id=flavor_id,
+            networks=[
+                {'port': base.LibvirtNeutronFixture.network_4_port_1['id']},
+            ],
+        )
+
 
 class GetServerDiagnosticsServerWithVfTestV21(_PCIServersTestBase):
 

nova/tests/unit/pci/test_stats.py

@@ -533,6 +533,30 @@ def test_remove_device(self):
         self.pci_stats.remove_device(dev2)
         self._assertPools()
 
+    def test_update_device(self):
+        # Update device type of one of the device from type-PCI to
+        # type-PF. Verify if the existing pool is updated and a new
+        # pool is created with dev_type type-PF.
+        self._create_pci_devices()
+        dev1 = self.pci_tagged_devices.pop()
+        dev1.dev_type = 'type-PF'
+        self.pci_stats.update_device(dev1)
+        self.assertEqual(3, len(self.pci_stats.pools))
+        self._assertPoolContent(self.pci_stats.pools[0], '1137', '0072',
+                                len(self.pci_untagged_devices))
+        self.assertEqual(self.pci_untagged_devices,
+                         self.pci_stats.pools[0]['devices'])
+        self._assertPoolContent(self.pci_stats.pools[1], '1137', '0071',
+                                len(self.pci_tagged_devices),
+                                physical_network='physnet1')
+        self.assertEqual(self.pci_tagged_devices,
+                         self.pci_stats.pools[1]['devices'])
+        self._assertPoolContent(self.pci_stats.pools[2], '1137', '0071',
+                                1,
+                                physical_network='physnet1')
+        self.assertEqual(dev1,
+                         self.pci_stats.pools[2]['devices'][0])
+
 
 class PciDeviceVFPFStatsTestCase(test.NoDBTestCase):