sound: remove ControlMessage parameter from release()

This commit removes ControlMessage from the release() method. The
backends process the release() command immediately thus not requiring the
audio backend to notify guest later. The notification happens in the
device after the cmd is processed. This also removes the need of
handling descriptors in the audio backend. This commit fixes the tests
in pw.

Signed-off-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
This commit is contained in:
Matias Ezequiel Vara Larsen 2023-11-23 10:51:40 +01:00 committed by Viresh Kumar
parent a045950d74
commit 6a072fa02c
5 changed files with 28 additions and 259 deletions

View File

@ -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(())
}

View File

@ -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<Direction> 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

View File

@ -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<Direction> 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<R: ByteValued>(
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::<R>() as u32,
VRING_DESC_F_NEXT as u16,
index + 1,
);
mem.write_obj::<R>(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::<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::<VirtioSndPcmSetParams>(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()
);
}
}

View File

@ -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 => {

View File

@ -214,62 +214,6 @@ impl TryFrom<Le32> 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<R: ByteValued>(
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::<R>() as u32,
VRING_DESC_F_NEXT as u16,
index + 1,
);
mem.write_obj::<R>(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::<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::<VirtioSndPcmSetParams>(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);
}
}