openstack
/
nova
Code Issues Proposed changes

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

Virtually all of the code for parsing 'hw:'-prefixed extra specs and
'hw_'-prefix image metadata properties lives in the 'nova.virt.hardware'
module. It makes sense for these to be included there. Do that.

Change-Id: I1fabdf1827af597f9e5fdb40d5aef244024dd015
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This commit is contained in:
Stephen Finucane 2021-05-20 12:56:34 +01:00
parent f42fb1241b
commit eacecc2433
7 changed files with 104 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
nova/api/openstack/compute/servers.py
View File

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

24
nova/compute/api.py
View File

@ -838,17 +838,10 @@ class API:
"""
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)
@ -858,19 +851,6 @@ class API:
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', {})

5
nova/exception.py
View File

@ -1861,11 +1861,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")

51
nova/tests/unit/compute/test_api.py
View File

@ -7158,57 +7158,6 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
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

50
nova/tests/unit/virt/test_hardware.py
View File

@ -5452,6 +5452,56 @@ class PCINUMAAffinityPolicyTest(test.NoDBTestCase):
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):

51
nova/virt/hardware.py
View File

@ -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',

9
nova/virt/libvirt/driver.py
View File

@ -5906,14 +5906,7 @@ class LibvirtDriver(driver.ComputeDriver):
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))