Merge "libvirt: Register defaults for undefined hw image properties"

2022-02-24 13:00:43 +00:00
parent f9ca5dba14 7ecdfb61a9
commit fa10c6ec6a
4 changed files with 683 additions and 97 deletions
@@ -0,0 +1,234 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import datetime
+from unittest import mock
+
+from oslo_utils.fixture import uuidsentinel as uuids
+
+from nova import context as nova_context
+from nova import objects
+from nova import test
+from nova.tests.functional.libvirt import base
+from nova.virt.libvirt import driver as libvirt_driver
+
+
+class LibvirtDeviceBusMigration(base.ServersTestBase):
+
+    microversion = 'latest'
+    # needed for move operations
+    ADMIN_API = True
+
+    def setUp(self):
+        super().setUp()
+        self.context = nova_context.get_admin_context()
+        self.compute_hostname = self.start_compute()
+        self.compute = self.computes[self.compute_hostname]
+
+    def _unset_stashed_image_properties(self, server_id, properties):
+        instance = objects.Instance.get_by_uuid(self.context, server_id)
+        for p in properties:
+            instance.system_metadata.pop(f'image_{p}')
+        instance.save()
+
+    def _assert_stashed_image_properties(self, server_id, properties):
+        instance = objects.Instance.get_by_uuid(self.context, server_id)
+        for p, value in properties.items():
+            self.assertEqual(instance.system_metadata.get(f'image_{p}'), value)
+
+    def _assert_stashed_image_properties_persist(self, server, properties):
+        # Assert the stashed properties persist across a host reboot
+        self.restart_compute_service(self.compute)
+        self._assert_stashed_image_properties(server['id'], properties)
+
+        # Assert the stashed properties persist across a guest reboot
+        self._reboot_server(server, hard=True)
+        self._assert_stashed_image_properties(server['id'], properties)
+
+        # Assert the stashed properties persist across a migration
+        if 'other_compute' not in self.computes:
+            self.start_compute('other_compute')
+        # TODO(stephenfin): The mock of 'migrate_disk_and_power_off' should
+        # probably be less...dumb
+        with mock.patch(
+            'nova.virt.libvirt.driver.LibvirtDriver'
+            '.migrate_disk_and_power_off', return_value='{}',
+        ):
+            self._migrate_server(server)
+        self._confirm_resize(server)
+        self._assert_stashed_image_properties(server['id'], properties)
+
+    def test_default_image_property_registration(self):
+        """Assert that the defaults for various hw image properties don't
+        change over the lifecycle of an instance.
+        """
+        default_image_properties = {
+            'hw_machine_type': 'pc',
+            'hw_cdrom_bus': 'ide',
+            'hw_disk_bus': 'virtio',
+            'hw_input_bus': 'usb',
+            'hw_pointer_model': 'usbtablet',
+            'hw_video_model': 'virtio',
+            'hw_vif_model': 'virtio',
+        }
+
+        server = self._create_server(networks='none')
+        self._assert_stashed_image_properties(
+            server['id'], default_image_properties)
+
+        # Unset the defaults here to ensure that init_host resets them
+        # when the compute restarts the libvirt driver
+        self._unset_stashed_image_properties(
+            server['id'], libvirt_driver.REGISTER_IMAGE_PROPERTY_DEFAULTS)
+
+        # Assert the defaults persist across a host reboot, guest reboot, and
+        # guest migration
+        self._assert_stashed_image_properties_persist(
+            server, default_image_properties)
+
+    def test_non_default_image_property_registration(self):
+        """Assert that non-default values for various hw image properties
+        don't change over the lifecycle of an instance.
+        """
+        non_default_image_properties = {
+            'hw_machine_type': 'q35',
+            'hw_cdrom_bus': 'sata',
+            'hw_disk_bus': 'sata',
+            'hw_input_bus': 'virtio',
+            'hw_video_model': 'qxl',
+            'hw_vif_model': 'e1000',
+        }
+        self.glance.create(
+            None,
+            {
+                'id': uuids.hw_bus_model_image_uuid,
+                'name': 'hw_bus_model_image',
+                'created_at': datetime.datetime(2011, 1, 1, 1, 2, 3),
+                'updated_at': datetime.datetime(2011, 1, 1, 1, 2, 3),
+                'deleted_at': None,
+                'deleted': False,
+                'status': 'active',
+                'is_public': False,
+                'container_format': 'bare',
+                'disk_format': 'qcow2',
+                'size': '74185822',
+                'min_ram': 0,
+                'min_disk': 0,
+                'protected': False,
+                'visibility': 'public',
+                'tags': [],
+                'properties': non_default_image_properties,
+            }
+        )
+        server = self._create_server(
+            networks='none', image_uuid=uuids.hw_bus_model_image_uuid)
+        self._assert_stashed_image_properties(
+            server['id'], non_default_image_properties)
+
+        # Assert the non defaults persist across a host reboot, guest reboot,
+        # and guest migration
+        self._assert_stashed_image_properties_persist(
+            server, non_default_image_properties)
+
+    def test_default_image_property_persists_across_osinfo_changes(self):
+        # Create a server with default image properties
+        default_image_properties = {
+            'hw_vif_model': 'virtio',
+            'hw_disk_bus': 'virtio',
+        }
+        server = self._create_server(networks='none')
+        self._assert_stashed_image_properties(
+            server['id'], default_image_properties)
+
+        with test.nested(
+            mock.patch('nova.virt.osinfo.HardwareProperties.network_model',
+                new=mock.PropertyMock()),
+            mock.patch('nova.virt.osinfo.HardwareProperties.disk_model',
+                new=mock.PropertyMock())
+        ) as (mock_nw_model, mock_disk_model):
+            # osinfo returning new things
+            mock_nw_model.return_value = 'e1000'
+            mock_disk_model.return_value = 'sata'
+
+            # Assert the defaults persist across a host reboot, guest reboot,
+            # and guest migration
+            self._assert_stashed_image_properties_persist(
+                server, default_image_properties)
+
+    def test_default_image_property_persists_across_host_flag_changes(self):
+        # Set the default to ps2 via host flag
+        self.flags(pointer_model='ps2mouse')
+        # Restart compute to pick up ps2 setting, which means the guest will
+        # not get a prescribed pointer device
+        self.restart_compute_service(self.compute)
+
+        # Create a server with default image properties
+        default_image_properties1 = {
+            'hw_pointer_model': None,
+            'hw_input_bus': None,
+        }
+        server1 = self._create_server(networks='none')
+        self._assert_stashed_image_properties(
+            server1['id'], default_image_properties1)
+
+        # Assert the defaults persist across a host flag change
+        self.flags(pointer_model='usbtablet')
+        # Restart compute to pick up usb setting
+        self.restart_compute_service(self.compute)
+        self._assert_stashed_image_properties(
+            server1['id'], default_image_properties1)
+
+        # Assert the defaults persist across a host reboot, guest reboot, and
+        # guest migration
+        self._assert_stashed_image_properties_persist(
+            server1, default_image_properties1)
+
+        # Create a server with new default image properties since the host flag
+        # change
+        default_image_properties2 = {
+            'hw_pointer_model': 'usbtablet',
+            'hw_input_bus': 'usb',
+        }
+        server2 = self._create_server(networks='none')
+        self._assert_stashed_image_properties(
+            server2['id'], default_image_properties2)
+
+        # Assert the defaults persist across a host reboot, guest reboot, and
+        # guest migration
+        self._assert_stashed_image_properties_persist(
+            server2, default_image_properties2)
+
+        # Finally, try changing the host flag again to None. Note that it is
+        # not possible for a user to specify None for this option:
+        # https://bugs.launchpad.net/nova/+bug/1866106
+        self.flags(pointer_model=None)
+        # Restart compute to pick up None setting
+        self.restart_compute_service(self.compute)
+        self._assert_stashed_image_properties(
+            server1['id'], default_image_properties1)
+        self._assert_stashed_image_properties(
+            server2['id'], default_image_properties2)
+
+        # Create a server since the host flag change to None. The defaults
+        # should be the same as for ps2mouse
+        server3 = self._create_server(networks='none')
+        self._assert_stashed_image_properties(
+            server3['id'], default_image_properties1)
+
+        # Assert the defaults persist across a host reboot, guest reboot, and
+        # guest migration for server1, server2, and server3
+        self._assert_stashed_image_properties_persist(
+            server1, default_image_properties1)
+        self._assert_stashed_image_properties_persist(
+            server2, default_image_properties2)
+        self._assert_stashed_image_properties_persist(
+            server3, default_image_properties1)
@@ -870,9 +870,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
            "'swtpm_enabled=True'"
        )

