diff --git a/nova/objects/pci_device.py b/nova/objects/pci_device.py index 265884cfdb7d..01e478c1696e 100644 --- a/nova/objects/pci_device.py +++ b/nova/objects/pci_device.py @@ -18,6 +18,7 @@ import copy from oslo_serialization import jsonutils from oslo_utils import versionutils +from nova import context from nova import db from nova import exception from nova import objects @@ -85,7 +86,8 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject): # Version 1.1: String attributes updated to support unicode # Version 1.2: added request_id field # Version 1.3: Added field to represent PCI device NUMA node - VERSION = '1.3' + # Version 1.4: Added parent_addr field + VERSION = '1.4' fields = { 'id': fields.IntegerField(), @@ -103,12 +105,30 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject): 'request_id': fields.StringField(nullable=True), 'extra_info': fields.DictOfStringsField(), 'numa_node': fields.IntegerField(nullable=True), + 'parent_addr': fields.StringField(nullable=True), } + @staticmethod + def _migrate_parent_addr(): + # NOTE(ndipanov): Only migrate parent_addr if all services are up to at + # least version 4 - this should only ever be called from save() + services = ('conductor', 'api') + min_parent_addr_version = 4 + + min_deployed = min(objects.Service.get_minimum_version( + context.get_admin_context(), 'nova-' + service) + for service in services) + return min_deployed >= min_parent_addr_version + def obj_make_compatible(self, primitive, target_version): target_version = versionutils.convert_version_to_tuple(target_version) if target_version < (1, 2) and 'request_id' in primitive: del primitive['request_id'] + if target_version < (1, 4) and 'parent_addr' in primitive: + if primitive['parent_addr'] is not None: + extra_info = primitive.get('extra_info', {}) + extra_info['phys_function'] = primitive['parent_addr'] + del primitive['parent_addr'] def update_device(self, dev_dict): """Sync the content from device dictionary to device object. @@ -156,6 +176,11 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject): pci_device.extra_info = jsonutils.loads(extra_info) pci_device._context = context pci_device.obj_reset_changes() + # NOTE(ndipanov): As long as there is PF data in the old location, we + # want to load it as it may have be the only place we have it + if 'phys_function' in pci_device.extra_info: + pci_device.parent_addr = pci_device.extra_info['phys_function'] + return pci_device @base.remotable_classmethod @@ -189,6 +214,24 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject): self.address) elif self.status != fields.PciDeviceStatus.DELETED: updates = self.obj_get_changes() + if not self._migrate_parent_addr(): + # NOTE(ndipanov): If we are not migrating data yet, make sure + # that any changes to parent_addr are also in the old location + # in extra_info + if 'parent_addr' in updates: + extra_update = updates.get('extra_info', {}) + if not extra_update and self.obj_attr_is_set('extra_info'): + extra_update = self.extra_info + extra_update['phys_function'] = updates['parent_addr'] + updates['extra_info'] = extra_update + else: + # NOTE(ndipanov): Once we start migrating, meaning all control + # plane has been upgraded - aggressively migrate on every save + pf_extra = self.extra_info.pop('phys_function', None) + if pf_extra and 'parent_addr' not in updates: + updates['parent_addr'] = pf_extra + updates['extra_info'] = self.extra_info + if 'extra_info' in updates: updates['extra_info'] = jsonutils.dumps(updates['extra_info']) if updates: @@ -270,6 +313,9 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject): else: instance.pci_devices.objects.remove(existed) + def is_available(self): + return self.status == fields.PciDeviceStatus.AVAILABLE + @base.NovaObjectRegistry.register class PciDeviceList(base.ObjectListBase, base.NovaObject): @@ -277,7 +323,8 @@ class PciDeviceList(base.ObjectListBase, base.NovaObject): # PciDevice <= 1.1 # Version 1.1: PciDevice 1.2 # Version 1.2: PciDevice 1.3 - VERSION = '1.2' + # Version 1.3: Adds get_by_parent_address + VERSION = '1.3' fields = { 'objects': fields.ListOfObjectsField('PciDevice'), @@ -299,3 +346,11 @@ class PciDeviceList(base.ObjectListBase, base.NovaObject): db_dev_list = db.pci_device_get_all_by_instance_uuid(context, uuid) return base.obj_make_list(context, cls(context), objects.PciDevice, db_dev_list) + + @base.remotable_classmethod + def get_by_parent_address(cls, context, node_id, parent_addr): + db_dev_list = db.pci_device_get_all_by_parent_addr(context, + node_id, + parent_addr) + return base.obj_make_list(context, cls(context), objects.PciDevice, + db_dev_list) diff --git a/nova/objects/service.py b/nova/objects/service.py index c67eb5c8375e..6ea5cc8f58c1 100644 --- a/nova/objects/service.py +++ b/nova/objects/service.py @@ -28,7 +28,7 @@ LOG = logging.getLogger(__name__) # NOTE(danms): This is the global service version counter -SERVICE_VERSION = 3 +SERVICE_VERSION = 4 # NOTE(danms): This is our SERVICE_VERSION history. The idea is that any @@ -58,6 +58,8 @@ SERVICE_VERSION_HISTORY = ( {'compute_rpc': '4.5'}, # Version 3: Add trigger_crash_dump method to compute rpc api {'compute_rpc': '4.6'}, + # Version 4: Add PciDevice.parent_addr (data migration needed) + {'compute_rpc': '4.6'}, ) diff --git a/nova/pci/devspec.py b/nova/pci/devspec.py index ea55f2129931..e2189d83b61e 100644 --- a/nova/pci/devspec.py +++ b/nova/pci/devspec.py @@ -161,19 +161,15 @@ class PciDeviceSpec(object): self.vendor_id in (ANY, dev_dict['vendor_id']), self.product_id in (ANY, dev_dict['product_id']), self.address.match(dev_dict['address'], - dev_dict.get('phys_function')) + dev_dict.get('parent_addr')) ] return all(conditions) def match_pci_obj(self, pci_obj): - if pci_obj.extra_info: - phy_func = pci_obj.extra_info.get('phys_function') - else: - phy_func = None return self.match({'vendor_id': pci_obj.vendor_id, 'product_id': pci_obj.product_id, 'address': pci_obj.address, - 'phys_function': phy_func}) + 'parent_addr': pci_obj.parent_addr}) def get_tags(self): return self.tags diff --git a/nova/tests/unit/objects/test_instance.py b/nova/tests/unit/objects/test_instance.py index 62425ee7c6af..2fc8bcd09b33 100644 --- a/nova/tests/unit/objects/test_instance.py +++ b/nova/tests/unit/objects/test_instance.py @@ -764,6 +764,7 @@ class _TestInstanceObject(object): 'label': 'l', 'instance_uuid': fake_uuid, 'request_id': None, + 'parent_addr': None, 'extra_info': '{}'}, { 'created_at': None, @@ -782,6 +783,7 @@ class _TestInstanceObject(object): 'label': 'l', 'instance_uuid': fake_uuid, 'request_id': None, + 'parent_addr': 'a1', 'extra_info': '{}'}, ] self.mox.StubOutWithMock(db, 'instance_get_by_uuid') diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index dcf13293f90b..cb5c34f23415 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -1169,8 +1169,8 @@ object_data = { 'NetworkList': '1.2-69eca910d8fa035dfecd8ba10877ee59', 'NetworkRequest': '1.1-7a3e4ca2ce1e7b62d8400488f2f2b756', 'NetworkRequestList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e', - 'PciDevice': '1.3-d92e0b17bbed61815b919af6b8d8998e', - 'PciDeviceList': '1.2-3757458c45591cbc92c72ee99e757c98', + 'PciDevice': '1.4-4f54e80054bbb6414e17eb9babc97a44', + 'PciDeviceList': '1.3-52ff14355491c8c580bdc0ba34c26210', 'PciDevicePool': '1.1-3f5ddc3ff7bfa14da7f6c7e9904cc000', 'PciDevicePoolList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e', 'Quotas': '1.2-1fe4cd50593aaf5d36a6dc5ab3f98fb3', diff --git a/nova/tests/unit/objects/test_pci_device.py b/nova/tests/unit/objects/test_pci_device.py index aaccbfb27bc6..45114c0ad291 100644 --- a/nova/tests/unit/objects/test_pci_device.py +++ b/nova/tests/unit/objects/test_pci_device.py @@ -15,11 +15,13 @@ import copy +import mock from oslo_utils import timeutils from nova import context from nova import db from nova import exception +from nova import objects from nova.objects import fields from nova.objects import instance from nova.objects import pci_device @@ -39,6 +41,7 @@ fake_db_dev = { 'updated_at': None, 'deleted_at': None, 'deleted': None, + 'parent_addr': None, 'id': 1, 'compute_node_id': 1, 'address': 'a', @@ -61,6 +64,7 @@ fake_db_dev_1 = { 'deleted_at': None, 'deleted': None, 'id': 2, + 'parent_addr': 'a', 'compute_node_id': 1, 'address': 'a1', 'vendor_id': 'v1', @@ -76,6 +80,28 @@ fake_db_dev_1 = { } +fake_db_dev_old = { + 'created_at': None, + 'updated_at': None, + 'deleted_at': None, + 'deleted': None, + 'id': 2, + 'parent_addr': None, + 'compute_node_id': 1, + 'address': 'a1', + 'vendor_id': 'v1', + 'product_id': 'p1', + 'numa_node': 1, + 'dev_type': fields.PciDeviceType.SRIOV_VF, + 'status': fields.PciDeviceStatus.AVAILABLE, + 'dev_id': 'i', + 'label': 'l', + 'instance_uuid': None, + 'extra_info': '{"phys_function": "blah"}', + 'request_id': None, + } + + class _TestPciDeviceObject(object): def _create_fake_instance(self): self.inst = instance.Instance() @@ -147,6 +173,13 @@ class _TestPciDeviceObject(object): self.assertEqual(self.pci_device.product_id, 'p') self.assertEqual(self.pci_device.obj_what_changed(), set()) + def test_from_db_obj_pre_1_4_format(self): + ctxt = context.get_admin_context() + dev = pci_device.PciDevice._from_db_object( + ctxt, pci_device.PciDevice(), fake_db_dev_old) + self.assertEqual('blah', dev.parent_addr) + self.assertEqual({'phys_function': 'blah'}, dev.extra_info) + def test_save(self): ctxt = context.get_admin_context() self._create_fake_pci_device(ctxt=ctxt) @@ -206,6 +239,49 @@ class _TestPciDeviceObject(object): self.pci_device.save() self.assertEqual(self.called, False) + @mock.patch.object(objects.Service, 'get_minimum_version', return_value=4) + def test_save_migrate_parent_addr(self, get_min_ver_mock): + ctxt = context.get_admin_context() + dev = pci_device.PciDevice._from_db_object( + ctxt, pci_device.PciDevice(), fake_db_dev_old) + with mock.patch.object(db, 'pci_device_update', + return_value=fake_db_dev_old) as update_mock: + dev.save() + update_mock.assert_called_once_with( + ctxt, dev.compute_node_id, dev.address, + {'extra_info': '{}', 'parent_addr': 'blah'}) + + @mock.patch.object(objects.Service, 'get_minimum_version', return_value=4) + def test_save_migrate_parent_addr_updated(self, get_min_ver_mock): + ctxt = context.get_admin_context() + dev = pci_device.PciDevice._from_db_object( + ctxt, pci_device.PciDevice(), fake_db_dev_old) + # Note that the pci manager code will never update parent_addr alone, + # but we want to make it future proof so we guard against it + dev.parent_addr = 'doh!' + with mock.patch.object(db, 'pci_device_update', + return_value=fake_db_dev_old) as update_mock: + dev.save() + update_mock.assert_called_once_with( + ctxt, dev.compute_node_id, dev.address, + {'extra_info': '{}', 'parent_addr': 'doh!'}) + + @mock.patch.object(objects.Service, 'get_minimum_version', return_value=2) + def test_save_dont_migrate_parent_addr(self, get_min_ver_mock): + ctxt = context.get_admin_context() + dev = pci_device.PciDevice._from_db_object( + ctxt, pci_device.PciDevice(), fake_db_dev_old) + dev.extra_info['other'] = "blahtoo" + with mock.patch.object(db, 'pci_device_update', + return_value=fake_db_dev_old) as update_mock: + dev.save() + self.assertEqual("blah", + update_mock.call_args[0][3]['parent_addr']) + self.assertIn("phys_function", + update_mock.call_args[0][3]['extra_info']) + self.assertIn("other", + update_mock.call_args[0][3]['extra_info']) + def test_update_numa_node(self): self.pci_device = pci_device.PciDevice.create(dev_dict) self.assertEqual(0, self.pci_device.numa_node) diff --git a/nova/tests/unit/pci/test_devspec.py b/nova/tests/unit/pci/test_devspec.py index be7c08065fd8..1ed6b1551667 100644 --- a/nova/tests/unit/pci/test_devspec.py +++ b/nova/tests/unit/pci/test_devspec.py @@ -23,7 +23,7 @@ from nova import test dev = {"vendor_id": "8086", "product_id": "5057", "address": "1234:5678:8988.5", - "phys_function": "0000:0a:00.0"} + "parent_addr": "0000:0a:00.0"} class PciAddressTestCase(test.NoDBTestCase): @@ -92,7 +92,7 @@ class PciAddressTestCase(test.NoDBTestCase): dev = {"vendor_id": "1137", "product_id": "0071", "address": "0000:0a:00.5", - "phys_function": "0000:0a:00.0"} + "parent_addr": "0000:0a:00.0"} self.assertTrue(pci.match(dev)) @mock.patch('nova.pci.utils.is_physical_function', return_value = True) @@ -169,6 +169,7 @@ class PciDevSpecTestCase(test.NoDBTestCase): 'product_id': '5057', 'vendor_id': '8086', 'status': 'available', + 'parent_addr': None, 'extra_k1': 'v1', } diff --git a/nova/tests/unit/pci/test_manager.py b/nova/tests/unit/pci/test_manager.py index e57efaf45731..d61e49de428e 100644 --- a/nova/tests/unit/pci/test_manager.py +++ b/nova/tests/unit/pci/test_manager.py @@ -60,13 +60,14 @@ fake_db_dev = { 'instance_uuid': None, 'extra_info': '{}', 'request_id': None, + 'parent_addr': None, } fake_db_dev_1 = dict(fake_db_dev, vendor_id='v1', product_id='p1', id=2, address='0000:00:00.2', numa_node=0) fake_db_dev_2 = dict(fake_db_dev, id=3, address='0000:00:00.3', - numa_node=None) + numa_node=None, parent_addr='0000:00:00.1') fake_db_devs = [fake_db_dev, fake_db_dev_1, fake_db_dev_2] @@ -272,7 +273,9 @@ class PciDevTrackerTestCase(test.NoDBTestCase): dev in self.tracker.pci_devs]), set(['v', 'v1'])) - def test_save(self): + @mock.patch.object(objects.PciDevice, '_migrate_parent_addr', + return_value=False) + def test_save(self, migrate_mock): self.stub_out( 'nova.db.pci_device_update', self._fake_pci_device_update) diff --git a/nova/tests/unit/pci/test_stats.py b/nova/tests/unit/pci/test_stats.py index d2860f2bf0a0..31ab37632e4b 100644 --- a/nova/tests/unit/pci/test_stats.py +++ b/nova/tests/unit/pci/test_stats.py @@ -218,6 +218,8 @@ class PciDeviceStatsWithTagsTestCase(test.NoDBTestCase): 'product_id': '0071', 'status': 'available', 'request_id': None, + 'dev_type': 'type-PCI', + 'parent_addr': None, 'numa_node': 0} self.pci_tagged_devices.append(objects.PciDevice.create(pci_dev)) @@ -229,6 +231,8 @@ class PciDeviceStatsWithTagsTestCase(test.NoDBTestCase): 'product_id': '0072', 'status': 'available', 'request_id': None, + 'dev_type': 'type-PCI', + 'parent_addr': None, 'numa_node': 0} self.pci_untagged_devices.append(objects.PciDevice.create(pci_dev)) @@ -286,6 +290,7 @@ class PciDeviceStatsWithTagsTestCase(test.NoDBTestCase): 'vendor_id': '2345', 'product_id': '0172', 'status': 'available', + 'parent_addr': None, 'request_id': None} pci_dev_obj = objects.PciDevice.create(pci_dev) self.pci_stats.add_device(pci_dev_obj) @@ -301,6 +306,7 @@ class PciDeviceStatsWithTagsTestCase(test.NoDBTestCase): 'vendor_id': '2345', 'product_id': '0172', 'status': 'available', + 'parent_addr': None, 'request_id': None} pci_dev_obj = objects.PciDevice.create(pci_dev) self.pci_stats.remove_device(pci_dev_obj) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index c4f89f16e6ce..c0fcd4d437c4 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -10042,7 +10042,7 @@ class LibvirtConnTestCase(test.NoDBTestCase): "vendor_id": '8086', "label": 'label_8086_1520', "dev_type": fields.PciDeviceType.SRIOV_VF, - "phys_function": '0000:04:00.3', + "parent_addr": '0000:04:00.3', } self.assertEqual(expect_vf, actualvf) @@ -10055,7 +10055,7 @@ class LibvirtConnTestCase(test.NoDBTestCase): "numa_node": 0, "label": 'label_8086_1520', "dev_type": fields.PciDeviceType.SRIOV_VF, - "phys_function": '0000:04:00.3', + "parent_addr": '0000:04:00.3', } self.assertEqual(expect_vf, actualvf) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index b1a873f3022f..f15340a08201 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -4824,7 +4824,7 @@ class LibvirtDriver(driver.ComputeDriver): fun_cap.device_addrs[0][3]) return { 'dev_type': fields.PciDeviceType.SRIOV_VF, - 'phys_function': phys_address, + 'parent_addr': phys_address, } # Note(moshele): libvirt < 1.3 reported virt_functions capability