diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index 2f0dde3c1927..616fa2b55f6c 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -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) diff --git a/nova/objects/pci_device.py b/nova/objects/pci_device.py index 5dbc949be00d..4f38829fe2fa 100644 --- a/nova/objects/pci_device.py +++ b/nova/objects/pci_device.py @@ -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, diff --git a/nova/pci/manager.py b/nova/pci/manager.py index 6d73c43d27f4..3decb431bcbd 100644 --- a/nova/pci/manager.py +++ b/nova/pci/manager.py @@ -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: diff --git a/nova/tests/unit/api/openstack/compute/test_pci.py b/nova/tests/unit/api/openstack/compute/test_pci.py index 074dd95e8fa6..a942b7f64482 100644 --- a/nova/tests/unit/api/openstack/compute/test_pci.py +++ b/nova/tests/unit/api/openstack/compute/test_pci.py @@ -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): diff --git a/nova/tests/unit/compute/test_tracker.py b/nova/tests/unit/compute/test_tracker.py index 326c731ca585..1fa8c9cc2e03 100644 --- a/nova/tests/unit/compute/test_tracker.py +++ b/nova/tests/unit/compute/test_tracker.py @@ -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)) diff --git a/nova/tests/unit/fake_network.py b/nova/tests/unit/fake_network.py index 10001d38d043..41baa5797c81 100644 --- a/nova/tests/unit/fake_network.py +++ b/nova/tests/unit/fake_network.py @@ -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 diff --git a/nova/tests/unit/objects/test_pci_device.py b/nova/tests/unit/objects/test_pci_device.py index 60217f1aaab0..789dde512647 100644 --- a/nova/tests/unit/objects/test_pci_device.py +++ b/nova/tests/unit/objects/test_pci_device.py @@ -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) diff --git a/nova/tests/unit/pci/test_manager.py b/nova/tests/unit/pci/test_manager.py index 665d20920ee4..23fffb268068 100644 --- a/nova/tests/unit/pci/test_manager.py +++ b/nova/tests/unit/pci/test_manager.py @@ -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()