diff --git a/nova/compute/api.py b/nova/compute/api.py index 5b91add65b06..17b5187c97d8 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1180,7 +1180,7 @@ class API(base.Base): block_device_mapping=block_device_mapping, tags=tags) - return (instances, reservation_id) + return instances, reservation_id @staticmethod def _cleanup_build_artifacts(instances, instances_to_build): diff --git a/nova/compute/claims.py b/nova/compute/claims.py index 1fad801ca7ce..099cb761f623 100644 --- a/nova/compute/claims.py +++ b/nova/compute/claims.py @@ -196,7 +196,7 @@ class Claim(NopClaim): if pci_requests.requests: stats = self.tracker.pci_tracker.stats if not stats.support_requests(pci_requests.requests): - return _('Claim pci failed.') + return _('Claim pci failed') def _test_numa_topology(self, resources, limit): host_topology = (resources.numa_topology @@ -219,12 +219,12 @@ class Claim(NopClaim): if requested_topology and not instance_topology: if pci_requests.requests: - return (_("Requested instance NUMA topology together with" - " requested PCI devices cannot fit the given" - " host NUMA topology")) + return (_("Requested instance NUMA topology together with " + "requested PCI devices cannot fit the given " + "host NUMA topology")) else: return (_("Requested instance NUMA topology cannot fit " - "the given host NUMA topology")) + "the given host NUMA topology")) elif instance_topology: self.claimed_numa_topology = instance_topology diff --git a/nova/compute/manager.py b/nova/compute/manager.py index c9906a5e2aa3..ccce48a29f3b 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1109,8 +1109,8 @@ class ComputeManager(manager.Manager): # if the configuration is wrong. whitelist.Whitelist(CONF.pci.passthrough_whitelist) - # NOTE(sbauza): We want the compute node to hard fail if it can't be - # able to provide its resources to the placement API, or it would not + # NOTE(sbauza): We want the compute node to hard fail if it won't be + # able to provide its resources to the placement API, or it will not # be able to be eligible as a destination. if CONF.placement.os_region_name is None: raise exception.PlacementNotConfigured() diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index 41a8047f02a4..3ed81fc77eb1 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -193,7 +193,7 @@ class ResourceTracker(object): overhead = self.driver.estimate_instance_overhead(instance) LOG.debug("Memory overhead for %(flavor)d MB instance; %(overhead)d " "MB", {'flavor': instance.flavor.memory_mb, - 'overhead': overhead['memory_mb']}) + 'overhead': overhead['memory_mb']}) LOG.debug("Disk overhead for %(flavor)d GB instance; %(overhead)d " "GB", {'flavor': instance.flavor.root_gb, 'overhead': overhead.get('disk_gb', 0)}) diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 523ea5739da7..f1ba89b142d5 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -1453,10 +1453,10 @@ class PciDevice(BASE, NovaBase, models.SoftDeleteMixin): parent_addr = Column(String(12), nullable=True) instance = orm.relationship(Instance, backref="pci_devices", - foreign_keys=instance_uuid, - primaryjoin='and_(' - 'PciDevice.instance_uuid == Instance.uuid,' - 'PciDevice.deleted == 0)') + foreign_keys=instance_uuid, + primaryjoin='and_(' + 'PciDevice.instance_uuid == Instance.uuid,' + 'PciDevice.deleted == 0)') class Tag(BASE, models.ModelBase): diff --git a/nova/exception.py b/nova/exception.py index 9b1cd9a16a6e..972c148590f6 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -1713,7 +1713,7 @@ class PciRequestAliasNotDefined(NovaException): class PciConfigInvalidWhitelist(Invalid): - msg_fmt = _("Invalid PCI devices Whitelist config %(reason)s") + msg_fmt = _("Invalid PCI devices Whitelist config: %(reason)s") # Cannot be templated, msg needs to be constructed when raised. diff --git a/nova/objects/pci_device.py b/nova/objects/pci_device.py index 86dfc84a3897..7fa96983bea7 100644 --- a/nova/objects/pci_device.py +++ b/nova/objects/pci_device.py @@ -303,9 +303,9 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject): parent.status = fields.PciDeviceStatus.UNCLAIMABLE else: LOG.debug('Physical function addr: %(pf_addr)s parent of ' - 'VF addr: %(vf_addr)s was not found', - {'pf_addr': self.parent_addr, - 'vf_addr': self.address}) + 'VF addr: %(vf_addr)s was not found', + {'pf_addr': self.parent_addr, + 'vf_addr': self.address}) self.status = fields.PciDeviceStatus.CLAIMED self.instance_uuid = instance_uuid diff --git a/nova/pci/devspec.py b/nova/pci/devspec.py index d330bc794a57..1aee64821628 100644 --- a/nova/pci/devspec.py +++ b/nova/pci/devspec.py @@ -31,22 +31,6 @@ ANY = '*' REGEX_ANY = '.*' -def get_pci_dev_info(pci_obj, property, max, hex_value): - a = getattr(pci_obj, property) - if a == ANY: - return - try: - v = int(a, 16) - except ValueError: - raise exception.PciConfigInvalidWhitelist( - reason = "invalid %s %s" % (property, a)) - if v > max: - raise exception.PciConfigInvalidWhitelist( - reason=_("invalid %(property)s %(attr)s") % - {'property': property, 'attr': a}) - setattr(pci_obj, property, hex_value % v) - - @six.add_metaclass(abc.ABCMeta) class PciAddressSpec(object): """Abstract class for all PCI address spec styles @@ -65,6 +49,23 @@ class PciAddressSpec(object): all(c in string.hexdigits for c in self.slot), all(c in string.hexdigits for c in self.func)]) + def _set_pci_dev_info(self, prop, maxval, hex_value): + a = getattr(self, prop) + if a == ANY: + return + try: + v = int(a, 16) + except ValueError: + raise exception.PciConfigInvalidWhitelist( + reason=_("property %(property)s ('%(attr)s') does not parse " + "as a hex number.") % {'property': prop, 'attr': a}) + if v > maxval: + raise exception.PciConfigInvalidWhitelist( + reason=_("property %(property)s (%(attr)s) is greater than " + "the maximum allowable value (%(max)X).") % + {'property': prop, 'attr': a, 'max': maxval}) + setattr(self, prop, hex_value % v) + class PhysicalPciAddress(PciAddressSpec): """Manages the address fields for a fully-qualified PCI address. @@ -82,10 +83,10 @@ class PhysicalPciAddress(PciAddressSpec): else: self.domain, self.bus, self.slot, self.func = ( utils.get_pci_address_fields(pci_addr)) - get_pci_dev_info(self, 'func', MAX_FUNC, '%1x') - get_pci_dev_info(self, 'domain', MAX_DOMAIN, '%04x') - get_pci_dev_info(self, 'bus', MAX_BUS, '%02x') - get_pci_dev_info(self, 'slot', MAX_SLOT, '%02x') + self._set_pci_dev_info('func', MAX_FUNC, '%1x') + self._set_pci_dev_info('domain', MAX_DOMAIN, '%04x') + self._set_pci_dev_info('bus', MAX_BUS, '%02x') + self._set_pci_dev_info('slot', MAX_SLOT, '%02x') except (KeyError, ValueError): raise exception.PciDeviceWrongAddressFormat(address=pci_addr) @@ -115,7 +116,7 @@ class PciAddressGlobSpec(PciAddressSpec): dbs, sep, func = pci_addr.partition('.') if func: self.func = func.strip() - get_pci_dev_info(self, 'func', MAX_FUNC, '%01x') + self._set_pci_dev_info('func', MAX_FUNC, '%01x') if dbs: dbs_fields = dbs.split(':') if len(dbs_fields) > 3: @@ -127,9 +128,9 @@ class PciAddressGlobSpec(PciAddressSpec): dbs_all.extend(dbs_fields) dbs_checked = [s.strip() or ANY for s in dbs_all] self.domain, self.bus, self.slot = dbs_checked - get_pci_dev_info(self, 'domain', MAX_DOMAIN, '%04x') - get_pci_dev_info(self, 'bus', MAX_BUS, '%02x') - get_pci_dev_info(self, 'slot', MAX_SLOT, '%02x') + self._set_pci_dev_info('domain', MAX_DOMAIN, '%04x') + self._set_pci_dev_info('bus', MAX_BUS, '%02x') + self._set_pci_dev_info('slot', MAX_SLOT, '%02x') def match(self, phys_pci_addr): conditions = [ @@ -176,7 +177,7 @@ class WhitelistPciAddress(object): This class checks the address fields of the pci.passthrough_whitelist configuration option, validating the address fields. - Example config are: + Example configs: | [pci] | passthrough_whitelist = {"address":"*:0a:00.*", @@ -226,8 +227,8 @@ class WhitelistPciAddress(object): # Try to match on the parent PCI address if the PciDeviceSpec is a # PF (sriov is available) and the device to match is a VF. This - # makes possible to specify the PCI address of a PF in the - # pci_passthrough_whitelist to match any of it's VFs PCI devices. + # makes it possible to specify the PCI address of a PF in the + # pci.passthrough_whitelist to match any of its VFs' PCI addresses. if self.is_physical_function and pci_phys_addr: pci_phys_addr_obj = PhysicalPciAddress(pci_phys_addr) if self.pci_address_spec.match(pci_phys_addr_obj): @@ -238,7 +239,7 @@ class WhitelistPciAddress(object): return self.pci_address_spec.match(pci_addr_obj) -class PciDeviceSpec(object): +class PciDeviceSpec(PciAddressSpec): def __init__(self, dev_spec): self.tags = dev_spec self._init_dev_details() @@ -253,8 +254,8 @@ class PciDeviceSpec(object): self.dev_name = self.tags.pop("devname", None) self.vendor_id = self.vendor_id.strip() - get_pci_dev_info(self, 'vendor_id', MAX_VENDOR_ID, '%04x') - get_pci_dev_info(self, 'product_id', MAX_PRODUCT_ID, '%04x') + self._set_pci_dev_info('vendor_id', MAX_VENDOR_ID, '%04x') + self._set_pci_dev_info('product_id', MAX_PRODUCT_ID, '%04x') if self.address and self.dev_name: raise exception.PciDeviceInvalidDeviceName() @@ -281,9 +282,9 @@ class PciDeviceSpec(object): def match_pci_obj(self, pci_obj): return self.match({'vendor_id': pci_obj.vendor_id, - 'product_id': pci_obj.product_id, - 'address': pci_obj.address, - 'parent_addr': pci_obj.parent_addr}) + 'product_id': pci_obj.product_id, + 'address': pci_obj.address, + 'parent_addr': pci_obj.parent_addr}) def get_tags(self): return self.tags diff --git a/nova/pci/manager.py b/nova/pci/manager.py index 8267d8da1150..ad015db5109b 100644 --- a/nova/pci/manager.py +++ b/nova/pci/manager.py @@ -38,14 +38,13 @@ class PciDevTracker(object): It's called by compute node resource tracker to allocate and free devices to/from instances, and to update the available pci passthrough - devices information from hypervisor periodically. + device information from the hypervisor periodically. - `pci_devs` attribute of this class is the in-memory "master copy" of all - devices on each compute host, and all data changes that happen when - claiming/allocating/freeing - devices HAVE TO be made against instances contained in `pci_devs` list, - because they are periodically flushed to the DB when the save() - method is called. + The `pci_devs` attribute of this class is the in-memory "master copy" of + all devices on each compute host, and all data changes that happen when + claiming/allocating/freeing devices HAVE TO be made against instances + contained in `pci_devs` list, because they are periodically flushed to the + DB when the save() method is called. It is unsafe to fetch PciDevice objects elsewhere in the code for update purposes as those changes will end up being overwritten when the `pci_devs` diff --git a/nova/pci/request.py b/nova/pci/request.py index cbdeb26f1789..84465982fe05 100644 --- a/nova/pci/request.py +++ b/nova/pci/request.py @@ -17,24 +17,24 @@ | [pci] | alias = '{ - | "name": "QuicAssist", + | "name": "QuickAssist", | "product_id": "0443", | "vendor_id": "8086", | "device_type": "type-PCI", | }' - Aliases with the same name and the same device_type are OR operation:: + Aliases with the same name and the same device_type are ORed:: | [pci] | alias = '{ - | "name": "QuicAssist", + | "name": "QuickAssist", | "product_id": "0442", | "vendor_id": "8086", | "device_type": "type-PCI", | }' - These 2 aliases define a device request meaning: vendor_id is "8086" and - product id is "0442" or "0443". + These two aliases define a device request meaning: vendor_id is "8086" and + product_id is "0442" or "0443". """ @@ -156,11 +156,11 @@ def get_pci_requests_from_flavor(flavor): 'alias_name_x:count, alias_name_y:count, ... '. The alias_name is 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 - three keys. The 'specs' gives the pci device properties - requirement, the 'count' gives the number of devices, and the - optional 'alias_name' is the corresponding alias definition name. + The flavor's requirement is translated into a pci requests list. + Each entry in the list is a dict-ish object with three keys/attributes. + The 'specs' gives the pci device properties requirement, the 'count' gives + the number of devices, and the optional 'alias_name' is the corresponding + alias definition name. Example: Assume alias configuration is:: diff --git a/nova/pci/stats.py b/nova/pci/stats.py index da3b48a67ba8..7754c068a973 100644 --- a/nova/pci/stats.py +++ b/nova/pci/stats.py @@ -156,12 +156,12 @@ class PciDeviceStats(object): # Failed to allocate the required number of devices # Return the devices already allocated back to their pools if sum([pool['count'] for pool in pools]) < count: - LOG.error("Failed to allocate PCI devices for instance." - " Unassigning devices back to pools." - " This should not happen, since the scheduler" - " should have accurate information, and allocation" - " during claims is controlled via a hold" - " on the compute node semaphore") + LOG.error("Failed to allocate PCI devices for instance. " + "Unassigning devices back to pools. " + "This should not happen, since the scheduler " + "should have accurate information, and allocation " + "during claims is controlled via a hold " + "on the compute node semaphore.") for d in range(len(alloc_devices)): self.add_device(alloc_devices.pop()) return None @@ -223,7 +223,7 @@ class PciDeviceStats(object): def _filter_non_requested_pfs(self, request, matching_pools): # Remove SRIOV_PFs from pools, unless it has been explicitly requested - # This is especially needed in cases where PFs and VFs has the same + # This is especially needed in cases where PFs and VFs have the same # product_id. if all(spec.get('dev_type') != fields.PciDeviceType.SRIOV_PF for spec in request.spec): diff --git a/nova/pci/utils.py b/nova/pci/utils.py index 3b004d09ed92..2ea64b14911c 100644 --- a/nova/pci/utils.py +++ b/nova/pci/utils.py @@ -72,12 +72,26 @@ def parse_address(address): def get_pci_address_fields(pci_addr): + """Parse a fully-specified PCI device address. + + Does not validate that the components are valid hex or wildcard values. + + :param pci_addr: A string of the form "::.". + :return: A 4-tuple of strings ("", "", "", "") + """ dbs, sep, func = pci_addr.partition('.') domain, bus, slot = dbs.split(':') - return (domain, bus, slot, func) + return domain, bus, slot, func def get_pci_address(domain, bus, slot, func): + """Assembles PCI address components into a fully-specified PCI address. + + Does not validate that the components are valid hex or wildcard values. + + :param domain, bus, slot, func: Hex or wildcard strings. + :return: A string of the form "::.". + """ return '%s:%s:%s.%s' % (domain, bus, slot, func) @@ -103,7 +117,6 @@ def is_physical_function(domain, bus, slot, function): dev_path = "/sys/bus/pci/devices/%(d)s:%(b)s:%(s)s.%(f)s/" % { "d": domain, "b": bus, "s": slot, "f": function} if os.path.isdir(dev_path): - sriov_totalvfs = 0 try: with open(dev_path + _SRIOV_TOTALVFS) as fd: sriov_totalvfs = int(fd.read()) @@ -119,12 +132,12 @@ def _get_sysfs_netdev_path(pci_addr, pf_interface): Assumes a networking device - will not check for the existence of the path. """ if pf_interface: - return "/sys/bus/pci/devices/%s/physfn/net" % (pci_addr) - return "/sys/bus/pci/devices/%s/net" % (pci_addr) + return "/sys/bus/pci/devices/%s/physfn/net" % pci_addr + return "/sys/bus/pci/devices/%s/net" % pci_addr def get_ifname_by_pci_address(pci_addr, pf_interface=False): - """Get the interface name based on a VF's pci address + """Get the interface name based on a VF's pci address. The returned interface name is either the parent PF's or that of the VF itself based on the argument of pf_interface. @@ -138,7 +151,7 @@ def get_ifname_by_pci_address(pci_addr, pf_interface=False): def get_mac_by_pci_address(pci_addr, pf_interface=False): - """Get the MAC address of the nic based on it's PCI address + """Get the MAC address of the nic based on its PCI address. Raises PciDeviceNotFoundById in case the pci device is not a NIC """ diff --git a/nova/pci/whitelist.py b/nova/pci/whitelist.py index ef7793f247fd..7133c4a38ca8 100644 --- a/nova/pci/whitelist.py +++ b/nova/pci/whitelist.py @@ -26,15 +26,16 @@ CONF = nova.conf.CONF class Whitelist(object): - """White list class to decide assignable pci devices. + """White list class to represent assignable pci devices. - Not all devices on compute node can be assigned to guest, the - cloud administrator decides the devices that can be assigned - based on vendor_id or product_id etc. If no white list specified, - no device will be assignable. + Not all devices on a compute node can be assigned to a guest. The + cloud administrator decides which devices can be assigned + based on vendor_id or product_id, etc. If no white list is specified, + no devices will be assignable. """ - def _parse_white_list_from_config(self, whitelists): + @staticmethod + def _parse_white_list_from_config(whitelists): """Parse and validate the pci whitelist from the nova config.""" specs = [] for jsonspec in whitelists: @@ -64,14 +65,16 @@ class Whitelist(object): def __init__(self, whitelist_spec=None): """White list constructor - For example, followed json string specifies that devices whose + For example, the following json string specifies that devices whose vendor_id is '8086' and product_id is '1520' can be assigned - to guest. + to guests. '[{"product_id":"1520", "vendor_id":"8086"}]' - :param whitelist_spec: A json string for a list of dictionaries, - each dictionary specifies the pci device - properties requirement. + :param whitelist_spec: A json string for a dictionary or list thereof. + Each dictionary specifies the pci device + properties requirement. See the definition of + passthrough_whitelist in nova.conf.pci for + details and examples. """ super(Whitelist, self).__init__() if whitelist_spec: diff --git a/nova/scheduler/host_manager.py b/nova/scheduler/host_manager.py index 52ca3f09141e..cfb0264a3cee 100644 --- a/nova/scheduler/host_manager.py +++ b/nova/scheduler/host_manager.py @@ -225,7 +225,7 @@ class HostState(object): self.updated = compute.updated_at self.numa_topology = compute.numa_topology self.pci_stats = pci_stats.PciDeviceStats( - compute.pci_device_pools) + stats=compute.pci_device_pools) # All virt drivers report host_ip self.host_ip = compute.host_ip @@ -264,7 +264,7 @@ class HostState(object): @set_update_time_on_success def _locked(self, spec_obj): # Scheduler API is inherently multi-threaded as every incoming RPC - # message will be dispatched in it's own green thread. So the + # message will be dispatched in its own green thread. So the # shared host state should be consumed in a consistent way to make # sure its data is valid under concurrent write operations. self._locked_consume_from_request(spec_obj) diff --git a/nova/tests/unit/pci/test_devspec.py b/nova/tests/unit/pci/test_devspec.py index ad35d190e28b..02162f13f447 100644 --- a/nova/tests/unit/pci/test_devspec.py +++ b/nova/tests/unit/pci/test_devspec.py @@ -210,7 +210,8 @@ class PciAddressTestCase(test.NoDBTestCase): pci_info = {"address": "0000:h4.12:6", "physical_network": "hr_net"} exc = self.assertRaises(exception.PciConfigInvalidWhitelist, devspec.PciDeviceSpec, pci_info) - msg = ('Invalid PCI devices Whitelist config invalid func 12:6') + msg = ("Invalid PCI devices Whitelist config: property func ('12:6') " + "does not parse as a hex number.") self.assertEqual(msg, six.text_type(exc)) def test_max_func(self): @@ -218,8 +219,9 @@ class PciAddressTestCase(test.NoDBTestCase): "physical_network": "hr_net"} exc = self.assertRaises(exception.PciConfigInvalidWhitelist, devspec.PciDeviceSpec, pci_info) - msg = ('Invalid PCI devices Whitelist config invalid func %x' - % (devspec.MAX_FUNC + 1)) + msg = ('Invalid PCI devices Whitelist config: property func (%x) is ' + 'greater than the maximum allowable value (%x).' + % (devspec.MAX_FUNC + 1, devspec.MAX_FUNC)) self.assertEqual(msg, six.text_type(exc)) def test_max_domain(self): @@ -227,8 +229,9 @@ class PciAddressTestCase(test.NoDBTestCase): "physical_network": "hr_net"} exc = self.assertRaises(exception.PciConfigInvalidWhitelist, devspec.PciDeviceSpec, pci_info) - msg = ('Invalid PCI devices Whitelist config invalid domain %x' - % (devspec.MAX_DOMAIN + 1)) + msg = ('Invalid PCI devices Whitelist config: property domain (%X) ' + 'is greater than the maximum allowable value (%X).' + % (devspec.MAX_DOMAIN + 1, devspec.MAX_DOMAIN)) self.assertEqual(msg, six.text_type(exc)) def test_max_bus(self): @@ -236,8 +239,9 @@ class PciAddressTestCase(test.NoDBTestCase): "physical_network": "hr_net"} exc = self.assertRaises(exception.PciConfigInvalidWhitelist, devspec.PciDeviceSpec, pci_info) - msg = ('Invalid PCI devices Whitelist config invalid bus %x' - % (devspec.MAX_BUS + 1)) + msg = ('Invalid PCI devices Whitelist config: property bus (%X) is ' + 'greater than the maximum allowable value (%X).' + % (devspec.MAX_BUS + 1, devspec.MAX_BUS)) self.assertEqual(msg, six.text_type(exc)) def test_max_slot(self): @@ -245,8 +249,9 @@ class PciAddressTestCase(test.NoDBTestCase): "physical_network": "hr_net"} exc = self.assertRaises(exception.PciConfigInvalidWhitelist, devspec.PciDeviceSpec, pci_info) - msg = ('Invalid PCI devices Whitelist config invalid slot %x' - % (devspec.MAX_SLOT + 1)) + msg = ('Invalid PCI devices Whitelist config: property slot (%X) is ' + 'greater than the maximum allowable value (%X).' + % (devspec.MAX_SLOT + 1, devspec.MAX_SLOT)) self.assertEqual(msg, six.text_type(exc)) def test_address_is_undefined(self): @@ -380,8 +385,10 @@ class PciDevSpecTestCase(test.NoDBTestCase): "product_id": "5057", "physical_network": "hr_net"} exc = self.assertRaises(exception.PciConfigInvalidWhitelist, devspec.PciDeviceSpec, pci_info) - self.assertEqual("Invalid PCI devices Whitelist config " - "invalid vendor_id 80860", six.text_type(exc)) + self.assertEqual( + "Invalid PCI devices Whitelist config: property vendor_id (80860) " + "is greater than the maximum allowable value (FFFF).", + six.text_type(exc)) def test_invalid_product_id(self): pci_info = {"vendor_id": "8086", "address": "*: *: *.5", @@ -394,8 +401,10 @@ class PciDevSpecTestCase(test.NoDBTestCase): "product_id": "50570", "physical_network": "hr_net"} exc = self.assertRaises(exception.PciConfigInvalidWhitelist, devspec.PciDeviceSpec, pci_info) - self.assertEqual("Invalid PCI devices Whitelist config " - "invalid product_id 50570", six.text_type(exc)) + self.assertEqual( + "Invalid PCI devices Whitelist config: property product_id " + "(50570) is greater than the maximum allowable value (FFFF).", + six.text_type(exc)) def test_devname_and_address(self): pci_info = {"devname": "eth0", "vendor_id": "8086",