libvirt: Record the machine_type of instances in system_metadata

This change starts to record the machine type of instances registered on
a given host during init_host or later in _configure_guest_by_virt_type
when spawning a new instance.

The machine type is recorded in the system metadata of the instance
under the ``image_hw_machine_type`` key as already used to store the
image metadata property ``hw_machine_type``. As a result no changes are
required to the code of libvirt_utils.get_machine_type that looks up the
machine type of an instance based on it's image metadata and host
config.

Functional tests are included to verify the basic behaviour of both this
new registration code and now this leads to instances being effectively
pinned to the machine type used during their initial spawn.

blueprint: libvirt-default-machine-type
Change-Id: I30c780a7729f1e7d791256bdc38d73b976c56268

nova/tests/functional/libvirt/test_machine_type.py

@@ -0,0 +1,180 @@
+# 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 copy
+import fixtures
+
+from oslo_utils.fixture import uuidsentinel
+
+from nova import context as nova_context
+from nova import objects
+from nova.tests.functional.libvirt import base
+
+
+class LibvirtMachineTypeTest(base.ServersTestBase):
+
+    microversion = 'latest'
+
+    def setUp(self):
+        super().setUp()
+        self.context = nova_context.get_admin_context()
+
+        # Add the q35 image to the glance fixture
+        hw_machine_type_q35_image = copy.deepcopy(self.glance.image1)
+        hw_machine_type_q35_image['id'] = uuidsentinel.q35_image_id
+        hw_machine_type_q35_image['properties']['hw_machine_type'] = 'q35'
+        self.glance.create(self.context, hw_machine_type_q35_image)
+
+        # Create a pass-through mock around _get_guest_config to capture the
+        # config of an instance so we can assert things about it later.
+        # TODO(lyarwood): This seems like a useful thing to do in the libvirt
+        # func tests for all computes we start?
+        self.start_compute()
+        self.guest_configs = {}
+        orig_get_config = self.computes['compute1'].driver._get_guest_config
+
+        def _get_guest_config(_self, *args, **kwargs):
+            guest_config = orig_get_config(*args, **kwargs)
+            instance = args[0]
+            self.guest_configs[instance.uuid] = guest_config
+            return self.guest_configs[instance.uuid]
+
+        self.useFixture(fixtures.MonkeyPatch(
+            'nova.virt.libvirt.LibvirtDriver._get_guest_config',
+            _get_guest_config))
+
+    def _create_servers(self):
+        server_with = self._create_server(
+            image_uuid=uuidsentinel.q35_image_id,
+            networks='none',
+        )
+        server_without = self._create_server(
+            image_uuid=self.glance.image1['id'],
+            networks='none',
+        )
+        return (server_with, server_without)
+
+    def _assert_machine_type(self, server_id, expected_machine_type):
+        instance = objects.Instance.get_by_uuid(self.context, server_id)
+        self.assertEqual(
+            expected_machine_type,
+            instance.image_meta.properties.hw_machine_type
+        )
+        self.assertEqual(
+            expected_machine_type,
+            instance.system_metadata['image_hw_machine_type']
+        )
+        self.assertEqual(
+            expected_machine_type,
+            self.guest_configs[server_id].os_mach_type
+        )
+
+    def test_init_host_register_machine_type(self):
+        """Assert that the machine type of an instance is recorded during
+           init_host if not already captured by an image prop.
+        """
+        self.flags(hw_machine_type='x86_64=pc', group='libvirt')
+
+        server_with, server_without = self._create_servers()
+        self._assert_machine_type(server_with['id'], 'q35')
+        self._assert_machine_type(server_without['id'], 'pc')
+
+        # Stop n-cpu and clear the recorded machine type from server_without to
+        # allow init_host to register the machine type.
+        self.computes['compute1'].stop()
+        instance_without = objects.Instance.get_by_uuid(
+            self.context,
+            server_without['id'],
+        )
+        instance_without.system_metadata.pop('image_hw_machine_type')
+        instance_without.save()
+
+        self.flags(hw_machine_type='x86_64=pc-q35-1.2.3', group='libvirt')
+
+        # Restart the compute
+        self.computes['compute1'].start()
+
+        # Assert the server_with remains pinned to q35
+        self._assert_machine_type(server_with['id'], 'q35')
+
+        # reboot the server so the config is rebuilt and _assert_machine_type
+        # is able to pass. This just keeps the tests clean.
+        self._reboot_server(server_without, hard=True)
+
+        # Assert server_without now has a machine type of pc-q35-1.2.3 picked
+        # up from [libvirt]hw_machine_type during init_host
+        self._assert_machine_type(server_without['id'], 'pc-q35-1.2.3')
+
+    def test_machine_type_after_config_change(self):
+        """Assert new instances pick up a new default machine type after the
+           config has been updated.
+        """
+        self.flags(hw_machine_type='x86_64=pc', group='libvirt')
+
+        server_with, server_without = self._create_servers()
+        self._assert_machine_type(server_with['id'], 'q35')
+        self._assert_machine_type(server_without['id'], 'pc')
+
+        self.flags(hw_machine_type='x86_64=pc-q35-1.2.3', group='libvirt')
+
+        server_with_new, server_without_new = self._create_servers()
+        self._assert_machine_type(server_with_new['id'], 'q35')
+        self._assert_machine_type(server_without_new['id'], 'pc-q35-1.2.3')
+
+    def test_machine_type_after_server_rebuild(self):
+        """Assert that the machine type of an instance changes with a full
+           rebuild of the instance pointing at a new image.
+        """
+        self.flags(hw_machine_type='x86_64=pc', group='libvirt')
+
+        server_with, server_without = self._create_servers()
+        self._assert_machine_type(server_with['id'], 'q35')
+        self._assert_machine_type(server_without['id'], 'pc')
+
+        # rebuild each server with the opposite image
+        self._rebuild_server(
+            server_with,
+            '155d900f-4e14-4e4c-a73d-069cbf4541e6',
+        )
+        self._rebuild_server(
+            server_without,
+            uuidsentinel.q35_image_id
+        )
+
+        # Assert that the machine types were updated during the rebuild
+        self._assert_machine_type(server_with['id'], 'pc')
+        self._assert_machine_type(server_without['id'], 'q35')
+
+    def _test_machine_type_after_server_reboot(self, hard=False):
+        """Assert that the recorded machine types don't change with the
+           reboot of a server, even when the underlying config changes.
+        """
+        self.flags(hw_machine_type='x86_64=pc', group='libvirt')
+
+        server_with, server_without = self._create_servers()
+        self._assert_machine_type(server_with['id'], 'q35')
+        self._assert_machine_type(server_without['id'], 'pc')
+
+        self.flags(hw_machine_type='x86_64=pc-q35-1.2.3', group='libvirt')
+
+        self._reboot_server(server_with, hard=hard)
+        self._reboot_server(server_without, hard=hard)
+
+        # Assert that the machine types don't change after a reboot
+        self._assert_machine_type(server_with['id'], 'q35')
+        self._assert_machine_type(server_without['id'], 'pc')
+
+    def test_machine_type_after_server_soft_reboot(self):
+        self._test_machine_type_after_server_reboot()
+
+    def test_machine_type_after_server_hard_reboot(self):
+        self._test_machine_type_after_server_reboot(hard=True)

