openstack/nova
Code Issues Proposed changes

pci: Move whitelist filtering inside PCI tracker

Browse Source 
Having the whitelist processing/filtering live inside the resource
tracker was leaking implementation details about the PCI device pool
handling out of the PCI code itself. This commit places the filtering of
devices via the whitelist parsing into the PCI device tracker object
itself, and removes it from the resource tracker.

This also changes the expected function return signature of the
nova.pci.whitelist.PciDeviceWhitelistTracker.device_assignable() method
to return a bool instead of None or a DeviceSpec object. No callers were
actually using the None or DeviceSpec object returned. All callers were
expecting a bool return.

Change-Id: Ieb6debc6a8857c9ab817d0f823a7519f322368ab
This commit is contained in:
Jay Pipes
2015-06-15 07:15:04 -04:00
parent a8e51e0e74
commit bc10ba2a69
6 changed files with 50 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
nova/compute/resource_tracker.py
View File

@@ -35,7 +35,6 @@ from nova.i18n import _, _LI, _LW
from nova import objects
from nova.objects import base as obj_base
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
@@ -72,7 +71,6 @@ 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)
@@ -420,20 +418,13 @@ class ResourceTracker(object):
return
if 'pci_passthrough_devices' in resources:
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)
# TODO(jaypipes): Move this into _init_compute_node()
if not self.pci_tracker:
n_id = self.compute_node['id'] if self.compute_node else None
self.pci_tracker = pci_manager.PciDevTracker(context,
node_id=n_id)
self.pci_tracker.set_hvdevs(devs)
dev_json = resources.pop('pci_passthrough_devices')
self.pci_tracker.update_devices_from_hypervisor_resources(dev_json)
# Grab all instances assigned to this node:
instances = objects.InstanceList.get_by_host_and_node(

20
nova/pci/manager.py
View File

@@ -17,6 +17,7 @@
import collections
from oslo_log import log as logging
from oslo_serialization import jsonutils
from nova import exception
from nova.i18n import _LW
@@ -24,6 +25,7 @@ from nova import objects
from nova.objects import fields
from nova.pci import device
from nova.pci import stats
from nova.pci import whitelist
from nova.virt import hardware
LOG = logging.getLogger(__name__)
@@ -53,6 +55,7 @@ class PciDevTracker(object):
self.stale = {}
self.node_id = node_id
self.stats = stats.PciDeviceStats()
self.dev_filter = whitelist.get_pci_devices_filter()
if node_id:
self.pci_devs = list(
objects.PciDeviceList.get_by_compute_node(context, node_id))
@@ -89,7 +92,7 @@ class PciDevTracker(object):
def pci_stats(self):
return self.stats
def set_hvdevs(self, devices):
def update_devices_from_hypervisor_resources(self, devices_json):
"""Sync the pci device tracker with hypervisor information.
To support pci device hot plug, we sync with the hypervisor
@@ -100,8 +103,23 @@ class PciDevTracker(object):
but possibly the hypervisor has no such guarantee. The best
we can do is to give a warning if a device is changed
or removed while assigned.
:param devices_json: The JSON-ified string of device information
that is returned from the virt driver's
get_available_resource() call in the
pci_passthrough_devices key.
"""
devices = []
for dev in jsonutils.loads(devices_json):
if dev.get('dev_type') == fields.PciDeviceType.SRIOV_PF:
continue
if self.dev_filter.device_assignable(dev):
devices.append(dev)
self._set_hvdevs(devices)
def _set_hvdevs(self, devices):
exist_addrs = set([dev.address for dev in self.pci_devs])
new_addrs = set([dev['address'] for dev in devices])

3
nova/pci/whitelist.py
View File

@@ -97,7 +97,8 @@ class PciHostDevicesWhiteList(object):
"""
for spec in self.specs:
if spec.match(dev):
return spec
return True
return False
def get_devspec(self, pci_dev):
for spec in self.specs:

12
nova/tests/unit/compute/test_claims.py
View File

@@ -200,7 +200,7 @@ class ClaimTestCase(test.NoDBTestCase):
'numa_node': 0,
'status': 'available'}
self.tracker.new_pci_tracker()
self.tracker.pci_tracker.set_hvdevs([dev_dict])
self.tracker.pci_tracker._set_hvdevs([dev_dict])
claim = self._claim()
request = objects.InstancePCIRequest(count=1,
spec=[{'vendor_id': 'v', 'product_id': 'p'}])
@@ -218,7 +218,7 @@ class ClaimTestCase(test.NoDBTestCase):
'numa_node': 1,
'status': 'available'}
self.tracker.new_pci_tracker()
self.tracker.pci_tracker.set_hvdevs([dev_dict])
self.tracker.pci_tracker._set_hvdevs([dev_dict])
claim = self._claim()
request = objects.InstancePCIRequest(count=1,
spec=[{'vendor_id': 'v', 'product_id': 'p'}])
@@ -236,7 +236,7 @@ class ClaimTestCase(test.NoDBTestCase):
'numa_node': 0,
'status': 'available'}
self.tracker.new_pci_tracker()
self.tracker.pci_tracker.set_hvdevs([dev_dict])
self.tracker.pci_tracker._set_hvdevs([dev_dict])
claim = self._claim()
self.assertIsNone(claim._test_pci())
@@ -281,7 +281,7 @@ class ClaimTestCase(test.NoDBTestCase):
'numa_node': 1,
'status': 'available'}
self.tracker.new_pci_tracker()
self.tracker.pci_tracker.set_hvdevs([dev_dict])
self.tracker.pci_tracker._set_hvdevs([dev_dict])
request = objects.InstancePCIRequest(count=1,
spec=[{'vendor_id': 'v', 'product_id': 'p'}])
mock_get.return_value = objects.InstancePCIRequests(
@@ -310,7 +310,7 @@ class ClaimTestCase(test.NoDBTestCase):
'numa_node': 2,
'status': 'available'}
self.tracker.new_pci_tracker()
self.tracker.pci_tracker.set_hvdevs([dev_dict, dev_dict2])
self.tracker.pci_tracker._set_hvdevs([dev_dict, dev_dict2])
request = objects.InstancePCIRequest(count=2,
spec=[{'vendor_id': 'v', 'product_id': 'p'}])
@@ -335,7 +335,7 @@ class ClaimTestCase(test.NoDBTestCase):
'numa_node': None,
'status': 'available'}
self.tracker.new_pci_tracker()
self.tracker.pci_tracker.set_hvdevs([dev_dict])
self.tracker.pci_tracker._set_hvdevs([dev_dict])
request = objects.InstancePCIRequest(count=1,
spec=[{'vendor_id': 'v', 'product_id': 'p'}])

22
nova/tests/unit/pci/test_manager.py
View File

@@ -16,6 +16,7 @@
import copy
import mock
from oslo_serialization import jsonutils
import nova
from nova.compute import vm_states
@@ -138,11 +139,20 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
self.tracker = manager.PciDevTracker(self.fake_context, node_id=1)
mock_get_cn.assert_called_once_with(self.fake_context, 1)
@mock.patch('nova.pci.whitelist.PciHostDevicesWhiteList.device_assignable',
return_value=True)
def test_update_devices_from_hypervisor_resources(self, _mock_dev_assign):
fake_pci_devs = [copy.deepcopy(fake_pci), copy.deepcopy(fake_pci_2)]
fake_pci_devs_json = jsonutils.dumps(fake_pci_devs)
tracker = manager.PciDevTracker(self.fake_context)
tracker.update_devices_from_hypervisor_resources(fake_pci_devs_json)
self.assertEqual(2, len(tracker.pci_devs))
def test_set_hvdev_new_dev(self):
fake_pci_3 = dict(fake_pci, address='0000:00:00.4', vendor_id='v2')
fake_pci_devs = [copy.deepcopy(fake_pci), copy.deepcopy(fake_pci_1),
copy.deepcopy(fake_pci_2), copy.deepcopy(fake_pci_3)]
self.tracker.set_hvdevs(fake_pci_devs)
self.tracker._set_hvdevs(fake_pci_devs)
self.assertEqual(len(self.tracker.pci_devs), 4)
self.assertEqual(set([dev.address for
dev in self.tracker.pci_devs]),
@@ -156,13 +166,13 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
fake_pci_v2 = dict(fake_pci, address='0000:00:00.2', vendor_id='v1')
fake_pci_devs = [copy.deepcopy(fake_pci), copy.deepcopy(fake_pci_2),
copy.deepcopy(fake_pci_v2)]
self.tracker.set_hvdevs(fake_pci_devs)
self.tracker._set_hvdevs(fake_pci_devs)
self.assertEqual(set([dev.vendor_id for
dev in self.tracker.pci_devs]),
set(['v', 'v1']))
def test_set_hvdev_remove(self):
self.tracker.set_hvdevs([fake_pci])
self.tracker._set_hvdevs([fake_pci])
self.assertEqual(len([dev for dev in self.tracker.pci_devs
if dev.status == 'removed']),
2)
@@ -175,7 +185,7 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
fake_pci_3 = dict(fake_pci, address='0000:00:00.2', vendor_id='v2')
fake_pci_devs = [copy.deepcopy(fake_pci), copy.deepcopy(fake_pci_2),
copy.deepcopy(fake_pci_3)]
self.tracker.set_hvdevs(fake_pci_devs)
self.tracker._set_hvdevs(fake_pci_devs)
self.assertEqual(len(self.tracker.stale), 1)
self.assertEqual(self.tracker.stale['0000:00:00.2']['vendor_id'], 'v2')
@@ -210,7 +220,7 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
fake_devs_numa = copy.deepcopy(fake_db_devs)
fake_devs_numa.append(fake_db_dev_3)
self.tracker = manager.PciDevTracker(1)
self.tracker.set_hvdevs(fake_devs_numa)
self.tracker._set_hvdevs(fake_devs_numa)
pci_requests = copy.deepcopy(fake_pci_requests)[:1]
pci_requests[0]['count'] = 2
self._create_pci_requests_object(mock_get, pci_requests)
@@ -269,7 +279,7 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
fake_pci_v3 = dict(fake_pci, address='0000:00:00.2', vendor_id='v3')
fake_pci_devs = [copy.deepcopy(fake_pci), copy.deepcopy(fake_pci_2),
copy.deepcopy(fake_pci_v3)]
self.tracker.set_hvdevs(fake_pci_devs)
self.tracker._set_hvdevs(fake_pci_devs)
self.update_called = 0
self.tracker.save(self.fake_context)
self.assertEqual(self.update_called, 3)

8
nova/tests/unit/pci/test_whitelist.py
View File

@@ -52,21 +52,21 @@ class PciHostDevicesWhiteListTestCase(test.NoDBTestCase):
def test_device_assignable(self):
white_list = '{"product_id":"0001", "vendor_id":"8086"}'
parsed = whitelist.PciHostDevicesWhiteList([white_list])
self.assertIsNotNone(parsed.device_assignable(dev_dict))
self.assertTrue(parsed.device_assignable(dev_dict))
def test_device_assignable_multiple(self):
white_list_1 = '{"product_id":"0001", "vendor_id":"8086"}'
white_list_2 = '{"product_id":"0002", "vendor_id":"8087"}'
parsed = whitelist.PciHostDevicesWhiteList(
[white_list_1, white_list_2])
self.assertIsNotNone(parsed.device_assignable(dev_dict))
self.assertTrue(parsed.device_assignable(dev_dict))
dev_dict1 = dev_dict.copy()
dev_dict1['vendor_id'] = '8087'
dev_dict1['product_id'] = '0002'
self.assertIsNotNone(parsed.device_assignable(dev_dict1))
self.assertTrue(parsed.device_assignable(dev_dict1))
def test_get_pci_devices_filter(self):
white_list_1 = '{"product_id":"0001", "vendor_id":"8086"}'
self.flags(pci_passthrough_whitelist=[white_list_1])
pci_filter = whitelist.get_pci_devices_filter()
self.assertIsNotNone(pci_filter.device_assignable(dev_dict))
self.assertTrue(pci_filter.device_assignable(dev_dict))