From f37cdf0c4182103ad81dbf39188ff39955da3850 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Tue, 8 Jul 2025 16:55:55 +0200 Subject: [PATCH] [PCI tracker]Remove non configured devs when freed The PCI tracker handles the case when a device spec is removed from the configuration while a device is still being allocated. It keeps the device until the VM is deleted to avoid inconsistencies. However the full removal of such a device needs not just the VM deletion, but also a nova-compute restart. The device tracker just frees the device during VM deletion but does not removed them until the next nova-compute startup. This allows the device to be re-allocated by another VM even though the device is not allowed by a device_spec. This change adds yet another in memory dict to the pci tracker to track these devices that are only kept until they are freed. Then during free() this list is consulted and if the device is in the list then the device is marked for removal as well. This kills two birds with one stone: * We prevent the re-allocation of the device as the state of the device will be set to REMOVED not AVAILABLE during VM deletion. * As PCI in Placement relies on the state of the device to decide what to track in placement, this change makes sure that a device that needs to be removed, is now removed from placement too. Note that we have another bug that prevents this removal for now. But at least the reproducers of that bug now starts to behave the same regardless of how many device belongs to the same RP in placement. Related-Bug: #2115905 Change-Id: I63c8fb2669a3c6b3adb77d210c0f9b39d3657c80 Signed-off-by: Balazs Gibizer --- nova/pci/manager.py | 18 ++- .../libvirt/test_pci_in_placement.py | 124 +++++------------- nova/tests/unit/pci/test_manager.py | 4 +- 3 files changed, 53 insertions(+), 93 deletions(-) diff --git a/nova/pci/manager.py b/nova/pci/manager.py index 3f3093bb8bce..2713466a5a17 100644 --- a/nova/pci/manager.py +++ b/nova/pci/manager.py @@ -68,6 +68,7 @@ class PciDevTracker(object): tracking. """ self.stale: ty.Dict[str, objects.PciDevice] = {} + self.to_be_removed_when_freed: ty.Dict[str, objects.PciDevice] = {} self.node_id: str = compute_node.id self.dev_filter = whitelist.Whitelist(CONF.pci.device_spec) numa_topology = compute_node.numa_topology @@ -254,6 +255,12 @@ class PciDevTracker(object): # device to a second vm. To prevent this bug we skip # deleting the device from the db in this iteration and # will try again on the next sync. + # NOTE(gibi): We keep a list of these devices in memory + # so that when the VM using the device is deleted then + # the tracker can not just free the device but also + # mark them for removal. This will prevent a bug where + # such a freed device is re-allocated before removed. + self.to_be_removed_when_freed[existed.address] = existed continue else: # Note(yjiang5): no need to update stats if an assigned @@ -395,8 +402,15 @@ class PciDevTracker(object): stale = self.stale.pop(dev.address, None) if stale: dev.update_device(stale) - for dev in freed_devs: - self.stats.add_device(dev) + + to_be_removed = self.to_be_removed_when_freed.pop(dev.address, None) + if to_be_removed: + dev.remove() + if dev in self.stats.get_free_devs(): + self.stats.remove_device(dev) + else: + for dev in freed_devs: + self.stats.add_device(dev) def free_instance_allocations( self, context: ctx.RequestContext, instance: 'objects.Instance', diff --git a/nova/tests/functional/libvirt/test_pci_in_placement.py b/nova/tests/functional/libvirt/test_pci_in_placement.py index ca10d00be325..14557d1bfbde 100644 --- a/nova/tests/functional/libvirt/test_pci_in_placement.py +++ b/nova/tests/functional/libvirt/test_pci_in_placement.py @@ -894,48 +894,26 @@ class PlacementPCIInventoryReportingTests(PlacementPCIReportingTests): ) self.stdlog.delete_stored_logs() - # Delete the server as the warning suggests - self._delete_server(server) + # Delete the server as the warning suggests. Unfortunately the deletion + # fails. This is bug https://bugs.launchpad.net/nova/+bug/2115905 + ex = self.assertRaises( + client.OpenStackApiException, self._delete_server, server) + self.assertIn("Unexpected API Error. Please report this", str(ex)) - # The deletion triggers a warning suggesting we have a bug. Indeed, - # this is part of https://bugs.launchpad.net/nova/+bug/2115905 - self.assertIn( - "WARNING [nova.compute.pci_placement_translator] " - "Device spec is not found for device 0000:81:00.0 in " - "[pci]device_spec. Ignoring device in Placement resource view. " - "This should not happen. Please file a bug", - self.stdlog.logger.output - ) + # The sever delete fails as nova tries to delete the RP while it still + # has allocations. + self.assertRegex( + self.stdlog.logger.output, + "ERROR .nova.scheduler.client.report..*Failed to delete " + "resource provider with UUID.*from the placement API. " + "Got 409.*Unable to delete resource provider.*Resource " + "provider has allocations.") - # The allocation successfully removed - compute1_expected_placement_view["usages"] = { - "0000:81:00.0": { - self.PF_RC: 0, - } - } - compute1_expected_placement_view["allocations"].pop(server["id"]) - # However the RP and the inventory are not removed from Placement - # due to pci tracker caching. The PciDevice remains in the DB until - # the next nova-compute restart and therefore the RP remains in - # Placement until too. This is a potential bug that keeps a device - # that seems to be available, but it should not as the device is not - # in the device spec anymore. - self.assert_placement_pci_view( - "compute1", **compute1_expected_placement_view) + # The instance is put into ERROR state. + server = self.api.get_server(server['id']) + self.assertEqual(server['status'], 'ERROR') - self.stdlog.delete_stored_logs() - self.restart_compute_service(hostname="compute1") - self._run_periodics() - - # The next compute restart not trigger PCI warning - self.assertNotIn( - "WARNING [nova.compute.pci_placement_translator]", - self.stdlog.logger.output) - - # And the device is now removed from Placement - compute1_expected_placement_view["inventories"].pop("0000:81:00.0") - compute1_expected_placement_view["traits"].pop("0000:81:00.0") - compute1_expected_placement_view["usages"].pop("0000:81:00.0") + # And the allocation is not removed. self.assert_placement_pci_view( "compute1", **compute1_expected_placement_view) @@ -1024,49 +1002,26 @@ class PlacementPCIInventoryReportingTests(PlacementPCIReportingTests): self.stdlog.logger.output, ) - self.stdlog.delete_stored_logs() - # Delete the server as the warning suggests - self._delete_server(server) + # Delete the server as the warning suggests. Unfortunately the deletion + # fails. This is bug https://bugs.launchpad.net/nova/+bug/2115905 + ex = self.assertRaises( + client.OpenStackApiException, self._delete_server, server) + self.assertIn("Unexpected API Error. Please report this", str(ex)) - # The deletion triggers a warning suggesting we have a bug. Indeed, - # this is part of https://bugs.launchpad.net/nova/+bug/2115905 - self.assertIn( - "WARNING [nova.compute.pci_placement_translator] " - "Device spec is not found for device 0000:81:00.1 in " - "[pci]device_spec. Ignoring device in Placement resource view. " - "This should not happen. Please file a bug", - self.stdlog.logger.output - ) + # The sever delete fails as nova tries to delete the RP while it still + # has allocations. + self.assertRegex( + self.stdlog.logger.output, + "ERROR .nova.scheduler.client.report..*Failed to delete " + "resource provider with UUID.*from the placement API. " + "Got 409.*Unable to delete resource provider.*Resource " + "provider has allocations.") - # The allocation successfully removed - compute1_expected_placement_view["usages"] = { - "0000:81:00.0": { - self.VF_RC: 0, - } - } - compute1_expected_placement_view["allocations"].pop(server["id"]) - # However the RP and the inventory are not removed from Placement - # due to pci tracker caching. The PciDevice remains in the DB until - # the next nova-compute restart and therefore the RP remains in - # Placement until too. This is a potential bug that keeps a device - # that seems to be available, but it should not as the device is not - # in the device spec anymore. - self.assert_placement_pci_view( - "compute1", **compute1_expected_placement_view) + # The instance is put into ERROR state. + server = self.api.get_server(server['id']) + self.assertEqual(server['status'], 'ERROR') - self.stdlog.delete_stored_logs() - self.restart_compute_service(hostname="compute1") - self._run_periodics() - - # The next compute restart not trigger PCI warning - self.assertNotIn( - "WARNING [nova.compute.pci_placement_translator]", - self.stdlog.logger.output) - - # And the device is now removed from Placement - compute1_expected_placement_view["inventories"].pop("0000:81:00.0") - compute1_expected_placement_view["traits"].pop("0000:81:00.0") - compute1_expected_placement_view["usages"].pop("0000:81:00.0") + # And the allocation is not removed. self.assert_placement_pci_view( "compute1", **compute1_expected_placement_view) @@ -1115,16 +1070,7 @@ class PlacementPCIInventoryReportingTests(PlacementPCIReportingTests): client.OpenStackApiException, self._delete_server, server) self.assertIn("Unexpected API Error. Please report this", str(ex)) - # The deletion triggers a warning as well suggesting we have a bug. - self.assertIn( - "WARNING [nova.compute.pci_placement_translator] " - "Device spec is not found for device 0000:81:00.2 in " - "[pci]device_spec. Ignoring device in Placement resource view. " - "This should not happen. Please file a bug", - self.stdlog.logger.output - ) - - # and the same RP deletion error as before. + # We have the same RP deletion error as before. self.assertRegex( self.stdlog.logger.output, "ERROR .nova.scheduler.client.report..*Failed to delete " diff --git a/nova/tests/unit/pci/test_manager.py b/nova/tests/unit/pci/test_manager.py index 0707d49f2a94..ab43596a1d60 100644 --- a/nova/tests/unit/pci/test_manager.py +++ b/nova/tests/unit/pci/test_manager.py @@ -516,9 +516,9 @@ class PciDevTrackerTestCase(test.NoDBTestCase): 0, len([dev for dev in self.tracker.pci_devs if dev.status == fields.PciDeviceStatus.REMOVED])) - # free the device that was allocated and update tracker again + + # free the device that was allocated self.tracker._free_device(claimed_dev) - self.tracker._set_hvdevs(copy.deepcopy(fake_pci_devs)) # and assert that one device is removed from the tracker self.assertEqual( 1,