diff --git a/nova/api/openstack/compute/volumes.py b/nova/api/openstack/compute/volumes.py index a97ae4e3718c..3947526ad70a 100644 --- a/nova/api/openstack/compute/volumes.py +++ b/nova/api/openstack/compute/volumes.py @@ -310,7 +310,7 @@ class VolumeAttachmentController(wsgi.Controller): assigned_mountpoint)} # TODO(mriedem): This API should return a 202 instead of a 200 response. - @wsgi.expected_errors((400, 404, 409)) + @wsgi.expected_errors((400, 403, 404, 409)) @validation.schema(volumes_schema.create_volume_attachment, '2.0', '2.48') @validation.schema(volumes_schema.create_volume_attachment_v249, '2.49') def create(self, req, server_id, body): @@ -352,6 +352,8 @@ class VolumeAttachmentController(wsgi.Controller): exception.MultiattachNotSupportedOldMicroversion, exception.MultiattachToShelvedNotSupported) as e: raise exc.HTTPBadRequest(explanation=e.format_message()) + except exception.TooManyDiskDevices as e: + raise exc.HTTPForbidden(explanation=e.format_message()) # The attach is async attachment = {} diff --git a/nova/exception.py b/nova/exception.py index a8fcd354ea45..86307456fccd 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -244,6 +244,12 @@ class InvalidBDMVolumeNotBootable(InvalidBDM): msg_fmt = _("Block Device %(id)s is not bootable.") +class TooManyDiskDevices(InvalidBDM): + msg_fmt = _('The maximum allowed number of disk devices (%(maximum)d) to ' + 'attach to a single instance has been exceeded.') + code = 403 + + class InvalidAttribute(Invalid): msg_fmt = _("Attribute not supported: %(attr)s") diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index d1af06c171c1..7d8bf5e06aa0 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -834,6 +834,31 @@ class ServersTest(ServersTestBase): created_server_id, post) self.assertEqual(403, ex.response.status_code) + def test_attach_vol_maximum_disk_devices_exceeded(self): + self.useFixture(nova_fixtures.CinderFixtureNewAttachFlow(self)) + + server = self._build_minimal_create_server_request() + created_server = self.api.post_server({"server": server}) + server_id = created_server['id'] + self._wait_for_state_change(created_server, 'BUILD') + + volume_id = '9a695496-44aa-4404-b2cc-ccab2501f87e' + LOG.info('Attaching volume %s to server %s', volume_id, server_id) + + # The fake driver doesn't implement get_device_name_for_instance, so + # we'll just raise the exception directly here, instead of simuluating + # an instance with 26 disk devices already attached. + with mock.patch.object(self.compute.driver, + 'get_device_name_for_instance') as mock_get: + mock_get.side_effect = exception.TooManyDiskDevices(maximum=26) + ex = self.assertRaises( + client.OpenStackApiException, self.api.post_server_volume, + server_id, dict(volumeAttachment=dict(volumeId=volume_id))) + expected = ('The maximum allowed number of disk devices (26) to ' + 'attach to a single instance has been exceeded.') + self.assertEqual(403, ex.response.status_code) + self.assertIn(expected, six.text_type(ex)) + class ServersTestV21(ServersTest): api_major_version = 'v2.1' diff --git a/nova/virt/driver.py b/nova/virt/driver.py index 8625bd7abcc6..f63652cb7515 100644 --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -491,7 +491,11 @@ class ComputeDriver(object): def attach_volume(self, context, connection_info, instance, mountpoint, disk_bus=None, device_type=None, encryption=None): - """Attach the disk to the instance at mountpoint using info.""" + """Attach the disk to the instance at mountpoint using info. + + :raises TooManyDiskDevices: if the maxmimum allowed devices to attach + to a single instance is exceeded. + """ raise NotImplementedError() def detach_volume(self, context, connection_info, instance, mountpoint, @@ -1014,6 +1018,8 @@ class ComputeDriver(object): :param disk_info: instance disk information :param migrate_data: a LiveMigrateData object :returns: migrate_data modified by the driver + :raises TooManyDiskDevices: if the maxmimum allowed devices to attach + to a single instance is exceeded. """ raise NotImplementedError() @@ -1706,12 +1712,18 @@ class ComputeDriver(object): The metadata of the image of the instance. :param nova.objects.BlockDeviceMapping root_bdm: The description of the root device. + :raises TooManyDiskDevices: if the maxmimum allowed devices to attach + to a single instance is exceeded. """ raise NotImplementedError() def default_device_names_for_instance(self, instance, root_device_name, *block_device_lists): - """Default the missing device names in the block device mapping.""" + """Default the missing device names in the block device mapping. + + :raises TooManyDiskDevices: if the maxmimum allowed devices to attach + to a single instance is exceeded. + """ raise NotImplementedError() def get_device_name_for_instance(self, instance, @@ -1728,6 +1740,8 @@ class ComputeDriver(object): implementation if not set. :returns: The chosen device name. + :raises TooManyDiskDevices: if the maxmimum allowed devices to attach + to a single instance is exceeded. """ raise NotImplementedError() diff --git a/nova/virt/libvirt/blockinfo.py b/nova/virt/libvirt/blockinfo.py index 429dc87f85b5..82ca0037243d 100644 --- a/nova/virt/libvirt/blockinfo.py +++ b/nova/virt/libvirt/blockinfo.py @@ -191,8 +191,7 @@ def find_disk_dev_for_disk_bus(mapping, bus, if disk_dev not in assigned_devices: return disk_dev - msg = _("No free disk device names for prefix '%s'") % dev_prefix - raise exception.InternalError(msg) + raise exception.TooManyDiskDevices(maximum=max_dev) def is_disk_bus_valid_for_virt(virt_type, disk_bus):