From 252494f7825bb2db79aaa8cddb55d81355bbf8ba Mon Sep 17 00:00:00 2001 From: Matias Ezequiel Vara Larsen Date: Wed, 22 Nov 2023 15:03:23 +0100 Subject: [PATCH] sound/alsa: handle errors and propagate to device This commit adds the corresponding handling and propagation of errors that may happen when device communicates with the alsa backend. Signed-off-by: Matias Ezequiel Vara Larsen --- .../src/audio_backends/alsa.rs | 35 +++++++++++++++---- staging/vhost-device-sound/src/lib.rs | 4 +++ staging/vhost-device-sound/src/stream.rs | 2 ++ 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/staging/vhost-device-sound/src/audio_backends/alsa.rs b/staging/vhost-device-sound/src/audio_backends/alsa.rs index 9355e14..618c886 100644 --- a/staging/vhost-device-sound/src/audio_backends/alsa.rs +++ b/staging/vhost-device-sound/src/audio_backends/alsa.rs @@ -22,7 +22,7 @@ use super::AudioBackend; use crate::{ stream::{PCMState, Stream}, virtio_sound::{self, VirtioSndPcmSetParams, VIRTIO_SND_S_BAD_MSG, VIRTIO_SND_S_NOT_SUPP}, - ControlMessage, Direction, Result as CrateResult, + ControlMessage, Direction, Error, Result as CrateResult, }; impl From for alsa::Direction { @@ -543,12 +543,18 @@ impl AudioBackend for AlsaBackend { stream_id, self.streams.read().unwrap().len() ); + return Err(Error::StreamWithIdNotFound(stream_id)); } if matches!( - self.streams.write().unwrap()[stream_id as usize].state, + self.streams.read().unwrap()[stream_id as usize].state, PCMState::Start | PCMState::Prepare ) { self.senders[stream_id as usize].send(true).unwrap(); + } else { + return Err(Error::Stream(crate::stream::Error::InvalidState( + "read", + self.streams.read().unwrap()[stream_id as usize].state, + ))); } Ok(()) } @@ -560,12 +566,18 @@ impl AudioBackend for AlsaBackend { stream_id, self.streams.read().unwrap().len() ); + return Err(Error::StreamWithIdNotFound(stream_id)); } if matches!( - self.streams.write().unwrap()[stream_id as usize].state, + self.streams.read().unwrap()[stream_id as usize].state, PCMState::Start | PCMState::Prepare ) { self.senders[stream_id as usize].send(true).unwrap(); + } else { + return Err(Error::Stream(crate::stream::Error::InvalidState( + "write", + self.streams.read().unwrap()[stream_id as usize].state, + ))); } Ok(()) } @@ -577,12 +589,13 @@ impl AudioBackend for AlsaBackend { stream_id, self.streams.read().unwrap().len() ); + return Err(Error::StreamWithIdNotFound(stream_id)); } if let Err(err) = self.streams.write().unwrap()[stream_id as usize] .state .start() { - log::error!("Stream {}: {}", stream_id, err); + return Err(Error::Stream(err)); } let pcm = &self.pcms[stream_id as usize]; let lck = pcm.lock().unwrap(); @@ -594,6 +607,7 @@ impl AudioBackend for AlsaBackend { stream_id, err ); + return Err(Error::UnexpectedAudioBackendError(err.to_string())); } } self.senders[stream_id as usize].send(true).unwrap(); @@ -607,12 +621,14 @@ impl AudioBackend for AlsaBackend { stream_id, self.streams.read().unwrap().len() as u32 ); + return Err(Error::StreamWithIdNotFound(stream_id)); } if let Err(err) = self.streams.write().unwrap()[stream_id as usize] .state .prepare() { log::error!("Stream {}: {}", stream_id, err); + return Err(Error::Stream(err)); } let pcm = &self.pcms[stream_id as usize]; let lck = pcm.lock().unwrap(); @@ -624,6 +640,7 @@ impl AudioBackend for AlsaBackend { stream_id, err ); + return Err(Error::UnexpectedAudioBackendError(err.to_string())); } } Ok(()) @@ -650,6 +667,7 @@ impl AudioBackend for AlsaBackend { self.streams.read().unwrap().len() as u32 ); msg.code = VIRTIO_SND_S_BAD_MSG; + return Err(Error::StreamWithIdNotFound(stream_id)); } let descriptors: Vec = msg.desc_chain.clone().collect(); let desc_request = &descriptors[0]; @@ -664,8 +682,10 @@ impl AudioBackend for AlsaBackend { if let Err(err) = st.state.set_parameters() { log::error!("Stream {} set_parameters {}", stream_id, err); msg.code = VIRTIO_SND_S_BAD_MSG; + return Err(Error::Stream(err)); } else if !st.supports_format(request.format) || !st.supports_rate(request.rate) { msg.code = VIRTIO_SND_S_NOT_SUPP; + return Err(Error::UnexpectedAudioBackendConfiguration); } else { st.params.buffer_bytes = request.buffer_bytes; st.params.period_bytes = request.period_bytes; @@ -694,14 +714,17 @@ impl AudioBackend for AlsaBackend { self.streams.read().unwrap().len() as u32 ); msg.code = VIRTIO_SND_S_BAD_MSG; + return Err(Error::StreamWithIdNotFound(stream_id)); } - // Stop worker thread - self.senders[stream_id as usize].send(false).unwrap(); 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 + self.senders[stream_id as usize].send(false).unwrap(); // Drop pending stream buffers to complete pending I/O messages // // This will release buffers even if state transition is invalid. If it is diff --git a/staging/vhost-device-sound/src/lib.rs b/staging/vhost-device-sound/src/lib.rs index a1aa851..ded4bd2 100644 --- a/staging/vhost-device-sound/src/lib.rs +++ b/staging/vhost-device-sound/src/lib.rs @@ -118,6 +118,10 @@ pub enum Error { SoundReqMissingData, #[error("Audio backend not supported")] AudioBackendNotSupported, + #[error("Audio backend unexpected error: {0}")] + UnexpectedAudioBackendError(String), + #[error("Audio backend configuration not supported")] + UnexpectedAudioBackendConfiguration, #[error("No memory configured")] NoMemoryConfigured, #[error("Invalid virtio_snd_hdr size, expected: {0}, found: {1}")] diff --git a/staging/vhost-device-sound/src/stream.rs b/staging/vhost-device-sound/src/stream.rs index 8dfb4cc..479364d 100644 --- a/staging/vhost-device-sound/src/stream.rs +++ b/staging/vhost-device-sound/src/stream.rs @@ -11,6 +11,8 @@ use crate::{virtio_sound::*, Direction, IOMessage, SUPPORTED_FORMATS, SUPPORTED_ /// Stream errors. #[derive(Debug, ThisError, PartialEq, Eq)] pub enum Error { + #[error("Guest driver request {0} in an invalid stream state {1}")] + InvalidState(&'static str, PCMState), #[error("Guest driver request an invalid stream state transition from {0} to {1}.")] InvalidStateTransition(PCMState, PCMState), #[error("Guest requested an invalid stream id: {0}")]