-    @mock.patch.object(
-        libvirt_driver.LibvirtDriver, '_register_instance_machine_type',
-        new=mock.Mock())
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    @mock.patch.object(
        host.Host, 'supports_secure_boot', new_callable=mock.PropertyMock)
    def test_driver_capabilities_secure_boot(self, mock_supports):
@@ -886,7 +886,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        mock_supports.assert_called_once_with()

    @mock.patch.object(
-        libvirt_driver.LibvirtDriver, '_register_instance_machine_type',
+        libvirt_driver.LibvirtDriver,
+        '_register_all_undefined_instance_details',
        new=mock.Mock())
    @mock.patch.object(
        host.Host, 'supports_remote_managed_ports',
@@ -1041,7 +1042,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        any_order=True)

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    @mock.patch.object(host.Host, "has_min_version")
    def test_min_version_start_ok(self, mock_version):
        mock_version.return_value = True
@@ -1057,7 +1059,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                          "dummyhost")

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    @mock.patch.object(fakelibvirt.Connection, 'getLibVersion',
                       return_value=versionutils.convert_version_to_int(
                            libvirt_driver.NEXT_MIN_LIBVIRT_VERSION) - 1)
@@ -1087,7 +1090,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        self.assertTrue(version_arg_found)

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    @mock.patch.object(fakelibvirt.Connection, 'getVersion',
                       return_value=versionutils.convert_version_to_int(
                            libvirt_driver.NEXT_MIN_QEMU_VERSION) - 1)