nova/tests/unit/virt/libvirt/test_driver.py

@@ -1264,6 +1264,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         ],
         any_order=True)
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', 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
@@ -1278,6 +1280,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                           drvr.init_host,
                           "dummyhost")
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', new=mock.Mock())
     @mock.patch.object(fakelibvirt.Connection, 'getLibVersion',
                        return_value=versionutils.convert_version_to_int(
                             libvirt_driver.NEXT_MIN_LIBVIRT_VERSION) - 1)
@@ -1306,6 +1310,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                 break
         self.assertTrue(version_arg_found)
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', new=mock.Mock())
     @mock.patch.object(fakelibvirt.Connection, 'getVersion',
                        return_value=versionutils.convert_version_to_int(
                             libvirt_driver.NEXT_MIN_QEMU_VERSION) - 1)
@@ -1334,6 +1340,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                 break
         self.assertTrue(version_arg_found)
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', new=mock.Mock())
     @mock.patch.object(fakelibvirt.Connection, 'getLibVersion',
                        return_value=versionutils.convert_version_to_int(
                             libvirt_driver.NEXT_MIN_LIBVIRT_VERSION))
@@ -1362,6 +1370,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                 break
         self.assertFalse(version_arg_found)
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', new=mock.Mock())
     @mock.patch.object(fakelibvirt.Connection, 'getVersion',
                        return_value=versionutils.convert_version_to_int(
                             libvirt_driver.NEXT_MIN_QEMU_VERSION))
