Merge "libvirt: Allow to specify granular CPU feature flags" into stable/ocata

nova/conf/libvirt.py

@@ -23,6 +23,8 @@ import itertools
 
 from oslo_config import cfg
 
+from oslo_config import types
+
 from nova.conf import paths
 
 # Downtime period in milliseconds
@@ -514,6 +516,58 @@ Related options:
 * ``cpu_mode``: Don't set this when ``cpu_mode`` is NOT set to ``custom``.
   This would result in an error and the instance won't be launched.
 * ``virt_type``: Only the virtualization types ``kvm`` and ``qemu`` use this.
+"""),
+    cfg.ListOpt(
+        'cpu_model_extra_flags',
+        item_type=types.String(
+            choices=['pcid'],
+            ignore_case=True,
+        ),
+        default=[],
+        help="""
+This allows specifying granular CPU feature flags when specifying CPU
+models.  For example, to explicitly specify the ``pcid``
+(Process-Context ID, an Intel processor feature) flag to the "IvyBridge"
+virtual CPU model::
+
+    [libvirt]
+    cpu_mode = custom
+    cpu_model = IvyBridge
+    cpu_model_extra_flags = pcid
+
+Currently, the choice is restricted to only one option: ``pcid`` (the
+option is case-insensitive, so ``PCID`` is also valid).  This flag is
+now required to address the guest performance degradation as a result of
+applying the "Meltdown" CVE fixes on certain Intel CPU models.
+
+Note that when using this config attribute to set the 'PCID' CPU flag,
+not all virtual (i.e. libvirt / QEMU) CPU models need it:
+
+* 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
+  to be explicitly specified when using the said virtual CPU models.
+
+For now, the ``cpu_model_extra_flags`` config attribute is valid only in
+combination with ``cpu_mode`` + ``cpu_model`` options.
+
+Besides ``custom``, the libvirt driver has two other CPU modes: The
+default, ``host-model``, tells it to do the right thing with respect to
+handling 'PCID' CPU flag for the guest -- *assuming* you are running
+updated processor microcode, host and guest kernel, libvirt, and QEMU.
+The other mode, ``host-passthrough``, checks if 'PCID' is available in
+the hardware, and if so directly passes it through to the Nova guests.
+Thus, in context of 'PCID', with either of these CPU modes
+(``host-model`` or ``host-passthrough``), there is no need to use the
+``cpu_model_extra_flags``.
+
+Related options:
+
+* cpu_mode
+* cpu_model
 """),
     cfg.StrOpt('snapshots_directory',
                default='$instances_path/snapshots',

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

@@ -269,6 +269,15 @@ class LibvirtConfigGuestCPUFeatureTest(LibvirtConfigBaseTest):
             <feature name="mtrr" policy="force"/>
         """)
 
+    def test_config_simple_pcid(self):
+        obj = config.LibvirtConfigGuestCPUFeature("pcid")
+        obj.policy = "require"
+
+        xml = obj.to_xml()
+        self.assertXmlEqual(xml, """
+            <feature name="pcid" policy="require"/>
+        """)
+
 
 class LibvirtConfigGuestCPUNUMATest(LibvirtConfigBaseTest):
 

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

@@ -5705,6 +5705,83 @@ class LibvirtConnTestCase(test.NoDBTestCase):
         self.assertEqual(conf.cpu.cores, 1)
         self.assertEqual(conf.cpu.threads, 1)
 
+    @mock.patch.object(libvirt_driver.LOG, 'warning')
+    def test_get_guest_cpu_config_custom_with_extra_flags(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_model="IvyBridge",
+                   cpu_model_extra_flags="pcid",
+                   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, 1),
+                                      image_meta, disk_info)
+        self.assertIsInstance(conf.cpu,
+                              vconfig.LibvirtConfigGuestCPU)
+        self.assertEqual(conf.cpu.mode, "custom")
+        self.assertEqual(conf.cpu.model, "IvyBridge")
+        self.assertIn(conf.cpu.features.pop().name, "pcid")
+        self.assertEqual(conf.cpu.sockets, instance_ref.flavor.vcpus)
+        self.assertEqual(conf.cpu.cores, 1)
+        self.assertEqual(conf.cpu.threads, 1)
+        self.assertFalse(mock_warn.called)
+
+    @mock.patch.object(libvirt_driver.LOG, 'warning')
+    def test_get_guest_cpu_config_host_model_with_extra_flags(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="host-model",
+                   cpu_model_extra_flags="pcid",
+                   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, 1),
+                                      image_meta, disk_info)
+        self.assertIsInstance(conf.cpu,
+                              vconfig.LibvirtConfigGuestCPU)
+        self.assertEqual(conf.cpu.mode, "host-model")
+        self.assertEqual(len(conf.cpu.features), 0)
+        self.assertEqual(conf.cpu.sockets, instance_ref.flavor.vcpus)
+        self.assertEqual(conf.cpu.cores, 1)
+        self.assertEqual(conf.cpu.threads, 1)
+        self.assertTrue(mock_warn.called)
+
+    @mock.patch.object(libvirt_driver.LOG, 'warning')
+    def test_get_guest_cpu_config_host_passthrough_with_extra_flags(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="host-passthrough",
+                   cpu_model_extra_flags="pcid",
+                   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, 1),
+                                      image_meta, disk_info)
+        self.assertIsInstance(conf.cpu,
+                              vconfig.LibvirtConfigGuestCPU)
+        self.assertEqual(conf.cpu.mode, "host-passthrough")
+        self.assertEqual(len(conf.cpu.features), 0)
+        self.assertEqual(conf.cpu.sockets, instance_ref.flavor.vcpus)
+        self.assertEqual(conf.cpu.cores, 1)
+        self.assertEqual(conf.cpu.threads, 1)
+        self.assertTrue(mock_warn.called)
+
     def test_get_guest_cpu_topology(self):
         instance_ref = objects.Instance(**self.test_instance)
         instance_ref.flavor.vcpus = 8

