vhost-user-backend: simplify the use of generics

In order to allow zero-cost exchanging of the concrete bitmap and vring
types, a lot of the generic code required using a tuple of
`<VringType, BitmapType>` for parameterizing the impl's. Once code is
also oblivious of the concrete backend (and a lot of code is), this
tuple turns into a triplet. Juggling these three single letter generic
parameters while making sure that all the type constraints (that are
also changing depending on the abstraction layer) does not feel very
ergonomic.

While one can argue that within this crate, this would be fine since
people probably know the internals, the design choice is leaking out
into every consumer of vhost-user-backend. Instead of just being able
to reference "some backend" one needs to copy a lot of boilerplate code
for also passing the other type parameters (again, while making sure
that all the constraints are met).

Instead, this commit changes things to utilize associated types [1].
This makes the Bitmap and Vring types part of the backend trait and
requires the implementations to spell them out. Code that just wants to
use the backend without needing to know the details can now just use
the trait without needing to specify the Bitmap and Vring types again.
Where needed, restricting Bitmap and Vring further is still possible
(though one no longer needs to copy all the existing restrictions and
can keep the code more maintainable by only listing new ones).

Overall, my main target was to improve the ergonomics of the consumers
of the crate. But I think the change also improves the readability and
maintainability within this crate. Combined, this hopefully justifies
the small code breakage in consumers.

No functional changes intended.
No change in type flexibility intended.

BREAKING CHANGE, consumers of the lib will need to adjust their code
(but it should improve the general readability).

Signed-off-by: Erik Schilling <[email protected]>

[1] https://doc.rust-lang.org/book/ch19-03-advanced-traits.html#specifying-placeholder-types-in-trait-definitions-with-associated-types

Author: Erik Schilling

crates/vhost-user-backend/CHANGELOG.md