@@ -1390,18 +1400,24 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                 break
         self.assertFalse(version_arg_found)
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', new=mock.Mock())
     @mock.patch.object(fields.Architecture, "from_host",
                        return_value=fields.Architecture.PPC64)
     def test_min_version_ppc_ok(self, mock_arch):
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
         drvr.init_host("dummyhost")
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', new=mock.Mock())
     @mock.patch.object(fields.Architecture, "from_host",
                        return_value=fields.Architecture.S390X)
     def test_min_version_s390_ok(self, mock_arch):
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
         drvr.init_host("dummyhost")
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', new=mock.Mock())
     def test_file_backed_memory_support_called(self):
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
         with mock.patch.object(drvr,
@@ -1455,6 +1471,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
             str(mock_log.call_args[0]),
         )
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', new=mock.Mock())
     def test__check_cpu_compatibility_start_ok(self):
         self.flags(cpu_mode="custom",
                    cpu_models=["Penryn"],
@@ -1486,6 +1504,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         self.assertRaises(exception.InvalidCPUInfo,
                           drvr.init_host, "dummyhost")
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', new=mock.Mock())
     def test__check_cpu_compatibility_with_flag(self):
         self.flags(cpu_mode="custom",
                    cpu_models=["Penryn"],
@@ -1527,6 +1547,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
         self.assertRaises(exception.Invalid, drvr.init_host, "dummyhost")
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', 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.
@@ -1630,6 +1652,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         )
         mock_getgrnam.assert_called_with('admins')
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', new=mock.Mock())
     @mock.patch('shutil.which')
     @mock.patch('pwd.getpwnam')
     @mock.patch('grp.getgrnam')
@@ -2603,6 +2627,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         result = drvr.get_volume_connector(volume)
         self.assertEqual(storage_ip, result['ip'])
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', new=mock.Mock())
     def test_lifecycle_event_registration(self):
         calls = []
 
@@ -2840,6 +2866,36 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         # i440fx is not pcie machine so there should be no pcie ports
         self.assertEqual(0, num_ports)
 
+    @mock.patch('nova.virt.libvirt.utils.get_default_machine_type',
+                new=mock.Mock(return_value='config-machine_type'))
+    def test_get_guest_config_records_machine_type_in_instance(self):
+        # Assert that the config derived machine type is used when it
+        # isn't present in the image_meta of an instance.
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        instance = objects.Instance(**self.test_instance)
+        image_meta = objects.ImageMeta.from_dict({})
+        disk_info = blockinfo.get_disk_info(
+            CONF.libvirt.virt_type,
+            instance,
+            image_meta
+        )
+
+        cfg = drvr._get_guest_config(
+            instance,
+            _fake_network_info(self),
+            image_meta,
+            disk_info
+        )
+
+        self.assertEqual(
+            'config-machine_type',
+            instance.system_metadata.get('image_hw_machine_type'),
+        )
+        self.assertEqual(
+            'config-machine_type',
+            cfg.os_mach_type,
+        )
+
     def test_get_guest_config_missing_ownership_info(self):
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
 
@@ -7581,9 +7637,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                                             instance_ref,
                                             image_meta)
 
-        return drvr._get_guest_config(instance_ref,
-                                      _fake_network_info(self),
-                                      image_meta, disk_info)
+        return drvr._get_guest_config(
+            instance_ref, _fake_network_info(self), image_meta, disk_info)
 
     def test_get_guest_config_machine_type_through_image_meta(self):
         cfg = self._get_guest_config_machine_type_through_image_meta(
@@ -15529,6 +15584,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                                          {'ifaces': '8.8.8.8, 75.75.75.75',
                                           'my_ip': mock.ANY})
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', 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:
@@ -15583,6 +15640,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                               drvr.init_host, ("wibble",))
             self.assertTrue(service_mock.disabled)
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', new=mock.Mock())
     def test_service_resume_after_broken_connection(self):
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
         service_mock = mock.MagicMock()
@@ -19793,6 +19852,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
             self.assertRaises(exception.NovaException,
                               driver.init_host, 'wibble')
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', new=mock.Mock())
     @mock.patch.object(fakelibvirt.Connection, 'getVersion',
                        return_value=versionutils.convert_version_to_int(
                            libvirt_driver.MIN_VIRTUOZZO_VERSION))
