diff options
author | Lee Yarwood <lyarwood@redhat.com> | 2020-05-01 14:20:04 +0100 |
---|---|---|
committer | Lee Yarwood <lyarwood@redhat.com> | 2020-07-29 16:05:48 +0000 |
commit | 5913bd889f9d3dfc8d154415e666c821054c229d (patch) | |
tree | e7743504c155477bdc1d8be6118210ddc6b0a5e5 | |
parent | 49c3ac7dfa7aca5504f9d19958ff82a40e47f5a1 (diff) | |
download | nova-5913bd889f9d3dfc8d154415e666c821054c229d.tar.gz |
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.
Closes-Bug: #1876301
Change-Id: I77b28b9cc8f99b159f628f4655d85ff305a71db8
-rw-r--r-- | api-ref/source/parameters.yaml | 7 | ||||
-rw-r--r-- | nova/api/openstack/compute/servers.py | 1 | ||||
-rw-r--r-- | nova/compute/api.py | 7 | ||||
-rw-r--r-- | nova/exception.py | 5 | ||||
-rw-r--r-- | nova/objects/fields.py | 2 | ||||
-rw-r--r-- | nova/tests/unit/api/openstack/compute/test_serversV21.py | 3 | ||||
-rw-r--r-- | nova/tests/unit/compute/test_compute_api.py | 17 |
7 files changed, 38 insertions, 4 deletions
diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 9ed259923e..cc36fcbe8a 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -2515,9 +2515,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 9517021f92..dafb4c3315 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -752,6 +752,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 410b9aedaf..9455194043 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1771,6 +1771,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 5c472fc298..a5dff45bac 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -251,6 +251,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 8891afb153..f94f2ba027 100644 --- a/nova/objects/fields.py +++ b/nova/objects/fields.py @@ -341,6 +341,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 cb911dfe51..e734fbbbad 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/nova/tests/unit/api/openstack/compute/test_serversV21.py @@ -5056,7 +5056,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 e98ea2aa67..961861743c 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -4616,6 +4616,23 @@ class _ComputeAPIUnitTestMixIn(object): bdms, image_cache, volumes) 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')]) + image_cache = volumes = {} + self.assertRaises(exception.InvalidBDMDiskBus, + self.compute_api._validate_bdm, + self.context, instance, objects.Flavor(), + bdms, image_cache, volumes) + def test_the_specified_volume_type_id_assignment_to_name(self): """Test _check_requested_volume_type method is called, if the user is using the volume type ID, assign volume_type to volume type name. |