Refactor vring abstraction

coverage_config_x86_64.json

@@ -1,5 +1,5 @@
 {
-  "coverage_score": 77.4,
+  "coverage_score": 79.1,
   "exclude_path": "",
   "crate_features": ""
 }

src/backend.rs

@@ -28,13 +28,18 @@ use vhost::vhost_user::SlaveFsCacheReq;
 use vm_memory::bitmap::Bitmap;
 use vmm_sys_util::eventfd::EventFd;
 
-use super::{Vring, GM};
+use super::vring::VringT;
+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 `the Send + Sync + 'static`.
 /// So there's no plan for support of "Rc<T>" and "RefCell<T>".
-pub trait VhostUserBackend<B: Bitmap + 'static = ()>: Send + Sync + 'static {
+pub trait VhostUserBackend<V, B = ()>: Send + Sync + 'static
+where
+    V: VringT<GM<B>>,
+    B: Bitmap + 'static,
+{
     /// Get number of queues supported.
     fn num_queues(&self) -> usize;
 
@@ -94,7 +99,7 @@ pub trait VhostUserBackend<B: Bitmap + 'static = ()>: Send + Sync + 'static {
     /// If an (`EventFd`, `token`) pair is returned, the returned `EventFd` will be monitored for IO
     /// events by using epoll with the specified `token`. When the returned EventFd is written to,
     /// the worker thread will exit.
-    fn exit_event(&self, _thread_index: usize) -> Option<(EventFd, u16)> {
+    fn exit_event(&self, _thread_index: usize) -> Option<EventFd> {
         None
     }
 
@@ -107,13 +112,17 @@ pub trait VhostUserBackend<B: Bitmap + 'static = ()>: Send + Sync + 'static {
         &self,
         device_event: u16,
         evset: epoll::Events,
-        vrings: &[Vring<GM<B>>],
+        vrings: &[V],
         thread_id: usize,
     ) -> result::Result<bool, io::Error>;
 }
 
 /// Trait without interior mutability for vhost user backend servers to implement concrete services.
-pub trait VhostUserBackendMut<B: Bitmap + 'static = ()>: Send + Sync + 'static {
+pub trait VhostUserBackendMut<V, B = ()>: Send + Sync + 'static
+where
+    V: VringT<GM<B>>,
+    B: Bitmap + 'static,
+{
     /// Get number of queues supported.
     fn num_queues(&self) -> usize;
 
@@ -173,7 +182,7 @@ pub trait VhostUserBackendMut<B: Bitmap + 'static = ()>: Send + Sync + 'static {
     /// If an (`EventFd`, `token`) pair is returned, the returned `EventFd` will be monitored for IO
     /// events by using epoll with the specified `token`. When the returned EventFd is written to,
     /// the worker thread will exit.
-    fn exit_event(&self, _thread_index: usize) -> Option<(EventFd, u16)> {
+    fn exit_event(&self, _thread_index: usize) -> Option<EventFd> {
         None
     }
 
@@ -186,12 +195,16 @@ pub trait VhostUserBackendMut<B: Bitmap + 'static = ()>: Send + Sync + 'static {
         &mut self,
         device_event: u16,
         evset: epoll::Events,
-        vrings: &[Vring<GM<B>>],
+        vrings: &[V],
         thread_id: usize,
     ) -> result::Result<bool, io::Error>;
 }
 
-impl<T: VhostUserBackend<B>, B: Bitmap + 'static> VhostUserBackend<B> for Arc<T> {
+impl<T: VhostUserBackend<V, B>, V, B> VhostUserBackend<V, B> for Arc<T>
+where
+    V: VringT<GM<B>>,
+    B: Bitmap + 'static,
+{
     fn num_queues(&self) -> usize {
         self.deref().num_queues()
     }
@@ -236,23 +249,27 @@ impl<T: VhostUserBackend<B>, B: Bitmap + 'static> VhostUserBackend<B> for Arc<T>
         self.deref().queues_per_thread()
     }
 
-    fn exit_event(&self, thread_index: usize) -> Option<(EventFd, u16)> {
+    fn exit_event(&self, thread_index: usize) -> Option<EventFd> {
         self.deref().exit_event(thread_index)
     }
 
     fn handle_event(
         &self,
         device_event: u16,
         evset: epoll::Events,
-        vrings: &[Vring<GM<B>>],
+        vrings: &[V],
         thread_id: usize,
     ) -> Result<bool, io::Error> {
         self.deref()
             .handle_event(device_event, evset, vrings, thread_id)
     }
 }
 
-impl<T: VhostUserBackendMut<B>, B: Bitmap + 'static> VhostUserBackend<B> for Mutex<T> {
+impl<T: VhostUserBackendMut<V, B>, V, B> VhostUserBackend<V, B> for Mutex<T>
+where
+    V: VringT<GM<B>>,
+    B: Bitmap + 'static,
+{
     fn num_queues(&self) -> usize {
         self.lock().unwrap().num_queues()
     }
@@ -297,15 +314,15 @@ impl<T: VhostUserBackendMut<B>, B: Bitmap + 'static> VhostUserBackend<B> for Mut
         self.lock().unwrap().queues_per_thread()
     }
 
-    fn exit_event(&self, thread_index: usize) -> Option<(EventFd, u16)> {
+    fn exit_event(&self, thread_index: usize) -> Option<EventFd> {
         self.lock().unwrap().exit_event(thread_index)
     }
 
     fn handle_event(
         &self,
         device_event: u16,
         evset: epoll::Events,
-        vrings: &[Vring<GM<B>>],
+        vrings: &[V],
         thread_id: usize,
     ) -> Result<bool, io::Error> {
         self.lock()
@@ -314,7 +331,11 @@ impl<T: VhostUserBackendMut<B>, B: Bitmap + 'static> VhostUserBackend<B> for Mut
     }
 }
 
-impl<T: VhostUserBackendMut<B>, B: Bitmap + 'static> VhostUserBackend<B> for RwLock<T> {
+impl<T: VhostUserBackendMut<V, B>, V, B> VhostUserBackend<V, B> for RwLock<T>
+where
+    V: VringT<GM<B>>,
+    B: Bitmap + 'static,
+{
     fn num_queues(&self) -> usize {
         self.read().unwrap().num_queues()
     }
@@ -359,15 +380,15 @@ impl<T: VhostUserBackendMut<B>, B: Bitmap + 'static> VhostUserBackend<B> for RwL
         self.read().unwrap().queues_per_thread()
     }
 
-    fn exit_event(&self, thread_index: usize) -> Option<(EventFd, u16)> {
+    fn exit_event(&self, thread_index: usize) -> Option<EventFd> {
         self.read().unwrap().exit_event(thread_index)
     }
 
     fn handle_event(
         &self,
         device_event: u16,
         evset: epoll::Events,
-        vrings: &[Vring<GM<B>>],
+        vrings: &[V],
         thread_id: usize,
     ) -> Result<bool, io::Error> {
         self.write()
@@ -379,6 +400,7 @@ impl<T: VhostUserBackendMut<B>, B: Bitmap + 'static> VhostUserBackend<B> for RwL
 #[cfg(test)]
 pub mod tests {
     use super::*;
+    use crate::VringRwLock;
     use epoll::Events;
     use std::io::Error;
     use std::sync::Mutex;
@@ -400,7 +422,7 @@ pub mod tests {
         }
     }
 
-    impl VhostUserBackendMut<()> for MockVhostBackend {
+    impl VhostUserBackendMut<VringRwLock, ()> for MockVhostBackend {
         fn num_queues(&self) -> usize {
             2
         }
@@ -453,17 +475,17 @@ pub mod tests {
             vec![1, 1]
         }
 
-        fn exit_event(&self, _thread_index: usize) -> Option<(EventFd, u16)> {
+        fn exit_event(&self, _thread_index: usize) -> Option<EventFd> {
             let event_fd = EventFd::new(0).unwrap();
 
-            Some((event_fd, 0x100))
+            Some(event_fd)
         }
 
         fn handle_event(
             &mut self,
             _device_event: u16,
             _evset: Events,
-            _vrings: &[Vring],
+            _vrings: &[VringRwLock],
             _thread_id: usize,
         ) -> Result<bool, Error> {
             self.events += 1;

src/event_loop.rs

@@ -6,13 +6,16 @@
 use std::fmt::{Display, Formatter};
 use std::fs::File;
 use std::io;
+use std::marker::PhantomData;
 use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
 use std::result;
 
 use vm_memory::bitmap::Bitmap;
 use vmm_sys_util::eventfd::EventFd;
 
-use super::{VhostUserBackend, Vring, GM};
+use super::backend::VhostUserBackend;
+use super::vring::VringT;
+use super::GM;
 
 /// Errors related to vring epoll event handling.
 #[derive(Debug)]
@@ -56,32 +59,39 @@ 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: VhostUserBackend<B>, B: Bitmap + 'static> {
+pub struct VringEpollHandler<S, V, B>
+where
+    S: VhostUserBackend<V, B>,
+    V: VringT<GM<B>>,
+    B: Bitmap + 'static,
+{
     epoll_file: File,
     backend: S,
-    vrings: Vec<Vring<GM<B>>>,
+    vrings: Vec<V>,
     thread_id: usize,
     exit_event_fd: Option<EventFd>,
-    exit_event_id: Option<u16>,
+    phantom: PhantomData<B>,
 }
 
-impl<S: VhostUserBackend<B>, B: Bitmap + 'static> VringEpollHandler<S, B> {
+impl<S, V, B> VringEpollHandler<S, V, B>
+where
+    S: VhostUserBackend<V, B>,
+    V: VringT<GM<B>>,
+    B: Bitmap + 'static,
+{
     /// Create a `VringEpollHandler` instance.
-    pub(crate) fn new(
-        backend: S,
-        vrings: Vec<Vring<GM<B>>>,
-        thread_id: usize,
-    ) -> VringEpollResult<Self> {
+    pub(crate) fn new(backend: S, vrings: Vec<V>, thread_id: usize) -> VringEpollResult<Self> {
         let epoll_fd = epoll::create(true).map_err(VringEpollError::EpollCreateFd)?;
         let epoll_file = unsafe { File::from_raw_fd(epoll_fd) };
 
         let handler = match backend.exit_event(thread_id) {
-            Some((exit_event_fd, exit_event_id)) => {
+            Some(exit_event_fd) => {
+                let id = backend.num_queues();
                 epoll::ctl(
                     epoll_file.as_raw_fd(),
                     epoll::ControlOptions::EPOLL_CTL_ADD,
                     exit_event_fd.as_raw_fd(),
-                    epoll::Event::new(epoll::Events::EPOLLIN, u64::from(exit_event_id)),
+                    epoll::Event::new(epoll::Events::EPOLLIN, id as u64),
                 )
                 .map_err(VringEpollError::RegisterExitEvent)?;
 
@@ -91,7 +101,7 @@ impl<S: VhostUserBackend<B>, B: Bitmap + 'static> VringEpollHandler<S, B> {
                     vrings,
                     thread_id,
                     exit_event_fd: Some(exit_event_fd),
-                    exit_event_id: Some(exit_event_id),
+                    phantom: PhantomData,
                 }
             }
             None => VringEpollHandler {
@@ -100,7 +110,7 @@ impl<S: VhostUserBackend<B>, B: Bitmap + 'static> VringEpollHandler<S, B> {
                 vrings,
                 thread_id,
                 exit_event_fd: None,
-                exit_event_id: None,
+                phantom: PhantomData,
             },
         };
 
@@ -123,6 +133,38 @@ impl<S: VhostUserBackend<B>, B: Bitmap + 'static> VringEpollHandler<S, B> {
         fd: RawFd,
         ev_type: epoll::Events,
         data: u64,
+    ) -> result::Result<(), io::Error> {
+        // `data` range [0...num_queues] is reserved for queues and exit event.
+        if data <= self.backend.num_queues() as u64 {
+            Err(io::Error::from_raw_os_error(libc::EINVAL))
+        } else {
+            self.register_event(fd, ev_type, data)
+        }
+    }
+
+    /// Unregister an event from the epoll fd.
+    ///
+    /// If the event is triggered after this function has been called, the event will be silently
+    /// dropped.
+    pub fn unregister_listener(
+        &self,
+        fd: RawFd,
+        ev_type: epoll::Events,
+        data: u64,
+    ) -> result::Result<(), io::Error> {
+        // `data` range [0...num_queues] is reserved for queues and exit event.
+        if data <= self.backend.num_queues() as u64 {
+            Err(io::Error::from_raw_os_error(libc::EINVAL))
+        } else {
+            self.unregister_event(fd, ev_type, data)
+        }
+    }
+
+    pub(crate) fn register_event(
+        &self,
+        fd: RawFd,
+        ev_type: epoll::Events,
+        data: u64,
     ) -> result::Result<(), io::Error> {
         epoll::ctl(
             self.epoll_file.as_raw_fd(),
@@ -132,11 +174,7 @@ impl<S: VhostUserBackend<B>, B: Bitmap + 'static> VringEpollHandler<S, B> {
         )
     }
 
-    /// Unregister an event from the epoll fd.
-    ///
-    /// If the event is triggered after this function has been called, the event will be silently
-    /// dropped.
-    pub fn unregister_listener(
+    pub(crate) fn unregister_event(
         &self,
         fd: RawFd,
         ev_type: epoll::Events,
@@ -199,7 +237,7 @@ impl<S: VhostUserBackend<B>, B: Bitmap + 'static> VringEpollHandler<S, B> {
     }
 
     fn handle_event(&self, device_event: u16, evset: epoll::Events) -> VringEpollResult<bool> {
-        if self.exit_event_id == Some(device_event) {
+        if self.exit_event_fd.is_some() && device_event as usize == self.backend.num_queues() {
             return Ok(true);
         }
 
@@ -224,6 +262,7 @@ impl<S: VhostUserBackend<B>, B: Bitmap + 'static> VringEpollHandler<S, B> {
 #[cfg(test)]
 mod tests {
     use super::super::backend::tests::MockVhostBackend;
+    use super::super::vring::VringRwLock;
     use super::*;
     use std::sync::{Arc, Mutex};
     use vm_memory::{GuestAddress, GuestMemoryAtomic, GuestMemoryMmap};
@@ -234,25 +273,32 @@ mod tests {
         let mem = GuestMemoryAtomic::new(
             GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0x100000), 0x10000)]).unwrap(),
         );
-        let vring = Vring::new(mem, 0x1000);
+        let vring = VringRwLock::new(mem, 0x1000);
         let backend = Arc::new(Mutex::new(MockVhostBackend::new()));
 
         let handler = VringEpollHandler::new(backend, vec![vring], 0x1).unwrap();
-        assert!(handler.exit_event_id.is_some());
 
         let eventfd = EventFd::new(0).unwrap();
         handler
-            .register_listener(eventfd.as_raw_fd(), epoll::Events::EPOLLIN, 1)
+            .register_listener(eventfd.as_raw_fd(), epoll::Events::EPOLLIN, 3)
             .unwrap();
         // Register an already registered fd.
+        handler
+            .register_listener(eventfd.as_raw_fd(), epoll::Events::EPOLLIN, 3)
+            .unwrap_err();
+        // Register an invalid data.
         handler
             .register_listener(eventfd.as_raw_fd(), epoll::Events::EPOLLIN, 1)
             .unwrap_err();
 
         handler
-            .unregister_listener(eventfd.as_raw_fd(), epoll::Events::EPOLLIN, 1)
+            .unregister_listener(eventfd.as_raw_fd(), epoll::Events::EPOLLIN, 3)
             .unwrap();
         // unregister an already unregistered fd.
+        handler
+            .unregister_listener(eventfd.as_raw_fd(), epoll::Events::EPOLLIN, 3)
+            .unwrap_err();
+        // unregister an invalid data.
         handler
             .unregister_listener(eventfd.as_raw_fd(), epoll::Events::EPOLLIN, 1)
             .unwrap_err();