add tests for QueueSync

bonzini · bonzini · commit 2e6808865e8c · 2021-10-26T17:14:25.000+02:00
For tests that do not use iter(), we can simply replace direct
access to the QueueState with lock() in order to retrieve the
guard.  However, "q.lock().a = q.lock().b" is illegal, because
it has two mutable borrows at the same time.

For tests that use iter(), the result of q.lock() must survive until
the test is done with the iterator.

Suggested-by: Laura Loghin &lt;lauralg@amazon.com&gt;
Signed-off-by: Paolo Bonzini &lt;pbonzini@redhat.com&gt;
diff --git a/crates/virtio-queue/src/lib.rs b/crates/virtio-queue/src/lib.rs
@@ -1211,12 +1211,10 @@ mod tests {
         }
     }
 
-    #[test]
-    fn test_queue_and_iterator() {
-        let m = &AddrSpace::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
+    fn do_test_queue_and_iterator<'a, Q: QueueT<&'a AddrSpace>>(m: &'a AddrSpace) {
         let vq = MockSplitQueue::new(m, 16);
 
-        let mut q: Queue<_> = vq.as_queue(m);
+        let mut q: Q = vq.as_queue(m);
 
         // q is currently valid
         assert!(q.is_valid());
@@ -1326,11 +1324,21 @@ mod tests {
     }
 
     #[test]
-    fn test_descriptor_and_iterator() {
+    fn test_queue_and_iterator() {
+        let m = &AddrSpace::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
+        do_test_queue_and_iterator::<Queue<&AddrSpace>>(m);
+    }
+
+    #[test]
+    fn test_queue_and_iterator_sync() {
         let m = &AddrSpace::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
+        do_test_queue_and_iterator::<QueueSync<&AddrSpace>>(m);
+    }
+
+    fn do_test_descriptor_and_iterator<'a, Q: QueueT<&'a AddrSpace>>(m: &'a AddrSpace) {
         let vq = MockSplitQueue::new(m, 16);
 
-        let mut q: Queue<_> = vq.as_queue(m);
+        let mut q: Q = vq.as_queue(m);
 
         // q is currently valid
         assert!(q.is_valid());
@@ -1390,11 +1398,21 @@ mod tests {
     }
 
     #[test]
-    fn test_add_used() {
+    fn test_descriptor_and_iterator() {
+        let m = &AddrSpace::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
+        do_test_descriptor_and_iterator::<Queue<&AddrSpace>>(m);
+    }
+
+    #[test]
+    fn test_descriptor_and_iterator_sync() {
         let m = &AddrSpace::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
+        do_test_descriptor_and_iterator::<QueueSync<&AddrSpace>>(m);
+    }
+
+    fn do_test_add_used<'a, Q: QueueT<&'a AddrSpace>>(m: &'a AddrSpace) {
         let vq = MockSplitQueue::new(m, 16);
 
-        let mut q: Queue<_> = vq.as_queue(m);
+        let mut q: Q = vq.as_queue(m);
 
         assert_eq!(vq.used().idx().load(), 0);
 
@@ -1412,6 +1430,18 @@ mod tests {
         assert_eq!(x.len, 0x1000);
     }
 
+    #[test]
+    fn test_add_used() {
+        let m = &AddrSpace::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
+        do_test_add_used::<Queue<&AddrSpace>>(m);
+    }
+
+    #[test]
+    fn test_add_used_sync() {
+        let m = &AddrSpace::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
+        do_test_add_used::<QueueSync<&AddrSpace>>(m);
+    }
+
     #[test]
     fn test_queue_guard() {
         let m = &GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
@@ -1427,11 +1457,31 @@ mod tests {
     }
 
     #[test]
-    fn test_reset_queue() {
-        let m = &AddrSpace::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
+    fn test_queue_guard_sync() {
+        let m = &GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
         let vq = MockSplitQueue::new(m, 16);
 
-        let mut q: Queue<_> = vq.as_queue(m);
+        // Note how the test_queue_guard above has "let mut" here.  A singlethreaded
+        // queue can only be accessed by one thread, and that one needs to have an
+        // exclusive reference; a multithreaded queue can be accessed by many thread
+        // and therefore it has to support interior mutability (via Mutex).  On the
+        // other hand, a multithreaded queue is reference counted and therefore it
+        // cannot be accessed via an exclusive reference *at all*.  This is the
+        // the reason why only Queue has the acquire() method, and only QueueSync
+        // has a lock() method.
+        let q: QueueSync<_> = vq.as_queue(m);
+        let mut qstate = q.lock();
+        qstate.ready = true;
+        qstate.reset();
+        assert_eq!(qstate.ready, false);
+        let mut iter = qstate.iter().unwrap();
+        assert!(iter.next().is_none());
+    }
+
+    fn do_test_reset_queue<'a, Q: QueueT<&'a AddrSpace>>(m: &'a AddrSpace) {
+        let vq = MockSplitQueue::new(m, 16);
+
+        let mut q: Q = vq.as_queue(m);
         q.with(|mut qstate| {
             qstate.size = 8;
             qstate.ready = true;
@@ -1442,12 +1492,22 @@ mod tests {
     }
 
     #[test]
-    fn test_needs_notification() {
+    fn test_reset_queue() {
+        let m = &AddrSpace::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
+        do_test_reset_queue::<Queue<&AddrSpace>>(m);
+    }
+
+    #[test]
+    fn test_reset_queue_sync() {
         let m = &AddrSpace::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
+        do_test_reset_queue::<QueueSync<&AddrSpace>>(m);
+    }
+
+    fn do_test_needs_notification<'a, Q: QueueT<&'a AddrSpace>>(m: &'a AddrSpace) {
         let qsize = 16;
         let vq = MockSplitQueue::new(m, qsize);
 
-        let mut q: Queue<_> = vq.as_queue(m);
+        let mut q: Q = vq.as_queue(m);
         let avail_addr = vq.avail_addr();
 
         // It should always return true when EVENT_IDX isn't enabled.
@@ -1491,11 +1551,21 @@ mod tests {
     }
 
     #[test]
-    fn test_enable_disable_notification() {
+    fn test_needs_notification() {
+        let m = &AddrSpace::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
+        do_test_needs_notification::<Queue<&AddrSpace>>(m);
+    }
+
+    #[test]
+    fn test_needs_notification_sync() {
         let m = &AddrSpace::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
+        do_test_needs_notification::<QueueSync<&AddrSpace>>(m);
+    }
+
+    fn do_test_enable_disable_notification<'a, Q: QueueT<&'a AddrSpace>>(m: &'a AddrSpace) {
         let vq = MockSplitQueue::new(m, 16);
 
-        let mut q: Queue<_> = vq.as_queue(m);
+        let mut q: Q = vq.as_queue(m);
         let used_addr = vq.used_addr();
 
         q.with(|qstate| assert_eq!(qstate.event_idx_enabled, false));
@@ -1526,4 +1596,16 @@ mod tests {
         q.with(|mut qstate| qstate.next_avail = Wrapping(8));
         assert_eq!(q.enable_notification().unwrap(), false);
     }
+
+    #[test]
+    fn test_enable_disable_notification() {
+        let m = &AddrSpace::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
+        do_test_enable_disable_notification::<Queue<&AddrSpace>>(m);
+    }
+
+    #[test]
+    fn test_enable_disable_notification_sync() {
+        let m = &AddrSpace::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
+        do_test_enable_disable_notification::<QueueSync<&AddrSpace>>(m);
+    }
 }
