diff --git a/nova/api/validation/extra_specs/hw.py b/nova/api/validation/extra_specs/hw.py index ab558d432c..c50355b794 100644 --- a/nova/api/validation/extra_specs/hw.py +++ b/nova/api/validation/extra_specs/hw.py @@ -539,6 +539,19 @@ feature_flag_validators = [ 'description': 'Whether to enable packed virtqueue', }, ), + base.ExtraSpecValidator( + name='hw:sound_model', + description=( + 'The model of the attached sound device. ' + 'Only supported by the libvirt virt driver. ' + 'If unset, no sound device is attached.' + ), + value={ + 'type': str, + 'description': 'A sound model', + 'enum': fields.SoundModelType.ALL, + }, + ), ] ephemeral_encryption_validators = [ diff --git a/nova/compute/api.py b/nova/compute/api.py index 5223b29b69..d13a71ff8b 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -122,9 +122,10 @@ SUPPORT_VNIC_TYPE_REMOTE_MANAGED = 61 MIN_COMPUTE_VDPA_ATTACH_DETACH = 62 MIN_COMPUTE_VDPA_HOTPLUG_LIVE_MIGRATION = 63 - SUPPORT_SHARES = 67 +MIN_COMPUTE_SOUND_MODEL_TRAITS = 69 + # FIXME(danms): Keep a global cache of the cells we find the # first time we look. This needs to be refreshed on a timer or # trigger. @@ -1049,6 +1050,12 @@ class API: flavor, root_bdm, validate_numa=validate_numa) + # Do we support adding a sound device? + image_meta = objects.ImageMeta.from_dict(image) + sound_model = hardware.get_sound_model(flavor, image_meta) + if sound_model and (min_comp_ver < MIN_COMPUTE_SOUND_MODEL_TRAITS): + raise exception.SoundModelRequestOldCompute() + def _check_support_vnic_accelerator( self, context, requested_networks, min_comp_ver): if requested_networks: diff --git a/nova/objects/service.py b/nova/objects/service.py index 6c4cca7909..60863e5f3a 100644 --- a/nova/objects/service.py +++ b/nova/objects/service.py @@ -31,7 +31,7 @@ LOG = logging.getLogger(__name__) # NOTE(danms): This is the global service version counter -SERVICE_VERSION = 68 +SERVICE_VERSION = 69 # NOTE(danms): This is our SERVICE_VERSION history. The idea is that any @@ -237,6 +237,9 @@ SERVICE_VERSION_HISTORY = ( # Version 68: Compute RPC v6.4: # Add support for shares {'compute_rpc': '6.4'}, + # Version 69: Compute RPC v6.4: + # Compute manager supports sound model traits + {'compute_rpc': '6.4'}, ) # This is the version after which we can rely on having a persistent diff --git a/nova/scheduler/request_filter.py b/nova/scheduler/request_filter.py index 6f61e861cc..935f2408e8 100644 --- a/nova/scheduler/request_filter.py +++ b/nova/scheduler/request_filter.py @@ -444,6 +444,27 @@ def ephemeral_encryption_filter( return True +@trace_request_filter +def virtio_sound_filter( + ctxt: nova_context.RequestContext, + request_spec: 'objects.RequestSpec' +) -> bool: + """Filter out hosts which do not support virtio sound devices + + This filter will only retain compute node resource providers that support + virtio sound devices. + """ + # Skip if the instance does not request a virtio sound device + model = hardware.get_sound_model(request_spec.flavor, request_spec.image) + if model != objects.fields.SoundModelType.VIRTIO: + LOG.debug('virtio_sound_filter skipped') + return False + + request_spec.root_required.add(os_traits.COMPUTE_SOUND_MODEL_VIRTIO) + LOG.debug('virtio_sound_filter added trait COMPUTE_SOUND_MODEL_VIRTIO') + return True + + ALL_REQUEST_FILTERS = [ require_tenant_aggregate, map_az_to_placement_aggregate, @@ -456,6 +477,7 @@ ALL_REQUEST_FILTERS = [ routed_networks_filter, remote_managed_ports_filter, ephemeral_encryption_filter, + virtio_sound_filter ] diff --git a/nova/tests/unit/scheduler/test_request_filter.py b/nova/tests/unit/scheduler/test_request_filter.py index 4a15f6b56a..86bb8d389d 100644 --- a/nova/tests/unit/scheduler/test_request_filter.py +++ b/nova/tests/unit/scheduler/test_request_filter.py @@ -724,3 +724,25 @@ class TestRequestFilter(test.NoDBTestCase): ot.COMPUTE_EPHEMERAL_ENCRYPTION_LUKS}, reqspec.root_required) self.assertEqual(set(), reqspec.root_forbidden) + + def test_virtio_sound_filter(self): + # First ensure that virtio_sound_filter is included + self.assertIn(request_filter.virtio_sound_filter, + request_filter.ALL_REQUEST_FILTERS) + + # Request filter puts the trait into the request spec + reqspec = objects.RequestSpec( + flavor=objects.Flavor( + extra_specs={ + 'hw:sound_model': 'virtio' + }), + image=objects.ImageMeta( + properties=objects.ImageMetaProps())) + self.assertEqual(set(), reqspec.root_required) + self.assertEqual(set(), reqspec.root_forbidden) + self.assertTrue( + request_filter.virtio_sound_filter(self.context, reqspec)) + self.assertEqual( + {ot.COMPUTE_SOUND_MODEL_VIRTIO}, + reqspec.root_required) + self.assertEqual(set(), reqspec.root_forbidden) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 044c8fb013..fefbda41b2 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -1004,6 +1004,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, inst = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) self.assertPublicAPISignatures(baseinst, inst) + @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_sound_model_traits') @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_cpu_traits') @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_storage_bus_traits') @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_video_model_traits') @@ -1011,12 +1012,22 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch.object(host.Host, "has_min_version") def test_static_traits( self, mock_version, mock_vif_traits, mock_video_traits, - mock_storage_traits, mock_cpu_traits, + mock_storage_traits, mock_cpu_traits, mock_sound_traits ): """Ensure driver capabilities are correctly retrieved and cached.""" # we don't mock out calls to os_traits intentionally, so we need to # return valid traits here + mock_sound_traits.return_value = { + 'COMPUTE_SOUND_MODEL_AC97': True, + 'COMPUTE_SOUND_MODEL_ES1370': True, + 'COMPUTE_SOUND_MODEL_ICH6': True, + 'COMPUTE_SOUND_MODEL_ICH9': True, + 'COMPUTE_SOUND_MODEL_PCSPK': True, + 'COMPUTE_SOUND_MODEL_SB16': True, + 'COMPUTE_SOUND_MODEL_USB': True, + 'COMPUTE_SOUND_MODEL_VIRTIO': True, + } mock_cpu_traits.return_value = {'HW_CPU_HYPERTHREADING': True} mock_storage_traits.return_value = {'COMPUTE_STORAGE_BUS_VIRTIO': True} mock_video_traits.return_value = {'COMPUTE_GRAPHICS_MODEL_VGA': True} @@ -1035,6 +1046,14 @@ class LibvirtConnTestCase(test.NoDBTestCase, 'COMPUTE_SECURITY_TPM_TIS': False, 'COMPUTE_SECURITY_TPM_CRB': False, 'COMPUTE_STORAGE_BUS_VIRTIO': True, + 'COMPUTE_SOUND_MODEL_AC97': True, + 'COMPUTE_SOUND_MODEL_ES1370': True, + 'COMPUTE_SOUND_MODEL_ICH6': True, + 'COMPUTE_SOUND_MODEL_ICH9': True, + 'COMPUTE_SOUND_MODEL_PCSPK': True, + 'COMPUTE_SOUND_MODEL_SB16': True, + 'COMPUTE_SOUND_MODEL_USB': True, + 'COMPUTE_SOUND_MODEL_VIRTIO': True, 'COMPUTE_VIOMMU_MODEL_AUTO': True, 'COMPUTE_VIOMMU_MODEL_INTEL': True, 'COMPUTE_VIOMMU_MODEL_SMMUV3': True, @@ -1049,7 +1068,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertEqual(expected, static_traits) for mock_traits in ( mock_vif_traits, mock_video_traits, mock_storage_traits, - mock_cpu_traits, + mock_cpu_traits, mock_sound_traits ): mock_traits.assert_called_once_with() mock_traits.reset_mock() @@ -1061,20 +1080,31 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertEqual(expected, static_traits) for mock_traits in ( mock_vif_traits, mock_video_traits, mock_storage_traits, - mock_cpu_traits, + mock_cpu_traits, mock_sound_traits ): mock_traits.assert_not_called() @mock.patch.object(libvirt_driver.LOG, 'debug') + @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_sound_model_traits') @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_cpu_traits') @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_storage_bus_traits') @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_video_model_traits') @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_vif_model_traits') def test_static_traits_invalid_trait( self, mock_vif_traits, mock_video_traits, mock_storage_traits, - mock_cpu_traits, mock_log, + mock_cpu_traits, mock_sound_traits, mock_log, ): """Ensure driver capabilities are correctly retrieved and cached.""" + mock_sound_traits.return_value = { + 'COMPUTE_SOUND_MODEL_AC97': True, + 'COMPUTE_SOUND_MODEL_ES1370': True, + 'COMPUTE_SOUND_MODEL_ICH6': True, + 'COMPUTE_SOUND_MODEL_ICH9': True, + 'COMPUTE_SOUND_MODEL_PCSPK': True, + 'COMPUTE_SOUND_MODEL_SB16': True, + 'COMPUTE_SOUND_MODEL_USB': True, + 'COMPUTE_SOUND_MODEL_VIRTIO': True, + } mock_cpu_traits.return_value = {'foo': True} mock_storage_traits.return_value = {'bar': True} mock_video_traits.return_value = {'baz': True} @@ -1088,6 +1118,14 @@ class LibvirtConnTestCase(test.NoDBTestCase, 'COMPUTE_SECURITY_TPM_2_0': False, 'COMPUTE_SECURITY_TPM_TIS': False, 'COMPUTE_SECURITY_TPM_CRB': False, + 'COMPUTE_SOUND_MODEL_AC97': True, + 'COMPUTE_SOUND_MODEL_ES1370': True, + 'COMPUTE_SOUND_MODEL_ICH6': True, + 'COMPUTE_SOUND_MODEL_ICH9': True, + 'COMPUTE_SOUND_MODEL_PCSPK': True, + 'COMPUTE_SOUND_MODEL_SB16': True, + 'COMPUTE_SOUND_MODEL_USB': True, + 'COMPUTE_SOUND_MODEL_VIRTIO': True, 'COMPUTE_VIOMMU_MODEL_AUTO': True, 'COMPUTE_VIOMMU_MODEL_INTEL': True, 'COMPUTE_VIOMMU_MODEL_SMMUV3': True, @@ -1245,6 +1283,47 @@ class LibvirtConnTestCase(test.NoDBTestCase, break self.assertFalse(version_arg_found) + @mock.patch.object(host.Host, 'has_min_version') + def test_libvirt_has_virtio_sound_but_not_qemu(self, mock_version): + def _fake_has_min_version(lv_ver=None, hv_ver=None, hv_type=None): + if lv_ver: + return True + return False + + mock_version.side_effect = _fake_has_min_version + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + static_traits = drvr.static_traits + self.assertFalse( + static_traits.get('COMPUTE_SOUND_MODEL_VIRTIO')) + + @mock.patch.object(host.Host, 'has_min_version') + def test_libvirt_no_virtio_sound(self, mock_version): + def _fake_has_min_version(lv_ver=None, hv_ver=None, hv_type=None): + if lv_ver: + return False + return False + + mock_version.side_effect = _fake_has_min_version + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + static_traits = drvr.static_traits + self.assertFalse( + static_traits.get('COMPUTE_SOUND_MODEL_VIRTIO')) + + @mock.patch.object(host.Host, 'has_min_version') + def test_libvirt_has_virtio_sound(self, mock_version): + def _fake_has_min_version(lv_ver=None, hv_ver=None, hv_type=None): + if lv_ver: + return True + return True + + mock_version.side_effect = _fake_has_min_version + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + static_traits = drvr.static_traits + self.assertTrue(static_traits.get('COMPUTE_SOUND_MODEL_VIRTIO')) + @mock.patch.object(libvirt_driver.LibvirtDriver, '_register_all_undefined_instance_details', new=mock.Mock()) @@ -7160,6 +7239,43 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertEqual(cfg.devices[7].model, 'tpm-crb') self.assertEqual(cfg.devices[7].secret_uuid, uuids.vtpm) + def test_get_guest_config_with_sound_model(self): + self.flags(virt_type='kvm', group='libvirt') + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + instance = objects.Instance(**self.test_instance) + image_meta = objects.ImageMeta.from_dict({ + 'disk_format': 'raw', + 'properties': { + 'hw_sound_model': 'sb16', + }, + }) + + disk_info = blockinfo.get_disk_info( + CONF.libvirt.virt_type, instance, image_meta) + cfg = drvr._get_guest_config(instance, [], image_meta, disk_info) + self.assertEqual(10, len(cfg.devices)) + self.assertIsInstance( + cfg.devices[0], vconfig.LibvirtConfigGuestDisk) + self.assertIsInstance( + cfg.devices[1], vconfig.LibvirtConfigGuestDisk) + self.assertIsInstance( + cfg.devices[2], vconfig.LibvirtConfigGuestSerial) + self.assertIsInstance( + cfg.devices[3], vconfig.LibvirtConfigGuestSound) + self.assertIsInstance( + cfg.devices[4], vconfig.LibvirtConfigGuestGraphics) + self.assertIsInstance( + cfg.devices[5], vconfig.LibvirtConfigGuestVideo) + self.assertIsInstance( + cfg.devices[6], vconfig.LibvirtConfigGuestInput) + self.assertIsInstance( + cfg.devices[7], vconfig.LibvirtConfigGuestRng) + self.assertIsInstance( + cfg.devices[8], vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance( + cfg.devices[9], vconfig.LibvirtConfigMemoryBalloon) + def test_get_guest_config_with_video_driver_vram(self): self.flags(enabled=False, group='vnc') self.flags(virt_type='kvm', group='libvirt') diff --git a/nova/virt/hardware.py b/nova/virt/hardware.py index d5506a6700..d35d17ff1c 100644 --- a/nova/virt/hardware.py +++ b/nova/virt/hardware.py @@ -2916,3 +2916,27 @@ def check_shares_supported(context, instance): ) ): raise exception.ForbiddenSharesNotConfiguredCorrectly() + + +def get_sound_model( + flavor: 'objects.Flavor', + image_meta: 'objects.ImageMeta', +) -> ty.Optional[str]: + """Get the sound device model, if any. + + :param flavor: ``nova.objects.Flavor`` instance + :param image_meta: ``nova.objects.ImageMeta`` instance + :raises: nova.exception.FlavorImageConflict if a value is specified in both + the flavor and the image, but the values do not match + :raises: nova.exception.Invalid if a value or combination of values is + invalid + :returns: A string containing the device model, else None. + """ + model = _get_unique_flavor_image_meta('sound_model', flavor, image_meta) + if model and model not in fields.SoundModelType.ALL: + raise exception.Invalid( + "Invalid sound device model %(model)r. Allowed values: %(valid)s." + % {'model': model, 'valid': ', '.join(fields.SoundModelType.ALL)} + ) + + return model diff --git a/nova/virt/libvirt/config.py b/nova/virt/libvirt/config.py index f3253c734f..fbb63a6e91 100644 --- a/nova/virt/libvirt/config.py +++ b/nova/virt/libvirt/config.py @@ -4036,3 +4036,15 @@ class LibvirtConfigGuestMetaNovaIp(LibvirtConfigObject): meta.set("address", str(self.address)) meta.set("ipVersion", str(self.ip_version)) return meta + + +class LibvirtConfigGuestSound(LibvirtConfigObject): + + def __init__(self, model): + super(LibvirtConfigGuestSound, self).__init__(root_name='sound') + self.model = model + + def format_dom(self): + meta = self._new_node('sound') + meta.set('model', str(self.model)) + return meta diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 17e6b3fc76..d6d55346a0 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -270,6 +270,10 @@ MIN_IGB_QEMU_VERSION = (8, 0, 0) MIN_VFIO_PCI_VARIANT_LIBVIRT_VERSION = (10, 0, 0) MIN_VFIO_PCI_VARIANT_QEMU_VERSION = (8, 2, 2) +# Minimum versions supporting the virtio sound model +MIN_VIRTIO_SOUND_LIBVIRT_VERSION = (10, 4, 0) +MIN_VIRTIO_SOUND_QEMU_VERSION = (8, 2, 0) + REGISTER_IMAGE_PROPERTY_DEFAULTS = [ 'hw_machine_type', 'hw_cdrom_bus', @@ -6882,6 +6886,22 @@ class LibvirtDriver(driver.ComputeDriver): vtpm = vconfig.LibvirtConfigGuestVTPM(vtpm_config, vtpm_secret_uuid) guest.add_device(vtpm) + def _add_sound_device( + self, + guest: vconfig.LibvirtConfigGuest, + flavor: 'objects.Flavor', + instance: 'objects.Instance', + image_meta: 'objects.ImageMeta', + ) -> None: + """Add a sound device to the guest, if requested.""" + # Enable sound support if required in the flavor or image. + sound_model = hardware.get_sound_model(flavor, image_meta) + if not sound_model: + return None + + sound_device = vconfig.LibvirtConfigGuestSound(sound_model) + guest.add_device(sound_device) + def _set_qemu_guest_agent(self, guest, flavor, instance, image_meta): # Enable qga only if the 'hw_qemu_guest_agent' is equal to yes if image_meta.properties.get('hw_qemu_guest_agent', False): @@ -7566,6 +7586,7 @@ class LibvirtDriver(driver.ComputeDriver): self._create_consoles(guest, instance, flavor, image_meta) self._guest_add_spice_channel(guest) + self._add_sound_device(guest, flavor, instance, image_meta) if self._guest_add_video_device(guest): self._add_video_driver(guest, image_meta, flavor) @@ -9625,6 +9646,7 @@ class LibvirtDriver(driver.ComputeDriver): traits.update(self._get_vif_model_traits()) traits.update(self._get_iommu_model_traits()) traits.update(self._get_tpm_traits()) + traits.update(self._get_sound_model_traits()) _, invalid_traits = ot.check_traits(traits) for invalid_trait in invalid_traits: @@ -13375,3 +13397,33 @@ class LibvirtDriver(driver.ComputeDriver): fs.source_dir = self._get_share_mount_path(instance, share) fs.target_dir = share.tag guest.add_device(fs) + + def _get_sound_model_traits(self) -> ty.Dict[str, bool]: + """Determine what sound models are supported. + + Not all traits generated by this function may be valid and the result + should be validated. + + :return: A dict of trait names mapped to boolean values. + """ + # NOTE(mikal): sadly libvirt's `getDomainCapabilities()` call does not + # include details of the supported sound models, so we need to instead + # infer it from libvirt versions. + + has_virtio = True + if not self._host.has_min_version(MIN_VIRTIO_SOUND_LIBVIRT_VERSION): + has_virtio = False + if not self._host.has_min_version( + hv_ver=MIN_VIRTIO_SOUND_QEMU_VERSION): + has_virtio = False + + return { + 'COMPUTE_SOUND_MODEL_SB16': True, + 'COMPUTE_SOUND_MODEL_ES1370': True, + 'COMPUTE_SOUND_MODEL_PCSPK': True, + 'COMPUTE_SOUND_MODEL_AC97': True, + 'COMPUTE_SOUND_MODEL_ICH6': True, + 'COMPUTE_SOUND_MODEL_ICH9': True, + 'COMPUTE_SOUND_MODEL_USB': True, + 'COMPUTE_SOUND_MODEL_VIRTIO': has_virtio + } diff --git a/releasenotes/notes/sound-model-extra-specs-2bcbe644b889005c.yaml b/releasenotes/notes/sound-model-extra-specs-2bcbe644b889005c.yaml new file mode 100644 index 0000000000..4ead886ea9 --- /dev/null +++ b/releasenotes/notes/sound-model-extra-specs-2bcbe644b889005c.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + The `hw:sound_model` flavor extra spec and the matching `hw_sound_model` + image property were added to allow the configuration of a sound device + within an instance. This is useful with the new spice-direct console + type. The default remains no sound device, but when using the libvirt + hypervisor driver you can select from `sb16`, `es1370`, `pcspk`, `ac97`, + `ich6`, `ich9`, `usb`, and `virtio`. For most use-cases `usb` is + likely to be the best choice unless you have at least libvirt 8.2.0 and + libvirt 10.4.0.