openstack/nova
Code Issues Proposed changes

pci: eliminate DB lookup PCI requests during claim

Browse Source 
The nova.pci.manager.PciDevTracker.claim_instance() accepted an Instance
object and called nova.objects.InstancePCIRequests.get_by_instance() to
retrieve the PCI requests for the instance. This caused a DB lookup of
the PCI requests for that instance, even though in all situations other
than for migration/resize, the instance's PCI requests were already
retrieved by the resource tracker.

This change removes that additional DB lookup during claim_instance() by
changing the instance parameter to instead be an InstancePCIRequests
object and an InstanceNUMATopology object.

Also in this patch is a change to nova.objects.PciDevice.claim() that
changes the single parameter to an instance UUID instead of an Instance
object, since nothing other than the instance's UUID was used in the
method.

Change-Id: I9ab10c3035628f083233114b47b43a9b9ecdd166
This commit is contained in:
Jay Pipes 2016-04-04 12:32:56 -07:00 committed by Moshe Levi
parent 74fbff8863
commit 1f259e2a94
8 changed files with 75 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
nova/compute/resource_tracker.py
View File

@ -133,18 +133,20 @@ class ResourceTracker(object):
claim = claims.Claim(context, instance_ref, self, self.compute_node,
pci_requests, overhead=overhead, limits=limits)
if self.pci_tracker:
# NOTE(jaypipes): ComputeNode.pci_device_pools is set below
# in _update_usage_from_instance().
self.pci_tracker.claim_instance(context, instance_ref)
# self._set_instance_host_and_node() will save instance_ref to the DB
# so set instance_ref['numa_topology'] first. We need to make sure
# that numa_topology is saved while under COMPUTE_RESOURCE_SEMAPHORE
# so that the resource audit knows about any cpus we've pinned.
instance_ref.numa_topology = claim.claimed_numa_topology
instance_numa_topology = claim.claimed_numa_topology
instance_ref.numa_topology = instance_numa_topology
self._set_instance_host_and_node(instance_ref)
if self.pci_tracker:
# NOTE(jaypipes): ComputeNode.pci_device_pools is set below
# in _update_usage_from_instance().
self.pci_tracker.claim_instance(context, pci_requests,
instance_numa_topology)
# Mark resources in-use and update stats
self._update_usage_from_instance(context, instance_ref)

4
nova/objects/pci_device.py
View File

