Libvirt: add support for vPMU configuration.

This change adds the ablity for a user or operator to contol
the virtualisation of a performance monitoring unit within a vm.
This change introduces a new "hw:pmu" extra spec and a corresponding
image metadata property "hw_pmu".

The glance image metadata doc will be updated seperately by:
https://review.opendev.org/#/c/675182

Change-Id: I5576fa2a67d2771614266022428b4a95487ab6d5
Implements: blueprint libvirt-pmu-configuration

doc/source/user/flavors.rst

@@ -373,6 +373,27 @@ Random-number generator
     the host's entropy per period.
   - RATE-PERIOD: (integer) Duration of the read period in seconds.
 
+.. _extra-specs-performance-monitoring-unit:
+
+Performance Monitoring Unit (vPMU)
+  If nova is deployed with the libvirt virt driver and ``[libvirt]/virt_type``
+  is set to ``qemu`` or ``kvm``, a vPMU can be enabled or disabled for an
+  instance using the ``hw:pmu`` extra_spec or the ``hw_pmu`` image property.
+  The supported values are ``True`` or ``False``. If the vPMU is not
+  explicitly enabled or disabled via the flavor or image, its presence is left
+  to QEMU to decide.
+
+  .. code-block:: console
+
+     $ openstack flavor set FLAVOR-NAME --property hw:pmu=True|False
+
+  The vPMU is used by tools like ``perf`` in the guest to provide more accurate
+  information for profiling application and monitoring guest performance.
+  For realtime workloads, the emulation of a vPMU can introduce additional
+  latency which may be undesirable. If the telemetry it provides is not
+  required, such workloads should set ``hw:pmu=False``. For most workloads
+  the default of unset or enabling the vPMU ``hw:pmu=True`` will be correct.
+
 .. _extra-specs-cpu-topology:
 
 CPU topology

nova/api/openstack/compute/servers.py