@@ -1117,7 +1121,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        self.assertTrue(version_arg_found)

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    @mock.patch.object(fakelibvirt.Connection, 'getLibVersion',
                       return_value=versionutils.convert_version_to_int(
                            libvirt_driver.NEXT_MIN_LIBVIRT_VERSION))
@@ -1147,7 +1152,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        self.assertFalse(version_arg_found)

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    @mock.patch.object(fakelibvirt.Connection, 'getVersion',
                       return_value=versionutils.convert_version_to_int(
                            libvirt_driver.NEXT_MIN_QEMU_VERSION))
@@ -1177,7 +1183,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        self.assertFalse(version_arg_found)

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    def test_min_version_ppc_ok(self):
        self.mock_uname.return_value = fakelibvirt.os_uname(
            'Linux', '', '5.4.0-0-generic', '', fields.Architecture.PPC64)
@@ -1185,7 +1192,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        drvr.init_host("dummyhost")

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    def test_min_version_s390_ok(self):
        self.mock_uname.return_value = fakelibvirt.os_uname(
            'Linux', '', '5.4.0-0-generic', '', fields.Architecture.S390X)
@@ -1193,7 +1201,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        drvr.init_host("dummyhost")

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    def test_file_backed_memory_support_called(self):
        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
        with mock.patch.object(drvr,
@@ -1248,7 +1257,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        )

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    def test__prepare_cpu_flag(self):
        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)

@@ -1278,7 +1288,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        self.assertXmlEqual(expected_xml, cpu.to_xml())

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    def test__check_cpu_compatibility_start_ok(self):
        self.flags(cpu_mode="custom",
                   cpu_models=["Penryn"],
@@ -1311,7 +1322,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                          drvr.init_host, "dummyhost")

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    def test__check_cpu_compatibility_with_flag(self):
        self.flags(cpu_mode="custom",
                   cpu_models=["Penryn"],
@@ -1370,7 +1382,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        self.assertRaises(exception.Invalid, drvr.init_host, "dummyhost")

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    def test__check_cpu_compatibility_aarch64_qemu_custom_start_OK(self):
        """Test getting CPU traits when using a virt_type that doesn't support
        the feature, only kvm and qemu supports reporting CPU traits.
@@ -1388,6 +1401,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
            drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
            drvr.init_host("dummyhost")

+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    def test__check_vtpm_support_non_qemu(self):
        """Test checking for vTPM support when we're not using QEMU or KVM."""
        self.flags(swtpm_enabled=True, virt_type='lxc', group='libvirt')
@@ -1475,7 +1491,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        mock_getgrnam.assert_called_with('admins')

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    @mock.patch('shutil.which')
    @mock.patch('pwd.getpwnam')
    @mock.patch('grp.getgrnam')
@@ -2450,7 +2467,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        self.assertEqual(storage_ip, result['ip'])

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    def test_lifecycle_event_registration(self):
        calls = []

@@ -14620,6 +14638,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        mock_stat.assert_called_once_with(path)
        mock_get_size.assert_called_once_with(path)

+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_undefined_instance_details',
+                       new=mock.Mock())
    def test_spawn_with_network_info(self, power_on=True):
        def fake_getLibVersion():
            return fakelibvirt.FAKE_LIBVIRT_VERSION
@@ -14780,6 +14801,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                   accel_info=accel_info, power_on=False)
        return instance

+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_undefined_instance_details',
+                       new=mock.Mock())
    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_guest_xml')
    def test_spawn_accels_no_accel_info(self, mock_get_guest_xml):
        # accel_info should be passed to get_guest_xml even if it is []
@@ -14790,6 +14814,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
            block_device_info=None, mdevs=mock.ANY,
            accel_info=[])

+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_undefined_instance_details',
+                       new=mock.Mock())
    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_guest_xml')
    def test_spawn_accels_with_accel_info(self, mock_get_guest_xml):
        # accel_info should be passed to get_guest_xml if it is not []
@@ -14800,6 +14827,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
            block_device_info=None, mdevs=mock.ANY,
            accel_info=accel_info)

+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_undefined_instance_details',
+                       new=mock.Mock())
    # Methods called directly by spawn()
    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_guest_xml')
    @mock.patch.object(libvirt_driver.LibvirtDriver,
@@ -14847,6 +14877,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        config_disk.import_file.assert_called_once_with(instance, mock.ANY,
                                                        'disk.config')

+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_undefined_instance_details',
+                       new=mock.Mock())
    def test_spawn_without_image_meta(self):
        instance_ref = self.test_instance
        instance_ref['image_ref'] = uuids.image_ref
@@ -14871,6 +14904,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        self.assertEqual(['disk', 'disk.local'],
                         sorted(backend.created_disks.keys()))

+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_undefined_instance_details',
+                       new=mock.Mock())
    def _test_spawn_disks(self, image_ref, block_device_info):
        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)

@@ -14931,6 +14967,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        # We should have created the root and ephemeral disks
        self.assertEqual(['disk', 'disk.local'], disks_created)

+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_undefined_instance_details',
+                       new=mock.Mock())
    def test_spawn_lxc_from_volume(self):
        self.flags(virt_type="lxc",
                   group='libvirt')
@@ -15021,6 +15060,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                             inst_obj.system_metadata.get(
                             'rootfs_device_name'))

