From 9af110a0cc86724e7c23d5fb7ef72d145de933e1 Mon Sep 17 00:00:00 2001
From: melanie witt <melwittt@gmail.com>
Date: Fri, 18 Jan 2019 16:30:40 +0000
Subject: [PATCH 02/10] Raise 403 instead of 500 error from attach volume API

Currently, the libvirt driver has a limit on the maximum number of
disk devices allowed to attach to a single instance of 26. If a user
attempts to attach a volume which would make the total number of
attached disk devices > 26 for the instance, the user receives a
500 error from the API.

This adds a new exception type TooManyDiskDevices and raises it for the
"No free disk devices names" condition, instead of InternalError, and
handles it in the attach volume API. We raise TooManyDiskDevices
directly from the libvirt driver because InternalError is ambiguous and
can be raised for different error reasons within the same method call.

During backport functional test was dropped due to high complexity of
backporting it.

Closes-Bug: #1770527

Change-Id: I1b08ed6826d7eb41ecdfc7102e5e8fcf3d1eb2e1
---
 nova/api/openstack/compute/volumes.py |  5 +++--
 nova/exception.py                     |  6 ++++++
 nova/virt/driver.py                   | 19 +++++++++++++++++--
 nova/virt/libvirt/blockinfo.py        |  4 +---
 4 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/nova/api/openstack/compute/volumes.py b/nova/api/openstack/compute/volumes.py
index db8855bbcf..beddf553d9 100644
--- a/nova/api/openstack/compute/volumes.py
+++ b/nova/api/openstack/compute/volumes.py
@@ -302,8 +302,7 @@ class VolumeAttachmentController(wsgi.Controller):
             instance.uuid,
             assigned_mountpoint)}
 
-    @extensions.expected_errors((400, 404, 409))
-    @validation.schema(volumes_schema.create_volume_attachment)
+    @extensions.expected_errors((400, 403, 404, 409))
     def create(self, req, server_id, body):
         """Attach a volume to an instance."""
         context = req.environ['nova.context']
@@ -337,6 +336,8 @@ class VolumeAttachmentController(wsgi.Controller):
         except (exception.InvalidVolume,
                 exception.InvalidDevicePath, exception.InvalidInput) 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 1d9d9b551a..1e0a5a3609 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -242,6 +242,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/virt/driver.py b/nova/virt/driver.py
index 7bd446451d..78b40a4b21 100644
--- a/nova/virt/driver.py
+++ b/nova/virt/driver.py
@@ -465,7 +465,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, connection_info, instance, mountpoint,
@@ -808,6 +812,9 @@ class ComputeDriver(object):
         :param network_info: instance network information
         :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()
 
@@ -1522,12 +1529,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,
@@ -1544,6 +1557,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 41e62d8108..d5c1f2fcba 100644
--- a/nova/virt/libvirt/blockinfo.py
+++ b/nova/virt/libvirt/blockinfo.py
@@ -192,9 +192,7 @@ def find_disk_dev_for_disk_bus(mapping, bus,
             if disk_dev not in assigned_devices:
                 return disk_dev
 
-    raise exception.NovaException(
-        _("No free disk device names for prefix '%s'") %
-        dev_prefix)
+    raise exception.TooManyDiskDevices(maximum=max_dev)
 
 
 def is_disk_bus_valid_for_virt(virt_type, disk_bus):
-- 
2.13.7

