diff --git a/staging/vhost-device-sound/src/audio_backends.rs b/staging/vhost-device-sound/src/audio_backends.rs index a2153e6..1592285 100644 --- a/staging/vhost-device-sound/src/audio_backends.rs +++ b/staging/vhost-device-sound/src/audio_backends.rs @@ -15,7 +15,7 @@ use self::alsa::AlsaBackend; use self::null::NullBackend; #[cfg(feature = "pw-backend")] use self::pipewire::PwBackend; -use crate::{stream::Stream, BackendType, ControlMessage, Result, VirtioSndPcmSetParams}; +use crate::{stream::Stream, BackendType, Result, VirtioSndPcmSetParams}; pub trait AudioBackend { fn write(&self, stream_id: u32) -> Result<()>; @@ -30,7 +30,7 @@ pub trait AudioBackend { Ok(()) } - fn release(&self, _stream_id: u32, _: ControlMessage) -> Result<()> { + fn release(&self, _stream_id: u32) -> Result<()> { Ok(()) } diff --git a/staging/vhost-device-sound/src/audio_backends/alsa.rs b/staging/vhost-device-sound/src/audio_backends/alsa.rs index ead8580..8e78d24 100644 --- a/staging/vhost-device-sound/src/audio_backends/alsa.rs +++ b/staging/vhost-device-sound/src/audio_backends/alsa.rs @@ -19,8 +19,8 @@ use alsa::{ use super::AudioBackend; use crate::{ stream::{PCMState, Stream}, - virtio_sound::{self, VirtioSndPcmSetParams, VIRTIO_SND_S_BAD_MSG}, - ControlMessage, Direction, Error, Result as CrateResult, + virtio_sound::{self, VirtioSndPcmSetParams}, + Direction, Error, Result as CrateResult, }; impl From for alsa::Direction { @@ -692,21 +692,18 @@ impl AudioBackend for AlsaBackend { Ok(()) } - fn release(&self, stream_id: u32, mut msg: ControlMessage) -> CrateResult<()> { + fn release(&self, stream_id: u32) -> CrateResult<()> { if stream_id >= self.streams.read().unwrap().len() as u32 { log::error!( "Received Release action for stream id {} but there are only {} PCM streams.", stream_id, self.streams.read().unwrap().len() as u32 ); - msg.code = VIRTIO_SND_S_BAD_MSG; return Err(Error::StreamWithIdNotFound(stream_id)); } let mut streams = self.streams.write().unwrap(); if let Err(err) = streams[stream_id as usize].state.release() { log::error!("Stream {}: {}", stream_id, err); - msg.code = VIRTIO_SND_S_BAD_MSG; - drop(msg); return Err(Error::Stream(err)); } // Stop worker thread diff --git a/staging/vhost-device-sound/src/audio_backends/pipewire.rs b/staging/vhost-device-sound/src/audio_backends/pipewire.rs index e14c904..71f6eaf 100644 --- a/staging/vhost-device-sound/src/audio_backends/pipewire.rs +++ b/staging/vhost-device-sound/src/audio_backends/pipewire.rs @@ -46,9 +46,9 @@ use crate::{ VIRTIO_SND_PCM_RATE_192000, VIRTIO_SND_PCM_RATE_22050, VIRTIO_SND_PCM_RATE_32000, VIRTIO_SND_PCM_RATE_384000, VIRTIO_SND_PCM_RATE_44100, VIRTIO_SND_PCM_RATE_48000, VIRTIO_SND_PCM_RATE_5512, VIRTIO_SND_PCM_RATE_64000, VIRTIO_SND_PCM_RATE_8000, - VIRTIO_SND_PCM_RATE_88200, VIRTIO_SND_PCM_RATE_96000, VIRTIO_SND_S_BAD_MSG, + VIRTIO_SND_PCM_RATE_88200, VIRTIO_SND_PCM_RATE_96000, }, - ControlMessage, Direction, Error, Result, Stream, + Direction, Error, Result, Stream, }; impl From for spa::Direction { @@ -485,22 +485,18 @@ impl AudioBackend for PwBackend { Ok(()) } - fn release(&self, stream_id: u32, mut msg: ControlMessage) -> Result<()> { + fn release(&self, stream_id: u32) -> Result<()> { debug!("pipewire backend, release function"); let release_result = self .stream_params .write() .unwrap() .get_mut(stream_id as usize) - .ok_or_else(|| { - msg.code = VIRTIO_SND_S_BAD_MSG; - Error::StreamWithIdNotFound(stream_id) - })? + .ok_or_else(|| Error::StreamWithIdNotFound(stream_id))? .state .release(); if let Err(err) = release_result { log::error!("Stream {} release {}", stream_id, err); - msg.code = VIRTIO_SND_S_BAD_MSG; return Err(Error::Stream(err)); } let lock_guard = self.thread_loop.lock(); @@ -575,86 +571,7 @@ mod test_utils; #[cfg(test)] mod tests { - use vhost_user_backend::{VringRwLock, VringT}; - use virtio_bindings::bindings::virtio_ring::{VRING_DESC_F_NEXT, VRING_DESC_F_WRITE}; - use virtio_queue::{mock::MockSplitQueue, Descriptor, Queue, QueueOwnedT}; - use vm_memory::{ - Address, ByteValued, Bytes, GuestAddress, GuestAddressSpace, GuestMemoryAtomic, - GuestMemoryMmap, - }; - use super::{test_utils::PipewireTestHarness, *}; - use crate::{ControlMessageKind, SoundDescriptorChain, VirtioSoundHeader}; - - // Prepares a single chain of descriptors for request queue - fn prepare_desc_chain( - start_addr: GuestAddress, - hdr: R, - response_len: u32, - ) -> SoundDescriptorChain { - let mem = &GuestMemoryMmap::<()>::from_ranges(&[(start_addr, 0x1000)]).unwrap(); - let vq = MockSplitQueue::new(mem, 16); - let mut next_addr = vq.desc_table().total_size() + 0x100; - let mut index = 0; - - let desc_out = Descriptor::new( - next_addr, - size_of::() as u32, - VRING_DESC_F_NEXT as u16, - index + 1, - ); - - mem.write_obj::(hdr, desc_out.addr()).unwrap(); - vq.desc_table().store(index, desc_out).unwrap(); - next_addr += u64::from(desc_out.len()); - index += 1; - - // In response descriptor - let desc_in = Descriptor::new(next_addr, response_len, VRING_DESC_F_WRITE as u16, 0); - vq.desc_table().store(index, desc_in).unwrap(); - - // Put the descriptor index 0 in the first available ring position. - mem.write_obj(0u16, vq.avail_addr().unchecked_add(4)) - .unwrap(); - - // Set `avail_idx` to 1. - mem.write_obj(1u16, vq.avail_addr().unchecked_add(2)) - .unwrap(); - - // Create descriptor chain from pre-filled memory - vq.create_queue::() - .unwrap() - .iter(GuestMemoryAtomic::new(mem.clone()).memory()) - .unwrap() - .next() - .unwrap() - } - - fn ctrlmsg() -> ControlMessage { - // set up minimal configuration for test - let hdr = VirtioSndPcmSetParams { - format: VIRTIO_SND_PCM_FMT_S16, - rate: VIRTIO_SND_PCM_RATE_11025, - channels: 1, - ..Default::default() - }; - let resp_len: u32 = 1; - let mem = &GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0), 0x1000)]).unwrap(); - let memr = GuestMemoryAtomic::new( - GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap(), - ); - let vq = MockSplitQueue::new(mem, 16); - let next_addr = vq.desc_table().total_size() + 0x100; - let index = 0; - let vring = VringRwLock::new(memr, 0x1000).unwrap(); - ControlMessage { - kind: ControlMessageKind::PcmInfo, - code: 0, - desc_chain: prepare_desc_chain::(GuestAddress(0), hdr, resp_len), - descriptor: Descriptor::new(next_addr, 0x200, VRING_DESC_F_NEXT as u16, index + 1), - vring, - } - } #[test] fn test_pipewire_backend_success() { @@ -679,9 +596,7 @@ mod tests { pw_backend.write(0).unwrap(); pw_backend.read(0).unwrap(); pw_backend.stop(0).unwrap(); - let release_msg = ctrlmsg(); - pw_backend.release(0, release_msg).unwrap(); - + pw_backend.release(0).unwrap(); let streams = streams.read().unwrap(); assert_eq!(streams[0].buffers.len(), 0); } @@ -712,13 +627,10 @@ mod tests { ); } - let msg = ctrlmsg(); - _ = pw_backend.release(0, msg.clone()); - let resp: VirtioSoundHeader = msg - .desc_chain - .memory() - .read_obj(msg.descriptor.addr()) - .unwrap(); - assert_eq!(resp.code, VIRTIO_SND_S_BAD_MSG); + let res = pw_backend.release(0); + assert_eq!( + res.unwrap_err().to_string(), + Error::StreamWithIdNotFound(0).to_string() + ); } } diff --git a/staging/vhost-device-sound/src/device.rs b/staging/vhost-device-sound/src/device.rs index 988e13e..c31567a 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::*, - ControlMessage, ControlMessageKind, Direction, Error, IOMessage, Result, SoundConfig, + ControlMessageKind, Direction, Error, IOMessage, Result, SoundConfig, }; pub struct VhostUserSoundThread { @@ -361,25 +361,16 @@ impl VhostUserSoundThread { if stream_id as usize >= self.streams_no { log::error!("{}", Error::from(StreamError::InvalidStreamId(stream_id))); resp.code = VIRTIO_SND_S_BAD_MSG.into(); - } else { - audio_backend - .write() - .unwrap() - .release( - stream_id, - ControlMessage { - kind: code, - code: VIRTIO_SND_S_OK, - desc_chain, - descriptor: desc_hdr, - vring: vring.clone(), - }, - ) - .unwrap(); - - // PcmRelease needs to flush IO messages; the audio backend will reply when - // it drops the ControlMessage. - continue; + } else if let Err(err) = audio_backend.write().unwrap().release(stream_id) { + match err { + Error::Stream(_) | Error::StreamWithIdNotFound(_) => { + resp.code = VIRTIO_SND_S_BAD_MSG.into() + } + _ => { + log::error!("{}", err); + resp.code = VIRTIO_SND_S_IO_ERR.into() + } + } } } ControlMessageKind::PcmStart => { diff --git a/staging/vhost-device-sound/src/lib.rs b/staging/vhost-device-sound/src/lib.rs index ded4bd2..50cd753 100644 --- a/staging/vhost-device-sound/src/lib.rs +++ b/staging/vhost-device-sound/src/lib.rs @@ -214,62 +214,6 @@ impl TryFrom for ControlMessageKind { } } -#[cfg_attr(test, derive(Clone))] -pub struct ControlMessage { - pub kind: ControlMessageKind, - pub code: u32, - pub desc_chain: SoundDescriptorChain, - pub descriptor: virtio_queue::Descriptor, - pub vring: VringRwLock, -} - -impl std::fmt::Debug for ControlMessage { - fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result { - fmt.debug_struct(stringify!(ControlMessage)) - .field("kind", &self.kind) - .field("code", &self.code) - .finish() - } -} - -impl Drop for ControlMessage { - fn drop(&mut self) { - log::trace!( - "dropping ControlMessage {:?} reply = {}", - self.kind, - match self.code { - virtio_sound::VIRTIO_SND_S_OK => "VIRTIO_SND_S_OK", - virtio_sound::VIRTIO_SND_S_BAD_MSG => "VIRTIO_SND_S_BAD_MSG", - virtio_sound::VIRTIO_SND_S_NOT_SUPP => "VIRTIO_SND_S_NOT_SUPP", - virtio_sound::VIRTIO_SND_S_IO_ERR => "VIRTIO_SND_S_IO_ERR", - _ => "other", - } - ); - let resp = VirtioSoundHeader { - code: self.code.into(), - }; - - if let Err(err) = self - .desc_chain - .memory() - .write_obj(resp, self.descriptor.addr()) - { - log::error!("Error::DescriptorWriteFailed: {}", err); - return; - } - if self - .vring - .add_used(self.desc_chain.head_index(), resp.as_slice().len() as u32) - .is_err() - { - log::error!("Couldn't add used"); - } - if self.vring.signal_used_queue().is_err() { - log::error!("Couldn't signal used queue"); - } - } -} - #[derive(Debug, Clone)] /// This structure is the public API through which an external program /// is allowed to configure the backend. @@ -367,77 +311,10 @@ pub fn start_backend_server(config: SoundConfig) { mod tests { use std::sync::Arc; - use vhost_user_backend::{VringRwLock, VringT}; - use virtio_bindings::bindings::virtio_ring::{VRING_DESC_F_NEXT, VRING_DESC_F_WRITE}; - use virtio_queue::{mock::MockSplitQueue, Descriptor, Queue, QueueOwnedT}; - use vm_memory::{ - Address, ByteValued, GuestAddress, GuestAddressSpace, GuestMemoryAtomic, GuestMemoryMmap, - }; + use vm_memory::{GuestMemoryAtomic, GuestMemoryMmap}; use super::*; - use crate::{ControlMessageKind, SoundDescriptorChain}; - - // Prepares a single chain of descriptors - fn prepare_desc_chain( - start_addr: GuestAddress, - hdr: R, - response_len: u32, - ) -> SoundDescriptorChain { - let mem = &GuestMemoryMmap::<()>::from_ranges(&[(start_addr, 0x1000)]).unwrap(); - let vq = MockSplitQueue::new(mem, 16); - let mut next_addr = vq.desc_table().total_size() + 0x100; - let mut index = 0; - - let desc_out = Descriptor::new( - next_addr, - std::mem::size_of::() as u32, - VRING_DESC_F_NEXT as u16, - index + 1, - ); - - mem.write_obj::(hdr, desc_out.addr()).unwrap(); - vq.desc_table().store(index, desc_out).unwrap(); - next_addr += u64::from(desc_out.len()); - index += 1; - - // In response descriptor - let desc_in = Descriptor::new(next_addr, response_len, VRING_DESC_F_WRITE as u16, 0); - vq.desc_table().store(index, desc_in).unwrap(); - - // Put the descriptor index 0 in the first available ring position. - mem.write_obj(0u16, vq.avail_addr().unchecked_add(4)) - .unwrap(); - - // Set `avail_idx` to 1. - mem.write_obj(1u16, vq.avail_addr().unchecked_add(2)) - .unwrap(); - - // Create descriptor chain from pre-filled memory - vq.create_queue::() - .unwrap() - .iter(GuestMemoryAtomic::new(mem.clone()).memory()) - .unwrap() - .next() - .unwrap() - } - - fn ctrl_msg() -> ControlMessage { - let hdr = VirtioSndPcmSetParams::default(); - let memr = GuestMemoryAtomic::new( - GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap(), - ); - let vring = VringRwLock::new(memr, 0x1000).unwrap(); - let mem = &GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0), 0x1000)]).unwrap(); - let vq = MockSplitQueue::new(mem, 16); - let next_addr = vq.desc_table().total_size() + 0x100; - ControlMessage { - kind: ControlMessageKind::JackInfo, - code: 0, - desc_chain: prepare_desc_chain::(GuestAddress(0), hdr, 1), - descriptor: Descriptor::new(next_addr, 0x200, VRING_DESC_F_NEXT as u16, 1), - vring, - } - } + use crate::ControlMessageKind; #[test] fn test_sound_server() { @@ -520,12 +397,4 @@ mod tests { let stream_error = stream::Error::DescriptorReadFailed; let _error: Error = stream_error.into(); } - - #[test] - fn test_debug_format() { - let ctrl_msg = ctrl_msg(); - let debug_output = format!("{:?}", ctrl_msg); - let expected_output = "ControlMessage { kind: JackInfo, code: 0 }".to_string(); - assert_eq!(debug_output, expected_output); - } }