+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_undefined_instance_details',
+                       new=mock.Mock())
    def test_spawn_with_pci_devices(self):
        class FakeLibvirtPciDevice(object):
            def dettach(self):
@@ -15065,6 +15107,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                               return_value=mock_connection):
            drvr.spawn(self.context, instance, image_meta, [], None, {})

+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_undefined_instance_details',
+                       new=mock.Mock())
    @mock.patch('nova.crypto.ensure_vtpm_secret')
    @mock.patch.object(hardware, 'get_vtpm_constraint')
    @mock.patch(
@@ -15859,7 +15904,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                                          'my_ip': mock.ANY})

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    def test_init_host_checks_ip(self):
        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
        with mock.patch.object(drvr, '_check_my_ip') as mock_check:
@@ -15915,7 +15961,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
            self.assertTrue(service_mock.disabled)

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    def test_service_resume_after_broken_connection(self):
        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
        service_mock = mock.MagicMock()
@@ -20053,7 +20100,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                              driver.init_host, 'wibble')

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    @mock.patch.object(fakelibvirt.Connection, 'getVersion',
                       return_value=versionutils.convert_version_to_int(
                           libvirt_driver.MIN_VIRTUOZZO_VERSION))
@@ -26030,7 +26078,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
                         drvr._get_existing_mdevs_not_assigned(parent=None))

    @mock.patch.object(libvirt_driver.LibvirtDriver,
-                       '_register_instance_machine_type', new=mock.Mock())
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
    @mock.patch('nova.compute.utils.get_machine_ips',
                new=mock.Mock(return_value=[]))
    @mock.patch.object(nova.privsep.libvirt, 'create_mdev')
@@ -26746,12 +26795,13 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
        mock_execute.assert_called_once_with('qemu-img', 'rebase',
                                             '-b', '', 'disk')

+    @mock.patch('nova.objects.instance.Instance.save')
    @mock.patch('nova.objects.instance.InstanceList.get_by_host')
    @mock.patch('nova.virt.libvirt.host.Host.get_hostname',
        new=mock.Mock(return_value=mock.sentinel.hostname))
    @mock.patch('nova.context.get_admin_context', new=mock.Mock())
    def test_register_machine_type_already_registered_image_metadata(
-        self, mock_get_by_host
+        self, mock_get_by_host, mock_instance_save,
    ):
        instance = self._create_instance(
            params={
@@ -26761,7 +26811,14 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
            }
        )
        mock_get_by_host.return_value = [instance]
-        self.drvr._register_instance_machine_type()
+
+        # We only care about hw_machine_type for this test
+        with mock.patch(
+            'nova.virt.libvirt.driver.REGISTER_IMAGE_PROPERTY_DEFAULTS',
+            ['hw_machine_type']
+        ):
+            self.drvr._register_all_undefined_instance_details()
+
        # Assert that we don't overwrite the existing type
        self.assertEqual(
            'existing_type',
@@ -26771,6 +26828,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
            'existing_type',
            instance.system_metadata.get('image_hw_machine_type')
        )
+        mock_instance_save.assert_not_called()

    @mock.patch('nova.objects.instance.Instance.save')
    @mock.patch('nova.objects.instance.InstanceList.get_by_host')
@@ -26783,7 +26841,14 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
    ):
        instance = self._create_instance()
        mock_get_by_host.return_value = [instance]
-        self.drvr._register_instance_machine_type()
+
+        # We only care about hw_machine_type for this test
+        with mock.patch(
+            'nova.virt.libvirt.driver.REGISTER_IMAGE_PROPERTY_DEFAULTS',
+            ['hw_machine_type']
+        ):
+            self.drvr._register_all_undefined_instance_details()
+
        mock_instance_save.assert_called_once()
        self.assertEqual(
            'conf_type',
@@ -26794,6 +26859,143 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
            instance.system_metadata.get('image_hw_machine_type')
        )

