Merge "Move 'hw:pmu', 'hw_pmu' parsing to nova.virt.hardware"

Author: Zuul

nova/api/openstack/compute/servers.py

@@ -68,7 +68,6 @@
     exception.ImageNUMATopologyIncomplete,
     exception.ImageNUMATopologyMemoryOutOfRange,
     exception.ImageNUMATopologyRebuildConflict,
-    exception.ImagePMUConflict,
     exception.ImageSerialPortNumberExceedFlavorValue,
     exception.ImageSerialPortNumberInvalid,
     exception.ImageVCPULimitsRangeExceeded,

nova/compute/api.py

@@ -840,17 +840,10 @@ def _validate_flavor_image_numa_pci(
         """
         image_meta = _get_image_meta_obj(image)
 
-        API._validate_flavor_image_mem_encryption(flavor, image_meta)
-
-        # validate PMU extra spec and image metadata
-        flavor_pmu = flavor.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_mem_encryption_constraint(flavor, image_meta)
+        hardware.get_pmu_constraint(flavor, image_meta)
         hardware.get_number_of_serial_ports(flavor, image_meta)
         hardware.get_realtime_cpu_constraint(flavor, image_meta)
         hardware.get_cpu_topology_constraints(flavor, image_meta)
@@ -860,19 +853,6 @@ def _validate_flavor_image_numa_pci(
         if validate_pci:
             pci_request.get_pci_requests_from_flavor(flavor)
 
-    @staticmethod
-    def _validate_flavor_image_mem_encryption(flavor, image):
-        """Validate that the flavor and image don't make contradictory
-        requests regarding memory encryption.
-
-        :param flavor: Flavor object
-        :param image: an ImageMeta object
-        :raises: nova.exception.FlavorImageConflict
-        """
-        # This library function will raise the exception for us if
-        # necessary; if not, we can ignore the result returned.
-        hardware.get_mem_encryption_constraint(flavor, image)
-
     def _get_image_defined_bdms(self, flavor, image_meta, root_device_name):
         image_properties = image_meta.get('properties', {})
 

nova/exception.py

@@ -1869,11 +1869,6 @@ 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/tests/unit/compute/test_api.py

@@ -7192,57 +7192,6 @@ def test__validate_numa_rebuild_alter_numa_toplogy(self):
                 self.compute_api._validate_numa_rebuild, instance,
                 image, flavor)
 
-    @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

nova/tests/unit/virt/test_hardware.py

@@ -5452,6 +5452,56 @@ def test_get_pci_numa_policy_invalid(self):
             image_meta.properties.hw_pci_numa_affinity_policy = "fake"
 
 
+class PMUEnabledTest(test.NoDBTestCase):
+
+    def test_pmu_image_and_flavor_conflict(self):
+        """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.
+        """
+        flavor = objects.Flavor(
+            name='foo', vcpus=1, memory_mb=512, root_gb=1,
+            extra_specs={'hw:pmu': "true"})
+        image_meta = objects.ImageMeta.from_dict({
+            'name': 'bar', 'properties': {'hw_pmu': False},
+        })
+        self.assertRaises(
+            exception.FlavorImageConflict,
+            hw.get_pmu_constraint,
+            flavor, image_meta)
+
+    def test_pmu_image_and_flavor_same_value(self):
+        # 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_meta = objects.ImageMeta.from_dict({
+            'properties': {'hw_pmu': True},
+        })
+        self.assertTrue(hw.get_pmu_constraint(flavor, image_meta))
+
+    def test_pmu_image_only(self):
+        # 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_meta = objects.ImageMeta.from_dict({
+            'properties': {'hw_pmu': 'yes'},
+        })
+        self.assertTrue(hw.get_pmu_constraint(flavor, image_meta))
+
+    def test_pmu_flavor_only(self):
+        # 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_meta = objects.ImageMeta.from_dict({'properties': {}})
+        self.assertTrue(hw.get_pmu_constraint(flavor, image_meta))
+
+
 @ddt.ddt
 class VIFMultiqueueEnabledTest(test.NoDBTestCase):
 

nova/virt/hardware.py

@@ -1784,6 +1784,57 @@ def get_pci_numa_policy_constraint(
     return policy
 
 
+def get_pmu_constraint(
+    flavor: 'objects.Flavor',
+    image_meta: 'objects.ImageMeta',
+) -> ty.Optional[bool]:
+    """Validate and return the requested vPMU configuration.
+
+    This one's a little different since we don't return False in the default
+    case: the PMU should only be configured if explicit configuration is
+    provided, otherwise we leave it to the hypervisor.
+
+    :param flavor: ``nova.objects.Flavor`` instance
+    :param image_meta: ``nova.objects.ImageMeta`` instance
+    :raises: nova.exception.FlavorImageConflict if a value is specified in both
+        the flavor and the image, but the values do not match
+    :raises: nova.exception.Invalid if a value or combination of values is
+        invalid
+    :returns: True if the virtual Performance Monitoring Unit must be enabled,
+        False if it should be disabled, or None if unconfigured.
+    """
+    flavor_value_str, image_value = _get_flavor_image_meta(
+        'pmu', flavor, image_meta)
+
+    flavor_value = None
+    if flavor_value_str is not None:
+        flavor_value = strutils.bool_from_string(flavor_value_str)
+
+    if (
+        image_value is not None and
+        flavor_value is not None and
+        image_value != flavor_value
+    ):
+        msg = _(
+            "Flavor %(flavor_name)s has %(prefix)s:%(key)s extra spec "
+            "explicitly set to %(flavor_val)s, conflicting with image "
+            "%(image_name)s which has %(prefix)s_%(key)s explicitly set to "
+            "%(image_val)s."
+        )
+        raise exception.FlavorImageConflict(
+            msg % {
+                'prefix': 'hw',
+                'key': 'pmu',
+                'flavor_name': flavor.name,
+                'flavor_val': flavor_value,
+                'image_name': image_meta.name,
+                'image_val': image_value,
+            },
+        )
+
+    return flavor_value if flavor_value is not None else image_value
+
+
 def get_vif_multiqueue_constraint(
     flavor: 'objects.Flavor',
     image_meta: 'objects.ImageMeta',

nova/virt/libvirt/driver.py

@@ -5934,14 +5934,7 @@ def _set_features(self, guest, os_type, image_meta, flavor):
                 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'))
+            pmu = hardware.get_pmu_constraint(flavor, image_meta)
             if pmu is not None:
                 guest.features.append(
                     vconfig.LibvirtConfigGuestFeaturePMU(pmu))