Merge pull request #90 from vireshk/i2c/testing

I2c/testing

coverage_config_x86_64.json

@@ -1,5 +1,5 @@
 {
-  "coverage_score": 73.9,
+  "coverage_score": 86.5,
   "exclude_path": "",
   "crate_features": ""
 }

i2c/src/i2c.rs

@@ -760,6 +760,14 @@ pub mod tests {
         verify_rdwr_data(&reqs);
     }
 
+    #[test]
+    fn test_verify_smbus_data() {
+        let data = I2cSmbusData { word: 0x050A };
+
+        assert_eq!(data.read_byte(), 0x0A);
+        assert_eq!(data.read_word(), 0x050A);
+    }
+
     #[test]
     fn test_smbus_transfer() {
         let adapter_config = AdapterConfig::try_from("1:3").unwrap();
@@ -836,6 +844,16 @@ pub mod tests {
         i2c_map.transfer(&mut reqs).unwrap();
         assert_eq!(reqs[1].buf[0], 1);
 
+        // I2C_SMBUS_WRITE (I2C_SMBUS_WORD_DATA) operation
+        let mut reqs: Vec<I2cReq> = vec![I2cReq {
+            addr: 0x3,
+            flags: 0,
+            len: 3,
+            buf: [7, 4, 3].to_vec(),
+        }];
+
+        i2c_map.transfer(&mut reqs).unwrap();
+
         // I2C_SMBUS_READ (I2C_SMBUS_WORD_DATA) operation
         let mut reqs = vec![
             I2cReq {
@@ -909,6 +927,19 @@ pub mod tests {
             Error::MessageLengthInvalid("write", 4)
         );
 
+        // I2C_SMBUS_WRITE (I2C_SMBUS_WORD_DATA) failure operation
+        let mut reqs: Vec<I2cReq> = vec![I2cReq {
+            addr: 0x3,
+            // Will cause failure
+            flags: I2C_M_RD,
+            len: 3,
+            buf: [7, 4, 3].to_vec(),
+        }];
+        assert_eq!(
+            i2c_map.transfer(&mut reqs).unwrap_err(),
+            Error::MessageLengthInvalid("read", 3)
+        );
+
         // I2C_SMBUS_READ (I2C_SMBUS_WORD_DATA) failure operation
         let mut reqs: Vec<I2cReq> = vec![
             I2cReq {

i2c/src/main.rs

@@ -171,7 +171,9 @@ impl TryFrom<ArgMatches> for I2cConfiguration {
     }
 }
 
-fn start_backend<D: 'static + I2cDevice + Send + Sync>(config: I2cConfiguration) -> Result<()> {
+fn start_backend<D: 'static + I2cDevice + Send + Sync>(cmd_args: ArgMatches) -> Result<()> {
+    let config = I2cConfiguration::try_from(cmd_args).unwrap();
+
     // The same i2c_map structure instance is shared between all the guests
     let i2c_map = Arc::new(I2cMap::<D>::new(&config.devices).map_err(Error::I2cFailure)?);
 
@@ -238,13 +240,13 @@ fn main() -> Result<()> {
     let yaml = load_yaml!("cli.yaml");
     let cmd_args = App::from(yaml).get_matches();
 
-    let config = I2cConfiguration::try_from(cmd_args).unwrap();
-    start_backend::<PhysDevice>(config)
+    start_backend::<PhysDevice>(cmd_args)
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::i2c::tests::DummyDevice;
 
     impl DeviceConfig {
         pub fn new_with(adapter_no: u32, addr: Vec<u16>) -> Self {
@@ -273,6 +275,25 @@ mod tests {
         app.try_get_matches_from(args).unwrap()
     }
 
+    #[test]
+    fn test_device_config() {
+        let mut config = DeviceConfig::new(5);
+        let invalid_addr = (MAX_I2C_VDEV + 1) as u16;
+
+        config.push(5).unwrap();
+        config.push(6).unwrap();
+
+        assert_eq!(
+            config.push(invalid_addr).unwrap_err(),
+            Error::ClientAddressInvalid(invalid_addr)
+        );
+
+        assert_eq!(
+            config.push(5).unwrap_err(),
+            Error::ClientAddressDuplicate(5)
+        );
+    }
+
     #[test]
     fn test_parse_failure() {
         let socket_name = Some("vi2c.sock");
@@ -362,4 +383,16 @@ mod tests {
             Error::AdapterDuplicate(1)
         );
     }
+
+    #[test]
+    fn test_fail_listener() {
+        // This will fail the listeners and thread will panic.
+        let socket_name = Some("~/path/not/present/i2c");
+        let cmd_args = get_cmd_args(socket_name, "1:4,3:5", Some("5"));
+
+        assert_eq!(
+            start_backend::<DummyDevice>(cmd_args).unwrap_err(),
+            Error::FailedJoiningThreads
+        );
+    }
 }

i2c/src/vhu_i2c.rs

@@ -114,7 +114,7 @@ impl<D: I2cDevice> VhostUserI2cBackend<D> {
     fn process_requests(
         &self,
         requests: Vec<I2cDescriptorChain>,
-        vring: Option<&VringRwLock>,
+        vring: &VringRwLock,
     ) -> Result<bool> {
         let mut reqs: Vec<I2cReq> = Vec::new();
 
@@ -239,10 +239,8 @@ impl<D: I2cDevice> VhostUserI2cBackend<D> {
                 .write_obj::<VirtioI2cInHdr>(in_hdr, desc_in_hdr.addr())
                 .map_err(|_| Error::DescriptorWriteFailed)?;
 
-            if let Some(vring) = vring {
-                if vring.add_used(desc_chain.head_index(), len).is_err() {
-                    warn!("Couldn't return used descriptors to the ring");
-                }
+            if vring.add_used(desc_chain.head_index(), len).is_err() {
+                warn!("Couldn't return used descriptors to the ring");
             }
         }
 
@@ -258,7 +256,7 @@ impl<D: I2cDevice> VhostUserI2cBackend<D> {
             .map_err(|_| Error::DescriptorNotFound)?
             .collect();
 
-        if self.process_requests(requests, Some(vring))? {
+        if self.process_requests(requests, vring)? {
             // Send notification once all the requests are processed
             vring
                 .signal_used_queue()
@@ -518,10 +516,14 @@ mod tests {
         let device_config = AdapterConfig::try_from("1:4,2:32:21,5:10:23").unwrap();
         let i2c_map = I2cMap::<DummyDevice>::new(&device_config).unwrap();
         let backend = VhostUserI2cBackend::new(Arc::new(i2c_map)).unwrap();
+        let mem = GuestMemoryAtomic::new(
+            GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0), 0x1000)]).unwrap(),
+        );
+        let vring = VringRwLock::new(mem, 0x1000);
 
         // Descriptor chain size zero, shouldn't fail
         backend
-            .process_requests(Vec::<I2cDescriptorChain>::new(), None)
+            .process_requests(Vec::<I2cDescriptorChain>::new(), &vring)
             .unwrap();
 
         // Valid single read descriptor
@@ -529,7 +531,9 @@ mod tests {
         let desc_chain = prepare_desc_chain(GuestAddress(0), &mut buf, VIRTIO_I2C_FLAGS_M_RD, 4);
         let desc_chains = vec![desc_chain];
 
-        backend.process_requests(desc_chains.clone(), None).unwrap();
+        backend
+            .process_requests(desc_chains.clone(), &vring)
+            .unwrap();
         validate_desc_chains(desc_chains, VIRTIO_I2C_MSG_OK);
 
         // Valid single write descriptor
@@ -537,7 +541,9 @@ mod tests {
         let desc_chain = prepare_desc_chain(GuestAddress(0), &mut buf, 0, 4);
         let desc_chains = vec![desc_chain];
 
-        backend.process_requests(desc_chains.clone(), None).unwrap();
+        backend
+            .process_requests(desc_chains.clone(), &vring)
+            .unwrap();
         validate_desc_chains(desc_chains, VIRTIO_I2C_MSG_OK);
 
         // Valid mixed read-write descriptors
@@ -557,7 +563,9 @@ mod tests {
             prepare_desc_chain(GuestAddress(0), &mut buf[5], VIRTIO_I2C_FLAGS_M_RD, 4),
         ];
 
-        backend.process_requests(desc_chains.clone(), None).unwrap();
+        backend
+            .process_requests(desc_chains.clone(), &vring)
+            .unwrap();
         validate_desc_chains(desc_chains, VIRTIO_I2C_MSG_OK);
     }
 
@@ -566,6 +574,10 @@ mod tests {
         let device_config = AdapterConfig::try_from("1:4,2:32:21,5:10:23").unwrap();
         let i2c_map = I2cMap::<DummyDevice>::new(&device_config).unwrap();
         let backend = VhostUserI2cBackend::new(Arc::new(i2c_map)).unwrap();
+        let mem = GuestMemoryAtomic::new(
+            GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0), 0x1000)]).unwrap(),
+        );
+        let vring = VringRwLock::new(mem, 0x1000);
 
         // One descriptors
         let flags: Vec<u16> = vec![0];
@@ -573,7 +585,7 @@ mod tests {
         let desc_chain = prepare_desc_chain_dummy(None, flags, len);
         assert_eq!(
             backend
-                .process_requests(vec![desc_chain], None)
+                .process_requests(vec![desc_chain], &vring)
                 .unwrap_err(),
             Error::UnexpectedDescriptorCount(1)
         );
@@ -584,7 +596,7 @@ mod tests {
         let desc_chain = prepare_desc_chain_dummy(None, flags, len);
         assert_eq!(
             backend
-                .process_requests(vec![desc_chain], None)
+                .process_requests(vec![desc_chain], &vring)
                 .unwrap_err(),
             Error::UnexpectedDescriptorCount(4)
         );
@@ -599,7 +611,7 @@ mod tests {
         let desc_chain = prepare_desc_chain_dummy(None, flags, len);
         assert_eq!(
             backend
-                .process_requests(vec![desc_chain], None)
+                .process_requests(vec![desc_chain], &vring)
                 .unwrap_err(),
             Error::UnexpectedWriteOnlyDescriptor(0)
         );
@@ -610,7 +622,7 @@ mod tests {
         let desc_chain = prepare_desc_chain_dummy(None, flags, len);
         assert_eq!(
             backend
-                .process_requests(vec![desc_chain], None)
+                .process_requests(vec![desc_chain], &vring)
                 .unwrap_err(),
             Error::UnexpectedDescriptorSize(size_of::<VirtioI2cOutHdr>(), 100)
         );
@@ -626,7 +638,7 @@ mod tests {
         let desc_chain = prepare_desc_chain_dummy(Some(addr), flags, len);
         assert_eq!(
             backend
-                .process_requests(vec![desc_chain], None)
+                .process_requests(vec![desc_chain], &vring)
                 .unwrap_err(),
             Error::DescriptorReadFailed
         );
@@ -641,7 +653,7 @@ mod tests {
         let desc_chain = prepare_desc_chain_dummy(None, flags, len);
         assert_eq!(
             backend
-                .process_requests(vec![desc_chain], None)
+                .process_requests(vec![desc_chain], &vring)
                 .unwrap_err(),
             Error::UnexpectedReadableDescriptor(2)
         );
@@ -652,7 +664,7 @@ mod tests {
         let desc_chain = prepare_desc_chain_dummy(None, flags, len);
         assert_eq!(
             backend
-                .process_requests(vec![desc_chain], None)
+                .process_requests(vec![desc_chain], &vring)
                 .unwrap_err(),
             Error::UnexpectedDescriptorSize(size_of::<u8>(), 100)
         );
@@ -668,7 +680,7 @@ mod tests {
         let desc_chain = prepare_desc_chain_dummy(Some(addr), flags, len);
         assert_eq!(
             backend
-                .process_requests(vec![desc_chain], None)
+                .process_requests(vec![desc_chain], &vring)
                 .unwrap_err(),
             Error::DescriptorWriteFailed
         );
@@ -683,7 +695,7 @@ mod tests {
         let desc_chain = prepare_desc_chain_dummy(None, flags, len);
         assert_eq!(
             backend
-                .process_requests(vec![desc_chain], None)
+                .process_requests(vec![desc_chain], &vring)
                 .unwrap_err(),
             Error::UnexpectedDescriptorSize(1, 0)
         );
@@ -699,17 +711,34 @@ mod tests {
         let desc_chain = prepare_desc_chain_dummy(Some(addr), flags, len);
         assert_eq!(
             backend
-                .process_requests(vec![desc_chain], None)
+                .process_requests(vec![desc_chain], &vring)
                 .unwrap_err(),
             Error::DescriptorReadFailed
         );
 
+        // Write only buf for write operation
+        let flags: Vec<u16> = vec![0, VIRTQ_DESC_F_WRITE, VIRTQ_DESC_F_WRITE];
+        let len: Vec<u32> = vec![
+            size_of::<VirtioI2cOutHdr>() as u32,
+            10,
+            size_of::<u8>() as u32,
+        ];
+        let desc_chain = prepare_desc_chain_dummy(None, flags, len);
+        assert_eq!(
+            backend
+                .process_requests(vec![desc_chain], &vring)
+                .unwrap_err(),
+            Error::UnexpectedWriteOnlyDescriptor(1)
+        );
+
         // Missing buffer for I2C rdwr transfer
         let mut buf = Vec::<u8>::new();
         let desc_chain = prepare_desc_chain(GuestAddress(0), &mut buf, VIRTIO_I2C_FLAGS_M_RD, 4);
         let desc_chains = vec![desc_chain];
 
-        backend.process_requests(desc_chains.clone(), None).unwrap();
+        backend
+            .process_requests(desc_chains.clone(), &vring)
+            .unwrap();
         validate_desc_chains(desc_chains, VIRTIO_I2C_MSG_ERR);
     }
 
@@ -729,5 +758,39 @@ mod tests {
 
         backend.set_event_idx(true);
         assert!(backend.event_idx);
+
+        assert!(backend.exit_event(0).is_some());
+
+        let mem = GuestMemoryAtomic::new(
+            GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0), 0x1000)]).unwrap(),
+        );
+        backend.update_memory(mem.clone()).unwrap();
+
+        let vring = VringRwLock::new(mem, 0x1000);
+        assert_eq!(
+            backend
+                .handle_event(0, EventSet::OUT, &[vring.clone()], 0)
+                .unwrap_err()
+                .kind(),
+            io::ErrorKind::Other
+        );
+
+        assert_eq!(
+            backend
+                .handle_event(1, EventSet::IN, &[vring.clone()], 0)
+                .unwrap_err()
+                .kind(),
+            io::ErrorKind::Other
+        );
+
+        // Hit the loop part
+        backend.set_event_idx(true);
+        backend
+            .handle_event(0, EventSet::IN, &[vring.clone()], 0)
+            .unwrap();
+
+        // Hit the non-loop part
+        backend.set_event_idx(false);
+        backend.handle_event(0, EventSet::IN, &[vring], 0).unwrap();
     }
 }