+    @mock.patch('nova.virt.libvirt.driver.LOG.exception')
+    @mock.patch('nova.objects.instance.InstanceList.get_by_host')
+    @mock.patch('nova.virt.libvirt.host.Host.get_hostname', new=mock.Mock())
+    def test_register_all_undefined_details_unknown_failure(
+        self, mock_get_by_host, mock_log_exc
+    ):
+        instance = self._create_instance()
+        mock_get_by_host.return_value = [instance]
+
+        # Assert that we swallow anything raised below us
+        with mock.patch.object(
+            self.drvr,
+            '_register_undefined_instance_details',
+            side_effect=test.TestingException()
+        ):
+            self.drvr._register_all_undefined_instance_details()
+
+        # Assert that we logged the failure
+        self.assertEqual(1, mock_log_exc.call_count)
+        self.assertIn('Ignoring unknown failure while attempting '
+                      'to save the defaults for unregistered image properties',
+                      mock_log_exc.call_args.args[0])
+
+    @mock.patch('nova.virt.libvirt.driver.LOG.exception')
+    @mock.patch('nova.objects.instance.Instance.save')
+    @mock.patch('nova.virt.libvirt.host.Host.get_guest', new=mock.Mock())
+    @mock.patch('nova.objects.block_device.BlockDeviceMappingList.'
+                'get_by_instance_uuid')
+    @mock.patch('nova.objects.instance.InstanceList.get_by_host')
+    @mock.patch('nova.virt.libvirt.host.Host.get_hostname', new=mock.Mock())
+    def test_register_all_undefined_details_unknown_failure_finding_default(
+        self, mock_get_by_host, mock_get_bdms, mock_save, mock_log_exc
+    ):
+        instance = self._create_instance()
+        mock_get_by_host.return_value = [instance]
+        mock_get_bdms.return_value = []
+
+        # Assert that we swallow anything raised below us
+        with mock.patch.object(
+            self.drvr,
+            '_find_default_for_image_property',
+            side_effect=test.TestingException()
+        ):
+            self.drvr._register_all_undefined_instance_details()
+
+        # Assert that we logged the failures (once for each unregistered
+        # image property)
+        self.assertEqual(len(libvirt_driver.REGISTER_IMAGE_PROPERTY_DEFAULTS),
+                         mock_log_exc.call_count)
+        self.assertIn('Ignoring unknown failure while attempting '
+                      'to find the default of',
+                      mock_log_exc.call_args.args[0])
+
+        # Assert that we updated the instance
+        mock_save.assert_called_once_with()
+
+    @mock.patch('nova.objects.instance.Instance.save',
+                new=mock.NonCallableMock())
+    @mock.patch('nova.virt.libvirt.host.Host.get_guest',
+                new=mock.NonCallableMock())
+    @mock.patch('nova.objects.block_device.BlockDeviceMappingList.'
+                'get_by_instance_uuid', new=mock.NonCallableMock())
+    def test_register_undefined_instance_details_nothing_to_register(self):
+        instance = self._create_instance()
+
+        # Set a value for all REGISTER_IMAGE_PROPERTY_DEFAULTS
+        for p in libvirt_driver.REGISTER_IMAGE_PROPERTY_DEFAULTS:
+            instance.system_metadata[f"image_{p}"] = 'foo'
+
+        # We should not have pulled bdms or updated the instance
+        self.drvr._register_undefined_instance_details(self.context, instance)
+
+    @mock.patch('nova.objects.instance.Instance.save')
+    @mock.patch('nova.virt.libvirt.host.Host.get_guest')
+    @mock.patch('nova.objects.block_device.BlockDeviceMappingList.'
+                'get_by_instance_uuid')
+    def test_register_undefined_instance_details_disk_info_and_guest_config(
+        self, mock_get_bdms, mock_get_guest, mock_save
+    ):
+        instance = self._create_instance()
+        mock_get_bdms.return_value = []
+
+        # Test all props unregistered
+        with mock.patch.object(
+            self.drvr,
+            '_find_default_for_image_property'
+        ) as mock_find:
+            self.drvr._register_undefined_instance_details(self.context,
+                                                           instance)
+        # We should have pulled bdms
+        mock_get_bdms.assert_called_once_with(self.context, instance.uuid)
+        # We should have pulled disk_info
+        self.assertIsNotNone(mock_find.call_args.args[2])
+        # We should have pulled guest config
+        mock_get_guest.return_value.get_config.assert_called_once_with()
+        self.assertIsNotNone(mock_find.call_args.args[3])
+
+        # Set one of ['hw_disk_bus', 'hw_cdrom_bus']
+        # Set one of ['hw_pointer_model', 'hw_input_bus']
+        mock_get_bdms.reset_mock()
+        mock_get_guest.reset_mock()
+        instance.system_metadata['image_hw_disk_bus'] = 'scsi'
+        instance.system_metadata['image_hw_pointer_model'] = None
+        with mock.patch.object(
+            self.drvr,
+            '_find_default_for_image_property'
+        ) as mock_find:
+            self.drvr._register_undefined_instance_details(self.context,
+                                                           instance)
+        # We should have pulled bdms
+        mock_get_bdms.assert_called_once_with(self.context, instance.uuid)
+        # We should have pulled disk_info
+        self.assertIsNotNone(mock_find.call_args.args[2])
+        # We should have pulled guest config
+        mock_get_guest.return_value.get_config.assert_called_once_with()
+        self.assertIsNotNone(mock_find.call_args.args[3])
+
+        # Set the other, now we have both ['hw_disk_bus', 'hw_cdrom_bus']
+        # Set the other, now we have both ['hw_pointer_model', 'hw_input_bus']
+        mock_get_bdms.reset_mock()
+        mock_get_guest.reset_mock()
+        instance.system_metadata['image_hw_cdrom_bus'] = 'scsi'
+        instance.system_metadata['image_hw_input_bus'] = None
+        with mock.patch.object(
+            self.drvr,
+            '_find_default_for_image_property'
+        ) as mock_find:
+            self.drvr._register_undefined_instance_details(self.context,
+                                                           instance)
+        # We should not have pulled bdms at all
+        mock_get_bdms.assert_not_called()
+        # And disk_info should not have been pulled
+        self.assertIsNone(mock_find.call_args.args[2])
+        # We should not have pulled guest config
+        mock_get_guest.return_value.assert_not_called()
+        self.assertIsNone(mock_find.call_args.args[3])
+

 class LibvirtVolumeUsageTestCase(test.NoDBTestCase):
    """Test for LibvirtDriver.get_all_volume_usage."""
