From 0908cfe375140731db16bb5b4926f56ddf3bb95b Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Thu, 2 Oct 2014 16:52:12 +0100 Subject: [PATCH] pci: move filtering of devices up into resource tracker There is no particular reason why each virt driver impl should have to remember todo filtering of PCI devices. It is simpler for each virt driver to just return the full list of PCI devices and then have the resource tracker do any filtering. Cleanup before blueprint: resource-objects Change-Id: I38372826a1c45247c9c24db55c48939f04f24b13 --- nova/compute/resource_tracker.py | 15 +++- .../unit/compute/test_resource_tracker.py | 80 ++++++++++++++++--- nova/tests/unit/virt/libvirt/test_driver.py | 23 ++---- nova/tests/unit/virt/xenapi/test_xenapi.py | 12 +-- nova/virt/libvirt/driver.py | 12 +-- nova/virt/xenapi/host.py | 11 +-- 6 files changed, 92 insertions(+), 61 deletions(-) diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index ca927a488f..d136238653 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -37,6 +37,7 @@ from nova import objects from nova.objects import base as obj_base from nova.openstack.common import log as logging from nova.pci import manager as pci_manager +from nova.pci import whitelist as pci_whitelist from nova import rpc from nova.scheduler import client as scheduler_client from nova import utils @@ -73,6 +74,7 @@ class ResourceTracker(object): self.host = host self.driver = driver self.pci_tracker = None + self.pci_filter = pci_whitelist.get_pci_devices_filter() self.nodename = nodename self.compute_node = None self.stats = importutils.import_object(CONF.compute_stats_class) @@ -336,8 +338,17 @@ class ResourceTracker(object): if 'pci_passthrough_devices' in resources: if not self.pci_tracker: self.pci_tracker = pci_manager.PciDevTracker() - self.pci_tracker.set_hvdevs(jsonutils.loads(resources.pop( - 'pci_passthrough_devices'))) + + devs = [] + for dev in jsonutils.loads(resources.pop( + 'pci_passthrough_devices')): + if dev['dev_type'] == 'type-PF': + continue + + if self.pci_filter.device_assignable(dev): + devs.append(dev) + + self.pci_tracker.set_hvdevs(devs) # Grab all instances assigned to this node: instances = objects.InstanceList.get_by_host_and_node( diff --git a/nova/tests/unit/compute/test_resource_tracker.py b/nova/tests/unit/compute/test_resource_tracker.py index 4654e3af06..fb1b19949f 100644 --- a/nova/tests/unit/compute/test_resource_tracker.py +++ b/nova/tests/unit/compute/test_resource_tracker.py @@ -91,19 +91,70 @@ class FakeVirtDriver(driver.ComputeDriver): self.memory_mb_used = 0 self.local_gb_used = 0 self.pci_support = pci_support - self.pci_devices = [{ - 'label': 'forza-napoli', - 'dev_type': 'foo', - 'compute_node_id': 1, - 'address': '0000:00:00.1', - 'product_id': 'p1', - 'vendor_id': 'v1', - 'status': 'available', - 'extra_k1': 'v1'}] if self.pci_support else [] - self.pci_stats = [{ - 'count': 1, - 'vendor_id': 'v1', - 'product_id': 'p1'}] if self.pci_support else [] + self.pci_devices = [ + { + 'label': 'label_8086_0443', + 'dev_type': 'type-VF', + 'compute_node_id': 1, + 'address': '0000:00:01.1', + 'product_id': '0443', + 'vendor_id': '8086', + 'status': 'available', + 'extra_k1': 'v1' + }, + { + 'label': 'label_8086_0443', + 'dev_type': 'type-VF', + 'compute_node_id': 1, + 'address': '0000:00:01.2', + 'product_id': '0443', + 'vendor_id': '8086', + 'status': 'available', + 'extra_k1': 'v1' + }, + { + 'label': 'label_8086_0443', + 'dev_type': 'type-PF', + 'compute_node_id': 1, + 'address': '0000:00:01.0', + 'product_id': '0443', + 'vendor_id': '8086', + 'status': 'available', + 'extra_k1': 'v1' + }, + { + 'label': 'label_8086_0123', + 'dev_type': 'type-PCI', + 'compute_node_id': 1, + 'address': '0000:00:01.0', + 'product_id': '0123', + 'vendor_id': '8086', + 'status': 'available', + 'extra_k1': 'v1' + }, + { + 'label': 'label_8086_7891', + 'dev_type': 'type-VF', + 'compute_node_id': 1, + 'address': '0000:00:01.0', + 'product_id': '7891', + 'vendor_id': '8086', + 'status': 'available', + 'extra_k1': 'v1' + }, + ] if self.pci_support else [] + self.pci_stats = [ + { + 'count': 2, + 'vendor_id': '8086', + 'product_id': '0443' + }, + { + 'count': 1, + 'vendor_id': '8086', + 'product_id': '7891' + }, + ] if self.pci_support else [] if stats is not None: self.stats = stats @@ -149,6 +200,9 @@ class BaseTestCase(test.TestCase): self.context = context.get_admin_context() + self.flags(pci_passthrough_whitelist=[ + '{"vendor_id": "8086", "product_id": "0443"}', + '{"vendor_id": "8086", "product_id": "7891"}']) self.flags(use_local=True, group='conductor') self.conductor = self.start_service('conductor', manager=CONF.conductor.manager) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 3adeea1068..6ebe3fa226 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -8410,17 +8410,6 @@ class LibvirtConnTestCase(test.NoDBTestCase): self.assertEqual(actualvf, expect_vf) - def test_pci_device_assignable(self): - conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - self.stubs.Set(conn.dev_filter, 'device_assignable', lambda x: True) - - fake_dev = {'dev_type': 'type-PF'} - self.assertFalse(conn._pci_device_assignable(fake_dev)) - fake_dev = {'dev_type': 'type-VF'} - self.assertTrue(conn._pci_device_assignable(fake_dev)) - fake_dev = {'dev_type': 'type-PCI'} - self.assertTrue(conn._pci_device_assignable(fake_dev)) - def test_list_devices_not_supported(self): conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) @@ -8464,13 +8453,12 @@ class LibvirtConnTestCase(test.NoDBTestCase): libvirt_driver.LibvirtDriver._conn.nodeDeviceLookupByName =\ fake_nodeDeviceLookupByName conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - self.stubs.Set(conn.dev_filter, 'device_assignable', lambda x: x) actjson = conn._get_pci_passthrough_devices() expectvfs = [ { "dev_id": "pci_0000_04_00_3", - "address": "0000:04:10.3", + "address": "0000:04:00.3", "product_id": '1521', "vendor_id": '8086', "dev_type": 'type-PF', @@ -8486,10 +8474,11 @@ class LibvirtConnTestCase(test.NoDBTestCase): } ] - actctualvfs = jsonutils.loads(actjson) - for key in actctualvfs[0].keys(): - if key not in ['phys_function', 'virt_functions', 'label']: - self.assertEqual(actctualvfs[0][key], expectvfs[1][key]) + actualvfs = jsonutils.loads(actjson) + for dev in range(len(actualvfs)): + for key in actualvfs[dev].keys(): + if key not in ['phys_function', 'virt_functions', 'label']: + self.assertEqual(actualvfs[dev][key], expectvfs[dev][key]) def _fake_caps_numa_topology(self): topology = vconfig.LibvirtConfigCapsNUMATopology() diff --git a/nova/tests/unit/virt/xenapi/test_xenapi.py b/nova/tests/unit/virt/xenapi/test_xenapi.py index 67f1857a8d..c23eda76e1 100644 --- a/nova/tests/unit/virt/xenapi/test_xenapi.py +++ b/nova/tests/unit/virt/xenapi/test_xenapi.py @@ -2070,17 +2070,9 @@ class XenAPIHostTestCase(stubs.XenAPITestBase): stats = self.conn.host_state.get_host_stats(True) self.assertEqual(stats['vcpus_used'], 4) - def test_pci_passthrough_devices_whitelist(self): - # NOTE(guillaume-thouvenin): This pci whitelist will be used to - # match with _plugin_xenhost_get_pci_device_details method in fake.py. - white_list = '{"vendor_id":"10de", "product_id":"11bf"}' - self.flags(pci_passthrough_whitelist=[white_list]) + def test_pci_passthrough_devices(self): stats = self.conn.host_state.get_host_stats(False) - self.assertEqual(len(stats['pci_passthrough_devices']), 1) - - def test_pci_passthrough_devices_no_whitelist(self): - stats = self.conn.host_state.get_host_stats(False) - self.assertEqual(len(stats['pci_passthrough_devices']), 0) + self.assertEqual(len(stats['pci_passthrough_devices']), 2) def test_host_state_missing_sr(self): # Must trigger construction of 'host_state' property diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 5f4268230a..c97b656dcf 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -78,7 +78,6 @@ from nova.openstack.common import log as logging from nova.openstack.common import loopingcall from nova.pci import manager as pci_manager from nova.pci import utils as pci_utils -from nova.pci import whitelist as pci_whitelist from nova import rpc from nova import utils from nova import version @@ -392,8 +391,6 @@ class LibvirtDriver(driver.ComputeDriver): self.volume_drivers = driver.driver_dict_from_config( CONF.libvirt.volume_drivers, self) - self.dev_filter = pci_whitelist.get_pci_devices_filter() - self._disk_cachemode = None self.image_cache_manager = imagecache.ImageCacheManager() self.image_backend = imagebackend.Backend(CONF.use_cow_images) @@ -4556,11 +4553,6 @@ class LibvirtDriver(driver.ComputeDriver): device.update(_get_device_type(cfgdev)) return device - def _pci_device_assignable(self, device): - if device['dev_type'] == 'type-PF': - return False - return self.dev_filter.device_assignable(device) - def _get_pci_passthrough_devices(self): """Get host PCI devices information. @@ -4596,9 +4588,7 @@ class LibvirtDriver(driver.ComputeDriver): pci_info = [] for name in dev_names: - pci_dev = self._get_pcidev_info(name) - if self._pci_device_assignable(pci_dev): - pci_info.append(pci_dev) + pci_info.append(self._get_pcidev_info(name)) return jsonutils.dumps(pci_info) diff --git a/nova/virt/xenapi/host.py b/nova/virt/xenapi/host.py index 02215e80fc..36ff094b5b 100644 --- a/nova/virt/xenapi/host.py +++ b/nova/virt/xenapi/host.py @@ -32,7 +32,6 @@ from nova import exception from nova.i18n import _, _LE, _LI, _LW from nova import objects from nova.openstack.common import log as logging -from nova.pci import whitelist as pci_whitelist from nova.virt.xenapi import pool_states from nova.virt.xenapi import vm_utils @@ -147,7 +146,6 @@ class HostState(object): super(HostState, self).__init__() self._session = session self._stats = {} - self._pci_device_filter = pci_whitelist.get_pci_devices_filter() self.update_status() def _get_passthrough_devices(self): @@ -155,10 +153,9 @@ class HostState(object): We use a plugin to get the output of the lspci command runs on dom0. From this list we will extract pci devices that are using the pciback - kernel driver. Then we compare this list to the pci whitelist to get - a new list of pci devices that can be used for pci passthrough. + kernel driver. - :returns: a list of pci devices available for pci passthrough. + :returns: a list of pci devices on the node """ def _compile_hex(pattern): """Return a compiled regular expression pattern into which we have @@ -217,9 +214,7 @@ class HostState(object): for dev_string_info in pci_list: if "Driver:\tpciback" in dev_string_info: new_dev = _parse_pci_device_string(dev_string_info) - - if self._pci_device_filter.device_assignable(new_dev): - passthrough_devices.append(new_dev) + passthrough_devices.append(new_dev) return passthrough_devices