@@ -66,6 +66,7 @@ INVALID_FLAVOR_IMAGE_EXCEPTIONS = (
     exception.ImageNUMATopologyForbidden,
     exception.ImageNUMATopologyIncomplete,
     exception.ImageNUMATopologyMemoryOutOfRange,
+    exception.ImagePMUConflict,
     exception.ImageSerialPortNumberExceedFlavorValue,
     exception.ImageSerialPortNumberInvalid,
     exception.ImageVCPULimitsRangeExceeded,

nova/compute/api.py

@@ -676,6 +676,13 @@ class API(base.Base):
         """
         image_meta = _get_image_meta_obj(image)
 
+        # validate PMU extra spec and image metadata
+        flavor_pmu = instance_type.extra_specs.get('hw:pmu')
+        image_pmu = image_meta.properties.get('hw_pmu')
+        if (flavor_pmu is not None and image_pmu is not None and
+                image_pmu != strutils.bool_from_string(flavor_pmu)):
+            raise exception.ImagePMUConflict()
+
         # Only validate values of flavor/image so the return results of
         # following 'get' functions are not used.
         hardware.get_number_of_serial_ports(instance_type, image_meta)

nova/exception.py

@@ -2045,6 +2045,11 @@ class ImageCPUThreadPolicyForbidden(Forbidden):
                 "override CPU thread pinning policy set against the flavor")
 
 
+class ImagePMUConflict(Forbidden):
+    msg_fmt = _("Image property 'hw_pmu' is not permitted to "
+                "override the PMU policy set in the flavor")
+
+
 class UnsupportedPolicyException(Invalid):
     msg_fmt = _("ServerGroup policy is not supported: %(reason)s")
 

nova/objects/image_meta.py

@@ -172,12 +172,15 @@ class ImageMetaProps(base.NovaObject):
     # Version 1.20: Added 'traits_required' list field
     # Version 1.21: Added 'hw_time_hpet' field
     # Version 1.22: Added 'gop', 'virtio' and 'none' to hw_video_model field
-    VERSION = '1.22'
+    # Version 1.23: Added 'hw_pmu' field
+    VERSION = '1.23'
 
     def obj_make_compatible(self, primitive, target_version):
         super(ImageMetaProps, self).obj_make_compatible(primitive,
                                                         target_version)
         target_version = versionutils.convert_version_to_tuple(target_version)
+        if target_version < (1, 23):
+            primitive.pop('hw_pmu', None)
         # NOTE(sean-k-mooney): unlike other nova object we version this object
         # when composed object are updated.
         if target_version < (1, 22):
@@ -327,6 +330,10 @@ class ImageMetaProps(base.NovaObject):
         # Generic property to specify the pointer model type.
         'hw_pointer_model': fields.PointerModelField(),
 
+        # boolean 'true' or 'false' to enable virtual performance
+        # monitoring unit (vPMU).
+        'hw_pmu': fields.FlexibleBooleanField(),
+
         # boolean 'yes' or 'no' to enable QEMU guest agent
         'hw_qemu_guest_agent': fields.FlexibleBooleanField(),
 

nova/tests/unit/api/openstack/compute/test_serversV21.py

@@ -6596,7 +6596,8 @@ class ServersControllerCreateTestV260(test.NoDBTestCase):
         get_flavor_mock = mock.patch(
             'nova.compute.flavors.get_flavor_by_flavor_id',
             return_value=fake_flavor.fake_flavor_obj(
-                context.get_admin_context(), flavorid='1'))
+                context.get_admin_context(), flavorid='1',
+                expected_attrs=['extra_specs']))
         get_flavor_mock.start()
         self.addCleanup(get_flavor_mock.stop)
         reqspec_create_mock = mock.patch(

nova/tests/unit/compute/test_compute_api.py

@@ -117,8 +117,8 @@ class _ComputeAPIUnitTestMixIn(object):
                  }
         if updates:
             flavor.update(updates)
-        return objects.Flavor._from_db_object(self.context, objects.Flavor(),
-                                              flavor)
+        return objects.Flavor._from_db_object(
+            self.context, objects.Flavor(extra_specs={}), flavor)
 
     def _create_instance_obj(self, params=None, flavor=None):
         """Create a test instance."""
@@ -6211,16 +6211,67 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
             self.assertEqual(['context-for-%s' % c for c in compute_api.CELLS],
                              cells)
 
+    @mock.patch('nova.pci.request.get_pci_requests_from_flavor')
+    def test_pmu_image_and_flavor_conflict(self, mock_request):
+        """Tests that calling _validate_flavor_image_nostatus()
+        with an image that conflicts with the flavor raises but no
+        exception is raised if there is no conflict.
+        """
+        image = {'id': uuids.image_id, 'status': 'foo',
+                 'properties': {'hw_pmu': False}}
+        flavor = objects.Flavor(
+            vcpus=1, memory_mb=512, root_gb=1, extra_specs={'hw:pmu': "true"})
+        self.assertRaises(
+            exception.ImagePMUConflict,
+            self.compute_api._validate_flavor_image_nostatus,
+            self.context, image, flavor, None)
+
+    @mock.patch('nova.pci.request.get_pci_requests_from_flavor')
+    def test_pmu_image_and_flavor_same_value(self, mock_request):
+        # assert that if both the image and flavor are set to the same value
+        # no exception is raised and the function returns nothing.
+        flavor = objects.Flavor(
+            vcpus=1, memory_mb=512, root_gb=1, extra_specs={'hw:pmu': "true"})
+
+        image = {'id': uuids.image_id, 'status': 'foo',
+                 'properties': {'hw_pmu': True}}
+        self.assertIsNone(self.compute_api._validate_flavor_image_nostatus(
+            self.context, image, flavor, None))
+
+    @mock.patch('nova.pci.request.get_pci_requests_from_flavor')
+    def test_pmu_image_only(self, mock_request):
+        # assert that if only the image metadata is set then it is valid
+        flavor = objects.Flavor(
+            vcpus=1, memory_mb=512, root_gb=1, extra_specs={})
+
+        # ensure string to bool conversion works for image metadata
+        # property by using "yes".
+        image = {'id': uuids.image_id, 'status': 'foo',
+                 'properties': {'hw_pmu': "yes"}}
+        self.assertIsNone(self.compute_api._validate_flavor_image_nostatus(
+            self.context, image, flavor, None))
+
+    @mock.patch('nova.pci.request.get_pci_requests_from_flavor')
+    def test_pmu_flavor_only(self, mock_request):
+        # assert that if only the flavor extra_spec is set then it is valid
+        # and test the string to bool conversion of "on" works.
+        flavor = objects.Flavor(
+            vcpus=1, memory_mb=512, root_gb=1, extra_specs={'hw:pmu': "on"})
+
+        image = {'id': uuids.image_id, 'status': 'foo', 'properties': {}}
+        self.assertIsNone(self.compute_api._validate_flavor_image_nostatus(
+            self.context, image, flavor, None))
+
     @mock.patch('nova.pci.request.get_pci_requests_from_flavor')
     def test_pci_validated(self, mock_request):
         """Tests that calling _validate_flavor_image_nostatus() with
         validate_pci=True results in get_pci_requests_from_flavor() being
         called.
         """
-        image = dict(id=uuids.image_id, status='foo')
+        image = {'id': uuids.image_id, 'status': 'foo'}
         flavor = self._create_flavor()
-        self.compute_api._validate_flavor_image_nostatus(self.context,
-            image, flavor, root_bdm=None, validate_pci=True)
+        self.compute_api._validate_flavor_image_nostatus(
+            self.context, image, flavor, root_bdm=None, validate_pci=True)
         mock_request.assert_called_once_with(flavor)
 
     def test_validate_and_build_base_options_translate_neutron_secgroup(self):

nova/tests/unit/objects/test_image_meta.py

@@ -403,3 +403,11 @@ class TestImageMetaProps(test.NoDBTestCase):
         obj = objects.ImageMetaProps(traits_required=['CUSTOM_TRUSTED'])
         primitive = obj.obj_to_primitive('1.19')
         self.assertNotIn('traits_required', primitive['nova_object.data'])
+
+    def test_obj_make_compatible_pmu(self):
+        """Tests that checks if we pop hw_pmu."""
+        obj = objects.ImageMetaProps(hw_pmu=True)
+        primitive = obj.obj_to_primitive()
+        old_primitive = obj.obj_to_primitive('1.22')
+        self.assertIn('hw_pmu', primitive['nova_object.data'])
+        self.assertNotIn('hw_pmu', old_primitive['nova_object.data'])

nova/tests/unit/objects/test_objects.py

@@ -1069,7 +1069,7 @@ object_data = {
     'HyperVLiveMigrateData': '1.4-e265780e6acfa631476c8170e8d6fce0',
     'IDEDeviceBus': '1.0-29d4c9f27ac44197f01b6ac1b7e16502',
     'ImageMeta': '1.8-642d1b2eb3e880a367f37d72dd76162d',
-    'ImageMetaProps': '1.22-b1c9ea78c43e8d7989a7d1f0f34d149a',
+    'ImageMetaProps': '1.23-ed659d0bb5dfb3b2c2c717850c732abc',
     'Instance': '2.6-5fefbcb483703c85e4d328b887c8af33',
     'InstanceAction': '1.2-9a5abc87fdd3af46f45731960651efb5',
     'InstanceActionEvent': '1.3-c749e1b3589e7117c81cb2aa6ac438d5',

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

@@ -2193,6 +2193,22 @@ class LibvirtConfigGuestFeatureTest(LibvirtConfigBaseTest):
             <vendor_id state="on" value="1234567890ab"/>
           </hyperv>""")
 
