From 326bc658eef04576230d5ba90d2a02bf32deee03 Mon Sep 17 00:00:00 2001 From: Sean Mooney Date: Wed, 17 Jul 2019 16:46:44 +0000 Subject: [PATCH] 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 | 21 ++++ nova/api/openstack/compute/servers.py | 1 + nova/compute/api.py | 7 ++ nova/exception.py | 5 + nova/objects/image_meta.py | 9 +- .../api/openstack/compute/test_serversV21.py | 3 +- nova/tests/unit/compute/test_compute_api.py | 61 ++++++++++- nova/tests/unit/objects/test_image_meta.py | 8 ++ nova/tests/unit/objects/test_objects.py | 2 +- nova/tests/unit/virt/libvirt/test_config.py | 16 +++ nova/tests/unit/virt/libvirt/test_driver.py | 100 ++++++++++++++++++ nova/virt/libvirt/config.py | 17 +++ nova/virt/libvirt/driver.py | 18 +++- ...rt-pmu-configuration-ec24904bddc84bef.yaml | 12 +++ 14 files changed, 270 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/libvirt-pmu-configuration-ec24904bddc84bef.yaml diff --git a/doc/source/user/flavors.rst b/doc/source/user/flavors.rst index 2201555e3635..f11189f71a17 100644 --- a/doc/source/user/flavors.rst +++ b/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 diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 06a54e198524..0e556d5bda38 100644 --- a/nova/api/openstack/compute/servers.py +++ b/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, diff --git a/nova/compute/api.py b/nova/compute/api.py index bac9546021e9..4b1b2f7b9e49 100644 --- a/nova/compute/api.py +++ b/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) diff --git a/nova/exception.py b/nova/exception.py index 478e5a6fe50e..16bd6422aa20 100644 --- a/nova/exception.py +++ b/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") diff --git a/nova/objects/image_meta.py b/nova/objects/image_meta.py index 862a64c67237..5263ad5f09d8 100644 --- a/nova/objects/image_meta.py +++ b/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(), diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_serversV21.py index 2f180aff9340..9ac9b7071549 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/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( diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index a568d0d9f47c..69ebbc66dbcc 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/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): diff --git a/nova/tests/unit/objects/test_image_meta.py b/nova/tests/unit/objects/test_image_meta.py index 6acd127901aa..b457bddd5348 100644 --- a/nova/tests/unit/objects/test_image_meta.py +++ b/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']) diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index b4e3c7c77fe2..1a5c96194a63 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/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', diff --git a/nova/tests/unit/virt/libvirt/test_config.py b/nova/tests/unit/virt/libvirt/test_config.py index a11d2df79f48..b69d026adc2b 100644 --- a/nova/tests/unit/virt/libvirt/test_config.py +++ b/nova/tests/unit/virt/libvirt/test_config.py @@ -2193,6 +2193,22 @@ class LibvirtConfigGuestFeatureTest(LibvirtConfigBaseTest): """) + 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, "") + for val in ("false", "off", "0", "no", False): + obj = config.LibvirtConfigGuestFeaturePMU(val) + xml = obj.to_xml() + self.assertXmlEqual(xml, "") + class LibvirtConfigGuestTest(LibvirtConfigBaseTest): diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 0dc24026cd1b..3b88138284c7 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/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. diff --git a/nova/virt/libvirt/config.py b/nova/virt/libvirt/config.py index 8542ccd6542a..01491698bf69 100644 --- a/nova/virt/libvirt/config.py +++ b/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 diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 76b2e2ddfd17..edab0969e45d 100644 --- a/nova/virt/libvirt/driver.py +++ b/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 diff --git a/releasenotes/notes/libvirt-pmu-configuration-ec24904bddc84bef.yaml b/releasenotes/notes/libvirt-pmu-configuration-ec24904bddc84bef.yaml new file mode 100644 index 000000000000..3a1d9afc3b7c --- /dev/null +++ b/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.