vhost_user: don't take ownership of Listener

The vhost-device devices all call VhostUserDaemon::serve in a loop, to
handle reconnections.  This is not ideal, because a new listener is
created each loop iteration, which means that each time, the old
socket is unlinked and a new one is created.  This means that there's
a potential race where a frontend attempts to connect to the backend
before the new socket is created.

A more robust way to achieve this would be to have the devices create
their own listeners, and pass the same one to VhostUserDaemon::start
on each loop iteration, instead of letting VhostUserDaemon::serve
create it repeatedly.  This was not previously possible though,
because VhostUserDaemon::start consumed the listener, even though it
didn't need to.

Because it's now possible to call VhostUserDaemon::start multiple
times with the same socket, I've removed the TODO about handling
reconnection.

Signed-off-by: Alyssa Ross <[email protected]>

Author: alyssais

vhost-user-backend/CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Added
 ### Changed
 - [[#308](https://github.com/rust-vmm/vhost/pull/308)] Replace Eventfd with EventNotifier/EventConsumer.
+- [[#321](https://github.com/rust-vmm/vhost/pull/321)] Don't take ownership of listener in `VhostUserDaemon::start`.
 
 ### Deprecated
 ### Fixed

vhost-user-backend/src/lib.rs

@@ -162,9 +162,7 @@ where
     ///
     /// *Note:* A convenience function [VhostUserDaemon::serve] exists that
     /// may be a better option than this for simple use-cases.
-    // TODO: the current implementation has limitations that only one incoming connection will be
-    // handled from the listener. Should it be enhanced to support reconnection?
-    pub fn start(&mut self, listener: Listener) -> Result<()> {
+    pub fn start(&mut self, listener: &mut Listener) -> Result<()> {
         let mut backend_listener = BackendListener::new(listener, self.handler.clone())
             .map_err(Error::CreateBackendListener)?;
         let backend_handler = self.accept(&mut backend_listener)?;
@@ -215,9 +213,9 @@ where
     /// *Note:* See [VhostUserDaemon::start] and [VhostUserDaemon::wait] if you
     /// need more flexibility.
     pub fn serve<P: AsRef<Path>>(&mut self, socket: P) -> Result<()> {
-        let listener = Listener::new(socket, true).map_err(Error::CreateVhostUserListener)?;
+        let mut listener = Listener::new(socket, true).map_err(Error::CreateVhostUserListener)?;
 
-        self.start(listener)?;
+        self.start(&mut listener)?;
         let result = self.wait();
 
         // Regardless of the result, we want to signal worker threads to exit
@@ -279,9 +277,9 @@ mod tests {
                 drop(socket)
             });
 
-            let listener = Listener::new(&path, false).unwrap();
+            let mut listener = Listener::new(&path, false).unwrap();
             barrier.wait();
-            daemon.start(listener).unwrap();
+            daemon.start(&mut listener).unwrap();
             barrier.wait();
             // Above process generates a `HandleRequest(PartialMessage)` error.
             daemon.wait().unwrap_err();

vhost-user-backend/tests/vhost-user-server.rs

@@ -260,9 +260,9 @@ fn vhost_user_server_with_fn<F: FnOnce(Arc<Mutex<MockVhostBackend>>, Arc<Barrier
     let path1 = path.clone();
     let thread = thread::spawn(move || cb(&path1, barrier2));
 
-    let listener = Listener::new(&path, false).unwrap();
+    let mut listener = Listener::new(&path, false).unwrap();
     barrier.wait();
-    daemon.start(listener).unwrap();
+    daemon.start(&mut listener).unwrap();
     barrier.wait();
 
     server_fn(backend, barrier);

vhost/CHANGELOG.md

@@ -7,6 +7,8 @@
   `From<UnixListener>` for `vhost_user::Listener`.
 
 ### Changed
+- [[#321](https://github.com/rust-vmm/vhost/pull/321)] Don't take ownership of listener in `BackendListener`.
+
 ### Deprecated
 ### Fixed
 - [[#304]](https://github.com/rust-vmm/vhost/pull/304) Fix building docs.

vhost/src/vhost_user/backend.rs

@@ -10,16 +10,16 @@ use super::message::*;
 use super::{BackendReqHandler, Result, VhostUserBackendReqHandler};
 
 /// Vhost-user backend side connection listener.
-pub struct BackendListener<S: VhostUserBackendReqHandler> {
-    listener: Listener,
+pub struct BackendListener<'a, S: VhostUserBackendReqHandler> {
+    listener: &'a mut Listener,
     backend: Option<Arc<S>>,
 }
 
 /// Sets up a listener for incoming frontend connections, and handles construction
 /// of a Backend on success.
-impl<S: VhostUserBackendReqHandler> BackendListener<S> {
+impl<'a, S: VhostUserBackendReqHandler> BackendListener<'a, S> {
     /// Create a unix domain socket for incoming frontend connections.
-    pub fn new(listener: Listener, backend: Arc<S>) -> Result<Self> {
+    pub fn new(listener: &'a mut Listener, backend: Arc<S>) -> Result<Self> {
         Ok(BackendListener {
             listener,
             backend: Some(backend),
@@ -55,9 +55,9 @@ mod tests {
     #[test]
     fn test_backend_listener_set_nonblocking() {
         let backend = Arc::new(Mutex::new(DummyBackendReqHandler::new()));
-        let listener =
+        let mut listener =
             Listener::new("/tmp/vhost_user_lib_unit_test_backend_nonblocking", true).unwrap();
-        let backend_listener = BackendListener::new(listener, backend).unwrap();
+        let backend_listener = BackendListener::new(&mut listener, backend).unwrap();
 
         backend_listener.set_nonblocking(true).unwrap();
         backend_listener.set_nonblocking(false).unwrap();
@@ -73,8 +73,8 @@ mod tests {
 
         let path = "/tmp/vhost_user_lib_unit_test_backend_accept";
         let backend = Arc::new(Mutex::new(DummyBackendReqHandler::new()));
-        let listener = Listener::new(path, true).unwrap();
-        let mut backend_listener = BackendListener::new(listener, backend).unwrap();
+        let mut listener = Listener::new(path, true).unwrap();
+        let mut backend_listener = BackendListener::new(&mut listener, backend).unwrap();
 
         backend_listener.set_nonblocking(true).unwrap();
         assert!(backend_listener.accept().unwrap().is_none());

vhost/src/vhost_user/mod.rs

@@ -293,8 +293,8 @@ mod tests {
         P: AsRef<Path>,
         S: VhostUserBackendReqHandler,
     {
-        let listener = Listener::new(&path, true).unwrap();
-        let mut backend_listener = BackendListener::new(listener, backend).unwrap();
+        let mut listener = Listener::new(&path, true).unwrap();
+        let mut backend_listener = BackendListener::new(&mut listener, backend).unwrap();
         let frontend = Frontend::connect(&path, 1).unwrap();
         (frontend, backend_listener.accept().unwrap().unwrap())
     }