@@ -244,6 +244,16 @@ LIBVIRT_PERF_EVENT_PREFIX = 'VIR_PERF_PARAM_'
 MIN_LIBVIRT_VDPA = (6, 9, 0)
 MIN_QEMU_VDPA = (5, 1, 0)

+REGISTER_IMAGE_PROPERTY_DEFAULTS = [
+    'hw_machine_type',
+    'hw_cdrom_bus',
+    'hw_disk_bus',
+    'hw_input_bus',
+    'hw_pointer_model',
+    'hw_video_model',
+    'hw_vif_model',
+]
+

 class AsyncDeviceEventsHandler:
    """A synchornization point between libvirt events an clients waiting for
@@ -805,7 +815,9 @@ class LibvirtDriver(driver.ComputeDriver):

        self._check_vtpm_support()

-        self._register_instance_machine_type()
+        # Set REGISTER_IMAGE_PROPERTY_DEFAULTS in the instance system_metadata
+        # to default values for properties that have not already been set.
+        self._register_all_undefined_instance_details()

    def _update_host_specific_capabilities(self) -> None:
        """Update driver capabilities based on capabilities of the host."""
@@ -817,34 +829,114 @@ class LibvirtDriver(driver.ComputeDriver):
            self._host.supports_remote_managed_ports
        })

-    def _register_instance_machine_type(self):
-        """Register the machine type of instances on this host
+    def _register_all_undefined_instance_details(self) -> None:
+        """Register the default image properties of instances on this host

        For each instance found on this host by InstanceList.get_by_host ensure
-        a machine type is registered within the system metadata of the instance
+        REGISTER_IMAGE_PROPERTY_DEFAULTS are registered within the system
+        metadata of the instance
        """
        context = nova_context.get_admin_context()
        hostname = self._host.get_hostname()
+        for instance in objects.InstanceList.get_by_host(
+            context, hostname, expected_attrs=['flavor', 'system_metadata']
+        ):
+            try:
+                self._register_undefined_instance_details(context, instance)
+            except Exception:
+                LOG.exception('Ignoring unknown failure while attempting '
+                              'to save the defaults for unregistered image '
+                              'properties', instance=instance)

-        for instance in objects.InstanceList.get_by_host(context, hostname):
-            # NOTE(lyarwood): Skip if hw_machine_type is set already in the
-            # image_meta of the instance. Note that this value comes from the
-            # system metadata of the instance where it is stored under the
-            # image_hw_machine_type key.
-            if instance.image_meta.properties.get('hw_machine_type'):
-                continue
+    def _register_undefined_instance_details(
+        self,
+        context: nova_context.RequestContext,
+        instance: 'objects.Instance',
+    ) -> None:
+        # Find any unregistered image properties against this instance
+        unregistered_image_props = [
+            p for p in REGISTER_IMAGE_PROPERTY_DEFAULTS
+            if f"image_{p}" not in instance.system_metadata
+        ]

-            # Fetch and record the machine type from the config
-            hw_machine_type = libvirt_utils.get_machine_type(
-                instance.image_meta)
-            # NOTE(lyarwood): As above this updates
-            # image_meta.properties.hw_machine_type within the instance and
-            # will be returned the next time libvirt_utils.get_machine_type is
-            # called for the instance image meta.
-            instance.system_metadata['image_hw_machine_type'] = hw_machine_type
-            instance.save()
-            LOG.debug("Instance machine_type updated to %s", hw_machine_type,
-                      instance=instance)
+        # Return if there's nothing left to register for this instance
+        if not unregistered_image_props:
+            return
+
+        LOG.debug(f'Attempting to register defaults for the following '
+                  f'image properties: {unregistered_image_props}',
+                  instance=instance)
+
+        # NOTE(lyarwood): Only build disk_info once per instance if we need it
+        # for hw_{disk,cdrom}_bus to avoid pulling bdms from the db etc.
+        requires_disk_info = ['hw_disk_bus', 'hw_cdrom_bus']
+        disk_info = None
+        if set(requires_disk_info) & set(unregistered_image_props):
+            bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
+                context, instance.uuid)
+            block_device_info = driver.get_block_device_info(instance, bdms)
+            disk_info = blockinfo.get_disk_info(
+                CONF.libvirt.virt_type, instance, instance.image_meta,
+                block_device_info)
+
+        # Only pull the guest config once per instance if we need it for
+        # hw_pointer_model or hw_input_bus.
+        requires_guest_config = ['hw_pointer_model', 'hw_input_bus']
+        guest_config = None
+        if set(requires_guest_config) & set(unregistered_image_props):
+            guest_config = self._host.get_guest(instance).get_config()
+
+        for image_prop in unregistered_image_props:
+            try:
+                default_value = self._find_default_for_image_property(
+                    instance, image_prop, disk_info, guest_config)
+                instance.system_metadata[f"image_{image_prop}"] = default_value
+
+                LOG.debug(f'Found default for {image_prop} of {default_value}',
+                          instance=instance)
+            except Exception:
+                LOG.exception(f'Ignoring unknown failure while attempting '
+                              f'to find the default of {image_prop}',
+                              instance=instance)
+        instance.save()
+
+    def _find_default_for_image_property(
+        self,
+        instance: 'objects.Instance',
+        image_property: str,
+        disk_info: ty.Optional[ty.Dict[str, ty.Any]],
+        guest_config: ty.Optional[vconfig.LibvirtConfigGuest],
+    ) -> ty.Optional[str]:
+        if image_property == 'hw_machine_type':
+            return libvirt_utils.get_machine_type(instance.image_meta)
+
+        if image_property == 'hw_disk_bus' and disk_info:
+            return disk_info.get('disk_bus')
+
+        if image_property == 'hw_cdrom_bus' and disk_info:
+            return disk_info.get('cdrom_bus')
+
+        if image_property == 'hw_input_bus' and guest_config:
+            _, default_input_bus = self._get_pointer_bus_and_model(
+                guest_config, instance.image_meta)
+            return default_input_bus
+
+        if image_property == 'hw_pointer_model' and guest_config:
+            default_pointer_model, _ = self._get_pointer_bus_and_model(
+                guest_config, instance.image_meta)
+            # hw_pointer_model is of type PointerModelType ('usbtablet' instead
+            # of 'tablet')
+            if default_pointer_model == 'tablet':
+                default_pointer_model = 'usbtablet'
+            return default_pointer_model
+
+        if image_property == 'hw_video_model':
+            return self._get_video_type(instance.image_meta)
+
+        if image_property == 'hw_vif_model':
+            return self.vif_driver.get_vif_model(instance.image_meta)
+
+        return None

    def _prepare_cpu_flag(self, flag):
        # NOTE(kchamart) This helper method will be used while computing
