Merge pull request #24 from aesteve-rh/virtio-sound-cli-backend

Change CLI backend option to ValueEnum

Cargo.lock

@@ -412,6 +412,12 @@ version = "0.3.28"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "76d3d132be6c0e6aa1534069c705a74a5997a356c0dc2f86a47765e5617c5b65"
 
+[[package]]
+name = "futures-timer"
+version = "3.0.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e64b03909df88034c26dc1547e8970b91f98bdb65165d6a4e9110d94263dbb2c"
+
 [[package]]
 name = "futures-util"
 version = "0.3.28"
@@ -868,12 +874,56 @@ version = "0.7.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "e5ea92a5b6195c6ef2a0295ea818b312502c6fc94dde986c5553242e18fd4ce2"
 
+[[package]]
+name = "relative-path"
+version = "1.9.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c707298afce11da2efef2f600116fa93ffa7a032b5d7b628aa17711ec81383ca"
+
+[[package]]
+name = "rstest"
+version = "0.18.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "97eeab2f3c0a199bc4be135c36c924b6590b88c377d416494288c14f2db30199"
+dependencies = [
+ "futures",
+ "futures-timer",
+ "rstest_macros",
+ "rustc_version",
+]
+
+[[package]]
+name = "rstest_macros"
+version = "0.18.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d428f8247852f894ee1be110b375111b586d4fa431f6c46e64ba5a0dcccbe605"
+dependencies = [
+ "cfg-if",
+ "glob",
+ "proc-macro2",
+ "quote",
+ "regex",
+ "relative-path",
+ "rustc_version",
+ "syn 2.0.26",
+ "unicode-ident",
+]
+
 [[package]]
 name = "rustc-hash"
 version = "1.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2"
 
+[[package]]
+name = "rustc_version"
+version = "0.4.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "bfa0f585226d2e68097d4f95d113b15b83a82e819ab25717ec0590d9584ef366"
+dependencies = [
+ "semver",
+]
+
 [[package]]
 name = "rustix"
 version = "0.37.23"
@@ -907,6 +957,12 @@ version = "1.2.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49"
 
+[[package]]
+name = "semver"
+version = "1.0.18"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b0293b4b29daaf487284529cc2f5675b8e57c61f70167ba415a463651fd6a918"
+
 [[package]]
 name = "serde"
 version = "1.0.163"
@@ -1211,6 +1267,7 @@ dependencies = [
  "log",
  "pipewire",
  "pipewire-sys",
+ "rstest",
  "serial_test",
  "thiserror",
  "vhost",

crates/sound/Cargo.toml

@@ -35,3 +35,4 @@ vmm-sys-util = "0.11"
 
 [dev-dependencies]
 serial_test = "1.0"
+rstest = "0.18.2"

crates/sound/README.md

@@ -16,7 +16,7 @@ generated with help2man target/debug/vhost-user-sound |mandoc
             vhost-user Unix domain socket path
 
      --backend <BACKEND>
-            audio backend to be used (supported: null)
+            audio backend to be used [possible values: null, pipewire, alsa]
 
      -h, --help
             Print help

crates/sound/src/audio_backends.rs

@@ -17,7 +17,7 @@ use self::alsa::AlsaBackend;
 use self::null::NullBackend;
 #[cfg(feature = "pw-backend")]
 use self::pipewire::PwBackend;
-use crate::{device::ControlMessage, stream::Stream, Error, Result};
+use crate::{device::ControlMessage, stream::Stream, BackendType, Error, Result};
 
 pub trait AudioBackend {
     fn write(&self, stream_id: u32) -> Result<()>;
@@ -46,17 +46,24 @@ pub trait AudioBackend {
 }
 
 pub fn alloc_audio_backend(
-    name: String,
-    streams: Arc<RwLock<Vec<Stream>>>,
+    backend: BackendType,
+    // Unused when compiled with no features.
+    #[allow(unused_variables)] streams: Arc<RwLock<Vec<Stream>>>,
 ) -> Result<Box<dyn AudioBackend + Send + Sync>> {
-    log::trace!("allocating audio backend {}", name);
-    match name.as_str() {
+    log::trace!("allocating audio backend {:?}", backend);
+    match backend {
         #[cfg(feature = "null-backend")]
-        "null" => Ok(Box::new(NullBackend::new(streams))),
+        BackendType::Null => Ok(Box::new(NullBackend::new(streams))),
         #[cfg(feature = "pw-backend")]
-        "pipewire" => Ok(Box::new(PwBackend::new(streams))),
+        BackendType::Pipewire => Ok(Box::new(PwBackend::new(streams))),
         #[cfg(feature = "alsa-backend")]
-        "alsa" => Ok(Box::new(AlsaBackend::new(streams))),
+        BackendType::Alsa => Ok(Box::new(AlsaBackend::new(streams))),
+        // By default all features are enabled and this branch is unreachable.
+        // Nonetheless, it is required when inidividual features (or no features
+        // at all) are enabled.
+        // To avoid having a complicated compilation condition and make the
+        // code more maintainable, we supress the unreachable_patterns warning.
+        #[allow(unreachable_patterns)]
         _ => Err(Error::AudioBackendNotSupported),
     }
 }

crates/sound/src/device.rs

@@ -557,7 +557,7 @@ impl VhostUserSoundBackend {
             )?)]
         };
 