+    def test_feature_pmu(self):
+        # NOTE(sean-k-moonmey): LibvirtConfigGuestFeaturePMU uses
+        # bool_from_string internally so assert that boolean and
+        # string inputs work. This does not need to be exhaustive
+        # as bool_from_string is tested in oslo so we just try
+        # some common values.
+
+        for val in ("true", "on", "1", "yes", True):
+            obj = config.LibvirtConfigGuestFeaturePMU(val)
+            xml = obj.to_xml()
+            self.assertXmlEqual(xml, "<pmu state='on'/>")
+        for val in ("false", "off", "0", "no", False):
+            obj = config.LibvirtConfigGuestFeaturePMU(val)
+            xml = obj.to_xml()
+            self.assertXmlEqual(xml, "<pmu state='off'/>")
+
 
 class LibvirtConfigGuestTest(LibvirtConfigBaseTest):
 

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

@@ -51,6 +51,7 @@ from oslo_service import loopingcall
 from oslo_utils import fileutils
 from oslo_utils import fixture as utils_fixture
 from oslo_utils.fixture import uuidsentinel as uuids
+from oslo_utils import strutils
 from oslo_utils import units
 from oslo_utils import uuidutils
 from oslo_utils import versionutils
@@ -6093,6 +6094,105 @@ class LibvirtConnTestCase(test.NoDBTestCase,
             any(isinstance(feature, vconfig.LibvirtConfigGuestFeatureKvmHidden)
                            for feature in cfg.features))
 
