Reviewed: https://review.openstack.org/291847 Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=66e79e1bbca66a6f0f7be14006a87426c4dda402 Submitter: Jenkins Branch: master
commit 66e79e1bbca66a6f0f7be14006a87426c4dda402 Author: Jay Pipes <[email protected]> Date: Fri Mar 11 13:16:40 2016 -0500 pci - Claim devices outside of Claim constructor During the nova.compute.Claim.__init__() call, there are a bunch of _test_XXX() methods that are called. These methods should test to see whether the requested resources of various types can be satisfied by the inventory on the ComputeNode. However, that inventory *should not* be claimed for a particular request during the Claim object's constructor. The Claim._test_pci() method was *actually* claiming the PCI device for the requested instance. Unfortunately, this meant that if an instance launch request's demand for a resource like RAM was not able to be satisfied by the compute node but the launch request's demand for PCI devices *was* able to be satisfied by the compute node, those PCI devices were actually claimed for the instance even though the claim itself would end up being aborted. This resulted in a data corruption/inconsistency where a PCI device would be claimed for an instance that actually was not running on the node. This patch moves the claim of PCI devices out of the _test_pci() method and into the ResourceTracker.instance_claim() method. In the process of fixing this bug, it was discovered that the unit tests for the Claim object with regards to PCI devices were just plain broken. They were testing for nothing at all because of the way the Claim constructor works. These unit tests were reworked completely, along with the MoveClaim unit tests which similarly were not testing the PCI code paths at all. An additional unit test was added on the resource tracker to verify that nova.pci.manager.PciDevTracker.claim_instance() is called when PCI requests are included and satisfied by the Claim. Change-Id: Icf75439a552de84ec31c1a47faeee3caf8a5b0a7 Closes-bug: #1549984 ** Changed in: nova Status: In Progress => Fix Released -- You received this bug notification because you are a member of Yahoo! Engineering Team, which is subscribed to OpenStack Compute (nova). https://bugs.launchpad.net/bugs/1549984 Title: PCI devices claimed on compute node during _claim_test() Status in OpenStack Compute (nova): Fix Released Bug description: The nova.compute.claims.Claim object is used to test whether a set of requested resources can be satisfied by the compute node. In the constructor of the Claim object, the Claim._claim_test() object is called: def __init__(self, context, instance, tracker, resources, overhead=None, limits=None): super(Claim, self).__init__() <snip> # Check claim at constructor to avoid mess code # Raise exception ComputeResourcesUnavailable if claim failed self._claim_test(resources, limits) If we take a look at _claim_test(), we see pretty clearly that resources are NOT supposed to be actually claimed -- instead, the method should only *check* to see if the request can be fulfilled: def _claim_test(self, resources, limits=None): """Test if this claim can be satisfied given available resources and optional oversubscription limits This should be called before the compute node actually consumes the resources required to execute the claim. :param resources: available local compute node resources :returns: Return true if resources are available to claim. """ <snip> reasons = [self._test_memory(resources, memory_mb_limit), self._test_disk(resources, disk_gb_limit), self._test_vcpus(resources, vcpus_limit), self._test_numa_topology(resources, numa_topology_limit), self._test_pci()] reasons = reasons + self._test_ext_resources(limits) reasons = [r for r in reasons if r is not None] if len(reasons) > 0: raise exception.ComputeResourcesUnavailable(reason= "; ".join(reasons)) Unfortunately, the PCI devices are *actually* claimed in the _test_pci() method: def _test_pci(self): pci_requests = objects.InstancePCIRequests.get_by_instance_uuid( self.context, self.instance.uuid) if pci_requests.requests: devs = self.tracker.pci_tracker.claim_instance(self.context, self.instance) if not devs: return _('Claim pci failed.') What this means is that if an instance is attempted to be launched on a compute node and that instance has PCI requests that can be satisfied by the compute host, but say, there isn't enough available RAM on the node, the Claim will raise ComputeResourcesUnavailable which will trigger a Retry operation to the scheduler, but the PCI devices will have already been marked as claimed by that instance in the PCI device tracker: devs = self.tracker.pci_tracker.claim_instance(self.context, self.instance) The above code actually marks one or more PCI devices on the compute host as claimed for the instance. This introduces inconsistent state into the system. Making things worse is the fact that the nova.pci.manager.PciDevTracker object uses the nova.pci.stats.PciDevStats object for tracking consumed quantities of "pools" of the PCI device types and both the stats aggregation AND the PciDevTracker.pci_devs PciDeviceList object have their state changed improperly. To manage notifications about this bug go to: https://bugs.launchpad.net/nova/+bug/1549984/+subscriptions -- Mailing list: https://launchpad.net/~yahoo-eng-team Post to : [email protected] Unsubscribe : https://launchpad.net/~yahoo-eng-team More help : https://help.launchpad.net/ListHelp
