From 4e5ed1c48916c2a76e4d7a1e640187dd3e555dc8 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 17 Aug 2016 17:47:43 +0100 Subject: [PATCH] conf: Move PCI options to a PCI group This allows us to remove the final TODO. Change-Id: I886045ab4e6bdb8418fd1ccdcd811417ecb4ad4a Implements: bp centralize-config-options-ocata --- nova/compute/manager.py | 4 +- nova/conf/pci.py | 53 +++++++++++-------- nova/pci/devspec.py | 9 ++-- nova/pci/manager.py | 2 +- nova/pci/request.py | 10 ++-- nova/pci/stats.py | 2 +- nova/pci/whitelist.py | 2 +- nova/tests/unit/compute/test_compute_mgr.py | 7 +-- nova/tests/unit/pci/test_request.py | 34 ++++++------ nova/tests/unit/pci/test_stats.py | 6 +-- ...-config-to-pci-group-5648cc0f307f24f8.yaml | 9 ++++ 11 files changed, 81 insertions(+), 57 deletions(-) create mode 100644 releasenotes/notes/add-pci-config-to-pci-group-5648cc0f307f24f8.yaml diff --git a/nova/compute/manager.py b/nova/compute/manager.py index d9bd3ff42393..3c843b4a1b68 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1123,7 +1123,7 @@ class ComputeManager(manager.Manager): def init_host(self): """Initialization for a standalone compute service.""" - if CONF.pci_passthrough_whitelist: + if CONF.pci.passthrough_whitelist: # Simply loading the PCI passthrough whitelist will do a bunch of # validation that would otherwise wait until the PciDevTracker is # constructed when updating available resources for the compute @@ -1131,7 +1131,7 @@ class ComputeManager(manager.Manager): # So load up the whitelist when starting the compute service to # flush any invalid configuration early so we can kill the service # if the configuration is wrong. - whitelist.Whitelist(CONF.pci_passthrough_whitelist) + whitelist.Whitelist(CONF.pci.passthrough_whitelist) self.driver.init_host(host=self.host) context = nova.context.get_admin_context() diff --git a/nova/conf/pci.py b/nova/conf/pci.py index 0c389d7d7003..fb0795cb4236 100644 --- a/nova/conf/pci.py +++ b/nova/conf/pci.py @@ -16,9 +16,15 @@ from oslo_config import cfg +pci_group = cfg.OptGroup( + name='pci', + title='PCI passthrough options') + pci_opts = [ - cfg.MultiStrOpt('pci_alias', + cfg.MultiStrOpt('alias', default=[], + deprecated_name='pci_alias', + deprecated_group='DEFAULT', help=""" An alias for a PCI passthrough device requirement. @@ -29,7 +35,7 @@ Possible Values: * A list of JSON values which describe the aliases. For example: - pci_alias = { + alias = { "name": "QuickAssist", "product_id": "0443", "vendor_id": "8086", @@ -45,8 +51,10 @@ Possible Values: * "device_type": Type of PCI device. Valid values are: "type-PCI", "type-PF" and "type-VF". """), - cfg.MultiStrOpt('pci_passthrough_whitelist', + cfg.MultiStrOpt('passthrough_whitelist', default=[], + deprecated_name='pci_passthrough_whitelist', + deprecated_group='DEFAULT', help=""" White list of PCI devices available to VMs. @@ -76,38 +84,37 @@ Possible values: Valid examples are: - pci_passthrough_whitelist = {"devname":"eth0", - "physical_network":"physnet"} - pci_passthrough_whitelist = {"address":"*:0a:00.*"} - pci_passthrough_whitelist = {"address":":0a:00.", - "physical_network":"physnet1"} - pci_passthrough_whitelist = {"vendor_id":"1137", - "product_id":"0071"} - pci_passthrough_whitelist = {"vendor_id":"1137", - "product_id":"0071", - "address": "0000:0a:00.1", - "physical_network":"physnet1"} + passthrough_whitelist = {"devname":"eth0", + "physical_network":"physnet"} + passthrough_whitelist = {"address":"*:0a:00.*"} + passthrough_whitelist = {"address":":0a:00.", + "physical_network":"physnet1"} + passthrough_whitelist = {"vendor_id":"1137", + "product_id":"0071"} + passthrough_whitelist = {"vendor_id":"1137", + "product_id":"0071", + "address": "0000:0a:00.1", + "physical_network":"physnet1"} The following are invalid, as they specify mutually exclusive options: - pci_passthrough_whitelist = {"devname":"eth0", - "physical_network":"physnet", - "address":"*:0a:00.*"} + passthrough_whitelist = {"devname":"eth0", + "physical_network":"physnet", + "address":"*:0a:00.*"} * A JSON list of JSON dictionaries corresponding to the above format. For example: - pci_passthrough_whitelist = [{"product_id":"0001", "vendor_id":"8086"}, - {"product_id":"0002", "vendor_id":"8086"}] + passthrough_whitelist = [{"product_id":"0001", "vendor_id":"8086"}, + {"product_id":"0002", "vendor_id":"8086"}] """) ] def register_opts(conf): - conf.register_opts(pci_opts) + conf.register_group(pci_group) + conf.register_opts(pci_opts, group=pci_group) def list_opts(): - # TODO(sfinucan): This should be moved into the PCI group and - # oslo_config.cfg.OptGroup used - return {'DEFAULT': pci_opts} + return {pci_group: pci_opts} diff --git a/nova/pci/devspec.py b/nova/pci/devspec.py index 78e3a9a9f1f0..8346367596b8 100644 --- a/nova/pci/devspec.py +++ b/nova/pci/devspec.py @@ -45,13 +45,14 @@ def get_pci_dev_info(pci_obj, property, max, hex_value): class PciAddress(object): """Manages the address fields of the whitelist. - This class checks the address fields of the pci_passthrough_whitelist + This class checks the address fields of the pci.passthrough_whitelist configuration option, validating the address fields. Example config are: - | pci_passthrough_whitelist = {"address":"*:0a:00.*", - | "physical_network":"physnet1"} - | pci_passthrough_whitelist = {"vendor_id":"1137","product_id":"0071"} + | [pci] + | passthrough_whitelist = {"address":"*:0a:00.*", + | "physical_network":"physnet1"} + | passthrough_whitelist = {"vendor_id":"1137","product_id":"0071"} This function class will validate the address fields, check for wildcards, and insert wildcards where the field is left blank. diff --git a/nova/pci/manager.py b/nova/pci/manager.py index 5e8d3d97d3ec..2e83eff2f0c7 100644 --- a/nova/pci/manager.py +++ b/nova/pci/manager.py @@ -64,7 +64,7 @@ class PciDevTracker(object): super(PciDevTracker, self).__init__() self.stale = {} self.node_id = node_id - self.dev_filter = whitelist.Whitelist(CONF.pci_passthrough_whitelist) + self.dev_filter = whitelist.Whitelist(CONF.pci.passthrough_whitelist) self.stats = stats.PciDeviceStats(dev_filter=self.dev_filter) self._context = context if node_id: diff --git a/nova/pci/request.py b/nova/pci/request.py index 3712f5877ec3..3c903969b272 100644 --- a/nova/pci/request.py +++ b/nova/pci/request.py @@ -15,7 +15,8 @@ """ Example of a PCI alias:: - | pci_alias = '{ + | [pci] + | alias = '{ | "name": "QuicAssist", | "product_id": "0443", | "vendor_id": "8086", @@ -24,7 +25,8 @@ Aliases with the same name and the same device_type are OR operation:: - | pci_alias = '{ + | [pci] + | alias = '{ | "name": "QuicAssist", | "product_id": "0442", | "vendor_id": "8086", @@ -97,7 +99,7 @@ _ALIAS_SCHEMA = { def _get_alias_from_config(): """Parse and validate PCI aliases from the nova config.""" - jaliases = CONF.pci_alias + jaliases = CONF.pci.alias aliases = {} # map alias name to alias spec list try: for jsonspecs in jaliases: @@ -150,7 +152,7 @@ def get_pci_requests_from_flavor(flavor): describes the flavor's pci requests, the key is 'pci_passthrough:alias' and the value has format 'alias_name_x:count, alias_name_y:count, ... '. The alias_name is - defined in 'pci_alias' configurations. + defined in 'pci.alias' configurations. The flavor's requirement is translated into pci requests list, each entry in the list is a dictionary. The dictionary has diff --git a/nova/pci/stats.py b/nova/pci/stats.py index 3ca70cfd3378..82d366dcc40e 100644 --- a/nova/pci/stats.py +++ b/nova/pci/stats.py @@ -65,7 +65,7 @@ class PciDeviceStats(object): for pci_pool in stats] if stats else [] self.pools.sort(key=lambda item: len(item)) self.dev_filter = dev_filter or whitelist.Whitelist( - CONF.pci_passthrough_whitelist) + CONF.pci.passthrough_whitelist) def _equal_properties(self, dev, entry, matching_keys): return all(dev.get(prop) == entry.get(prop) diff --git a/nova/pci/whitelist.py b/nova/pci/whitelist.py index 9bc537dcdb33..5c3549064ad5 100644 --- a/nova/pci/whitelist.py +++ b/nova/pci/whitelist.py @@ -96,5 +96,5 @@ class Whitelist(object): def get_pci_device_devspec(pci_dev): - dev_filter = Whitelist(CONF.pci_passthrough_whitelist) + dev_filter = Whitelist(CONF.pci.passthrough_whitelist) return dev_filter.get_devspec(pci_dev) diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 2936a7e97541..b05f51cbf2a3 100755 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -2730,11 +2730,12 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): self.assertIn('appears to not be owned by this host', msg[0][0]) def test_init_host_pci_passthrough_whitelist_validation_failure(self): - # Tests that we fail init_host if there is a pci_passthrough_whitelist + # Tests that we fail init_host if there is a pci.passthrough_whitelist # configured incorrectly. - self.flags(pci_passthrough_whitelist=[ + self.flags(passthrough_whitelist=[ # it's invalid to specify both in the same devspec - jsonutils.dumps({'address': 'foo', 'devname': 'bar'})]) + jsonutils.dumps({'address': 'foo', 'devname': 'bar'})], + group='pci') self.assertRaises(exception.PciDeviceInvalidDeviceName, self.compute.init_host) diff --git a/nova/tests/unit/pci/test_request.py b/nova/tests/unit/pci/test_request.py index 8424c4445cd6..b3850fc77c20 100644 --- a/nova/tests/unit/pci/test_request.py +++ b/nova/tests/unit/pci/test_request.py @@ -55,7 +55,7 @@ _fake_alias3 = """{ class AliasTestCase(test.NoDBTestCase): def test_good_alias(self): - self.flags(pci_alias=[_fake_alias1]) + self.flags(alias=[_fake_alias1], group='pci') als = request._get_alias_from_config() self.assertIsInstance(als['QuicAssist'], list) expect_dict = { @@ -67,7 +67,7 @@ class AliasTestCase(test.NoDBTestCase): self.assertEqual(expect_dict, als['QuicAssist'][0]) def test_multispec_alias(self): - self.flags(pci_alias=[_fake_alias1, _fake_alias11]) + self.flags(alias=[_fake_alias1, _fake_alias11], group='pci') als = request._get_alias_from_config() self.assertIsInstance(als['QuicAssist'], list) expect_dict1 = { @@ -87,48 +87,51 @@ class AliasTestCase(test.NoDBTestCase): self.assertEqual(expect_dict2, als['QuicAssist'][1]) def test_wrong_type_aliase(self): - self.flags(pci_alias=[_fake_alias2]) + self.flags(alias=[_fake_alias2], group='pci') self.assertRaises(exception.PciInvalidAlias, request._get_alias_from_config) def test_wrong_product_id_aliase(self): - self.flags(pci_alias=[ + self.flags(alias=[ """{ "name": "xxx", "capability_type": "pci", "product_id": "g111", "vendor_id": "1111", "device_type": "NIC" - }"""]) + }"""], + group='pci') self.assertRaises(exception.PciInvalidAlias, request._get_alias_from_config) def test_wrong_vendor_id_aliase(self): - self.flags(pci_alias=[ + self.flags(alias=[ """{ "name": "xxx", "capability_type": "pci", "product_id": "1111", "vendor_id": "0xg111", "device_type": "NIC" - }"""]) + }"""], + group='pci') self.assertRaises(exception.PciInvalidAlias, request._get_alias_from_config) def test_wrong_cap_type_aliase(self): - self.flags(pci_alias=[ + self.flags(alias=[ """{ "name": "xxx", "capability_type": "usb", "product_id": "1111", "vendor_id": "8086", "device_type": "NIC" - }"""]) + }"""], + group='pci') self.assertRaises(exception.PciInvalidAlias, request._get_alias_from_config) def test_dup_aliase(self): - self.flags(pci_alias=[ + self.flags(alias=[ """{ "name": "xxx", "capability_type": "pci", @@ -142,7 +145,8 @@ class AliasTestCase(test.NoDBTestCase): "product_id": "1111", "vendor_id": "8086", "device_type": "type-PCI" - }"""]) + }"""], + group='pci') self.assertRaises( exception.PciInvalidAlias, request._get_alias_from_config) @@ -155,7 +159,7 @@ class AliasTestCase(test.NoDBTestCase): self.assertEqual(exp['spec'], real.spec) def test_aliase_2_request(self): - self.flags(pci_alias=[_fake_alias1, _fake_alias3]) + self.flags(alias=[_fake_alias1, _fake_alias3], group='pci') expect_request = [ {'count': 3, 'spec': [{'vendor_id': '8086', 'product_id': '4443', @@ -175,13 +179,13 @@ class AliasTestCase(test.NoDBTestCase): self._verify_result(expect_request, requests) def test_aliase_2_request_invalid(self): - self.flags(pci_alias=[_fake_alias1, _fake_alias3]) + self.flags(alias=[_fake_alias1, _fake_alias3], group='pci') self.assertRaises(exception.PciRequestAliasNotDefined, request._translate_alias_to_requests, "QuicAssistX : 3") def test_get_pci_requests_from_flavor(self): - self.flags(pci_alias=[_fake_alias1, _fake_alias3]) + self.flags(alias=[_fake_alias1, _fake_alias3], group='pci') expect_request = [ {'count': 3, 'spec': [{'vendor_id': '8086', 'product_id': '4443', @@ -203,7 +207,7 @@ class AliasTestCase(test.NoDBTestCase): self._verify_result(expect_request, requests.requests) def test_get_pci_requests_from_flavor_no_extra_spec(self): - self.flags(pci_alias=[_fake_alias1, _fake_alias3]) + self.flags(alias=[_fake_alias1, _fake_alias3], group='pci') flavor = {} requests = request.get_pci_requests_from_flavor(flavor) self.assertEqual([], requests.requests) diff --git a/nova/tests/unit/pci/test_stats.py b/nova/tests/unit/pci/test_stats.py index 2e00ddc9e57e..1e4d6c80cb56 100644 --- a/nova/tests/unit/pci/test_stats.py +++ b/nova/tests/unit/pci/test_stats.py @@ -212,7 +212,7 @@ class PciDeviceStatsTestCase(test.NoDBTestCase): 'nova.pci.whitelist.Whitelist._parse_white_list_from_config') def test_white_list_parsing(self, mock_whitelist_parse): white_list = '{"product_id":"0001", "vendor_id":"8086"}' - CONF.set_override('pci_passthrough_whitelist', white_list) + CONF.set_override('passthrough_whitelist', white_list, 'pci') pci_stats = stats.PciDeviceStats() pci_stats.add_device(self.fake_dev_2) pci_stats.remove_device(self.fake_dev_2) @@ -226,7 +226,7 @@ class PciDeviceStatsWithTagsTestCase(test.NoDBTestCase): white_list = ['{"vendor_id":"1137","product_id":"0071",' '"address":"*:0a:00.*","physical_network":"physnet1"}', '{"vendor_id":"1137","product_id":"0072"}'] - self.flags(pci_passthrough_whitelist=white_list) + self.flags(passthrough_whitelist=white_list, group='pci') dev_filter = whitelist.Whitelist(white_list) self.pci_stats = stats.PciDeviceStats(dev_filter=dev_filter) @@ -356,7 +356,7 @@ class PciDeviceVFPFStatsTestCase(test.NoDBTestCase): super(PciDeviceVFPFStatsTestCase, self).setUp() white_list = ['{"vendor_id":"8086","product_id":"1528"}', '{"vendor_id":"8086","product_id":"1515"}'] - self.flags(pci_passthrough_whitelist=white_list) + self.flags(passthrough_whitelist=white_list, group='pci') self.pci_stats = stats.PciDeviceStats() def _create_pci_devices(self, vf_product_id=1515, pf_product_id=1528): diff --git a/releasenotes/notes/add-pci-config-to-pci-group-5648cc0f307f24f8.yaml b/releasenotes/notes/add-pci-config-to-pci-group-5648cc0f307f24f8.yaml new file mode 100644 index 000000000000..4efe5012bb0b --- /dev/null +++ b/releasenotes/notes/add-pci-config-to-pci-group-5648cc0f307f24f8.yaml @@ -0,0 +1,9 @@ +--- +upgrade: + - | + All pci configuration options have been added to the 'pci' group. They + should no longer be included in the 'DEFAULT' group. These options are as + below: + + - pci_alias (now pci.alias) + - pci_passthrough_whitelist (now pci.passthrough_whitelist)