diff --git a/staging/coverage_config_x86_64.json b/staging/coverage_config_x86_64.json index 87c142cba..7e449b28a 100644 --- a/staging/coverage_config_x86_64.json +++ b/staging/coverage_config_x86_64.json @@ -1,5 +1,5 @@ { - "coverage_score": 67.98, + "coverage_score": 71.65, "exclude_path": "", "crate_features": "" } diff --git a/staging/vhost-device-sound/src/audio_backends.rs b/staging/vhost-device-sound/src/audio_backends.rs index 1592285c3..05c0048af 100644 --- a/staging/vhost-device-sound/src/audio_backends.rs +++ b/staging/vhost-device-sound/src/audio_backends.rs @@ -41,6 +41,9 @@ pub trait AudioBackend { fn stop(&self, _stream_id: u32) -> Result<()> { Ok(()) } + + #[cfg(test)] + fn as_any(&self) -> &dyn std::any::Any; } pub fn alloc_audio_backend( @@ -56,3 +59,35 @@ pub fn alloc_audio_backend( BackendType::Alsa => Ok(Box::new(AlsaBackend::new(streams))), } } + +#[cfg(test)] +mod tests { + use std::any::TypeId; + + use super::*; + + #[test] + fn test_alloc_audio_backend() { + crate::init_logger(); + { + let v = BackendType::Null; + let value = alloc_audio_backend(v, Default::default()).unwrap(); + assert_eq!(TypeId::of::(), value.as_any().type_id()); + } + #[cfg(feature = "pw-backend")] + { + use pipewire::{test_utils::PipewireTestHarness, *}; + + let _test_harness = PipewireTestHarness::new(); + let v = BackendType::Pipewire; + let value = alloc_audio_backend(v, Default::default()).unwrap(); + assert_eq!(TypeId::of::(), value.as_any().type_id()); + } + #[cfg(feature = "alsa-backend")] + { + let v = BackendType::Alsa; + let value = alloc_audio_backend(v, Default::default()).unwrap(); + assert_eq!(TypeId::of::(), value.as_any().type_id()); + } + } +} diff --git a/staging/vhost-device-sound/src/audio_backends/alsa.rs b/staging/vhost-device-sound/src/audio_backends/alsa.rs index 8e78d24d1..04150df84 100644 --- a/staging/vhost-device-sound/src/audio_backends/alsa.rs +++ b/staging/vhost-device-sound/src/audio_backends/alsa.rs @@ -645,12 +645,14 @@ impl AudioBackend for AlsaBackend { } fn stop(&self, id: u32) -> CrateResult<()> { - if let Some(Err(err)) = self + if let Err(err) = self .streams .write() .unwrap() .get_mut(id as usize) - .map(|s| s.state.stop()) + .ok_or_else(|| Error::StreamWithIdNotFound(id))? + .state + .stop() { log::error!("Stream {} stop {}", id, err); } @@ -716,4 +718,518 @@ impl AudioBackend for AlsaBackend { std::mem::take(&mut streams[stream_id as usize].buffers); Ok(()) } + + #[cfg(test)] + fn as_any(&self) -> &dyn std::any::Any { + self + } +} + +#[cfg(test)] +/// Utilities for temporarily setting a test-specific alsa config. +pub mod test_utils; + +#[cfg(test)] +mod tests { + use super::{test_utils::setup_alsa_conf, *}; + use crate::{stream::PcmParams, virtio_sound::*}; + + const RATES: [u8; _VIRTIO_SND_PCM_RATE_MAX as usize] = [ + virtio_sound::VIRTIO_SND_PCM_RATE_5512, + virtio_sound::VIRTIO_SND_PCM_RATE_8000, + virtio_sound::VIRTIO_SND_PCM_RATE_11025, + virtio_sound::VIRTIO_SND_PCM_RATE_16000, + virtio_sound::VIRTIO_SND_PCM_RATE_22050, + virtio_sound::VIRTIO_SND_PCM_RATE_32000, + virtio_sound::VIRTIO_SND_PCM_RATE_44100, + virtio_sound::VIRTIO_SND_PCM_RATE_48000, + virtio_sound::VIRTIO_SND_PCM_RATE_64000, + virtio_sound::VIRTIO_SND_PCM_RATE_88200, + virtio_sound::VIRTIO_SND_PCM_RATE_96000, + virtio_sound::VIRTIO_SND_PCM_RATE_176400, + virtio_sound::VIRTIO_SND_PCM_RATE_192000, + virtio_sound::VIRTIO_SND_PCM_RATE_384000, + ]; + + const FORMATS: [u8; _VIRTIO_SND_PCM_FMT_MAX as usize] = [ + virtio_sound::VIRTIO_SND_PCM_FMT_IMA_ADPCM, + virtio_sound::VIRTIO_SND_PCM_FMT_MU_LAW, + virtio_sound::VIRTIO_SND_PCM_FMT_A_LAW, + virtio_sound::VIRTIO_SND_PCM_FMT_S8, + virtio_sound::VIRTIO_SND_PCM_FMT_U8, + virtio_sound::VIRTIO_SND_PCM_FMT_S16, + virtio_sound::VIRTIO_SND_PCM_FMT_U16, + virtio_sound::VIRTIO_SND_PCM_FMT_S18_3, + virtio_sound::VIRTIO_SND_PCM_FMT_U18_3, + virtio_sound::VIRTIO_SND_PCM_FMT_S20_3, + virtio_sound::VIRTIO_SND_PCM_FMT_U20_3, + virtio_sound::VIRTIO_SND_PCM_FMT_S24_3, + virtio_sound::VIRTIO_SND_PCM_FMT_U24_3, + virtio_sound::VIRTIO_SND_PCM_FMT_S20, + virtio_sound::VIRTIO_SND_PCM_FMT_U20, + virtio_sound::VIRTIO_SND_PCM_FMT_S24, + virtio_sound::VIRTIO_SND_PCM_FMT_U24, + virtio_sound::VIRTIO_SND_PCM_FMT_S32, + virtio_sound::VIRTIO_SND_PCM_FMT_U32, + virtio_sound::VIRTIO_SND_PCM_FMT_FLOAT, + virtio_sound::VIRTIO_SND_PCM_FMT_FLOAT64, + virtio_sound::VIRTIO_SND_PCM_FMT_DSD_U8, + virtio_sound::VIRTIO_SND_PCM_FMT_DSD_U16, + virtio_sound::VIRTIO_SND_PCM_FMT_DSD_U32, + virtio_sound::VIRTIO_SND_PCM_FMT_IEC958_SUBFRAME, + ]; + + #[test] + fn test_alsa_trait_impls() { + crate::init_logger(); + let _harness = setup_alsa_conf(); + + let _: alsa::Direction = Direction::Output.into(); + let _: alsa::Direction = Direction::Input.into(); + + let backend = AlsaBackend::new(Default::default()); + #[allow(clippy::redundant_clone)] + let _ = backend.clone(); + + _ = format!("{:?}", backend); + } + + #[test] + fn test_alsa_ops() { + crate::init_logger(); + let _harness = setup_alsa_conf(); + + let streams = Arc::new(RwLock::new(vec![ + Stream::default(), + Stream { + id: 1, + direction: Direction::Input, + ..Stream::default() + }, + ])); + let backend = AlsaBackend::new(streams); + let request = VirtioSndPcmSetParams { + hdr: VirtioSoundPcmHeader { + stream_id: 0.into(), + hdr: VirtioSoundHeader { code: 0.into() }, + }, + format: VIRTIO_SND_PCM_FMT_S16, + rate: VIRTIO_SND_PCM_RATE_44100, + channels: 2, + features: 0.into(), + buffer_bytes: 8192.into(), + period_bytes: 4096.into(), + padding: 0, + }; + backend.set_parameters(0, request).unwrap(); + backend.prepare(0).unwrap(); + backend.start(0).unwrap(); + backend.write(0).unwrap(); + backend.read(0).unwrap(); + backend.stop(0).unwrap(); + backend.release(0).unwrap(); + } + + #[test] + fn test_alsa_invalid_stream_id() { + crate::init_logger(); + let _harness = setup_alsa_conf(); + + let streams = Arc::new(RwLock::new(vec![ + Stream::default(), + Stream { + id: 1, + direction: Direction::Input, + ..Stream::default() + }, + ])); + let backend = AlsaBackend::new(streams); + let request = VirtioSndPcmSetParams { + hdr: VirtioSoundPcmHeader { + stream_id: 3.into(), + hdr: VirtioSoundHeader { code: 0.into() }, + }, + format: VIRTIO_SND_PCM_FMT_S16, + rate: VIRTIO_SND_PCM_RATE_44100, + channels: 2, + features: 0.into(), + buffer_bytes: 8192.into(), + period_bytes: 4096.into(), + padding: 0, + }; + backend.set_parameters(3, request).unwrap_err(); + backend.prepare(3).unwrap_err(); + backend.start(3).unwrap_err(); + backend.write(3).unwrap_err(); + backend.read(3).unwrap_err(); + backend.stop(3).unwrap_err(); + backend.release(3).unwrap_err(); + } + + #[test] + fn test_alsa_invalid_state_transitions() { + crate::init_logger(); + let _harness = setup_alsa_conf(); + + let streams = Arc::new(RwLock::new(vec![ + Stream::default(), + Stream { + id: 1, + direction: Direction::Input, + ..Stream::default() + }, + ])); + let request = VirtioSndPcmSetParams { + hdr: VirtioSoundPcmHeader { + stream_id: 3.into(), + hdr: VirtioSoundHeader { code: 0.into() }, + }, + format: VIRTIO_SND_PCM_FMT_S16, + rate: VIRTIO_SND_PCM_RATE_44100, + channels: 2, + features: 0.into(), + buffer_bytes: 8192.into(), + period_bytes: 4096.into(), + padding: 0, + }; + { + let backend = AlsaBackend::new(streams.clone()); + + // Invalid, but we allow it. + backend.stop(0).unwrap(); + // Invalid, but we don't allow it. + backend.release(0).unwrap_err(); + backend.start(0).unwrap_err(); + backend.release(0).unwrap_err(); + } + { + let backend = AlsaBackend::new(streams.clone()); + + // set_parameters -> set_parameters | VALID + backend.set_parameters(0, request).unwrap(); + + // set_parameters -> prepare | VALID + backend.prepare(0).unwrap(); + + // Invalid, but we allow it. + // prepare -> stop | INVALID + backend.stop(0).unwrap(); + // prepare -> release | VALID + backend.release(0).unwrap(); + + // release -> start | INVALID + backend.start(0).unwrap_err(); + // release -> stop | VALID + backend.stop(0).unwrap(); + // release -> prepare | VALID + backend.prepare(0).unwrap(); + + // prepare -> start | VALID + backend.start(0).unwrap(); + + // start -> start | INVALID + backend.start(0).unwrap_err(); + // start -> set_parameters | INVALID + backend.set_parameters(0, request).unwrap_err(); + // start -> prepare | INVALID + backend.prepare(0).unwrap_err(); + // start -> release | INVALID + backend.release(0).unwrap_err(); + // start -> stop | VALID + backend.stop(0).unwrap(); + // stop -> start | VALID + backend.start(0).unwrap(); + // start -> stop | VALID + backend.stop(0).unwrap(); + // stop -> prepare | INVALID + backend.prepare(0).unwrap_err(); + // stop -> set_parameters | INVALID + backend.set_parameters(0, request).unwrap_err(); + // stop -> release | VALID + backend.release(0).unwrap(); + } + + // Redundant checks? Oh well. + // + // Generated with: + // + // ```python + // import itertools + // states = ["SetParameters", "Prepare", "Release", "Start", "Stop"] + // combs = set(itertools.product(states, repeat=2)) + // ``` + { + let backend = AlsaBackend::new(streams.clone()); + backend.set_parameters(0, request).unwrap(); + backend.prepare(0).unwrap(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.prepare(0).unwrap(); + backend.stop(0).unwrap(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.start(0).unwrap(); + backend.start(0).unwrap_err(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.prepare(0).unwrap_err(); + backend.start(0).unwrap_err(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.stop(0).unwrap(); + backend.release(0).unwrap(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.stop(0).unwrap(); + backend.prepare(0).unwrap(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.release(0).unwrap(); + backend.set_parameters(0, request).unwrap(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.start(0).unwrap_err(); + backend.set_parameters(0, request).unwrap(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.prepare(0).unwrap(); + backend.set_parameters(0, request).unwrap(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.release(0).unwrap_err(); + backend.read(0).unwrap_err(); + backend.write(0).unwrap_err(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.set_parameters(0, request).unwrap(); + backend.stop(0).unwrap(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.release(0).unwrap_err(); + backend.prepare(0).unwrap(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.set_parameters(0, request).unwrap(); + backend.start(0).unwrap_err(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.start(0).unwrap_err(); + backend.release(0).unwrap_err(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.prepare(0).unwrap(); + backend.release(0).unwrap(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.start(0).unwrap_err(); + backend.prepare(0).unwrap(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.stop(0).unwrap(); + backend.stop(0).unwrap(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.prepare(0).unwrap(); + backend.prepare(0).unwrap(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.stop(0).unwrap(); + backend.start(0).unwrap(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.set_parameters(0, request).unwrap_err(); + backend.release(0).unwrap_err(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.release(0).unwrap_err(); + backend.stop(0).unwrap(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.stop(0).unwrap(); + backend.set_parameters(0, request).unwrap_err(); + } + { + let backend = AlsaBackend::new(streams.clone()); + backend.release(0).unwrap(); + backend.start(0).unwrap_err(); + } + { + let backend = AlsaBackend::new(streams); + backend.start(0).unwrap_err(); + backend.stop(0).unwrap(); + } + } + + #[test] + fn test_alsa_worker() { + crate::init_logger(); + let _harness = setup_alsa_conf(); + + let streams = Arc::new(RwLock::new(vec![ + Stream::default(), + Stream { + id: 1, + direction: Direction::Input, + ..Stream::default() + }, + ])); + let (sender, receiver) = channel(); + let pcm = Arc::new(Mutex::new( + PCM::new("null", Direction::Output.into(), false).unwrap(), + )); + + let mtx = Arc::clone(&pcm); + let streams = Arc::clone(&streams); + let _handle = + thread::spawn(move || alsa_worker(mtx.clone(), streams.clone(), &receiver, 0)); + sender.send(false).unwrap(); + } + + #[test] + fn test_alsa_valid_parameters() { + crate::init_logger(); + let _harness = setup_alsa_conf(); + + let streams = Arc::new(RwLock::new(vec![ + Stream::default(), + Stream { + id: 1, + direction: Direction::Input, + ..Stream::default() + }, + ])); + let mut request = VirtioSndPcmSetParams { + hdr: VirtioSoundPcmHeader { + stream_id: 0.into(), + hdr: VirtioSoundHeader { code: 0.into() }, + }, + format: VIRTIO_SND_PCM_FMT_S16, + rate: VIRTIO_SND_PCM_RATE_44100, + channels: 2, + features: 0.into(), + buffer_bytes: 8192.into(), + period_bytes: 4096.into(), + padding: 0, + }; + + for rate in RATES + .iter() + .cloned() + .filter(|rt| ((1 << *rt) & crate::SUPPORTED_RATES) > 0) + { + request.rate = rate; + let backend = AlsaBackend::new(streams.clone()); + backend.set_parameters(0, request).unwrap(); + } + + for rate in RATES + .iter() + .cloned() + .filter(|rt| ((1 << *rt) & crate::SUPPORTED_RATES) == 0) + { + request.rate = rate; + let backend = AlsaBackend::new(streams.clone()); + backend.set_parameters(0, request).unwrap_err(); + } + request.rate = VIRTIO_SND_PCM_RATE_44100; + + for format in FORMATS + .iter() + .cloned() + .filter(|fmt| ((1 << *fmt) & crate::SUPPORTED_FORMATS) > 0) + { + request.format = format; + let backend = AlsaBackend::new(streams.clone()); + backend.set_parameters(0, request).unwrap(); + } + + for format in FORMATS + .iter() + .cloned() + .filter(|fmt| ((1 << *fmt) & crate::SUPPORTED_FORMATS) == 0) + { + request.format = format; + let backend = AlsaBackend::new(streams.clone()); + backend.set_parameters(0, request).unwrap_err(); + } + + { + for format in FORMATS + .iter() + .cloned() + .filter(|fmt| ((1 << *fmt) & crate::SUPPORTED_FORMATS) > 0) + { + let streams = Arc::new(RwLock::new(vec![Stream { + params: PcmParams { + format, + ..PcmParams::default() + }, + ..Stream::default() + }])); + let pcm = Arc::new(Mutex::new( + PCM::new("null", Direction::Output.into(), false).unwrap(), + )); + update_pcm(&pcm, 0, &streams).unwrap(); + } + } + } + + #[test] + #[should_panic(expected = "unreachable")] + fn test_alsa_invalid_rate() { + crate::init_logger(); + let _harness = setup_alsa_conf(); + + let streams = Arc::new(RwLock::new(vec![Stream { + params: PcmParams { + rate: _VIRTIO_SND_PCM_RATE_MAX, + ..PcmParams::default() + }, + ..Stream::default() + }])); + let pcm = Arc::new(Mutex::new( + PCM::new("null", Direction::Output.into(), false).unwrap(), + )); + update_pcm(&pcm, 0, &streams).unwrap(); + } + + #[test] + #[should_panic(expected = "unreachable")] + fn test_alsa_invalid_fmt() { + crate::init_logger(); + let _harness = setup_alsa_conf(); + + let streams = Arc::new(RwLock::new(vec![Stream { + params: PcmParams { + format: _VIRTIO_SND_PCM_FMT_MAX, + ..PcmParams::default() + }, + ..Stream::default() + }])); + let pcm = Arc::new(Mutex::new( + PCM::new("null", Direction::Output.into(), false).unwrap(), + )); + update_pcm(&pcm, 0, &streams).unwrap(); + } } diff --git a/staging/vhost-device-sound/src/audio_backends/alsa/test_utils.rs b/staging/vhost-device-sound/src/audio_backends/alsa/test_utils.rs new file mode 100644 index 000000000..0d4f0cd7b --- /dev/null +++ b/staging/vhost-device-sound/src/audio_backends/alsa/test_utils.rs @@ -0,0 +1,128 @@ +// Manos Pitsidianakis +// SPDX-License-Identifier: Apache-2.0 or BSD-3-Clause + +use std::{ + path::PathBuf, + sync::{ + atomic::{AtomicU8, Ordering}, + Arc, Mutex, Once, + }, +}; + +use tempfile::{tempdir, TempDir}; + +static mut TEST_HARNESS: Option = None; +static INIT_ALSA_CONF: Once = Once::new(); + +#[must_use] +pub fn setup_alsa_conf() -> AlsaTestHarnessRef<'static> { + INIT_ALSA_CONF.call_once(|| + // SAFETY: + // This is only called once, because of.. `Once`, so it's safe to + // access the static value mutably. + unsafe { + TEST_HARNESS = Some(AlsaTestHarness::new()); + }); + let retval = AlsaTestHarnessRef( + // SAFETY: + // The unsafe { } block is needed because TEST_HARNESS is a mutable static. The inner + // operations are protected by atomics. + unsafe { TEST_HARNESS.as_ref().unwrap() }, + ); + retval.0.inc_ref(); + retval +} + +/// The alsa test harness. It must only be constructed via +/// `AlsaTestHarness::new()`. +#[non_exhaustive] +pub struct AlsaTestHarness { + pub tempdir: Arc>>, + pub conf_path: PathBuf, + pub ref_count: AtomicU8, +} + +/// Ref counted alsa test harness ref. +#[repr(transparent)] +#[non_exhaustive] +pub struct AlsaTestHarnessRef<'a>(&'a AlsaTestHarness); + +impl<'a> Drop for AlsaTestHarnessRef<'a> { + fn drop(&mut self) { + self.0.dec_ref(); + } +} + +impl AlsaTestHarness { + pub fn new() -> Self { + let tempdir = tempdir().unwrap(); + let conf_path = tempdir.path().join("alsa.conf"); + + std::fs::write( + &conf_path, + b"pcm.!default {\n type null \n }\n\nctl.!default {\n type null\n }\n\npcm.null {\n type null \n }\n\nctl.null {\n type null\n }\n", + ).unwrap(); + + std::env::set_var("ALSA_CONFIG_PATH", &conf_path); + println!( + "INFO: setting ALSA_CONFIG_PATH={} in PID {} and TID {:?}", + conf_path.display(), + std::process::id(), + std::thread::current().id() + ); + + Self { + tempdir: Arc::new(Mutex::new(Some(tempdir))), + conf_path, + ref_count: 0.into(), + } + } + + #[inline] + pub fn inc_ref(&self) { + let old_val = self.ref_count.fetch_add(1, Ordering::SeqCst); + assert!( + old_val != u8::MAX, + "ref_count overflowed on 8bits when increasing by 1" + ); + } + + #[inline] + pub fn dec_ref(&self) { + let old_val = self.ref_count.fetch_sub(1, Ordering::SeqCst); + if old_val == 1 { + let mut lck = self.tempdir.lock().unwrap(); + println!( + "INFO: unsetting ALSA_CONFIG_PATH={} in PID {} and TID {:?}", + self.conf_path.display(), + std::process::id(), + std::thread::current().id() + ); + std::env::remove_var("ALSA_CONFIG_PATH"); + _ = lck.take(); + } + } +} + +impl Drop for AlsaTestHarness { + fn drop(&mut self) { + let ref_count = self.ref_count.load(Ordering::SeqCst); + if ref_count != 0 { + println!( + "ERROR: ref_count is {ref_count} when dropping {}", + stringify!(AlsaTestHarness) + ); + } + if self + .tempdir + .lock() + .map(|mut l| l.take().is_some()) + .unwrap_or(false) + { + println!( + "ERROR: tempdir held a value when dropping {}", + stringify!(AlsaTestHarness) + ); + } + } +} diff --git a/staging/vhost-device-sound/src/audio_backends/null.rs b/staging/vhost-device-sound/src/audio_backends/null.rs index ef5aa2275..05175f9c2 100644 --- a/staging/vhost-device-sound/src/audio_backends/null.rs +++ b/staging/vhost-device-sound/src/audio_backends/null.rs @@ -26,6 +26,11 @@ impl AudioBackend for NullBackend { log::trace!("NullBackend read stream_id {}", _id); Ok(()) } + + #[cfg(test)] + fn as_any(&self) -> &dyn std::any::Any { + self + } } #[cfg(test)] @@ -34,6 +39,7 @@ mod tests { #[test] fn test_null_backend_write() { + crate::init_logger(); let streams = Arc::new(RwLock::new(vec![Stream::default()])); let null_backend = NullBackend::new(streams.clone()); @@ -45,6 +51,7 @@ mod tests { #[test] fn test_null_backend_read() { + crate::init_logger(); let streams = Arc::new(RwLock::new(vec![Stream::default()])); let null_backend = NullBackend::new(streams.clone()); diff --git a/staging/vhost-device-sound/src/audio_backends/pipewire.rs b/staging/vhost-device-sound/src/audio_backends/pipewire.rs index 71f6eafbb..3cabbd2a3 100644 --- a/staging/vhost-device-sound/src/audio_backends/pipewire.rs +++ b/staging/vhost-device-sound/src/audio_backends/pipewire.rs @@ -562,12 +562,17 @@ impl AudioBackend for PwBackend { lock_guard.unlock(); Ok(()) } + + #[cfg(test)] + fn as_any(&self) -> &dyn std::any::Any { + self + } } #[cfg(test)] /// Utilities for building a temporary Dbus session and a pipewire instance for /// testing. -mod test_utils; +pub mod test_utils; #[cfg(test)] mod tests { @@ -575,6 +580,7 @@ mod tests { #[test] fn test_pipewire_backend_success() { + crate::init_logger(); let streams = Arc::new(RwLock::new(vec![Stream::default()])); let stream_params = streams.clone(); @@ -603,6 +609,7 @@ mod tests { #[test] fn test_pipewire_backend_invalid_stream() { + crate::init_logger(); let stream_params = Arc::new(RwLock::new(vec![])); let _test_harness = PipewireTestHarness::new(); diff --git a/staging/vhost-device-sound/src/device.rs b/staging/vhost-device-sound/src/device.rs index c31567a15..9325c9b15 100644 --- a/staging/vhost-device-sound/src/device.rs +++ b/staging/vhost-device-sound/src/device.rs @@ -30,7 +30,7 @@ use crate::{ audio_backends::{alloc_audio_backend, AudioBackend}, stream::{Buffer, Error as StreamError, Stream}, virtio_sound::*, - ControlMessageKind, Direction, Error, IOMessage, Result, SoundConfig, + ControlMessageKind, Direction, Error, IOMessage, QueueIdx, Result, SoundConfig, }; pub struct VhostUserSoundThread { @@ -38,7 +38,7 @@ pub struct VhostUserSoundThread { event_idx: bool, chmaps: Arc>>, jacks: Arc>>, - queue_indexes: Vec, + queue_indexes: Vec, streams: Arc>>, streams_no: usize, } @@ -49,11 +49,11 @@ impl VhostUserSoundThread { pub fn new( chmaps: Arc>>, jacks: Arc>>, - mut queue_indexes: Vec, + mut queue_indexes: Vec, streams: Arc>>, streams_no: usize, ) -> Result { - queue_indexes.sort(); + queue_indexes.sort_by_key(|idx| *idx as u16); Ok(Self { event_idx: false, @@ -70,7 +70,7 @@ impl VhostUserSoundThread { let mut queues_per_thread = 0u64; for idx in self.queue_indexes.iter() { - queues_per_thread |= 1u64 << idx + queues_per_thread |= 1u64 << *idx as u16 } queues_per_thread @@ -91,8 +91,13 @@ impl VhostUserSoundThread { vrings: &[VringRwLock], audio_backend: &RwLock>, ) -> IoResult<()> { - let vring = &vrings[device_event as usize]; - let queue_idx = self.queue_indexes[device_event as usize]; + let vring = &vrings + .get(device_event as usize) + .ok_or_else(|| Error::HandleUnknownEvent(device_event))?; + let queue_idx = self + .queue_indexes + .get(device_event as usize) + .ok_or_else(|| Error::HandleUnknownEvent(device_event))?; if self.event_idx { // vm-virtio's Queue implementation only checks avail_index // once, so to properly support EVENT_IDX we need to keep @@ -101,11 +106,10 @@ impl VhostUserSoundThread { loop { vring.disable_notification().unwrap(); match queue_idx { - CONTROL_QUEUE_IDX => self.process_control(vring, audio_backend), - EVENT_QUEUE_IDX => self.process_event(vring), - TX_QUEUE_IDX => self.process_io(vring, audio_backend, Direction::Output), - RX_QUEUE_IDX => self.process_io(vring, audio_backend, Direction::Input), - _ => Err(Error::HandleUnknownEvent.into()), + QueueIdx::Control => self.process_control(vring, audio_backend), + QueueIdx::Event => self.process_event(vring), + QueueIdx::Tx => self.process_io(vring, audio_backend, Direction::Output), + QueueIdx::Rx => self.process_io(vring, audio_backend, Direction::Input), }?; if !vring.enable_notification().unwrap() { break; @@ -114,11 +118,10 @@ impl VhostUserSoundThread { } else { // Without EVENT_IDX, a single call is enough. match queue_idx { - CONTROL_QUEUE_IDX => self.process_control(vring, audio_backend), - EVENT_QUEUE_IDX => self.process_event(vring), - TX_QUEUE_IDX => self.process_io(vring, audio_backend, Direction::Output), - RX_QUEUE_IDX => self.process_io(vring, audio_backend, Direction::Input), - _ => Err(Error::HandleUnknownEvent.into()), + QueueIdx::Control => self.process_control(vring, audio_backend), + QueueIdx::Event => self.process_event(vring), + QueueIdx::Tx => self.process_io(vring, audio_backend, Direction::Output), + QueueIdx::Rx => self.process_io(vring, audio_backend, Direction::Input), }?; } Ok(()) @@ -633,21 +636,21 @@ impl VhostUserSoundBackend { RwLock::new(VhostUserSoundThread::new( chmaps.clone(), jacks.clone(), - vec![CONTROL_QUEUE_IDX, EVENT_QUEUE_IDX], + vec![QueueIdx::Control, QueueIdx::Event], streams.clone(), streams_no, )?), RwLock::new(VhostUserSoundThread::new( chmaps.clone(), jacks.clone(), - vec![TX_QUEUE_IDX], + vec![QueueIdx::Tx], streams.clone(), streams_no, )?), RwLock::new(VhostUserSoundThread::new( chmaps, jacks, - vec![RX_QUEUE_IDX], + vec![QueueIdx::Rx], streams.clone(), streams_no, )?), @@ -657,10 +660,10 @@ impl VhostUserSoundBackend { chmaps, jacks, vec![ - CONTROL_QUEUE_IDX, - EVENT_QUEUE_IDX, - TX_QUEUE_IDX, - RX_QUEUE_IDX, + QueueIdx::Control, + QueueIdx::Event, + QueueIdx::Tx, + QueueIdx::Rx, ], streams.clone(), streams_no, @@ -825,11 +828,12 @@ mod tests { #[test] fn test_sound_thread_success() { + crate::init_logger(); let config = SoundConfig::new(SOCKET_PATH.to_string(), false, BackendType::Null); let chmaps = Arc::new(RwLock::new(vec![])); let jacks = Arc::new(RwLock::new(vec![])); - let queue_indexes = vec![1, 2, 3]; + let queue_indexes = vec![QueueIdx::Event, QueueIdx::Tx, QueueIdx::Rx]; let streams = vec![Stream::default()]; let streams_no = streams.len(); let streams = Arc::new(RwLock::new(streams)); @@ -919,11 +923,12 @@ mod tests { #[test] fn test_sound_thread_failure() { + crate::init_logger(); let config = SoundConfig::new(SOCKET_PATH.to_string(), false, BackendType::Null); let chmaps = Arc::new(RwLock::new(vec![])); let jacks = Arc::new(RwLock::new(vec![])); - let queue_indexes = vec![1, 2, 3]; + let queue_indexes = vec![QueueIdx::Event, QueueIdx::Tx, QueueIdx::Rx]; let streams = Arc::new(RwLock::new(vec![])); let streams_no = 0; let thread = @@ -993,6 +998,7 @@ mod tests { #[test] fn test_sound_backend() { + crate::init_logger(); let test_dir = tempdir().expect("Could not create a temp test directory."); let socket_path = test_dir.path().join(SOCKET_PATH).display().to_string(); let config = SoundConfig::new(socket_path, false, BackendType::Null); @@ -1002,68 +1008,74 @@ mod tests { assert_eq!(backend.max_queue_size(), 64); assert_ne!(backend.features(), 0); assert!(!backend.protocol_features().is_empty()); - backend.set_event_idx(false); + for event_idx in [true, false] { + backend.set_event_idx(event_idx); - // Mock memory - let mem = GuestMemoryAtomic::new( - GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap(), - ); - - // Mock Vring for queues - let vrings = [ - VringRwLock::new(mem.clone(), 0x1000).unwrap(), - VringRwLock::new(mem.clone(), 0x1000).unwrap(), - VringRwLock::new(mem.clone(), 0x1000).unwrap(), - VringRwLock::new(mem.clone(), 0x1000).unwrap(), - ]; - vrings[CONTROL_QUEUE_IDX as usize] - .set_queue_info(0x100, 0x200, 0x300) - .unwrap(); - vrings[CONTROL_QUEUE_IDX as usize].set_queue_ready(true); - vrings[EVENT_QUEUE_IDX as usize] - .set_queue_info(0x100, 0x200, 0x300) - .unwrap(); - vrings[EVENT_QUEUE_IDX as usize].set_queue_ready(true); - vrings[TX_QUEUE_IDX as usize] - .set_queue_info(0x1100, 0x1200, 0x1300) - .unwrap(); - vrings[TX_QUEUE_IDX as usize].set_queue_ready(true); - vrings[RX_QUEUE_IDX as usize] - .set_queue_info(0x100, 0x200, 0x300) - .unwrap(); - vrings[RX_QUEUE_IDX as usize].set_queue_ready(true); - - backend.update_memory(mem).unwrap(); - - let queues_per_thread = backend.queues_per_thread(); - assert_eq!(queues_per_thread.len(), 1); - assert_eq!(queues_per_thread[0], 0xf); - - let config = backend.get_config(0, 8); - assert_eq!(config.len(), 8); - - let exit = backend.exit_event(0); - assert!(exit.is_some()); - exit.unwrap().write(1).unwrap(); + // Mock memory + let mem = GuestMemoryAtomic::new( + GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap(), + ); - backend - .handle_event(CONTROL_QUEUE_IDX, EventSet::IN, &vrings, 0) - .unwrap(); - backend - .handle_event(EVENT_QUEUE_IDX, EventSet::IN, &vrings, 0) - .unwrap(); - backend - .handle_event(TX_QUEUE_IDX, EventSet::IN, &vrings, 0) - .unwrap(); - backend - .handle_event(RX_QUEUE_IDX, EventSet::IN, &vrings, 0) - .unwrap(); + // Mock Vring for queues + let vrings = [ + VringRwLock::new(mem.clone(), 0x1000).unwrap(), + VringRwLock::new(mem.clone(), 0x1000).unwrap(), + VringRwLock::new(mem.clone(), 0x1000).unwrap(), + VringRwLock::new(mem.clone(), 0x1000).unwrap(), + ]; + vrings[CONTROL_QUEUE_IDX as usize] + .set_queue_info(0x100, 0x200, 0x300) + .unwrap(); + vrings[CONTROL_QUEUE_IDX as usize].set_queue_ready(true); + vrings[EVENT_QUEUE_IDX as usize] + .set_queue_info(0x100, 0x200, 0x300) + .unwrap(); + vrings[EVENT_QUEUE_IDX as usize].set_queue_ready(true); + vrings[TX_QUEUE_IDX as usize] + .set_queue_info(0x1100, 0x1200, 0x1300) + .unwrap(); + vrings[TX_QUEUE_IDX as usize].set_queue_ready(true); + vrings[RX_QUEUE_IDX as usize] + .set_queue_info(0x100, 0x200, 0x300) + .unwrap(); + vrings[RX_QUEUE_IDX as usize].set_queue_ready(true); + + backend.update_memory(mem).unwrap(); + + let queues_per_thread = backend.queues_per_thread(); + assert_eq!(queues_per_thread.len(), 1); + assert_eq!(queues_per_thread[0], 0xf); + + let config = backend.get_config(0, 8); + assert_eq!(config.len(), 8); + + let exit = backend.exit_event(0); + assert!(exit.is_some()); + exit.unwrap().write(1).unwrap(); + + backend + .handle_event(CONTROL_QUEUE_IDX, EventSet::IN, &vrings, 0) + .unwrap(); + backend + .handle_event(EVENT_QUEUE_IDX, EventSet::IN, &vrings, 0) + .unwrap(); + backend + .handle_event(TX_QUEUE_IDX, EventSet::IN, &vrings, 0) + .unwrap(); + backend + .handle_event(RX_QUEUE_IDX, EventSet::IN, &vrings, 0) + .unwrap(); + backend + .handle_event(RX_QUEUE_IDX * 2, EventSet::IN, &vrings, 0) + .unwrap_err(); + } test_dir.close().unwrap(); } #[test] fn test_sound_backend_failures() { + crate::init_logger(); let test_dir = tempdir().expect("Could not create a temp test directory."); let socket_path = test_dir @@ -1116,6 +1128,13 @@ mod tests { Error::HandleEventNotEpollIn.to_string() ); + // Unknown event. + let ret = backend.handle_event(RX_QUEUE_IDX * 2, EventSet::IN, &vrings, 0); + assert_eq!( + ret.unwrap_err().to_string(), + Error::HandleUnknownEvent(RX_QUEUE_IDX * 2).to_string() + ); + test_dir.close().unwrap(); } } diff --git a/staging/vhost-device-sound/src/lib.rs b/staging/vhost-device-sound/src/lib.rs index 50cd7532d..969215851 100644 --- a/staging/vhost-device-sound/src/lib.rs +++ b/staging/vhost-device-sound/src/lib.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 or BSD-3-Clause // #![deny( + clippy::undocumented_unsafe_blocks, /* groups */ clippy::correctness, clippy::suspicious, @@ -30,6 +31,12 @@ clippy::significant_drop_tightening )] +#[cfg(test)] +pub fn init_logger() { + std::env::set_var("RUST_LOG", "trace"); + let _ = env_logger::builder().is_test(true).try_init(); +} + pub mod audio_backends; pub mod device; pub mod stream; @@ -88,7 +95,43 @@ impl TryFrom for Direction { Ok(match val { virtio_sound::VIRTIO_SND_D_OUTPUT => Self::Output, virtio_sound::VIRTIO_SND_D_INPUT => Self::Input, - other => return Err(Error::InvalidMessageValue(stringify!(Direction), other)), + other => { + return Err(Error::InvalidMessageValue( + stringify!(Direction), + other.into(), + )) + } + }) + } +} + +/// Queue index. +/// +/// Type safe enum for CONTROL_QUEUE_IDX, EVENT_QUEUE_IDX, TX_QUEUE_IDX, +/// RX_QUEUE_IDX. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[repr(u16)] +pub enum QueueIdx { + #[doc(alias = "CONTROL_QUEUE_IDX")] + Control = virtio_sound::CONTROL_QUEUE_IDX, + #[doc(alias = "EVENT_QUEUE_IDX")] + Event = virtio_sound::EVENT_QUEUE_IDX, + #[doc(alias = "TX_QUEUE_IDX")] + Tx = virtio_sound::TX_QUEUE_IDX, + #[doc(alias = "RX_QUEUE_IDX")] + Rx = virtio_sound::RX_QUEUE_IDX, +} + +impl TryFrom for QueueIdx { + type Error = Error; + + fn try_from(val: u16) -> std::result::Result { + Ok(match val { + virtio_sound::CONTROL_QUEUE_IDX => Self::Control, + virtio_sound::EVENT_QUEUE_IDX => Self::Event, + virtio_sound::TX_QUEUE_IDX => Self::Tx, + virtio_sound::RX_QUEUE_IDX => Self::Rx, + other => return Err(Error::InvalidMessageValue(stringify!(QueueIdx), other)), }) } } @@ -106,12 +149,12 @@ pub enum Error { DescriptorWriteFailed, #[error("Failed to handle event other than EPOLLIN event")] HandleEventNotEpollIn, - #[error("Failed to handle unknown event")] - HandleUnknownEvent, + #[error("Failed to handle unknown event with id {0}")] + HandleUnknownEvent(u16), #[error("Invalid control message code {0}")] InvalidControlMessage(u32), #[error("Invalid value in {0}: {1}")] - InvalidMessageValue(&'static str, u8), + InvalidMessageValue(&'static str, u16), #[error("Failed to create a new EventFd")] EventFdCreate(IoError), #[error("Request missing data buffer")] @@ -319,6 +362,7 @@ mod tests { #[test] fn test_sound_server() { const SOCKET_PATH: &str = "vsound.socket"; + crate::init_logger(); let config = SoundConfig::new(SOCKET_PATH.to_string(), false, BackendType::Null); @@ -341,6 +385,7 @@ mod tests { #[test] fn test_control_message_kind_try_from() { + crate::init_logger(); assert_eq!( ControlMessageKind::try_from(>::into(VIRTIO_SND_R_JACK_INFO)), Ok(ControlMessageKind::JackInfo) @@ -361,6 +406,7 @@ mod tests { #[test] fn test_control_message_kind_try_from_invalid() { + crate::init_logger(); // Test an invalid value that should result in an InvalidControlMessage error let invalid_value: u32 = 0x1101; assert_eq!( @@ -371,6 +417,7 @@ mod tests { #[test] fn test_try_from_valid_output() { + crate::init_logger(); let val = virtio_sound::VIRTIO_SND_D_OUTPUT; assert_eq!(Direction::try_from(val).unwrap(), Direction::Output); @@ -379,10 +426,30 @@ mod tests { let val = 42; Direction::try_from(val).unwrap_err(); + + assert_eq!( + QueueIdx::try_from(virtio_sound::CONTROL_QUEUE_IDX).unwrap(), + QueueIdx::Control + ); + assert_eq!( + QueueIdx::try_from(virtio_sound::EVENT_QUEUE_IDX).unwrap(), + QueueIdx::Event + ); + assert_eq!( + QueueIdx::try_from(virtio_sound::TX_QUEUE_IDX).unwrap(), + QueueIdx::Tx + ); + assert_eq!( + QueueIdx::try_from(virtio_sound::RX_QUEUE_IDX).unwrap(), + QueueIdx::Rx + ); + let val = virtio_sound::NUM_QUEUES; + QueueIdx::try_from(val).unwrap_err(); } #[test] fn test_display() { + crate::init_logger(); let error = InvalidControlMessage(42); let formatted_error = format!("{}", error); assert_eq!(formatted_error, "Invalid control message code 42"); @@ -390,6 +457,7 @@ mod tests { #[test] fn test_into_error() { + crate::init_logger(); let error = InvalidControlMessage(42); let _error: Error = error.into(); diff --git a/staging/vhost-device-sound/src/main.rs b/staging/vhost-device-sound/src/main.rs index 522662685..9bb31a7a4 100644 --- a/staging/vhost-device-sound/src/main.rs +++ b/staging/vhost-device-sound/src/main.rs @@ -53,8 +53,14 @@ mod tests { } } + fn init_logger() { + std::env::set_var("RUST_LOG", "trace"); + let _ = env_logger::builder().is_test(true).try_init(); + } + #[test] fn test_sound_config_setup() { + init_logger(); let args = SoundArgs::from_args("/tmp/vhost-sound.socket"); let config = SoundConfig::try_from(args); diff --git a/staging/vhost-device-sound/src/virtio_sound.rs b/staging/vhost-device-sound/src/virtio_sound.rs index bb9744cf9..a666b3f19 100644 --- a/staging/vhost-device-sound/src/virtio_sound.rs +++ b/staging/vhost-device-sound/src/virtio_sound.rs @@ -89,6 +89,7 @@ pub const VIRTIO_SND_PCM_FMT_DSD_U8: u8 = 21; pub const VIRTIO_SND_PCM_FMT_DSD_U16: u8 = 22; pub const VIRTIO_SND_PCM_FMT_DSD_U32: u8 = 23; pub const VIRTIO_SND_PCM_FMT_IEC958_SUBFRAME: u8 = 24; +pub(crate) const _VIRTIO_SND_PCM_FMT_MAX: u8 = 25; // supported PCM frame rates @@ -106,6 +107,7 @@ pub const VIRTIO_SND_PCM_RATE_96000: u8 = 10; pub const VIRTIO_SND_PCM_RATE_176400: u8 = 11; pub const VIRTIO_SND_PCM_RATE_192000: u8 = 12; pub const VIRTIO_SND_PCM_RATE_384000: u8 = 13; +pub(crate) const _VIRTIO_SND_PCM_RATE_MAX: u8 = 14; // standard channel position definition