nova/virt/libvirt/driver.py

@@ -3563,6 +3563,7 @@ class LibvirtDriver(driver.ComputeDriver):
     def _get_guest_cpu_model_config(self):
         mode = CONF.libvirt.cpu_mode
         model = CONF.libvirt.cpu_model
+        extra_flags = CONF.libvirt.cpu_model_extra_flags
 
         if (CONF.libvirt.virt_type == "kvm" or
             CONF.libvirt.virt_type == "qemu"):
@@ -3589,14 +3590,50 @@ class LibvirtDriver(driver.ComputeDriver):
             msg = _("A CPU model name should not be set when a "
                     "host CPU model is requested")
             raise exception.Invalid(msg)
+        # FIXME (kchamart): We're intentionally restricting the choices
+        # (in the conf/libvirt.py) for 'extra_flags` to just 'PCID', to
+        # address the immediate guest performance degradation caused by
+        # "Meltdown" CVE fixes on certain Intel CPU models.  In a future
+        # patch, we will:
+        # (a) Remove the restriction of choices for 'extra_flags',
+        #     allowing to add / remove additional CPU flags, as it will
+        #     make way for other useful features.
+        # (b) Remove the below check for "host-model", as it is a
+        #     valid configuration to supply additional CPU flags to it.
+        # (c) Revisit and fix the warnings / exception handling for
+        #     different combinations of CPU modes and 'extra_flags'.
+        elif ((mode == "host-model" or mode == "host-passthrough") and
+              extra_flags):
+            extra_flags = []
+            LOG.warning(
+              _LW("Setting extra CPU flags is only valid in "
+                  "combination with a custom CPU model. Refer "
+                  "to the 'nova.conf' documentation for "
+                  "'[libvirt]/cpu_model_extra_flags'"))
 
-        LOG.debug("CPU mode '%(mode)s' model '%(model)s' was chosen",
-                  {'mode': mode, 'model': (model or "")})
+        LOG.debug("CPU mode '%(mode)s' model '%(model)s' was chosen, "
+                  "with extra flags: '%(extra_flags)s'",
+                  {'mode': mode,
+                   'model': (model or ""),
+                   'extra_flags': (extra_flags or "")})
 
         cpu = vconfig.LibvirtConfigGuestCPU()
         cpu.mode = mode
         cpu.model = model
 
+        # NOTE (kchamart): Currently there's no existing way to ask if a
+        # given CPU model + CPU flags combination is supported by KVM &
+        # a specific QEMU binary.  However, libvirt runs the 'CPUID'
+        # command upfront -- before even a Nova instance (a QEMU
+        # process) is launched -- to construct CPU models and check
+        # their validity; so we are good there.  In the long-term,
+        # upstream libvirt intends to add an additional new API that can
+        # 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).
+        for flag in extra_flags:
+            cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag))
+
         return cpu
 
     def _get_guest_cpu_config(self, flavor, image_meta,

releasenotes/notes/libvirt-cpu-model-extra-flags-a23085f58bd22d27.yaml

@@ -0,0 +1,21 @@
+---
+fixes:
+  - |
+    The libvirt driver now allows specifying individual CPU feature
+    flags for guests, via a new configuration attribute
+    ``[libvirt]/cpu_model_extra_flags`` -- only with ``custom`` as the
+    ``[libvirt]/cpu_model``.  Refer to its documentation in
+    ``nova.conf`` for usage details.
+
+    One of the motivations for this is to alleviate the performance
+    degradation (caused as a result of applying the "Meltdown" CVE
+    fixes) for guests running with certain Intel-based virtual CPU
+    models.  This guest performance impact is reduced by exposing the
+    CPU feature flag 'PCID' ("Process-Context ID") to the *guest* CPU,
+    assuming that it is available in the physical hardware itself.
+
+    Note that besides ``custom``, Nova's libvirt driver has two other
+    CPU modes: ``host-model`` (which is the default), and
+    ``host-passthrough``.  Refer to the
+    ``[libvirt]/cpu_model_extra_flags`` documentation for what to do
+    when you are using either of those CPU modes in context of 'PCID'.