diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index 6ba55df8bf6d..3f0cb198465a 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -60,6 +60,7 @@ from nova.objects import compute_node as compute_node_obj from nova.objects import host_mapping as host_mapping_obj from nova.objects import instance as instance_obj from nova.objects import instance_mapping as instance_mapping_obj +from nova.objects import pci_device as pci_device_obj from nova.objects import quotas as quotas_obj from nova.objects import virtual_interface as virtual_interface_obj from nova import rpc @@ -135,6 +136,8 @@ class DbCommands(object): virtual_interface_obj.fill_virtual_interface_list, # Added in Stein instance_mapping_obj.populate_user_id, + # Added in Victoria + pci_device_obj.PciDevice.populate_dev_uuids, ) def __init__(self): diff --git a/nova/objects/pci_device.py b/nova/objects/pci_device.py index cadbacd42ae6..14db00ebc3e9 100644 --- a/nova/objects/pci_device.py +++ b/nova/objects/pci_device.py @@ -198,6 +198,22 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject): def __ne__(self, other): return not (self == other) + @classmethod + def populate_dev_uuids(cls, context, count): + @db_api.pick_context_manager_reader + def get_devs_no_uuid(context): + return context.session.query(db_models.PciDevice).\ + filter_by(uuid=None).limit(count).all() + + db_devs = get_devs_no_uuid(context) + + done = 0 + for db_dev in db_devs: + cls._create_uuid(context, db_dev['id']) + done += 1 + + return done, done + @classmethod def _from_db_object(cls, context, pci_device, db_dev): for key in pci_device.fields: diff --git a/nova/tests/unit/objects/test_pci_device.py b/nova/tests/unit/objects/test_pci_device.py index 4a5835ede4c3..88f5f2c1a838 100644 --- a/nova/tests/unit/objects/test_pci_device.py +++ b/nova/tests/unit/objects/test_pci_device.py @@ -724,7 +724,7 @@ class TestPciDeviceUUIDMigration(test.TestCase): @staticmethod @db_api.pick_context_manager_writer - def _create_legacy_dev(context): + def _create_db_dev(context, deleted=False, **updates): """Create a PCI device with no UUID.""" values = copy.copy(dev_dict) # 'PciDeviceList.get_by_instance_uuid' expected devices to be allocated @@ -732,15 +732,21 @@ class TestPciDeviceUUIDMigration(test.TestCase): values['label'] = 'l' # Why is this necessary? values['instance_uuid'] = uuids.instance_uuid values['extra_info'] = '{}' + values.update(updates) + dev_ref = db_models.PciDevice() dev_ref.update(values) dev_ref.save(context.session) + + if deleted: + dev_ref.soft_delete(context.session) + return dev_ref @mock.patch.object(objects.PciDevice, '_create_uuid', wraps=objects.PciDevice._create_uuid) def test_populate_uuid(self, mock_create_uuid): - self._create_legacy_dev(self.context) + self._create_db_dev(self.context) devs = objects.PciDeviceList.get_by_instance_uuid( self.context, uuids.instance_uuid) @@ -769,7 +775,7 @@ class TestPciDeviceUUIDMigration(test.TestCase): # demonstrate that the compare-and-swap works correctly, and we trust # the correctness of the database for the rest. - db_dev = self._create_legacy_dev(self.context) + db_dev = self._create_db_dev(self.context) uuid1 = objects.PciDevice._create_uuid(self.context, db_dev.id) dev = objects.PciDeviceList.get_by_instance_uuid( @@ -782,3 +788,40 @@ class TestPciDeviceUUIDMigration(test.TestCase): uuid2 = objects.PciDevice._create_uuid(self.context, dev.id) self.assertEqual(uuid1, uuid2) + + def _assert_online_migration(self, expected_total, expected_done, + limit=10): + total, done = objects.PciDevice.populate_dev_uuids( + self.context, limit) + self.assertEqual(expected_total, total) + self.assertEqual(expected_done, done) + + def test_online_migration(self): + self._assert_online_migration(0, 0) + + # Create 2 PCI devices, one with a uuid and one without. We need to + # specify an address due to unique constraints in the database + self._create_db_dev(self.context, address='a', uuid=None) + self._create_db_dev(self.context, address='b', uuid=uuids.dev_uuid) + + # Run the online migration. We should find 1 and update 1 + self._assert_online_migration(1, 1) + + # Fetch the BDMs and check we didn't modify the uuid of bdm2 + devs = objects.PciDeviceList.get_by_instance_uuid( + self.context, uuids.instance_uuid) + dev_uuids = [dev.uuid for dev in devs] + self.assertIn(uuids.dev_uuid, dev_uuids) + self.assertNotIn(None, dev_uuids) + + # Run the online migration again to see nothing was processed + self._assert_online_migration(0, 0) + + # Assert that we assign a uuid to a deleted bdm. + self._create_db_dev(self.context, address='c', deleted=True) + self._assert_online_migration(1, 1) + + # Test that we don't migrate more than the limit + for i in range(0, 3): + self._create_db_dev(self.context, address=str(i)) + self._assert_online_migration(2, 2, limit=2)