@@ -24913,6 +24974,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
         self.assertEqual(set([uuids.mdev1]),
                          drvr._get_existing_mdevs_not_assigned(parent=None))
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_instance_machine_type', new=mock.Mock())
     @mock.patch('nova.compute.utils.get_machine_ips',
                 new=mock.Mock(return_value=[]))
     @mock.patch.object(nova.privsep.libvirt, 'create_mdev')
@@ -25579,6 +25642,54 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
         mock_execute.assert_called_once_with('qemu-img', 'rebase',
                                              '-b', '', 'disk')
 
+    @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
+    ):
+        instance = self._create_instance(
+            params={
+                'system_metadata': {
+                    'image_hw_machine_type': 'existing_type',
+                }
+            }
+        )
+        mock_get_by_host.return_value = [instance]
+        self.drvr._register_instance_machine_type()
+        # Assert that we don't overwrite the existing type
+        self.assertEqual(
+            'existing_type',
+            instance.image_meta.properties.hw_machine_type
+        )
+        self.assertEqual(
+            'existing_type',
+            instance.system_metadata.get('image_hw_machine_type')
+        )
+
+    @mock.patch('nova.objects.instance.Instance.save')
+    @mock.patch('nova.objects.instance.InstanceList.get_by_host')
+    @mock.patch('nova.virt.libvirt.utils.get_machine_type',
+        new=mock.Mock(return_value='conf_type'))
+    @mock.patch('nova.virt.libvirt.host.Host.get_hostname', new=mock.Mock())
+    @mock.patch('nova.context.get_admin_context', new=mock.Mock())
+    def test_register_machine_type(
+        self, mock_get_by_host, mock_instance_save,
+    ):
+        instance = self._create_instance()
+        mock_get_by_host.return_value = [instance]
+        self.drvr._register_instance_machine_type()
+        mock_instance_save.assert_called_once()
+        self.assertEqual(
+            'conf_type',
+            instance.image_meta.properties.hw_machine_type
+        )
+        self.assertEqual(
+            'conf_type',
+            instance.system_metadata.get('image_hw_machine_type')
+        )
+
 
 class LibvirtVolumeUsageTestCase(test.NoDBTestCase):
     """Test for LibvirtDriver.get_all_volume_usage."""

nova/virt/libvirt/driver.py

@@ -647,6 +647,37 @@ class LibvirtDriver(driver.ComputeDriver):
 
         self._check_vtpm_support()
 
+        self._register_instance_machine_type()
+
+    def _register_instance_machine_type(self):
+        """Register the machine type 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
+        """
+        context = nova_context.get_admin_context()
+        hostname = self._host.get_hostname()
+
+        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
+
+            # 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)
+
     def _check_cpu_compatibility(self):
         mode = CONF.libvirt.cpu_mode
         models = CONF.libvirt.cpu_models
@@ -5679,7 +5710,22 @@ class LibvirtDriver(driver.ComputeDriver):
                     guest.os_loader_type = "pflash"
                 else:
                     raise exception.UEFINotSupported()
-            guest.os_mach_type = libvirt_utils.get_machine_type(image_meta)
+
+            mtype = libvirt_utils.get_machine_type(image_meta)
+            guest.os_mach_type = mtype
+
+            # NOTE(lyarwood): If the machine type isn't recorded in the stashed
+            # image metadata then record it through the system metadata table.
+            # This will allow the host configuration to change in the future
+            # without impacting existing instances.
+            # NOTE(lyarwood): The value of ``hw_machine_type`` within the
+            # stashed image metadata of the instance actually comes from the
+            # system metadata table under the ``image_hw_machine_type`` key via
+            # nova.objects.ImageMeta.from_instance and the
+            # nova.utils.get_image_from_system_metadata function.
+            if image_meta.properties.get('hw_machine_type') is None:
+                instance.system_metadata['image_hw_machine_type'] = mtype
+
             if image_meta.properties.get('hw_boot_menu') is None:
                 guest.os_bootmenu = strutils.bool_from_string(
                     flavor.extra_specs.get('hw:boot_menu', 'no'))

releasenotes/notes/libvirt-store-and-change-default-machine-type-bf86b4973c4dee4c.yaml

@@ -0,0 +1,10 @@
+---
+upgrade:
+  - |
+    The libvirt virt driver will now attempt to record the machine type of an
+    instance at startup and when launching an instance if the machine type is
+    not already recorded in the image metadata associated with the instance.
+
+    This machine type will then be used when the instance is restarted or
+    migrated as it will now appear as an image metadata property associated
+    with the instance.