diff --git a/nova/compute/provider_tree.py b/nova/compute/provider_tree.py index 7be5d8b91a1d..06bb0521e13a 100644 --- a/nova/compute/provider_tree.py +++ b/nova/compute/provider_tree.py @@ -25,6 +25,7 @@ import os_traits from oslo_concurrency import lockutils from oslo_log import log as logging from oslo_utils import uuidutils +import six from nova.i18n import _ @@ -232,7 +233,12 @@ class ProviderTree(object): def __init__(self): """Create an empty provider tree.""" self.lock = lockutils.internal_lock(_LOCK_NAME) - self.roots = [] + self.roots_by_uuid = {} + self.roots_by_name = {} + + @property + def roots(self): + return six.itervalues(self.roots_by_uuid) def get_provider_uuids(self, name_or_uuid=None): """Return a list, in top-down traversable order, of the UUIDs of all @@ -343,7 +349,8 @@ class ProviderTree(object): provider = _Provider.from_dict(pd) if parent_uuid is None: - self.roots.append(provider) + self.roots_by_uuid[provider.uuid] = provider + self.roots_by_name[provider.name] = provider else: parent = self._find_with_lock(parent_uuid) parent.add_child(provider) @@ -357,7 +364,8 @@ class ProviderTree(object): parent = self._find_with_lock(found.parent_uuid) parent.remove_child(found) else: - self.roots.remove(found) + del self.roots_by_uuid[found.uuid] + del self.roots_by_name[found.name] def remove(self, name_or_uuid): """Safely removes the provider identified by the supplied name_or_uuid @@ -393,10 +401,21 @@ class ProviderTree(object): raise ValueError(err % uuid) p = _Provider(name, uuid=uuid, generation=generation) - self.roots.append(p) + self.roots_by_uuid[uuid] = p + self.roots_by_name[name] = p return p.uuid def _find_with_lock(self, name_or_uuid, return_root=False): + # Optimization for large number of roots (e.g. ironic): if name_or_uuid + # represents a root, this is O(1). + found = self.roots_by_uuid.get(name_or_uuid) + if found: + return found + found = self.roots_by_name.get(name_or_uuid) + if found: + return found + + # Okay, it's a child; do it the hard way. for root in self.roots: found = root.find(name_or_uuid) if found: diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 1b1a6b381191..60476a6fef32 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -21490,7 +21490,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): self.assertRaises(exception.ComputeResourcesUnavailable, drvr._allocate_mdevs, allocations=allocations) - def test_allocate_mdevs_with_no_idea_of_the_provider(self): + @mock.patch.object(libvirt_driver.LOG, 'warning') + def test_allocate_mdevs_with_no_idea_of_the_provider(self, mock_warning): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) # Mock the fact update_provider_tree() should have run drvr.provider_tree = self._get_fake_provider_tree_with_vgpu() @@ -21517,6 +21518,9 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): # Remember, rp2 has a wrong naming convention self.assertRaises(exception.ComputeResourcesUnavailable, drvr._allocate_mdevs, allocations=allocations) + mock_warning.assert_called_once_with( + "pGPU device name %(name)s can't be guessed from the ProviderTree " + "roots %(roots)s", {'name': 'oops_I_did_it_again', 'roots': 'cn'}) @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_mediated_devices') @mock.patch.object(libvirt_driver.LibvirtDriver, diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index ad1aa6293934..a982960f7e4d 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -6366,7 +6366,7 @@ class LibvirtDriver(driver.ComputeDriver): rp_name = allocated_rp.name # There can be multiple roots, we need to find the root name # to guess the physical device name - roots = self.provider_tree.roots + roots = list(self.provider_tree.roots) for root in roots: if rp_name.startswith(root.name + '_'): # The RP name convention is : @@ -6374,10 +6374,11 @@ class LibvirtDriver(driver.ComputeDriver): parent_device = rp_name[len(root.name) + 1:] break else: - LOG.warning("pGPU device name %(name)s can't be guessed from " - "the ProviderTree " - "roots %(roots)s", {'name': rp_name, - 'roots': roots}) + LOG.warning( + "pGPU device name %(name)s can't be guessed from the " + "ProviderTree roots %(roots)s", + {'name': rp_name, + 'roots': ', '.join([root.name for root in roots])}) # We f... have no idea what was the parent device # If we can't find devices having available VGPUs, just raise raise exception.ComputeResourcesUnavailable(