From bcd6b42047ea9422a58a4273d831e23f2ea27092 Mon Sep 17 00:00:00 2001 From: Kashyap Chamarthy Date: Wed, 3 Feb 2021 10:44:44 +0100 Subject: [PATCH] libvirt: Allow disabling CPU flags via `cpu_model_extra_flags` Parse a comma-separated list of CPU flags from `[libvirt]/cpu_model_extra_flags`. If the CPU flag starts with '+', enable the feature in Nova guest CPU guest XML, or if it starts with '-', disable the feature. If neither '+' nor '-' is specified, enable the flag. For example, on a compute node that is running hardware (e.g. an Intel server that supports TSX) and virtualization software that supports the given CPU flags, if a user provides this config: [libvirt] cpu_mode = custom cpu_models = Cascadelake-Server cpu_model_extra_flags = -hle, -rtm, +ssbd, mtrr Then Nova should generate this CPU for the guest: Cascadelake-Server Intel This ability to selectively disable CPU flags lets you avoid any CPU flags that need to be disabled for any number of reasons. E.g. disable a CPU flag that is a potential security risk, or disable one that causes a performance penalty. blueprint: allow-disabling-cpu-flags Change-Id: I2ef7c5bef87bd64c087f3b136c2faac9a3865f10 Signed-off-by: Patrick Uiterwijk Signed-off-by: Kashyap Chamarthy --- nova/conf/libvirt.py | 74 +++++++++-------- nova/tests/unit/virt/libvirt/test_driver.py | 83 +++++++++++++++++++ nova/virt/libvirt/config.py | 4 +- nova/virt/libvirt/driver.py | 32 ++++++- ...-disabling-cpu-flags-cc861a3bdfffadf8.yaml | 13 +++ 5 files changed, 167 insertions(+), 39 deletions(-) create mode 100644 releasenotes/notes/allow-disabling-cpu-flags-cc861a3bdfffadf8.yaml diff --git a/nova/conf/libvirt.py b/nova/conf/libvirt.py index d23bcd6427a3..954cbbe420f1 100644 --- a/nova/conf/libvirt.py +++ b/nova/conf/libvirt.py @@ -532,9 +532,8 @@ richer that the previous CPU model. Possible values: -* The named CPU models listed in ``/usr/share/libvirt/cpu_map.xml`` for - libvirt prior to version 4.7.0 or ``/usr/share/libvirt/cpu_map/*.xml`` - for version 4.7.0 and higher. +* The named CPU models can be found via ``virsh cpu-models ARCH``, where + ARCH is your host architecture. Related options: @@ -554,53 +553,60 @@ Related options: ), default=[], help=""" -This allows specifying granular CPU feature flags when configuring CPU -models. For example, to explicitly specify the ``pcid`` -(Process-Context ID, an Intel processor feature -- which is now required -to address the guest performance degradation as a result of applying the -"Meltdown" CVE fixes to certain Intel CPU models) flag to the -"IvyBridge" virtual CPU model:: +Enable or disable guest CPU flags. + +To explicitly enable or disable CPU flags, use the ``+flag`` or +``-flag`` notation -- the ``+`` sign will enable the CPU flag for the +guest, while a ``-`` sign will disable it. If neither ``+`` nor ``-`` +is specified, the flag will be enabled, which is the default behaviour. +For example, if you specify the following (assuming the said CPU model +and features are supported by the host hardware and software):: [libvirt] cpu_mode = custom - cpu_models = IvyBridge - cpu_model_extra_flags = pcid + cpu_models = Cascadelake-Server + cpu_model_extra_flags = -hle, -rtm, +ssbd, mtrr -To specify multiple CPU flags (e.g. the Intel ``VMX`` to expose the -virtualization extensions to the guest, or ``pdpe1gb`` to configure 1GB -huge pages for CPU models that do not provide it):: +Nova will disable the ``hle`` and ``rtm`` flags for the guest; and it +will enable ``ssbd`` and ``mttr`` (because it was specified with neither +``+`` nor ``-`` prefix). + +The CPU flags are case-insensitive. In the following example, the +``pdpe1gb`` flag will be disabled for the guest; ``vmx`` and ``pcid`` +flags will be enabled:: [libvirt] cpu_mode = custom cpu_models = Haswell-noTSX-IBRS - cpu_model_extra_flags = PCID, VMX, pdpe1gb + cpu_model_extra_flags = -PDPE1GB, +VMX, pcid -As it can be noticed from above, the ``cpu_model_extra_flags`` config -attribute is case insensitive. And specifying extra flags is valid in -combination with all the three possible values for ``cpu_mode``: -``custom`` (this also requires an explicit ``cpu_models`` to be -specified), ``host-model``, or ``host-passthrough``. A valid example -for allowing extra CPU flags even for ``host-passthrough`` mode is that -sometimes QEMU may disable certain CPU features -- e.g. Intel's -"invtsc", Invariable Time Stamp Counter, CPU flag. And if you need to -expose that CPU flag to the Nova instance, the you need to explicitly -ask for it. +Specifying extra CPU flags is valid in combination with all the three +possible values of ``cpu_mode`` config attribute: ``custom`` (this also +requires an explicit CPU model to be specified via the ``cpu_models`` +config attribute), ``host-model``, or ``host-passthrough``. + +There can be scenarios where you may need to configure extra CPU flags +even for ``host-passthrough`` CPU mode, because sometimes QEMU may +disable certain CPU features. An example of this is Intel's "invtsc" +(Invariable Time Stamp Counter) CPU flag -- if you need to expose this +flag to a Nova instance, you need to explicitly enable it. The possible values for ``cpu_model_extra_flags`` depends on the CPU -model in use. Refer to ``/usr/share/libvirt/cpu_map.xml`` for libvirt -prior to version 4.7.0 or ``/usr/share/libvirt/cpu_map/*.xml`` thereafter -for possible CPU feature flags for a given CPU model. +model in use. Refer to `/usr/share/libvirt/cpu_map/*.xml`` for possible +CPU feature flags for a given CPU model. -Note that when using this config attribute to set the 'PCID' CPU flag -with the ``custom`` CPU mode, not all virtual (i.e. libvirt / QEMU) CPU -models need it: +A special note on a particular CPU flag: ``pcid`` (an Intel processor +feature that alleviates guest performance degradation as a result of +applying the 'Meltdown' CVE fixes). When configuring this flag with the +``custom`` CPU mode, not all CPU models (as defined by QEMU and libvirt) +need it: -* The only virtual CPU models that include the 'PCID' capability are +* The only virtual CPU models that include the ``pcid`` capability are Intel "Haswell", "Broadwell", and "Skylake" variants. * The libvirt / QEMU CPU models "Nehalem", "Westmere", "SandyBridge", - and "IvyBridge" will _not_ expose the 'PCID' capability by default, - even if the host CPUs by the same name include it. I.e. 'PCID' needs + and "IvyBridge" will _not_ expose the ``pcid`` capability by default, + even if the host CPUs by the same name include it. I.e. 'PCID' needs to be explicitly specified when using the said virtual CPU models. The libvirt driver's default CPU mode, ``host-model``, will do the right diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 29bbf9d4917b..16210baac5de 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -1474,6 +1474,36 @@ 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__prepare_cpu_flag(self): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + + # The `+` means the guest "require[s]" (i.e. enable in libvirt + # parlance) the said CPU feature; and `-` will disable it + feat1 = drvr._prepare_cpu_flag('+md-clear') + feat2 = drvr._prepare_cpu_flag('pdpe1gb') + feat3 = drvr._prepare_cpu_flag('-ssbd') + + self.assertIsInstance(feat1, vconfig.LibvirtConfigGuestCPUFeature) + self.assertIsInstance(feat2, vconfig.LibvirtConfigGuestCPUFeature) + self.assertIsInstance(feat3, vconfig.LibvirtConfigGuestCPUFeature) + + cpu = vconfig.LibvirtConfigGuestCPU() + cpu.add_feature(feat1) + cpu.add_feature(feat2) + cpu.add_feature(feat3) + + # Verify that the resulting guest XML records both enabled + # _and_ disabled CPU features + expected_xml = ''' + + + + + ''' + self.assertXmlEqual(expected_xml, cpu.to_xml()) + @mock.patch.object(libvirt_driver.LibvirtDriver, '_register_instance_machine_type', new=mock.Mock()) def test__check_cpu_compatibility_start_ok(self): @@ -1530,6 +1560,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch('nova.virt.libvirt.host.libvirt.Connection.compareCPU') 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) self.flags(cpu_mode="custom", cpu_models=["Broadwell-noTSX"], @@ -1539,6 +1571,20 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertRaises(exception.InvalidCPUInfo, drvr.init_host, "dummyhost") + @mock.patch('nova.virt.libvirt.host.libvirt.Connection.compareCPU') + def test__check_cpu_compatibility_enabled_and_disabled_flags( + self, mocked_compare + ): + mocked_compare.side_effect = (2, 0) + self.flags( + cpu_mode="custom", + cpu_models=["Cascadelake-Server"], + cpu_model_extra_flags = ["-hle", "-rtm", "+ssbd", "mttr"], + group="libvirt") + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + self.assertRaises(exception.InvalidCPUInfo, + drvr.init_host, "dummyhost") + def test__check_cpu_compatibility_invalid_virt_type(self): """Test getting CPU traits when using a virt_type that doesn't support the feature, only kvm and qemu supports reporting CPU traits. @@ -8108,6 +8154,43 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertEqual(conf.cpu.threads, 1) mock_warn.assert_not_called() + @mock.patch.object(libvirt_driver.LOG, 'warning') + def test_get_guest_cpu_config_custom_flags_enabled_and_disabled( + self, mock_warn + ): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + instance_ref = objects.Instance(**self.test_instance) + image_meta = objects.ImageMeta.from_dict(self.test_image_meta) + + self.flags( + cpu_mode="custom", + cpu_models=["Cascadelake-Server"], + cpu_model_extra_flags=['-hle', '-rtm', '+ssbd', 'mtrr'], + group='libvirt') + disk_info = blockinfo.get_disk_info( + CONF.libvirt.virt_type, + instance_ref, + image_meta) + conf = drvr._get_guest_config( + instance_ref, + _fake_network_info(self), + image_meta, disk_info) + features = [(feature.name, feature.policy) + for feature in conf.cpu.features] + self.assertIsInstance( + conf.cpu, + vconfig.LibvirtConfigGuestCPU) + self.assertEqual(conf.cpu.mode, "custom") + self.assertEqual(conf.cpu.model, "Cascadelake-Server") + self.assertIn(("ssbd", 'require'), features) + self.assertIn(("mtrr", 'require'), features) + self.assertIn(("hle", 'disable'), features) + self.assertIn(("rtm", 'disable'), features) + self.assertEqual(conf.cpu.sockets, instance_ref.flavor.vcpus) + self.assertEqual(conf.cpu.cores, 1) + self.assertEqual(conf.cpu.threads, 1) + mock_warn.assert_not_called() + def test_get_guest_cpu_config_custom_upper_cpu_model(self): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) instance_ref = objects.Instance(**self.test_instance) diff --git a/nova/virt/libvirt/config.py b/nova/virt/libvirt/config.py index 8e54d9d0dcf4..02c2c60f8fe0 100644 --- a/nova/virt/libvirt/config.py +++ b/nova/virt/libvirt/config.py @@ -765,10 +765,10 @@ class LibvirtConfigCPU(LibvirtConfigObject): class LibvirtConfigGuestCPUFeature(LibvirtConfigCPUFeature): - def __init__(self, name=None, **kwargs): + def __init__(self, name=None, policy="require", **kwargs): super(LibvirtConfigGuestCPUFeature, self).__init__(name, **kwargs) - self.policy = "require" + self.policy = policy def format_dom(self): ft = super(LibvirtConfigGuestCPUFeature, self).format_dom() diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 17a90ef5602b..80d1d39de058 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -678,6 +678,25 @@ class LibvirtDriver(driver.ComputeDriver): LOG.debug("Instance machine_type updated to %s", hw_machine_type, instance=instance) + def _prepare_cpu_flag(self, flag): + # NOTE(kchamart) This helper method will be used while computing + # guest CPU compatibility. It will take into account a + # comma-separated list of CPU flags from + # `[libvirt]cpu_model_extra_flags`. If the CPU flag starts + # with '+', it is enabled for the guest; if it starts with '-', + # it is disabled. If neither '+' nor '-' is specified, the CPU + # flag is enabled. + if flag.startswith('-'): + flag = flag.lstrip('-') + policy_value = 'disable' + else: + flag = flag.lstrip('+') + policy_value = 'require' + + cpu_feature = vconfig.LibvirtConfigGuestCPUFeature( + flag, policy=policy_value) + return cpu_feature + def _check_cpu_compatibility(self): mode = CONF.libvirt.cpu_mode models = CONF.libvirt.cpu_models @@ -717,7 +736,8 @@ class LibvirtDriver(driver.ComputeDriver): 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.add_feature(vconfig.LibvirtConfigCPUFeature(flag)) + 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: @@ -4621,9 +4641,15 @@ class LibvirtDriver(driver.ComputeDriver): # do fine-grained validation of a certain CPU model + CPU flags # against a specific QEMU binary (the libvirt RFE bug for that: # https://bugzilla.redhat.com/show_bug.cgi?id=1559832). + # + # NOTE(kchamart) Similar to what was done in + # _check_cpu_compatibility(), the below parses a comma-separated + # list of CPU flags from `[libvirt]cpu_model_extra_flags` and + # will selectively enable or disable a given CPU flag for the + # guest, before it is launched by Nova. for flag in extra_flags: - cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag)) - + cpu_feature = self._prepare_cpu_flag(flag) + cpu.add_feature(cpu_feature) return cpu def _match_cpu_model_by_flags(self, models, flags): diff --git a/releasenotes/notes/allow-disabling-cpu-flags-cc861a3bdfffadf8.yaml b/releasenotes/notes/allow-disabling-cpu-flags-cc861a3bdfffadf8.yaml new file mode 100644 index 000000000000..f0535135efb0 --- /dev/null +++ b/releasenotes/notes/allow-disabling-cpu-flags-cc861a3bdfffadf8.yaml @@ -0,0 +1,13 @@ +--- +features: + - | + The libvirt driver now allows explicitly disabling CPU flags for + guests via the ``[libvirt]cpu_model_extra_flags`` config attribute. + This is possible via a ``+`` / ``-`` notation, where if you specify + a CPU flag prefixed with a ``+`` sign (without quotes), it will be + enabled for the guest, while a prefix of ``-`` will disable it. If + neither ``+`` nor ``-`` is specified, the CPU flag will be enabled, + which is the default behaviour. + + Refer to the ``[libvirt]cpu_model_extra_flags`` documentation for + more information.