diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 527fbdcd5e32..ed447469243a 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -712,6 +712,8 @@ class ServersController(wsgi.Controller): exception.AutoDiskConfigDisabledByImage, exception.ImageCPUPinningForbidden, exception.ImageCPUThreadPolicyForbidden, + exception.InvalidCPUAllocationPolicy, + exception.InvalidCPUThreadAllocationPolicy, exception.ImageNUMATopologyIncomplete, exception.ImageNUMATopologyForbidden, exception.ImageNUMATopologyAsymmetric, diff --git a/nova/exception.py b/nova/exception.py index 23c929e21ec6..33575f730b57 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -2279,6 +2279,16 @@ class InvalidEmulatorThreadsPolicy(Invalid): "given: '%(requested)s', available: '%(available)s'.") +class InvalidCPUAllocationPolicy(Invalid): + msg_fmt = _("CPU policy requested from '%(source)s' is invalid, " + "given: '%(requested)s', available: '%(available)s'.") + + +class InvalidCPUThreadAllocationPolicy(Invalid): + msg_fmt = _("CPU thread policy requested from '%(source)s' is invalid, " + "given: '%(requested)s', available: '%(available)s'.") + + class BadRequirementEmulatorThreadsPolicy(Invalid): msg_fmt = _("An isolated CPU emulator threads option requires a dedicated " "CPU policy option.") diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_serversV21.py index f5f69a0596b7..170a54c7ba0c 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/nova/tests/unit/api/openstack/compute/test_serversV21.py @@ -4357,12 +4357,16 @@ class ServersControllerCreateTest(test.TestCase): @mock.patch('nova.virt.hardware.numa_get_constraints') def _test_create_instance_numa_topology_wrong(self, exc, numa_constraints_mock): - numa_constraints_mock.side_effect = exc(**{'name': None, - 'cpunum': 0, - 'cpumax': 0, - 'cpuset': None, - 'memsize': 0, - 'memtotal': 0}) + numa_constraints_mock.side_effect = exc(**{ + 'name': None, + 'source': 'flavor', + 'requested': 'dummy', + 'available': str(objects.fields.CPUAllocationPolicy.ALL), + 'cpunum': 0, + 'cpumax': 0, + 'cpuset': None, + 'memsize': 0, + 'memtotal': 0}) self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create, self.req, body=self.body) @@ -4374,6 +4378,8 @@ class ServersControllerCreateTest(test.TestCase): exception.ImageNUMATopologyCPUOutOfRange, exception.ImageNUMATopologyCPUDuplicates, exception.ImageNUMATopologyCPUsUnassigned, + exception.InvalidCPUAllocationPolicy, + exception.InvalidCPUThreadAllocationPolicy, exception.ImageNUMATopologyMemoryOutOfRange]: self._test_create_instance_numa_topology_wrong(exc) diff --git a/nova/tests/unit/virt/test_hardware.py b/nova/tests/unit/virt/test_hardware.py index 15c862fd1be0..d5f11b5153aa 100644 --- a/nova/tests/unit/virt/test_hardware.py +++ b/nova/tests/unit/virt/test_hardware.py @@ -359,7 +359,7 @@ class VCPUTopologyTest(test.NoDBTestCase): image_meta = objects.ImageMeta.from_dict(topo_test["image"]) if type(topo_test["expect"]) == tuple: (preferred, - maximum) = hw._get_cpu_topology_constraints( + maximum) = hw.get_cpu_topology_constraints( topo_test["flavor"], image_meta) self.assertEqual(topo_test["expect"][0], preferred.sockets) @@ -370,10 +370,32 @@ class VCPUTopologyTest(test.NoDBTestCase): self.assertEqual(topo_test["expect"][5], maximum.threads) else: self.assertRaises(topo_test["expect"], - hw._get_cpu_topology_constraints, + hw.get_cpu_topology_constraints, topo_test["flavor"], image_meta) + def test_invalid_flavor_values(self): + extra_specs = { + # test non-integer value of these extra specs + "hw:cpu_sockets": "8", + "hw:cpu_cores": "2", + "hw:cpu_threads": "1", + "hw:cpu_max_sockets": "8", + "hw:cpu_max_cores": "2", + "hw:cpu_max_threads": "1", + } + image_meta = objects.ImageMeta.from_dict({"properties": {}}) + + for key in extra_specs: + extra_specs_invalid = extra_specs.copy() + extra_specs_invalid[key] = 'foo' + flavor = objects.Flavor(vcpus=16, memory_mb=2048, + extra_specs=extra_specs_invalid) + self.assertRaises(exception.InvalidRequest, + hw.get_cpu_topology_constraints, + flavor, + image_meta) + def test_possible_topologies(self): testdata = [ { @@ -1248,7 +1270,42 @@ class NUMATopologyTest(test.NoDBTestCase): cpu_policy=fields.CPUAllocationPolicy.DEDICATED, )]), }, - + { + # Invalid CPU pinning policy + "flavor": objects.Flavor(vcpus=4, memory_mb=2048, + extra_specs={ + "hw:cpu_policy": "foo", + }), + "image": { + "properties": {} + }, + "expect": exception.InvalidCPUAllocationPolicy, + }, + { + # Invalid CPU thread pinning + "flavor": objects.Flavor(vcpus=4, memory_mb=2048, + extra_specs={ + "hw:cpu_policy": fields.CPUAllocationPolicy.DEDICATED, + "hw:cpu_thread_policy": "foo", + }), + "image": { + "properties": {} + }, + "expect": exception.InvalidCPUThreadAllocationPolicy, + }, + { + # Invalid CPU thread pinning in image + "flavor": objects.Flavor(vcpus=4, memory_mb=2048, + extra_specs={ + "hw:cpu_policy": fields.CPUAllocationPolicy.SHARED, + }), + "image": { + "properties": { + "hw_cpu_policy": fields.CPUAllocationPolicy.DEDICATED, + } + }, + "expect": exception.ImageCPUPinningForbidden, + }, ] for testitem in testdata: diff --git a/nova/virt/hardware.py b/nova/virt/hardware.py index a6ae6ae3d44c..a86b97637a48 100644 --- a/nova/virt/hardware.py +++ b/nova/virt/hardware.py @@ -188,6 +188,10 @@ def get_number_of_serial_ports(flavor, image_meta): :param flavor: Flavor object to read extra specs from :param image_meta: nova.objects.ImageMeta object instance + :raises: exception.ImageSerialPortNumberInvalid if the serial port count + is not a valid integer + :raises: exception.ImageSerialPortNumberExceedFlavorValue if the serial + port count defined in image is greater than that of flavor :returns: number of serial ports """ flavor_num_ports, image_num_ports = _get_flavor_image_meta( @@ -252,7 +256,7 @@ def _score_cpu_topology(topology, wanttopology): return score -def _get_cpu_topology_constraints(flavor, image_meta): +def get_cpu_topology_constraints(flavor, image_meta): """Get the topology constraints declared in flavor or image Extracts the topology constraints from the configuration defined in @@ -287,8 +291,8 @@ def _get_cpu_topology_constraints(flavor, image_meta): :raises: exception.ImageVCPUTopologyRangeExceeded if the preferred counts set against the image exceed the maximum counts set against the image or flavor - :raises: ValueError if one of the provided flavor properties is a - non-integer + :raises: exception.InvalidRequest if one of the provided flavor properties + is a non-integer :returns: A two-tuple of objects.VirtCPUTopology instances. The first element corresponds to the preferred topology, while the latter corresponds to the maximum topology, @@ -301,9 +305,13 @@ def _get_cpu_topology_constraints(flavor, image_meta): flavor_max_threads, image_max_threads = _get_flavor_image_meta( 'cpu_max_threads', flavor, image_meta, 0) # image metadata is already of the correct type - flavor_max_sockets = int(flavor_max_sockets) - flavor_max_cores = int(flavor_max_cores) - flavor_max_threads = int(flavor_max_threads) + try: + flavor_max_sockets = int(flavor_max_sockets) + flavor_max_cores = int(flavor_max_cores) + flavor_max_threads = int(flavor_max_threads) + except ValueError as e: + msg = _('Invalid flavor extra spec. Error: %s') % six.text_type(e) + raise exception.InvalidRequest(msg) LOG.debug("Flavor limits %(sockets)d:%(cores)d:%(threads)d", {"sockets": flavor_max_sockets, @@ -337,9 +345,13 @@ def _get_cpu_topology_constraints(flavor, image_meta): 'cpu_cores', flavor, image_meta, 0) flavor_threads, image_threads = _get_flavor_image_meta( 'cpu_threads', flavor, image_meta, 0) - flavor_sockets = int(flavor_sockets) - flavor_cores = int(flavor_cores) - flavor_threads = int(flavor_threads) + try: + flavor_sockets = int(flavor_sockets) + flavor_cores = int(flavor_cores) + flavor_threads = int(flavor_threads) + except ValueError as e: + msg = _('Invalid flavor extra spec. Error: %s') % six.text_type(e) + raise exception.InvalidRequest(msg) LOG.debug("Flavor pref %(sockets)d:%(cores)d:%(threads)d", {"sockets": flavor_sockets, @@ -562,7 +574,7 @@ def _get_desirable_cpu_topologies(flavor, image_meta, allow_threads=True, {"flavor": flavor, "image_meta": image_meta, "threads": allow_threads}) - preferred, maximum = _get_cpu_topology_constraints(flavor, image_meta) + preferred, maximum = get_cpu_topology_constraints(flavor, image_meta) LOG.debug("Topology preferred %(preferred)s, maximum %(maximum)s", {"preferred": preferred, "maximum": maximum}) @@ -1294,11 +1306,25 @@ def _get_cpu_policy_constraint(flavor, image_meta): :param image_meta: ``nova.objects.ImageMeta`` instance :raises: exception.ImageCPUPinningForbidden if policy is defined on both image and flavor and these policies conflict. + :raises: exception.InvalidCPUAllocationPolicy if policy is defined with + invalid value in image or flavor. :returns: The CPU policy requested. """ flavor_policy, image_policy = _get_flavor_image_meta( 'cpu_policy', flavor, image_meta) + if flavor_policy and (flavor_policy not in fields.CPUAllocationPolicy.ALL): + raise exception.InvalidCPUAllocationPolicy( + source='flavor extra specs', + requested=flavor_policy, + available=str(fields.CPUAllocationPolicy.ALL)) + + if image_policy and (image_policy not in fields.CPUAllocationPolicy.ALL): + raise exception.InvalidCPUAllocationPolicy( + source='image properties', + requested=image_policy, + available=str(fields.CPUAllocationPolicy.ALL)) + if flavor_policy == fields.CPUAllocationPolicy.DEDICATED: cpu_policy = flavor_policy elif flavor_policy == fields.CPUAllocationPolicy.SHARED: @@ -1321,11 +1347,27 @@ def _get_cpu_thread_policy_constraint(flavor, image_meta): :param image_meta: ``nova.objects.ImageMeta`` instance :raises: exception.ImageCPUThreadPolicyForbidden if policy is defined on both image and flavor and these policies conflict. + :raises: exception.InvalidCPUThreadAllocationPolicy if policy is defined + with invalid value in image or flavor. :returns: The CPU thread policy requested. """ flavor_policy, image_policy = _get_flavor_image_meta( 'cpu_thread_policy', flavor, image_meta) + if flavor_policy and ( + flavor_policy not in fields.CPUThreadAllocationPolicy.ALL): + raise exception.InvalidCPUThreadAllocationPolicy( + source='flavor extra specs', + requested=flavor_policy, + available=str(fields.CPUThreadAllocationPolicy.ALL)) + + if image_policy and ( + image_policy not in fields.CPUThreadAllocationPolicy.ALL): + raise exception.InvalidCPUThreadAllocationPolicy( + source='image properties', + requested=image_policy, + available=str(fields.CPUThreadAllocationPolicy.ALL)) + if flavor_policy in [None, fields.CPUThreadAllocationPolicy.PREFER]: policy = flavor_policy or image_policy elif image_policy and image_policy != flavor_policy: @@ -1496,6 +1538,12 @@ def numa_get_constraints(flavor, image_meta): policy conflicts with CPU allocation policy :raises: exception.ImageCPUThreadPolicyForbidden if a CPU thread policy specified in a flavor conflicts with one defined in image metadata + :raises: exception.BadRequirementEmulatorThreadsPolicy if CPU emulator + threads policy conflicts with CPU allocation policy + :raises: exception.InvalidCPUAllocationPolicy if policy is defined with + invalid value in image or flavor. + :raises: exception.InvalidCPUThreadAllocationPolicy if policy is defined + with invalid value in image or flavor. :returns: objects.InstanceNUMATopology, or None """ numa_topology = None