From d3274444ad0dd9c0ab03ea6c5e3806de7cc5e166 Mon Sep 17 00:00:00 2001
From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Date: Mon, 2 Dec 2024 13:36:03 +0200
Subject: [PATCH] sound: make PwBackend::new() return Result

Add a pipewire-specific error enum and make the backend's new() method
return a Result instead of panicking with expect() on error.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 vhost-device-sound/src/audio_backends.rs      |  6 +++-
 .../src/audio_backends/pipewire.rs            | 28 +++++++++++++------
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/vhost-device-sound/src/audio_backends.rs b/vhost-device-sound/src/audio_backends.rs
index e5fcfce..27ebf06 100644
--- a/vhost-device-sound/src/audio_backends.rs
+++ b/vhost-device-sound/src/audio_backends.rs
@@ -54,7 +54,11 @@ pub fn alloc_audio_backend(
     match backend {
         BackendType::Null => Ok(Box::new(NullBackend::new(streams))),
         #[cfg(all(feature = "pw-backend", target_env = "gnu"))]
-        BackendType::Pipewire => Ok(Box::new(PwBackend::new(streams))),
+        BackendType::Pipewire => {
+            Ok(Box::new(PwBackend::new(streams).map_err(|err| {
+                crate::Error::UnexpectedAudioBackendError(err.into())
+            })?))
+        }
         #[cfg(all(feature = "alsa-backend", target_env = "gnu"))]
         BackendType::Alsa => Ok(Box::new(AlsaBackend::new(streams))),
     }
diff --git a/vhost-device-sound/src/audio_backends/pipewire.rs b/vhost-device-sound/src/audio_backends/pipewire.rs
index 32f989d..46fbad9 100644
--- a/vhost-device-sound/src/audio_backends/pipewire.rs
+++ b/vhost-device-sound/src/audio_backends/pipewire.rs
@@ -33,6 +33,7 @@ use spa::{
         SPA_AUDIO_FORMAT_UNKNOWN,
     },
 };
+use thiserror::Error as ThisError;
 
 use super::AudioBackend;
 use crate::{
@@ -64,6 +65,17 @@ impl From<Direction> for spa::utils::Direction {
     }
 }
 
+/// Error type for the Pipewire backend
+#[derive(Debug, ThisError)]
+pub enum PwError {
+    #[error("Failed to create Pipewire context: {0}")]
+    CreateContext(pw::Error),
+    #[error("Failed to connect to Pipewire core: {0}")]
+    ConnectToCore(pw::Error),
+    #[error("Failed to trigger sync event with Pipewire server: {0}")]
+    TriggerSyncEvent(pw::Error),
+}
+
 // SAFETY: Safe as the structure can be sent to another thread.
 unsafe impl Send for PwBackend {}
 
@@ -84,7 +96,7 @@ pub struct PwBackend {
 }
 
 impl PwBackend {
-    pub fn new(stream_params: Arc<RwLock<Vec<Stream>>>) -> Self {
+    pub fn new(stream_params: Arc<RwLock<Vec<Stream>>>) -> std::result::Result<Self, PwError> {
         pw::init();
 
         // SAFETY: safe as the thread loop cannot access objects associated
@@ -93,9 +105,9 @@ impl PwBackend {
 
         let lock_guard = thread_loop.lock();
 
-        let context = Context::new(&thread_loop).expect("failed to create context");
+        let context = Context::new(&thread_loop).map_err(PwError::CreateContext)?;
         thread_loop.start();
-        let core = context.connect(None).expect("Failed to connect to core");
+        let core = context.connect(None).map_err(PwError::ConnectToCore)?;
 
         // Create new reference for the variable so that it can be moved into the
         // closure.
@@ -104,7 +116,7 @@ impl PwBackend {
         // Trigger the sync event. The server's answer won't be processed until we start
         // the thread loop, so we can safely do this before setting up a
         // callback. This lets us avoid using a Cell.
-        let pending = core.sync(0).expect("sync failed");
+        let pending = core.sync(0).map_err(PwError::TriggerSyncEvent)?;
         let _listener_core = core
             .add_listener_local()
             .done(move |id, seq| {
@@ -119,14 +131,14 @@ impl PwBackend {
 
         log::trace!("pipewire backend running");
 
-        Self {
+        Ok(Self {
             stream_params,
             thread_loop,
             core,
             context,
             stream_hash: RwLock::new(HashMap::new()),
             stream_listener: RwLock::new(HashMap::new()),
-        }
+        })
     }
 }
 
@@ -594,7 +606,7 @@ mod tests {
 
         let _test_harness = PipewireTestHarness::new();
 
-        let pw_backend = PwBackend::new(stream_params);
+        let pw_backend = PwBackend::new(stream_params).unwrap();
         assert_eq!(pw_backend.stream_hash.read().unwrap().len(), 0);
         assert_eq!(pw_backend.stream_listener.read().unwrap().len(), 0);
         // set up minimal configuration for test
@@ -622,7 +634,7 @@ mod tests {
 
         let _test_harness = PipewireTestHarness::new();
 
-        let pw_backend = PwBackend::new(stream_params);
+        let pw_backend = PwBackend::new(stream_params).unwrap();
 
         let request = VirtioSndPcmSetParams::default();
         let res = pw_backend.set_parameters(0, request);