@ -262,7 +262,7 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject):
for dev in dev_list:
dev.status = status
def claim(self, instance):
def claim(self, instance_uuid):
if self.status != fields.PciDeviceStatus.AVAILABLE:
raise exception.PciDeviceInvalidStatus(
compute_node_id=self.compute_node_id,
@ -314,7 +314,7 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject):
'vf_addr': self.address})
self.status = fields.PciDeviceStatus.CLAIMED
self.instance_uuid = instance['uuid']
self.instance_uuid = instance_uuid
def allocate(self, instance):
ok_statuses = (fields.PciDeviceStatus.AVAILABLE,

30
nova/pci/manager.py
View File

@ -169,13 +169,7 @@ class PciDevTracker(object):
self.pci_devs.objects.append(dev_obj)
self.stats.add_device(dev_obj)
def _claim_instance(self, context, instance, prefix=''):
pci_requests = objects.InstancePCIRequests.get_by_instance(
context, instance)
if not pci_requests.requests:
return None
instance_numa_topology = hardware.instance_topology_from_instance(
instance)
def _claim_instance(self, context, pci_requests, instance_numa_topology):
instance_cells = None
if instance_numa_topology:
instance_cells = instance_numa_topology.cells
@ -185,13 +179,14 @@ class PciDevTracker(object):
if not devs:
return None
instance_uuid = pci_requests.instance_uuid
for dev in devs:
dev.claim(instance)
dev.claim(instance_uuid)
if instance_numa_topology and any(
dev.numa_node is None for dev in devs):
LOG.warning(_LW("Assigning a pci device without numa affinity to"
"instance %(instance)s which has numa topology"),
{'instance': instance['uuid']})
{'instance': instance_uuid})
return devs
def _allocate_instance(self, instance, devs):
@ -204,13 +199,15 @@ class PciDevTracker(object):
if devs:
self.allocations[instance['uuid']] += devs
def claim_instance(self, context, instance):
if not self.pci_devs:
def claim_instance(self, context, pci_requests, instance_numa_topology):
if not self.pci_devs or not pci_requests.requests:
return
devs = self._claim_instance(context, instance)
instance_uuid = pci_requests.instance_uuid
devs = self._claim_instance(context, pci_requests,
instance_numa_topology)
if devs:
self.claims[instance['uuid']] = devs
self.claims[instance_uuid] = devs
return devs
return None
@ -260,8 +257,13 @@ class PciDevTracker(object):
the claims when sign is -1
"""
uuid = instance['uuid']
pci_requests = objects.InstancePCIRequests.get_by_instance(
context, instance)
instance_numa_topology = hardware.instance_topology_from_instance(
instance)
if sign == 1 and uuid not in self.claims:
devs = self._claim_instance(context, instance, 'new_')
devs = self._claim_instance(context, pci_requests,
instance_numa_topology)
if devs:
self.claims[uuid] = devs
if sign == -1 and uuid in self.claims:

2
nova/tests/unit/api/openstack/compute/test_pci.py
View File

@ -61,7 +61,7 @@ class PciServerControllerTestV21(test.NoDBTestCase):
}]}
self._create_fake_instance()
self._create_fake_pci_device()
self.pci_device.claim(self.inst)
self.pci_device.claim(self.inst.uuid)
self.pci_device.allocate(self.inst)
def _create_fake_instance(self):

3
nova/tests/unit/compute/test_tracker.py
View File

@ -1381,7 +1381,8 @@ class TestInstanceClaim(BaseTestCase):
self.rt.instance_claim(self.ctx, self.instance, None)
update_mock.assert_called_once_with(self.elevated)
pci_manager_mock.assert_called_once_with(mock.ANY, # context...
self.instance)
pci_mock.return_value,
None)
self.assertTrue(obj_base.obj_equal_prims(expected,
self.rt.compute_node))

4
nova/tests/unit/fake_network.py
View File

@ -453,11 +453,11 @@ def _get_instances_with_cached_ips(orig_func, *args, **kwargs):
if isinstance(instances, (list, obj_base.ObjectListBase)):
for instance in instances:
_info_cache_for(instance)
fake_device.claim(instance)
fake_device.claim(instance.uuid)
fake_device.allocate(instance)
else:
_info_cache_for(instances)
fake_device.claim(instances)
fake_device.claim(instances.uuid)
fake_device.allocate(instances)
return instances

34
nova/tests/unit/objects/test_pci_device.py
View File

@ -209,11 +209,11 @@ class _TestPciDeviceObject(object):
ctxt = context.get_admin_context()
self._create_fake_pci_device(ctxt=ctxt)
return_dev = dict(fake_db_dev, status=fields.PciDeviceStatus.AVAILABLE,
instance_uuid='fake-uuid-3')
instance_uuid=uuids.instance3)
self.pci_device.status = fields.PciDeviceStatus.ALLOCATED
self.pci_device.instance_uuid = 'fake-uuid-2'
self.pci_device.instance_uuid = uuids.instance2
expected_updates = dict(status=fields.PciDeviceStatus.ALLOCATED,
instance_uuid='fake-uuid-2')
instance_uuid=uuids.instance2)
self.mox.StubOutWithMock(db, 'pci_device_update')
db.pci_device_update(ctxt, 1, 'a',
expected_updates).AndReturn(return_dev)
@ -222,11 +222,11 @@ class _TestPciDeviceObject(object):
self.assertEqual(self.pci_device.status,
fields.PciDeviceStatus.AVAILABLE)
self.assertEqual(self.pci_device.instance_uuid,
'fake-uuid-3')
uuids.instance3)
def test_save_no_extra_info(self):
return_dev = dict(fake_db_dev, status=fields.PciDeviceStatus.AVAILABLE,
instance_uuid='fake-uuid-3')
instance_uuid=uuids.instance3)
def _fake_update(ctxt, node_id, addr, updates):
self.extra_info = updates.get('extra_info')
@ -350,7 +350,7 @@ class _TestPciDeviceObject(object):
def test_claim_device(self):
self._create_fake_instance()
devobj = pci_device.PciDevice.create(None, dev_dict)
devobj.claim(self.inst)
devobj.claim(self.inst.uuid)
self.assertEqual(devobj.status,
fields.PciDeviceStatus.CLAIMED)
self.assertEqual(devobj.instance_uuid,
@ -367,7 +367,7 @@ class _TestPciDeviceObject(object):
def test_allocate_device(self):
self._create_fake_instance()
devobj = pci_device.PciDevice.create(None, dev_dict)
devobj.claim(self.inst)
devobj.claim(self.inst.uuid)
devobj.allocate(self.inst)
self.assertEqual(devobj.status,
fields.PciDeviceStatus.ALLOCATED)
@ -390,14 +390,14 @@ class _TestPciDeviceObject(object):
inst_2 = instance.Instance()
inst_2.uuid = uuids.instance_2
devobj = pci_device.PciDevice.create(None, dev_dict)
devobj.claim(self.inst)
devobj.claim(self.inst.uuid)
self.assertRaises(exception.PciDeviceInvalidOwner,
devobj.allocate, inst_2)
def test_free_claimed_device(self):
self._create_fake_instance()
devobj = pci_device.PciDevice.create(None, dev_dict)
devobj.claim(self.inst)
devobj.claim(self.inst.uuid)
devobj.free(self.inst)
self.assertEqual(devobj.status,
fields.PciDeviceStatus.AVAILABLE)
@ -408,7 +408,7 @@ class _TestPciDeviceObject(object):
ctx = context.get_admin_context()
devobj = pci_device.PciDevice._from_db_object(
ctx, pci_device.PciDevice(), fake_db_dev)
devobj.claim(self.inst)
devobj.claim(self.inst.uuid)
devobj.allocate(self.inst)
self.assertEqual(len(self.inst.pci_devices), 1)
devobj.free(self.inst)
@ -433,7 +433,7 @@ class _TestPciDeviceObject(object):
def test_remove_device_fail(self):
self._create_fake_instance()
devobj = pci_device.PciDevice.create(None, dev_dict)
devobj.claim(self.inst)
devobj.claim(self.inst.uuid)
self.assertRaises(exception.PciDeviceInvalidStatus, devobj.remove)
@ -555,7 +555,7 @@ class _TestSRIOVPciDeviceObject(object):
side_effect=self._fake_get_by_parent_address):
self._create_pci_devices()
devobj = self.sriov_pf_devices[0]
devobj.claim(self.inst)
devobj.claim(self.inst.uuid)
self.assertEqual(devobj.status,
fields.PciDeviceStatus.CLAIMED)
self.assertEqual(devobj.instance_uuid,
@ -573,7 +573,7 @@ class _TestSRIOVPciDeviceObject(object):
side_effect=self._fake_pci_device_get_by_addr):
self._create_pci_devices()
devobj = self.sriov_vf_devices[0]
devobj.claim(self.inst)
devobj.claim(self.inst.uuid)
self.assertEqual(devobj.status,
fields.PciDeviceStatus.CLAIMED)
self.assertEqual(devobj.instance_uuid,
@ -591,7 +591,7 @@ class _TestSRIOVPciDeviceObject(object):
side_effect=self._fake_get_by_parent_address):
self._create_pci_devices()
devobj = self.sriov_pf_devices[0]
devobj.claim(self.inst)
devobj.claim(self.inst.uuid)
devobj.allocate(self.inst)
self.assertEqual(devobj.status,
fields.PciDeviceStatus.ALLOCATED)
@ -610,7 +610,7 @@ class _TestSRIOVPciDeviceObject(object):
side_effect=self._fake_pci_device_get_by_addr):
self._create_pci_devices()
devobj = self.sriov_vf_devices[0]
devobj.claim(self.inst)
devobj.claim(self.inst.uuid)
devobj.allocate(self.inst)
self.assertEqual(devobj.status,
fields.PciDeviceStatus.ALLOCATED)
@ -677,7 +677,7 @@ class _TestSRIOVPciDeviceObject(object):
side_effect=self._fake_get_by_parent_address):
self._create_pci_devices()
devobj = self.sriov_pf_devices[0]
devobj.claim(self.inst)
devobj.claim(self.inst.uuid)
devobj.allocate(self.inst)
devobj.free(self.inst)
self.assertEqual(devobj.status,
@ -701,7 +701,7 @@ class _TestSRIOVPciDeviceObject(object):
dependents = self._fake_get_by_parent_address(None, None,
vf.parent_addr)
for devobj in dependents:
devobj.claim(self.inst)
devobj.claim(self.inst.uuid)
devobj.allocate(self.inst)
self.assertEqual(devobj.status,
fields.PciDeviceStatus.ALLOCATED)

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

@ -26,6 +26,7 @@ from nova.objects import fields
from nova.pci import manager
from nova import test
from nova.tests.unit.pci import fakes as pci_fakes
from nova.tests import uuidsentinel
fake_pci = {
@ -82,7 +83,7 @@ fake_pci_requests = [
class PciDevTrackerTestCase(test.NoDBTestCase):
def _create_fake_instance(self):
self.inst = objects.Instance()
self.inst.uuid = 'fake-inst-uuid'
self.inst.uuid = uuidsentinel.instance1
self.inst.pci_devices = objects.PciDeviceList()
self.inst.vm_state = vm_states.ACTIVE
self.inst.task_state = None
@ -100,13 +101,17 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
def _fake_pci_device_destroy(self, ctxt, node_id, address):
self.destroy_called += 1
def _create_pci_requests_object(self, mock_get, requests):
def _create_pci_requests_object(self, mock_get, requests,
instance_uuid=None):
instance_uuid = instance_uuid or uuidsentinel.instance1
pci_reqs = []
for request in requests:
pci_req_obj = objects.InstancePCIRequest(count=request['count'],
spec=request['spec'])
pci_reqs.append(pci_req_obj)
mock_get.return_value = objects.InstancePCIRequests(requests=pci_reqs)
mock_get.return_value = objects.InstancePCIRequests(
instance_uuid=instance_uuid,
requests=pci_reqs)
def setUp(self):
super(PciDevTrackerTestCase, self).setUp()
@ -181,7 +186,7 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
def test_set_hvdev_changed_stal(self, mock_get):
self._create_pci_requests_object(mock_get,
[{'count': 1, 'spec': [{'vendor_id': 'v1'}]}])
self.tracker._claim_instance(mock.sentinel.context, self.inst)
self.tracker._claim_instance(None, mock_get.return_value, None)
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)]
@ -193,7 +198,7 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
def test_update_pci_for_instance_active(self, mock_get):
self._create_pci_requests_object(mock_get, fake_pci_requests)
self.tracker.claim_instance(None, self.inst)
self.tracker.claim_instance(None, mock_get.return_value, None)
self.assertEqual(len(self.tracker.claims[self.inst['uuid']]), 2)
self.tracker.update_pci_for_instance(None, self.inst, sign=1)
self.assertEqual(len(self.tracker.allocations[self.inst['uuid']]), 2)
@ -206,7 +211,7 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
pci_requests = copy.deepcopy(fake_pci_requests)
pci_requests[0]['count'] = 4
self._create_pci_requests_object(mock_get, pci_requests)
self.tracker.claim_instance(None, self.inst)
self.tracker.claim_instance(None, mock_get.return_value, None)
self.assertEqual(len(self.tracker.claims[self.inst['uuid']]), 0)
devs = self.tracker.update_pci_for_instance(None,
self.inst,
@ -227,7 +232,8 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
self.inst.numa_topology = objects.InstanceNUMATopology(
cells=[objects.InstanceNUMACell(
id=1, cpuset=set([1, 2]), memory=512)])
self.tracker.claim_instance(None, self.inst)
self.tracker.claim_instance(None, mock_get.return_value,
self.inst.numa_topology)
free_devs = self.tracker.pci_stats.get_free_devs()
self.assertEqual(2, len(free_devs))
self.assertEqual('v1', free_devs[0].vendor_id)
@ -239,12 +245,14 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
self.inst.numa_topology = objects.InstanceNUMATopology(
cells=[objects.InstanceNUMACell(
id=1, cpuset=set([1, 2]), memory=512)])
self.assertIsNone(self.tracker.claim_instance(None, self.inst))
self.assertIsNone(self.tracker.claim_instance(
None, mock_get.return_value,
self.inst.numa_topology))
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance')
def test_update_pci_for_instance_deleted(self, mock_get):
self._create_pci_requests_object(mock_get, fake_pci_requests)
self.tracker.claim_instance(None, self.inst)
self.tracker.claim_instance(None, mock_get.return_value, None)
free_devs = self.tracker.pci_stats.get_free_devs()
self.assertEqual(len(free_devs), 1)
self.inst.vm_state = vm_states.DELETED
@ -307,17 +315,18 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance')
def test_clean_usage(self, mock_get):
inst_2 = copy.copy(self.inst)
inst_2.uuid = 'uuid5'
inst_2.uuid = uuidsentinel.instance2
migr = {'instance_uuid': 'uuid2', 'vm_state': vm_states.BUILDING}
orph = {'uuid': 'uuid3', 'vm_state': vm_states.BUILDING}
self._create_pci_requests_object(mock_get,
[{'count': 1, 'spec': [{'vendor_id': 'v'}]}])
self.tracker.claim_instance(None, self.inst)
self.tracker.claim_instance(None, mock_get.return_value, None)
self.tracker.update_pci_for_instance(None, self.inst, sign=1)
self._create_pci_requests_object(mock_get,
[{'count': 1, 'spec': [{'vendor_id': 'v1'}]}])
self.tracker.claim_instance(None, inst_2)
[{'count': 1, 'spec': [{'vendor_id': 'v1'}]}],
instance_uuid=inst_2.uuid)
self.tracker.claim_instance(None, mock_get.return_value, None)
self.tracker.update_pci_for_instance(None, inst_2, sign=1)
free_devs = self.tracker.pci_stats.get_free_devs()
self.assertEqual(len(free_devs), 1)
@ -333,16 +342,17 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance')
def test_clean_usage_claims(self, mock_get):
inst_2 = copy.copy(self.inst)
inst_2.uuid = 'uuid5'
inst_2.uuid = uuidsentinel.instance2
migr = {'instance_uuid': 'uuid2', 'vm_state': vm_states.BUILDING}
orph = {'uuid': 'uuid3', 'vm_state': vm_states.BUILDING}
self._create_pci_requests_object(mock_get,
[{'count': 1, 'spec': [{'vendor_id': 'v'}]}])
self.tracker.claim_instance(None, self.inst)
self.tracker.claim_instance(None, mock_get.return_value, None)
self.tracker.update_pci_for_instance(None, self.inst, sign=1)
self._create_pci_requests_object(mock_get,
[{'count': 1, 'spec': [{'vendor_id': 'v1'}]}])
[{'count': 1, 'spec': [{'vendor_id': 'v1'}]}],
instance_uuid=inst_2.uuid)
self.tracker.update_pci_for_migration(None, inst_2)
free_devs = self.tracker.pci_stats.get_free_devs()
self.assertEqual(len(free_devs), 1)
@ -373,7 +383,7 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
def test_free_devices(self, mock_get):
self._create_pci_requests_object(mock_get,
[{'count': 1, 'spec': [{'vendor_id': 'v'}]}])
self.tracker.claim_instance(None, self.inst)
self.tracker.claim_instance(None, mock_get.return_value, None)
self.tracker.update_pci_for_instance(None, self.inst, sign=1)
free_devs = self.tracker.pci_stats.get_free_devs()