+    def test_get_guest_config_with_pmu(self):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        instance_ref = objects.Instance(**self.test_instance)
+        for virt_type in ('qemu', 'kvm'):
+            self.flags(virt_type=virt_type, group='libvirt')
+
+            for state in ("true", "false"):
+                # assert that values set in flavor are reflected in
+                # the xml generated.
+                flavor = fake_flavor.fake_flavor_obj(self.context,
+                    extra_specs={"hw:pmu": state},
+                    expected_attrs={"extra_specs"})
+                image_meta = objects.ImageMeta.from_dict({
+                    "disk_format": "raw"})
+
+                instance_ref.flavor = flavor
+                disk_info = blockinfo.get_disk_info(
+                    CONF.libvirt.virt_type, instance_ref, image_meta)
+                cfg = drvr._get_guest_config(
+                    instance_ref, [], image_meta, disk_info)
+                self.assertTrue(any(isinstance(
+                    feature, vconfig.LibvirtConfigGuestFeaturePMU) and
+                    feature.state == strutils.bool_from_string(state)
+                    for feature in cfg.features))
+
+                # assert that values set in image are reflected in
+                # the xml generated.
+                instance_ref.flavor.extra_specs.pop('hw:pmu')
+                image_meta = objects.ImageMeta.from_dict({
+                    "disk_format": "raw",
+                    "properties": {"hw_pmu": state}})
+
+                disk_info = blockinfo.get_disk_info(
+                    CONF.libvirt.virt_type, instance_ref, image_meta)
+                cfg = drvr._get_guest_config(
+                    instance_ref, [], image_meta, disk_info)
+                self.assertTrue(any(isinstance(
+                    feature, vconfig.LibvirtConfigGuestFeaturePMU) and
+                    feature.state == strutils.bool_from_string(state)
+                    for feature in cfg.features))
+
+    def test_get_guest_config_with_pmu_unset(self):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        instance_ref = objects.Instance(**self.test_instance)
+        for virt_type in ('qemu', 'kvm'):
+            self.flags(virt_type=virt_type, group='libvirt')
+
+            # assert that if not set in image or flavor no pmu feature object
+            # is created.
+            flavor = fake_flavor.fake_flavor_obj(
+                self.context, extra_specs={}, expected_attrs={"extra_specs"})
+            image_meta = objects.ImageMeta.from_dict({"disk_format": "raw"})
+
+            instance_ref.flavor = flavor
+            disk_info = blockinfo.get_disk_info(
+                CONF.libvirt.virt_type, instance_ref, image_meta)
+            cfg = drvr._get_guest_config(
+                instance_ref, [], image_meta, disk_info)
+            self.assertFalse(
+                any(isinstance(feature, vconfig.LibvirtConfigGuestFeaturePMU)
+                    for feature in cfg.features))
+
+    def test_get_guest_config_with_pmu_non_qemu_kvm(self):
+        # assert that for virt_types other then qemu and kvm
+        # the xml the element is not generated.
+        self.flags(virt_type='lxc', group='libvirt')
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        instance_ref = objects.Instance(**self.test_instance)
+
+        for state in ("true", "false"):
+            flavor = fake_flavor.fake_flavor_obj(self.context,
+                extra_specs={"hw:pmu": state},
+                expected_attrs={"extra_specs"})
+            image_meta = objects.ImageMeta.from_dict({
+                "disk_format": "raw"})
+
+            instance_ref.flavor = flavor
+            disk_info = blockinfo.get_disk_info(
+                CONF.libvirt.virt_type, instance_ref, image_meta)
+            cfg = drvr._get_guest_config(
+                instance_ref, [], image_meta, disk_info)
+            self.assertFalse(
+                any(isinstance(feature, vconfig.LibvirtConfigGuestFeaturePMU)
+                for feature in cfg.features))
+
+            # assert that values set in image are also ignored.
+            instance_ref.flavor.extra_specs.pop('hw:pmu')
+            image_meta = objects.ImageMeta.from_dict({
+                "disk_format": "raw",
+                "properties": {"hw_pmu": state}})
+
+            disk_info = blockinfo.get_disk_info(
+                CONF.libvirt.virt_type, instance_ref, image_meta)
+            cfg = drvr._get_guest_config(
+                instance_ref, [], image_meta, disk_info)
+            self.assertFalse(
+                any(isinstance(feature, vconfig.LibvirtConfigGuestFeaturePMU)
+                    for feature in cfg.features))
+
     def _test_get_guest_config_disk_cachemodes(self, images_type):
         # Verify that the configured cachemodes are propagated to the device
         # configurations.