-        let audio_backend = alloc_audio_backend(config.audio_backend_name, streams)?;
+        let audio_backend = alloc_audio_backend(config.audio_backend, streams)?;
 
         Ok(Self {
             threads,

crates/sound/src/lib.rs

@@ -12,6 +12,7 @@ use std::{
     sync::Arc,
 };
 
+use clap::ValueEnum;
 use log::{info, warn};
 pub use stream::Stream;
 use thiserror::Error as ThisError;
@@ -90,6 +91,14 @@ impl From<stream::Error> for Error {
     }
 }
 
+#[derive(ValueEnum, Clone, Copy, Default, Debug, Eq, PartialEq)]
+pub enum BackendType {
+    #[default]
+    Null,
+    Pipewire,
+    Alsa,
+}
+
 #[derive(Debug)]
 pub struct InvalidControlMessage(u32);
 
@@ -203,18 +212,18 @@ pub struct SoundConfig {
     socket: String,
     /// use multiple threads to hanlde the virtqueues
     multi_thread: bool,
-    /// audio backend name
-    audio_backend_name: String,
+    /// audio backend variant
+    audio_backend: BackendType,
 }
 
 impl SoundConfig {
     /// Create a new instance of the SoundConfig struct, containing the
     /// parameters to be fed into the sound-backend server.
-    pub fn new(socket: String, multi_thread: bool, audio_backend_name: String) -> Self {
+    pub fn new(socket: String, multi_thread: bool, audio_backend: BackendType) -> Self {
         Self {
             socket,
             multi_thread,
-            audio_backend_name,
+            audio_backend,
         }
     }
 
@@ -223,6 +232,10 @@ impl SoundConfig {
     pub fn get_socket_path(&self) -> String {
         String::from(&self.socket)
     }
+
+    pub fn get_audio_backend(&self) -> BackendType {
+        self.audio_backend
+    }
 }
 
 pub struct IOMessage {

crates/sound/src/main.rs

@@ -4,7 +4,7 @@
 use std::convert::TryFrom;
 
 use clap::Parser;
-use vhost_user_sound::{start_backend_server, Error, Result, SoundConfig};
+use vhost_user_sound::{start_backend_server, BackendType, Error, Result, SoundConfig};
 
 #[derive(Parser, Debug)]
 #[clap(version, about, long_about = None)]
@@ -12,9 +12,10 @@ struct SoundArgs {
     /// vhost-user Unix domain socket path.
     #[clap(long)]
     socket: String,
-    /// audio backend to be used (supported: null)
+    /// audio backend to be used
     #[clap(long)]
-    backend: String,
+    #[clap(value_enum)]
+    backend: BackendType,
 }
 
 impl TryFrom<SoundArgs> for SoundConfig {
@@ -22,9 +23,8 @@ impl TryFrom<SoundArgs> for SoundConfig {
 
     fn try_from(cmd_args: SoundArgs) -> Result<Self> {
         let socket = cmd_args.socket.trim().to_string();
-        let backend = cmd_args.backend.trim().to_string();
 
-        Ok(SoundConfig::new(socket, false, backend))
+        Ok(SoundConfig::new(socket, false, cmd_args.backend))
     }
 }
 
@@ -43,19 +43,20 @@ mod tests {
     use serial_test::serial;
 
     use super::*;
+    use rstest::*;
 
     impl SoundArgs {
         fn from_args(socket: &str) -> Self {
             SoundArgs {
                 socket: socket.to_string(),
-                backend: "null".to_string(),
+                backend: BackendType::default(),
             }
         }
     }
 
     #[test]
     #[serial]
-    fn test_vsock_config_setup() {
+    fn test_sound_config_setup() {
         let args = SoundArgs::from_args("/tmp/vhost-sound.socket");
 
         let config = SoundConfig::try_from(args);
@@ -64,4 +65,25 @@ mod tests {
         let config = config.unwrap();
         assert_eq!(config.get_socket_path(), "/tmp/vhost-sound.socket");
     }
+
+    #[rstest]
+    #[serial]
+    #[case::null_backend("null", BackendType::Null)]
+    #[case::pipewire("pipewire", BackendType::Pipewire)]
+    #[case::alsa("alsa", BackendType::Alsa)]
+    fn test_cli_backend_arg(#[case] backend_name: &str, #[case] backend: BackendType) {
+        let args: SoundArgs = Parser::parse_from([
+            "",
+            "--socket",
+            "/tmp/vhost-sound.socket ",
+            "--backend",
+            backend_name,
+        ]);
+
+        let config = SoundConfig::try_from(args);
+        assert!(config.is_ok());
+
+        let config = config.unwrap();
+        assert_eq!(config.get_audio_backend(), backend);
+    }
 }