From 9caaaf1f221063a4329c72c8b67a6015648644a2 Mon Sep 17 00:00:00 2001 From: Kashyap Chamarthy Date: Tue, 17 Jan 2023 11:15:37 +0100 Subject: [PATCH] libvirt: At start-up rework compareCPU() usage with a workaround In this patch: - Remove the first compareCPU() call (called via the wrapper _compare_cpu()) in _check_cpu_compatibility(), and let libvirt handle it. (QEMU >=2.9 and libvirt >= 4.4.0 are the mininum required versions, and upstream Nova satisfies them by a good margin.) - Validate the user-configured CPU models from _get_cpu_model_mapping(). And take into account all the CPU flags before calling _compare_cpu(). (Suggested-by: Sean Mooney -- thanks!) - Add a workaround to allow skipping the remaining compareCPU() call in _check_cpu_compatibility() as a potential future-proof (because we cannot test all possible CPU models and hardware). Unlike the removed first call, this call takes into account the extra CPU flags provided by the user into account when evaluating guest CPU model compatibility. As a follow up comes the patch[1] that replaces the older libvirt CPU API with the newer one. [1] https://review.opendev.org/c/openstack/nova/+/869950 -- libvirt: Replace usage of compareCPU() with compareHypervisorCPU() Change-Id: I8ef9db851b37c5249d2efbe09a15a1ddbae8205d Signed-off-by: Kashyap Chamarthy --- nova/conf/workarounds.py | 7 +++ nova/tests/unit/virt/libvirt/test_driver.py | 22 ++++++++-- nova/virt/libvirt/driver.py | 47 +++++++++------------ 3 files changed, 46 insertions(+), 30 deletions(-) diff --git a/nova/conf/workarounds.py b/nova/conf/workarounds.py index 1116664d36..e485ae673a 100644 --- a/nova/conf/workarounds.py +++ b/nova/conf/workarounds.py @@ -410,6 +410,13 @@ with the destination host. When using QEMU >= 2.9 and libvirt >= 4.4.0, libvirt will do the correct thing with respect to checking CPU compatibility on the destination host during live migration. """), + cfg.BoolOpt('skip_cpu_compare_at_startup', + default=False, + help=""" +This will skip the CPU comparison call at the startup of Compute +service and lets libvirt handle it. +"""), + cfg.BoolOpt( 'skip_hypervisor_version_check_on_lm', default=False, diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 1c5f79dc89..9d45ba017c 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -1330,6 +1330,22 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertRaises(exception.InvalidCPUInfo, drvr.init_host, "dummyhost") + @mock.patch.object(libvirt_driver.LibvirtDriver, + '_register_all_undefined_instance_details', + new=mock.Mock()) + @mock.patch('nova.virt.libvirt.host.libvirt.Connection.compareCPU') + def test__check_cpu_compatibility_skip_compare_at_init( + self, mocked_compare + ): + self.flags(group='workarounds', skip_cpu_compare_at_startup=True) + self.flags(cpu_mode="custom", + cpu_models=["Icelake-Server-noTSX"], + cpu_model_extra_flags = ["-mpx"], + group="libvirt") + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr.init_host("dummyhost") + mocked_compare.assert_not_called() + @mock.patch.object(libvirt_driver.LibvirtDriver, '_register_all_undefined_instance_details', new=mock.Mock()) @@ -1343,7 +1359,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch('nova.virt.libvirt.host.libvirt.Connection.compareCPU') def test__check_cpu_compatibility_advance_flag(self, mocked_compare): - mocked_compare.side_effect = (2, 0) + mocked_compare.side_effect = (-1, 0) self.flags(cpu_mode="custom", cpu_models=["qemu64"], cpu_model_extra_flags = ["avx", "avx2"], @@ -1356,7 +1372,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, def test__check_cpu_compatibility_wrong_flag(self, mocked_compare): # here, and in the surrounding similar tests, the non-zero error # code in the compareCPU() side effect indicates error - mocked_compare.side_effect = (2, 0) + mocked_compare.side_effect = (-1, 0) self.flags(cpu_mode="custom", cpu_models=["Broadwell-noTSX"], cpu_model_extra_flags = ["a v x"], @@ -1369,7 +1385,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, def test__check_cpu_compatibility_enabled_and_disabled_flags( self, mocked_compare ): - mocked_compare.side_effect = (2, 0) + mocked_compare.side_effect = (-1, 0) self.flags( cpu_mode="custom", cpu_models=["Cascadelake-Server"], diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index d7fe0fcc93..ff503de7e6 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -984,33 +984,26 @@ class LibvirtDriver(driver.ComputeDriver): msg = _("The cpu_models option is required when cpu_mode=custom") raise exception.Invalid(msg) - cpu = vconfig.LibvirtConfigGuestCPU() - for model in models: - cpu.model = self._get_cpu_model_mapping(model) - try: - self._compare_cpu(cpu, self._get_cpu_info(), None) - except exception.InvalidCPUInfo as e: - msg = (_("Configured CPU model: %(model)s is not " - "compatible with host CPU. Please correct your " - "config and try again. %(e)s") % { - 'model': model, 'e': e}) - raise exception.InvalidCPUInfo(msg) - - # Use guest CPU model to check the compatibility between guest CPU and - # configured extra_flags - cpu = vconfig.LibvirtConfigGuestCPU() - cpu.model = self._host.get_capabilities().host.cpu.model - for flag in set(x.lower() for x in CONF.libvirt.cpu_model_extra_flags): - cpu_feature = self._prepare_cpu_flag(flag) - cpu.add_feature(cpu_feature) - try: - self._compare_cpu(cpu, self._get_cpu_info(), None) - except exception.InvalidCPUInfo as e: - msg = (_("Configured extra flag: %(flag)s it not correct, or " - "the host CPU does not support this flag. Please " - "correct the config and try again. %(e)s") % { - 'flag': flag, 'e': e}) - raise exception.InvalidCPUInfo(msg) + if not CONF.workarounds.skip_cpu_compare_at_startup: + # Use guest CPU model to check the compatibility between + # guest CPU and configured extra_flags + for model in models: + cpu = vconfig.LibvirtConfigGuestCPU() + cpu.model = self._get_cpu_model_mapping(model) + for flag in set(x.lower() for + x in CONF.libvirt.cpu_model_extra_flags): + cpu_feature = self._prepare_cpu_flag(flag) + cpu.add_feature(cpu_feature) + try: + self._compare_cpu(cpu, self._get_cpu_info(), None) + except exception.InvalidCPUInfo as e: + msg = (_("Configured CPU model: %(model)s " + "and CPU Flags %(flags)s ar not " + "compatible with host CPU. Please correct your " + "config and try again. %(e)s") % { + 'model': model, 'e': e, + 'flags': CONF.libvirt.cpu_model_extra_flags}) + raise exception.InvalidCPUInfo(msg) def _check_vtpm_support(self) -> None: # TODO(efried): A key manager must be configured to create/retrieve