nova/virt/libvirt/config.py

@@ -26,6 +26,7 @@ helpers for populating up config object instances.
 import time
 
 from lxml import etree
+from oslo_utils import strutils
 from oslo_utils import units
 import six
 
@@ -2350,6 +2351,22 @@ class LibvirtConfigGuestFeatureKvmHidden(LibvirtConfigGuestFeature):
         return root
 
 
+class LibvirtConfigGuestFeaturePMU(LibvirtConfigGuestFeature):
+
+    def __init__(self, state, **kwargs):
+        super(LibvirtConfigGuestFeaturePMU, self).__init__("pmu", **kwargs)
+        # NOTE(sean-k-mooney): bool_from_string is needed to handle the raw
+        # flavor exta_sepc value. bool_from_string internally checks if the
+        # value is already a bool and returns it. As such it's safe to use
+        # with the image metadata property too, so we call it unconditionally.
+        self.state = strutils.bool_from_string(state)
+
+    def format_dom(self):
+        root = super(LibvirtConfigGuestFeaturePMU, self).format_dom()
+        root.attrib['state'] = "on" if self.state else "off"
+        return root
+
+
 class LibvirtConfigGuestFeatureHyperV(LibvirtConfigGuestFeature):
 
     # QEMU requires at least this value to be set

nova/virt/libvirt/driver.py

@@ -4775,8 +4775,22 @@ class LibvirtDriver(driver.ComputeDriver):
 
             guest.features.append(hv)
 
-        if (virt_type in ("qemu", "kvm") and hide_hypervisor_id):
-            guest.features.append(vconfig.LibvirtConfigGuestFeatureKvmHidden())
+        if virt_type in ("qemu", "kvm"):
+            if hide_hypervisor_id:
+                guest.features.append(
+                    vconfig.LibvirtConfigGuestFeatureKvmHidden())
+
+            # NOTE(sean-k-mooney): we validate that the image and flavor
+            # cannot have conflicting values in the compute API
+            # so we just use the values directly. If it is not set in
+            # either the flavor or image pmu will be none and we should
+            # not generate the element to allow qemu to decide if a vPMU
+            # should be provided for backwards compatibility.
+            pmu = (flavor.extra_specs.get('hw:pmu') or
+                   image_meta.properties.get('hw_pmu'))
+            if pmu is not None:
+                guest.features.append(
+                    vconfig.LibvirtConfigGuestFeaturePMU(pmu))
 
     def _check_number_of_serial_console(self, num_ports):
         virt_type = CONF.libvirt.virt_type

releasenotes/notes/libvirt-pmu-configuration-ec24904bddc84bef.yaml

@@ -0,0 +1,12 @@
+---
+features:
+  - |
+    The libvirt driver has been extended to support user configurable
+    performance monitoring unit (vPMU) virtualization.
+    This is particularly useful for real-time workloads.
+    A pair of boolean flavor extra spec and image metadata properties
+    ``hw:pmu`` and ``hw_pmu`` have been added to control the emulation
+    of the vPMU. By default the behavior of vPMU emulation has
+    not been changed. To take advantage of this new feature, the operator
+    or tenant will need to update their flavors or images to define the
+    new property.