@@ -7,6 +7,8 @@
 ### Changed
 - Change uses of master/slave for frontend/backend in the codebase.
 - [[#192]](https://github.com/rust-vmm/vhost/pull/192) vhost-user-backend: remove return value from handle_event
+- [[#155]](https://github.com/rust-vmm/vhost/pull/155) Converted generic type
+  parameters of VhostUserBackend into associated types.
 
 ### Fixed
 

crates/vhost-user-backend/src/backend.rs

@@ -34,11 +34,10 @@ use super::GM;
 /// Trait with interior mutability for vhost user backend servers to implement concrete services.
 ///
 /// To support multi-threading and asynchronous IO, we enforce `Send + Sync` bound.
-pub trait VhostUserBackend<V, B = ()>: Send + Sync
-where
-    V: VringT<GM<B>>,
-    B: Bitmap + 'static,
-{
+pub trait VhostUserBackend: Send + Sync {
+    type Bitmap: Bitmap + 'static;
+    type Vring: VringT<GM<Self::Bitmap>>;
+
     /// Get number of queues supported.
     fn num_queues(&self) -> usize;
 
@@ -74,7 +73,7 @@ where
     }
 
     /// Update guest memory regions.
-    fn update_memory(&self, mem: GM<B>) -> Result<()>;
+    fn update_memory(&self, mem: GM<Self::Bitmap>) -> Result<()>;
 
     /// Set handler for communicating with the frontend by the backend communication channel.
     ///
@@ -108,17 +107,16 @@ where
         &self,
         device_event: u16,
         evset: EventSet,
-        vrings: &[V],
+        vrings: &[Self::Vring],
         thread_id: usize,
     ) -> Result<()>;
 }
 
 /// Trait without interior mutability for vhost user backend servers to implement concrete services.
-pub trait VhostUserBackendMut<V, B = ()>: Send + Sync
-where
-    V: VringT<GM<B>>,
-    B: Bitmap + 'static,
-{
+pub trait VhostUserBackendMut: Send + Sync {
+    type Bitmap: Bitmap + 'static;
+    type Vring: VringT<GM<Self::Bitmap>>;
+
     /// Get number of queues supported.
     fn num_queues(&self) -> usize;
 
@@ -154,7 +152,7 @@ where
     }
 
     /// Update guest memory regions.
-    fn update_memory(&mut self, mem: GM<B>) -> Result<()>;
+    fn update_memory(&mut self, mem: GM<Self::Bitmap>) -> Result<()>;
 
     /// Set handler for communicating with the frontend by the backend communication channel.
     ///
@@ -189,16 +187,15 @@ where
         &mut self,
         device_event: u16,
         evset: EventSet,
-        vrings: &[V],
+        vrings: &[Self::Vring],
         thread_id: usize,
     ) -> Result<()>;
 }
 
-impl<T: VhostUserBackend<V, B>, V, B> VhostUserBackend<V, B> for Arc<T>
-where
-    V: VringT<GM<B>>,
-    B: Bitmap + 'static,
-{
+impl<T: VhostUserBackend> VhostUserBackend for Arc<T> {
+    type Bitmap = T::Bitmap;
+    type Vring = T::Vring;
+
     fn num_queues(&self) -> usize {
         self.deref().num_queues()
     }
@@ -231,7 +228,7 @@ where
         self.deref().set_config(offset, buf)
     }
 
-    fn update_memory(&self, mem: GM<B>) -> Result<()> {
+    fn update_memory(&self, mem: GM<Self::Bitmap>) -> Result<()> {
         self.deref().update_memory(mem)
     }
 
@@ -251,19 +248,18 @@ where
         &self,
         device_event: u16,
         evset: EventSet,
-        vrings: &[V],
+        vrings: &[Self::Vring],
         thread_id: usize,
     ) -> Result<()> {
         self.deref()
             .handle_event(device_event, evset, vrings, thread_id)
     }
 }
 
-impl<T: VhostUserBackendMut<V, B>, V, B> VhostUserBackend<V, B> for Mutex<T>
-where
-    V: VringT<GM<B>>,
-    B: Bitmap + 'static,
-{
+impl<T: VhostUserBackendMut> VhostUserBackend for Mutex<T> {
+    type Bitmap = T::Bitmap;
+    type Vring = T::Vring;
+
     fn num_queues(&self) -> usize {
         self.lock().unwrap().num_queues()
     }
@@ -296,7 +292,7 @@ where
         self.lock().unwrap().set_config(offset, buf)
     }
 
-    fn update_memory(&self, mem: GM<B>) -> Result<()> {
+    fn update_memory(&self, mem: GM<Self::Bitmap>) -> Result<()> {
         self.lock().unwrap().update_memory(mem)
     }
 
@@ -316,7 +312,7 @@ where
         &self,
         device_event: u16,
         evset: EventSet,
-        vrings: &[V],
+        vrings: &[Self::Vring],
         thread_id: usize,
     ) -> Result<()> {
         self.lock()
@@ -325,11 +321,10 @@ where
     }
 }
 
-impl<T: VhostUserBackendMut<V, B>, V, B> VhostUserBackend<V, B> for RwLock<T>
-where
-    V: VringT<GM<B>>,
-    B: Bitmap + 'static,
-{
+impl<T: VhostUserBackendMut> VhostUserBackend for RwLock<T> {
+    type Bitmap = T::Bitmap;
+    type Vring = T::Vring;
+
     fn num_queues(&self) -> usize {
         self.read().unwrap().num_queues()
     }
@@ -362,7 +357,7 @@ where
         self.write().unwrap().set_config(offset, buf)
     }
 
-    fn update_memory(&self, mem: GM<B>) -> Result<()> {
+    fn update_memory(&self, mem: GM<Self::Bitmap>) -> Result<()> {
         self.write().unwrap().update_memory(mem)
     }
 
@@ -382,7 +377,7 @@ where
         &self,
         device_event: u16,
         evset: EventSet,
-        vrings: &[V],
+        vrings: &[Self::Vring],
         thread_id: usize,
     ) -> Result<()> {
         self.write()
@@ -426,7 +421,10 @@ pub mod tests {
         }
     }
 
-    impl VhostUserBackendMut<VringRwLock, ()> for MockVhostBackend {
+    impl VhostUserBackendMut for MockVhostBackend {
+        type Bitmap = ();
+        type Vring = VringRwLock;
+
         fn num_queues(&self) -> usize {
             2
         }

crates/vhost-user-backend/src/event_loop.rs

@@ -8,13 +8,11 @@ use std::io::{self, Result};
 use std::marker::PhantomData;
 use std::os::unix::io::{AsRawFd, RawFd};
 
-use vm_memory::bitmap::Bitmap;
 use vmm_sys_util::epoll::{ControlOperation, Epoll, EpollEvent, EventSet};
 use vmm_sys_util::eventfd::EventFd;
 
 use super::backend::VhostUserBackend;
 use super::vring::VringT;
-use super::GM;
 
 /// Errors related to vring epoll event handling.
 #[derive(Debug)]
@@ -58,16 +56,16 @@ pub type VringEpollResult<T> = std::result::Result<T, VringEpollError>;
 /// - add file descriptors to be monitored by the epoll fd
 /// - remove registered file descriptors from the epoll fd
 /// - run the event loop to handle pending events on the epoll fd
-pub struct VringEpollHandler<S, V, B> {
+pub struct VringEpollHandler<T: VhostUserBackend> {
     epoll: Epoll,
-    backend: S,
-    vrings: Vec<V>,
+    backend: T,
+    vrings: Vec<T::Vring>,
     thread_id: usize,
     exit_event_fd: Option<EventFd>,
-    phantom: PhantomData<B>,
+    phantom: PhantomData<T::Bitmap>,
 }
 
-impl<S, V, B> VringEpollHandler<S, V, B> {
+impl<T: VhostUserBackend> VringEpollHandler<T> {
     /// Send `exit event` to break the event loop.
     pub fn send_exit_event(&self) {
         if let Some(eventfd) = self.exit_event_fd.as_ref() {
@@ -76,14 +74,16 @@ impl<S, V, B> VringEpollHandler<S, V, B> {
     }
 }
 
-impl<S, V, B> VringEpollHandler<S, V, B>
+impl<T> VringEpollHandler<T>
 where
-    S: VhostUserBackend<V, B>,
-    V: VringT<GM<B>>,
-    B: Bitmap + 'static,
+    T: VhostUserBackend,
 {
     /// Create a `VringEpollHandler` instance.
-    pub(crate) fn new(backend: S, vrings: Vec<V>, thread_id: usize) -> VringEpollResult<Self> {
+    pub(crate) fn new(
+        backend: T,
+        vrings: Vec<T::Vring>,
+        thread_id: usize,
+    ) -> VringEpollResult<Self> {
         let epoll = Epoll::new().map_err(VringEpollError::EpollCreateFd)?;
         let exit_event_fd = backend.exit_event(thread_id);
 
@@ -217,7 +217,7 @@ where
     }
 }
 
-impl<S, V, B> AsRawFd for VringEpollHandler<S, V, B> {
+impl<T: VhostUserBackend> AsRawFd for VringEpollHandler<T> {
     fn as_raw_fd(&self) -> RawFd {
         self.epoll.as_raw_fd()
     }

crates/vhost-user-backend/src/handler.rs

@@ -20,7 +20,6 @@ use vhost::vhost_user::{
 };
 use virtio_bindings::bindings::virtio_ring::VIRTIO_RING_F_EVENT_IDX;
 use virtio_queue::{Error as VirtQueError, QueueT};
-use vm_memory::bitmap::Bitmap;
 use vm_memory::mmap::NewBitmap;
 use vm_memory::{GuestAddress, GuestAddressSpace, GuestMemoryMmap, GuestRegionMmap};
 use vmm_sys_util::epoll::EventSet;
@@ -74,9 +73,9 @@ struct AddrMapping {
     gpa_base: u64,
 }
 
-pub struct VhostUserHandler<S, V, B: Bitmap + 'static> {
-    backend: S,
-    handlers: Vec<Arc<VringEpollHandler<S, V, B>>>,
+pub struct VhostUserHandler<T: VhostUserBackend> {
+    backend: T,
+    handlers: Vec<Arc<VringEpollHandler<T>>>,
     owned: bool,
     features_acked: bool,
     acked_features: u64,
@@ -85,26 +84,26 @@ pub struct VhostUserHandler<S, V, B: Bitmap + 'static> {
     max_queue_size: usize,
     queues_per_thread: Vec<u64>,
     mappings: Vec<AddrMapping>,
-    atomic_mem: GM<B>,
-    vrings: Vec<V>,
+    atomic_mem: GM<T::Bitmap>,
+    vrings: Vec<T::Vring>,
     worker_threads: Vec<thread::JoinHandle<VringEpollResult<()>>>,
 }
 
 // Ensure VhostUserHandler: Clone + Send + Sync + 'static.
-impl<S, V, B> VhostUserHandler<S, V, B>
+impl<T> VhostUserHandler<T>
 where
-    S: VhostUserBackend<V, B> + Clone + 'static,
-    V: VringT<GM<B>> + Clone + Send + Sync + 'static,
-    B: Bitmap + Clone + Send + Sync + 'static,
+    T: VhostUserBackend + Clone + 'static,
+    T::Vring: Clone + Send + Sync + 'static,
+    T::Bitmap: Clone + Send + Sync + 'static,
 {
-    pub(crate) fn new(backend: S, atomic_mem: GM<B>) -> VhostUserHandlerResult<Self> {
+    pub(crate) fn new(backend: T, atomic_mem: GM<T::Bitmap>) -> VhostUserHandlerResult<Self> {
         let num_queues = backend.num_queues();
         let max_queue_size = backend.max_queue_size();
         let queues_per_thread = backend.queues_per_thread();
 
         let mut vrings = Vec::new();
         for _ in 0..num_queues {
-            let vring = V::new(atomic_mem.clone(), max_queue_size as u16)
+            let vring = T::Vring::new(atomic_mem.clone(), max_queue_size as u16)
                 .map_err(VhostUserHandlerError::CreateVring)?;
             vrings.push(vring);
         }
@@ -151,7 +150,7 @@ where
     }
 }
 
-impl<S, V, B: Bitmap> VhostUserHandler<S, V, B> {
+impl<T: VhostUserBackend> VhostUserHandler<T> {
     pub(crate) fn send_exit_event(&self) {
         for handler in self.handlers.iter() {
             handler.send_exit_event();
@@ -169,25 +168,23 @@ impl<S, V, B: Bitmap> VhostUserHandler<S, V, B> {
     }
 }
 
-impl<S, V, B> VhostUserHandler<S, V, B>
+impl<T> VhostUserHandler<T>
 where
-    S: VhostUserBackend<V, B>,
-    V: VringT<GM<B>>,
-    B: Bitmap,
+    T: VhostUserBackend,
 {
-    pub(crate) fn get_epoll_handlers(&self) -> Vec<Arc<VringEpollHandler<S, V, B>>> {
+    pub(crate) fn get_epoll_handlers(&self) -> Vec<Arc<VringEpollHandler<T>>> {
         self.handlers.clone()
     }
 
-    fn vring_needs_init(&self, vring: &V) -> bool {
+    fn vring_needs_init(&self, vring: &T::Vring) -> bool {
         let vring_state = vring.get_ref();
 
         // If the vring wasn't initialized and we already have an EventFd for
         // VRING_KICK, initialize it now.
         !vring_state.get_queue().ready() && vring_state.get_kick().is_some()
     }
 
-    fn initialize_vring(&self, vring: &V, index: u8) -> VhostUserResult<()> {
+    fn initialize_vring(&self, vring: &T::Vring, index: u8) -> VhostUserResult<()> {
         assert!(vring.get_ref().get_kick().is_some());
 
         if let Some(fd) = vring.get_ref().get_kick() {
@@ -218,11 +215,9 @@ where
     }
 }
 
-impl<S, V, B> VhostUserBackendReqHandlerMut for VhostUserHandler<S, V, B>
+impl<T: VhostUserBackend> VhostUserBackendReqHandlerMut for VhostUserHandler<T>
 where
-    S: VhostUserBackend<V, B>,
-    V: VringT<GM<B>>,
-    B: NewBitmap + Clone,
+    T::Bitmap: NewBitmap + Clone,
 {
     fn set_owner(&mut self) -> VhostUserResult<()> {
         if self.owned {
@@ -604,7 +599,7 @@ where
     }
 }
 
-impl<S, V, B: Bitmap> Drop for VhostUserHandler<S, V, B> {
+impl<T: VhostUserBackend> Drop for VhostUserHandler<T> {
     fn drop(&mut self) {
         // Signal all working threads to exit.
         self.send_exit_event();