From 50355c4595e08f293f610da32247e405b20c1c5b Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Mon, 23 Nov 2015 23:43:07 -0500 Subject: [PATCH] objects: adding a parent_addr field to the PciDevice object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parent_addr field will help identify the relationship between Physical and Virtual pci device functions. This information was already available to the PciDevice objects and was stored in extra_info['phys_function'] field, so we add code that will migrate old data on the fly, for live upgrade scenarios where we still have running older compute nodes alongside new ones. We don't want to migrate, however, if there are still any API service instances running that might access this info directly (without the help of the conductor) but have not been upgraded yet. Co-authored-by: Nikola Đipanov Partially implements blueprint sriov-physical-function-passthrough Change-Id: I94e8ce2c2a3d1c9e8b4aa1b245076eba84f37f45 --- nova/objects/pci_device.py | 59 +++++++++++++++- nova/objects/service.py | 4 +- nova/pci/devspec.py | 8 +-- nova/tests/unit/objects/test_instance.py | 2 + nova/tests/unit/objects/test_objects.py | 4 +- nova/tests/unit/objects/test_pci_device.py | 76 +++++++++++++++++++++ nova/tests/unit/pci/test_devspec.py | 5 +- nova/tests/unit/pci/test_manager.py | 7 +- nova/tests/unit/pci/test_stats.py | 6 ++ nova/tests/unit/virt/libvirt/test_driver.py | 4 +- nova/virt/libvirt/driver.py | 2 +- 11 files changed, 159 insertions(+), 18 deletions(-) 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