vhost-device-sound: reuse socket between requests

This removes a race where there's a moment in between connections
where the socket is unlinked and recreated, so there's no socket
available for clients to connect to.  This unfortunately requires
changing the public API.

Signed-off-by: Alyssa Ross <hi@alyssa.is>

vhost-device-sound/CHANGELOG.md

@@ -5,6 +5,22 @@
 
 ### Changed
 
+- [[#907]](https://github.com/rust-vmm/vhost-device/pull/907)
+  `vhost_device_sound::start_backend_server` now mutably borrows a
+  `vhost::vhost_user::Listener`, so the socket isn't removed and
+  re-created between each connection, and there's no longer a short
+  window of time where there's no socket for clients to connect to.
+
+  As a consequence of this change:
+
+  - `vhost_device_sound::SoundConfig::new` no longer takes a `socket` argument.
+  - `vhost_device_sound::SoundConfig::get_socket_path` has been removed.
+  - `vhost_device_sound::SoundConfig` no longer implements
+    `From<vhost_device_sound::args::SoundArgs>` (since the `socket`
+    argument should be handled separately).
+  - `vhost_device_sound::start_backend_server` now additionally takes
+    a `listener` argument.
+
 ### Fixed
 
 ### Deprecated

vhost-device-sound/src/device.rs

@@ -682,8 +682,6 @@ impl VhostUserBackend for VhostUserSoundBackend {
 
 #[cfg(test)]
 mod tests {
-    use std::path::PathBuf;
-
     use tempfile::tempdir;
     use virtio_bindings::virtio_ring::VRING_DESC_F_WRITE;
     use virtio_queue::{
@@ -697,8 +695,6 @@ mod tests {
     use super::*;
     use crate::BackendType;
 
-    const SOCKET_PATH: &str = "vsound.socket";
-
     fn setup_descs(descs: &[RawDescriptor]) -> (VringRwLock, GuestMemoryAtomic<GuestMemoryMmap>) {
         let mem = GuestMemoryAtomic::new(
             GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0), 0x1000_0000)]).unwrap(),
@@ -739,7 +735,7 @@ mod tests {
     #[test]
     fn test_sound_thread_success() {
         crate::init_logger();
-        let config = SoundConfig::new(PathBuf::from(SOCKET_PATH), false, BackendType::Null);
+        let config = SoundConfig::new(false, BackendType::Null);
 
         let chmaps = Arc::new(RwLock::new(vec![]));
         let jacks = Arc::new(RwLock::new(vec![]));
@@ -849,7 +845,7 @@ mod tests {
     #[test]
     fn test_sound_thread_failure() {
         crate::init_logger();
-        let config = SoundConfig::new(PathBuf::from(SOCKET_PATH), false, BackendType::Null);
+        let config = SoundConfig::new(false, BackendType::Null);
 
         let chmaps = Arc::new(RwLock::new(vec![]));
         let jacks = Arc::new(RwLock::new(vec![]));
@@ -938,8 +934,7 @@ mod tests {
     fn test_sound_backend() {
         crate::init_logger();
         let test_dir = tempdir().expect("Could not create a temp test directory.");
-        let socket_path = test_dir.path().join(SOCKET_PATH);
+        let config = SoundConfig::new(false, BackendType::Null);
-        let config = SoundConfig::new(socket_path, false, BackendType::Null);
         let backend = VhostUserSoundBackend::new(config).expect("Could not create backend.");
 
         assert_eq!(backend.num_queues(), NUM_QUEUES as usize);
@@ -1017,9 +1012,7 @@ mod tests {
         crate::init_logger();
         let test_dir = tempdir().expect("Could not create a temp test directory.");
 
-        let socket_path = test_dir.path().join("sound_failures.socket");
+        let config = SoundConfig::new(false, BackendType::Null);
-
-        let config = SoundConfig::new(socket_path, false, BackendType::Null);
         let backend = VhostUserSoundBackend::new(config);
 
         let backend = backend.unwrap();

vhost-device-sound/src/lib.rs

@@ -14,7 +14,7 @@ pub mod device;
 pub mod stream;
 pub mod virtio_sound;
 
-use std::{convert::TryFrom, io::Error as IoError, mem::size_of, path::PathBuf, sync::Arc};
+use std::{convert::TryFrom, io::Error as IoError, mem::size_of, sync::Arc};
 
 pub use args::BackendType;
 pub use stream::Stream;
@@ -212,39 +212,22 @@ impl TryFrom<Le32> for ControlMessageKind {
 /// This structure is the public API through which an external program
 /// is allowed to configure the backend.
 pub struct SoundConfig {
-    /// vhost-user Unix domain socket
-    socket: PathBuf,
     /// use multiple threads to handle the virtqueues
     multi_thread: bool,
     /// audio backend variant
     audio_backend: BackendType,
 }
 
-impl From<args::SoundArgs> for SoundConfig {
-    fn from(cmd_args: args::SoundArgs) -> Self {
-        let args::SoundArgs { socket, backend } = cmd_args;
-
-        Self::new(socket, false, backend)
-    }
-}
-
 impl SoundConfig {
     /// Create a new instance of the SoundConfig struct, containing the
     /// parameters to be fed into the sound-backend server.
-    pub const fn new(socket: PathBuf, multi_thread: bool, audio_backend: BackendType) -> Self {
+    pub const fn new(multi_thread: bool, audio_backend: BackendType) -> Self {
         Self {
-            socket,
             multi_thread,
             audio_backend,
         }
     }
 
-    /// Return the path of the unix domain socket which is listening to
-    /// requests from the guest.
-    pub fn get_socket_path(&self) -> PathBuf {
-        self.socket.clone()
-    }
-
     pub const fn get_audio_backend(&self) -> BackendType {
         self.audio_backend
     }
@@ -308,9 +291,8 @@ impl Drop for IOMessage {
 
 /// This is the public API through which an external program starts the
 /// vhost-device-sound backend server.
-pub fn start_backend_server(config: SoundConfig) {
+pub fn start_backend_server(listener: &mut Listener, config: SoundConfig) {
     log::trace!("Using config {:?}.", &config);
-    let socket = config.get_socket_path();
     let backend = Arc::new(VhostUserSoundBackend::new(config).unwrap());
 
     let mut daemon = VhostUserDaemon::new(
@@ -322,9 +304,7 @@ pub fn start_backend_server(config: SoundConfig) {
 
     log::trace!("Starting daemon.");
 
-    let mut listener = Listener::new(socket, true).unwrap();
+    daemon.start(listener).unwrap();
-
-    daemon.start(&mut listener).unwrap();
     let result = daemon.wait();
 
     backend.send_exit_event();
@@ -350,10 +330,9 @@ mod tests {
 
     #[test]
     fn test_sound_server() {
-        const SOCKET_PATH: &str = "vsound.socket";
         crate::init_logger();
 
-        let config = SoundConfig::new(PathBuf::from(SOCKET_PATH), false, BackendType::Null);
+        let config = SoundConfig::new(false, BackendType::Null);
 
         let backend = Arc::new(VhostUserSoundBackend::new(config).unwrap());
         let daemon = VhostUserDaemon::new(

vhost-device-sound/src/main.rs

@@ -3,48 +3,29 @@
 // SPDX-License-Identifier: Apache-2.0 or BSD-3-Clause
 
 use clap::Parser;
+use vhost::vhost_user::Listener;
 use vhost_device_sound::{args::SoundArgs, start_backend_server, SoundConfig};
 
 fn main() {
     env_logger::init();
 
-    let config = SoundConfig::from(SoundArgs::parse());
+    let args = SoundArgs::parse();
+    let config = SoundConfig::new(false, args.backend);
+    let mut listener = Listener::new(args.socket, true).unwrap();
 
     loop {
-        start_backend_server(config.clone());
+        start_backend_server(&mut listener, config.clone());
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use std::path::{Path, PathBuf};
-
     use clap::Parser;
     use rstest::*;
     use vhost_device_sound::BackendType;
 
     use super::*;
 
-    fn init_logger() {
-        std::env::set_var("RUST_LOG", "trace");
-        let _ = env_logger::builder().is_test(true).try_init();
-    }
-
-    #[test]
-    fn test_sound_config_setup() {
-        init_logger();
-        let args = SoundArgs {
-            socket: PathBuf::from("/tmp/vhost-sound.socket"),
-            backend: BackendType::default(),
-        };
-        let config = SoundConfig::from(args);
-
-        assert_eq!(
-            config.get_socket_path(),
-            Path::new("/tmp/vhost-sound.socket")
-        );
-    }
-
     #[rstest]
     #[case::null_backend("null", BackendType::Null)]
     #[cfg_attr(
@@ -68,7 +49,7 @@ mod tests {
             backend_name,
         ]);
 
-        let config = SoundConfig::from(args);
+        let config = SoundConfig::new(false, args.backend);
         assert_eq!(config.get_audio_backend(), backend);
     }
 }