compute: Validate a BDMs disk_bus when provided

Previously disk_bus values were never validated and could easily end up
being ignored by the underlying virt driver and hypervisor.

For example, a common mistake made by users is to request a virtio-scsi
disk_bus when using the libvirt virt driver. This however isn't a valid
bus and is ignored, defaulting back to the virtio (virtio-blk) bus.

This change adds a simple validation in the compute API using the
potential disk_bus values provided by the DiskBus field class as used
when validating the hw_*_bus image properties.

Conflicts:
    nova/tests/unit/compute/test_compute_api.py

NOTE(lyarwood): Conflict as If9c459a9a0aa752c478949e4240286cbdb146494 is
not present in stable/train. test_validate_bdm_disk_bus is also updated
as Ib31ba2cbff0ebb22503172d8801b6e0c3d2aa68a is not present in
stable/train.

Closes-Bug: #1876301
Change-Id: I77b28b9cc8f99b159f628f4655d85ff305a71db8
(cherry picked from commit 5913bd889f9d3dfc8d154415e666c821054c229d)
(cherry picked from commit fb31ae430a2e4f8869e77e31ea0d6a9478f6aa61)

7 files changed, 37 insertions, 4 deletions

diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml
index 2a0858f09a..8bc699f575 100644
--- a/api-ref/source/parameters.yaml
+++ b/api-ref/source/parameters.yaml
@@ -2449,9 +2449,10 @@ disk_available_least_total:
 disk_bus:
   description: |
     Disk bus type, some hypervisors (currently only libvirt) support
-    specify this parameter. Some example disk_bus values can be: `ide`,
-    `usb`, `virtio`, `scsi`. This is not an exhaustive list as it depends
-    on the virtualization driver, and may change as more support is added.
+    specify this parameter. Some example disk_bus values can be: ``fdc``,
+    ``ide``, ``sata``, ``scsi``, ``usb``, ``virtio``, ``xen``, ``lxc``
+    and ``uml``. Support for each bus type depends on the virtualization driver
+    and underlying hypervisor.
   in: body
   required: false
   type: string
diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py
index 42308af931..75e145245d 100644
--- a/nova/api/openstack/compute/servers.py
+++ b/nova/api/openstack/compute/servers.py
@@ -762,6 +762,7 @@ class ServersController(wsgi.Controller):
                 exception.InvalidBDMEphemeralSize,
                 exception.InvalidBDMFormat,
                 exception.InvalidBDMSwapSize,
+                exception.InvalidBDMDiskBus,
                 exception.VolumeTypeNotFound,
                 exception.AutoDiskConfigDisabledByImage,
                 exception.InstanceGroupNotFound,
diff --git a/nova/compute/api.py b/nova/compute/api.py
index 69837f62d7..803c9df992 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -1664,6 +1664,13 @@ class API(base.Base):
                     "(source: 'blank', dest: 'volume') need to have non-zero "
                     "size"))
 
+            # NOTE(lyarwood): Ensure the disk_bus is at least known to Nova.
+            # The virt driver may reject this later but for now just ensure
+            # it's listed as an acceptable value of the DiskBus field class.
+            disk_bus = bdm.disk_bus if 'disk_bus' in bdm else None
+            if disk_bus and disk_bus not in fields_obj.DiskBus.ALL:
+                raise exception.InvalidBDMDiskBus(disk_bus=disk_bus)
+
         ephemeral_size = sum(bdm.volume_size or instance_type['ephemeral_gb']
                 for bdm in block_device_mappings
                 if block_device.new_format_is_ephemeral(bdm))
diff --git a/nova/exception.py b/nova/exception.py
index c98dc0b15d..5fcd3f0baf 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -254,6 +254,11 @@ class TooManyDiskDevices(InvalidBDM):
     code = 403
 
 
+class InvalidBDMDiskBus(InvalidBDM):
+    msg_fmr = _("Block Device Mapping is invalid: The provided disk bus "
+                "%(disk_bus)s is not valid.")
+
+
 class InvalidAttribute(Invalid):
     msg_fmt = _("Attribute not supported: %(attr)s")
 
diff --git a/nova/objects/fields.py b/nova/objects/fields.py
index a768d0236a..366a970737 100644
--- a/nova/objects/fields.py
+++ b/nova/objects/fields.py
@@ -339,6 +339,8 @@ class DiskBus(BaseNovaEnum):
 
     # NOTE(aspiers): If you change this, don't forget to update the
     # docs and metadata for hw_*_bus in glance.
+    # NOTE(lyarwood): Also update the possible values in the api-ref for the
+    # block_device_mapping_v2.disk_bus parameter.
     FDC = "fdc"
     IDE = "ide"
     SATA = "sata"
diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_serversV21.py
index 80af96ebb6..d5736cecce 100644
--- a/nova/tests/unit/api/openstack/compute/test_serversV21.py
+++ b/nova/tests/unit/api/openstack/compute/test_serversV21.py
@@ -4993,7 +4993,8 @@ class ServersControllerCreateTest(test.TestCase):
                           (exception.InvalidBDMVolume, {'id': 'fake'}),
                           (exception.InvalidBDMImage, {'id': 'fake'}),
                           (exception.InvalidBDMBootSequence, {}),
-                          (exception.InvalidBDMLocalsLimit, {}))
+                          (exception.InvalidBDMLocalsLimit, {}),
+                          (exception.InvalidBDMDiskBus, {'disk_bus': 'foo'}))
 
         ex_iter = iter(bdm_exceptions)
 
diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py
index d17a9b5492..f1b1a19310 100644
--- a/nova/tests/unit/compute/test_compute_api.py
+++ b/nova/tests/unit/compute/test_compute_api.py
@@ -4469,6 +4469,22 @@ class _ComputeAPIUnitTestMixIn(object):
                           bdms)
         mock_get_image.assert_called_once_with(self.context, image_id)
 
+    @mock.patch('nova.compute.api.API._get_image')
+    def test_validate_bdm_disk_bus(self, mock_get_image):
+        """Tests that _validate_bdm fail if an invalid disk_bus is provided
+        """
+        instance = self._create_instance_obj()
+        bdms = objects.BlockDeviceMappingList(objects=[
+            objects.BlockDeviceMapping(
+                boot_index=0, image_id=instance.image_ref,
+                source_type='image', destination_type='volume',
+                volume_type=None, snapshot_id=None, volume_id=None,
+                volume_size=1, disk_bus='virtio-scsi')])
+        self.assertRaises(exception.InvalidBDMDiskBus,
+                          self.compute_api._validate_bdm,
+                          self.context, instance, objects.Flavor(),
+                          bdms)
+
     @mock.patch.object(objects.service, 'get_minimum_version_all_cells',
                        return_value=compute_api.MIN_COMPUTE_VOLUME_TYPE)
     def test_the_specified_volume_type_id_assignment_to_name(