@@ -4247,6 +4339,11 @@ class LibvirtDriver(driver.ComputeDriver):
        else:
            LOG.info("Instance spawned successfully.", instance=instance)

+        # Finally register defaults for any undefined image properties so that
+        # future changes by QEMU, libvirt or within this driver don't change
+        # the ABI of the instance.
+        self._register_undefined_instance_details(context, instance)
+
    def _get_console_output_file(self, instance, console_log):
        bytes_to_read = MAX_CONSOLE_BYTES
        log_data = b""  # The last N read bytes
@@ -5954,40 +6051,7 @@ class LibvirtDriver(driver.ComputeDriver):

    def _add_video_driver(self, guest, image_meta, flavor):
        video = vconfig.LibvirtConfigGuestVideo()
-        # NOTE(ldbragst): The following logic sets the video.type
-        # depending on supported defaults given the architecture,
-        # virtualization type, and features. The video.type attribute can
-        # be overridden by the user with image_meta.properties, which
-        # is carried out in the next if statement below this one.
-        guestarch = libvirt_utils.get_arch(image_meta)
-        if CONF.libvirt.virt_type == 'parallels':
-            video.type = 'vga'
-        # NOTE(kchamart): 'virtio' is a sensible default whether or not
-        # the guest has the native kernel driver (called "virtio-gpu" in
-        # Linux) -- i.e. if the guest has the VirtIO GPU driver, it'll
-        # be used; otherwise, the 'virtio' model will gracefully
-        # fallback to VGA compatibiliy mode.
-        elif (guestarch in (fields.Architecture.I686,
-                            fields.Architecture.X86_64) and not
-                            CONF.spice.enabled):
-            video.type = 'virtio'
-        elif guestarch in (fields.Architecture.PPC,
-                           fields.Architecture.PPC64,
-                           fields.Architecture.PPC64LE):
-            # NOTE(ldbragst): PowerKVM doesn't support 'cirrus' be default
-            # so use 'vga' instead when running on Power hardware.
-            video.type = 'vga'
-        elif guestarch == fields.Architecture.AARCH64:
-            # NOTE(kevinz): Only virtio device type is supported by AARCH64
-            # so use 'virtio' instead when running on AArch64 hardware.
-            video.type = 'virtio'
-        elif CONF.spice.enabled:
-            video.type = 'qxl'
-        if image_meta.properties.get('hw_video_model'):
-            video.type = image_meta.properties.hw_video_model
-            if not self._video_model_supported(video.type):
-                raise exception.InvalidVideoMode(model=video.type)
-
+        video.type = self._get_video_type(image_meta) or video.type
        # Set video memory, only if the flavor's limit is set
        video_ram = image_meta.properties.get('hw_video_ram', 0)
        max_vram = int(flavor.extra_specs.get('hw_video:ram_max_mb', 0))
@@ -6002,6 +6066,62 @@ class LibvirtDriver(driver.ComputeDriver):
        # for simpler testing.
        return video

