From 0208be629c3853863bcd49b8bdbe2b9889b85012 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Wed, 2 Jul 2025 18:01:41 +0200 Subject: [PATCH] Fix pci_tracker.save to delete all removed devs The bug is that the save code modifies the self.pci_devs.object list while iterating it. This resulted in only half of the removed devs was deleted at each save() run. Iterating on a shallow copy of that list fixes it. Closes-Bug: #2115729 Change-Id: I2711be9605618c1b93104d1dbddd8c7ee73b577e Signed-off-by: Balazs Gibizer --- nova/pci/manager.py | 4 +++- nova/tests/unit/pci/test_manager.py | 10 ---------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/nova/pci/manager.py b/nova/pci/manager.py index 3f3093bb8bce..2cbce90516d6 100644 --- a/nova/pci/manager.py +++ b/nova/pci/manager.py @@ -15,6 +15,7 @@ # under the License. import collections +import copy import typing as ty from oslo_config import cfg @@ -97,7 +98,8 @@ class PciDevTracker(object): self.stats.add_device(dev) def save(self, context: ctx.RequestContext) -> None: - for dev in self.pci_devs: + devs = copy.copy(self.pci_devs.objects) + for dev in devs: if dev.obj_what_changed(): with dev.obj_alternate_context(context): dev.save() diff --git a/nova/tests/unit/pci/test_manager.py b/nova/tests/unit/pci/test_manager.py index fc5765b10e36..7826c3e72935 100644 --- a/nova/tests/unit/pci/test_manager.py +++ b/nova/tests/unit/pci/test_manager.py @@ -885,18 +885,8 @@ class PciDevTrackerTestCase(test.NoDBTestCase): dev2.remove() self.tracker.save(self.fake_context) - # This is https://bugs.launchpad.net/nova/+bug/2115729 as - # only one half of the removed devices are destroyed. - self.assertEqual(len(self.tracker.pci_devs), 2) - self.assertEqual(self.destroy_called, 1) - # a subsequent save will destroy half of the remaining removed devices - self.tracker.save(self.fake_context) self.assertEqual(len(self.tracker.pci_devs), 1) self.assertEqual(self.destroy_called, 2) - # after the fix we should see that a single save causes all the - # removed devices destroyed - # self.assertEqual(len(self.tracker.pci_devs), 1) - # self.assertEqual(self.destroy_called, 2) def test_clean_usage(self): inst_2 = copy.copy(self.inst)