+    def _get_video_type(
+        self,
+        image_meta: objects.ImageMeta,
+    ) -> ty.Optional[str]:
+        # NOTE(ldbragst): The following logic returns the video type
+        # depending on supported defaults given the architecture,
+        # virtualization type, and features. The video type can
+        # be overridden by the user with image_meta.properties, which
+        # is carried out first.
+        if image_meta.properties.get('hw_video_model'):
+            video_type = image_meta.properties.hw_video_model
+            if not self._video_model_supported(video_type):
+                raise exception.InvalidVideoMode(model=video_type)
+            return video_type
+
+        guestarch = libvirt_utils.get_arch(image_meta)
+
+        if CONF.libvirt.virt_type == 'parallels':
+            return 'vga'
+
+        # NOTE(kchamart): 'virtio' is a sensible default whether or not
+        # the guest has the native kernel driver (called "virtio-gpu" in
+        # Linux) -- i.e. if the guest has the VirtIO GPU driver, it'll
+        # be used; otherwise, the 'virtio' model will gracefully
+        # fallback to VGA compatibiliy mode.
+        if (
+            guestarch in (
+                fields.Architecture.I686,
+                fields.Architecture.X86_64
+            ) and not CONF.spice.enabled
+        ):
+            return 'virtio'
+
+        if (
+            guestarch in (
+                fields.Architecture.PPC,
+                fields.Architecture.PPC64,
+                fields.Architecture.PPC64LE
+            )
+        ):
+            # NOTE(ldbragst): PowerKVM doesn't support 'cirrus' be default
+            # so use 'vga' instead when running on Power hardware.
+            return 'vga'
+
+        if guestarch == fields.Architecture.AARCH64:
+            # NOTE(kevinz): Only virtio device type is supported by AARCH64
+            # so use 'virtio' instead when running on AArch64 hardware.
+            return 'virtio'
+
+        if CONF.spice.enabled:
+            return 'qxl'
+
+        # NOTE(lyarwood): Return None and default to the default of
+        # LibvirtConfigGuestVideo.type that is currently virtio
+        return None
+
    def _add_qga_device(self, guest, instance):
        qga = vconfig.LibvirtConfigGuestChannel()
        qga.type = "unix"
@@ -6975,13 +7095,11 @@ class LibvirtDriver(driver.ComputeDriver):

        return add_video_driver

-    def _guest_add_pointer_device(self, guest, image_meta):
-        """Build the pointer device to add to the instance.
-
-        The configuration is determined by examining the 'hw_input_bus' image
-        metadata property, the 'hw_pointer_model' image metadata property, and
-        the '[DEFAULT] pointer_model' config option in that order.
-        """
+    def _get_pointer_bus_and_model(
+        self,
+        guest: vconfig.LibvirtConfigGuest,
+        image_meta: objects.ImageMeta,
+    ) -> ty.Tuple[ty.Optional[str], ty.Optional[str]]:
        pointer_bus = image_meta.properties.get('hw_input_bus')
        pointer_model = image_meta.properties.get('hw_pointer_model')

@@ -6995,7 +7113,7 @@ class LibvirtDriver(driver.ComputeDriver):
        else:
            # If the user hasn't requested anything and the host config says to
            # use something other than a USB tablet, there's nothing to do
-            return
+            return None, None

        # For backward compatibility, we don't want to error out if the host
        # configuration requests a USB tablet but the virtual machine mode is
@@ -7005,7 +7123,7 @@ class LibvirtDriver(driver.ComputeDriver):
                'USB tablet requested for guests on non-HVM host; '
                'in order to accept this request the machine mode should '
                'be configured as HVM.')
-            return
+            return None, None

        # Ditto for using a USB tablet when the SPICE agent is enabled, since
        # that has a paravirt mouse builtin which drastically reduces overhead;
@@ -7019,15 +7137,32 @@ class LibvirtDriver(driver.ComputeDriver):
                'USB tablet requested for guests but the SPICE agent is '
                'enabled; ignoring request in favour of default '
                'configuration.')
-            return
+            return None, None

-        pointer = vconfig.LibvirtConfigGuestInput()
-        pointer.type = pointer_model
-        pointer.bus = pointer_bus
-        guest.add_device(pointer)
+        return pointer_model, pointer_bus

-        # returned for unit testing purposes
-        return pointer
+    def _guest_add_pointer_device(
+        self,
+        guest: vconfig.LibvirtConfigGuest,
+        image_meta: objects.ImageMeta
+    ) -> None:
+        """Build the pointer device to add to the instance.
+
+        The configuration is determined by examining the 'hw_input_bus' image
+        metadata property, the 'hw_pointer_model' image metadata property, and
+        the '[DEFAULT] pointer_model' config option in that order.
+        """
+        pointer_model, pointer_bus = self._get_pointer_bus_and_model(
+            guest, image_meta)
+
+        if pointer_model and pointer_bus:
+            pointer = vconfig.LibvirtConfigGuestInput()
+            pointer.type = pointer_model
+            pointer.bus = pointer_bus
+            guest.add_device(pointer)
+
+            # returned for unit testing purposes
+            return pointer

    def _guest_add_keyboard_device(self, guest, image_meta):
        """Add keyboard for graphical console use."""
@@ -0,0 +1,15 @@
+other:
+  - |
+    Default image properties for device buses and models are now persisted in
+    the instance system metadata for the following image properties:
+
+    * ``hw_cdrom_bus``
+    * ``hw_disk_bus``
+    * ``hw_input_bus``
+    * ``hw_pointer_model``
+    * ``hw_video_model``
+    * ``hw_vif_model``
+
+    Instance device buses and models will now remain stable across reboots and
+    will not be changed by new defaults in libosinfo or the OpenStack Nova
+    libvirt driver.