From aab3a25816317db6b077c8c4028688ec80a2b5fa Mon Sep 17 00:00:00 2001 From: Andreea Florescu Date: Wed, 22 Sep 2021 22:21:25 +0300 Subject: [PATCH] [i2c] revert to a simplified main program Remove the dry run related functionality. The main function now just having boilerplate code that we should test via integration tests. We can test the parsing by validating the `TryFrom` implementation. Everything else is not possible to test with unit tests because they need I/O (such as working with sockets), and thus are more appropriate for integration tests. Added more tests for validating the expected behavior of parsing. Signed-off-by: Andreea Florescu --- src/i2c/src/i2c.rs | 14 +++++ src/i2c/src/main.rs | 137 ++++++++++++++++++++------------------------ 2 files changed, 77 insertions(+), 74 deletions(-) diff --git a/src/i2c/src/i2c.rs b/src/i2c/src/i2c.rs index 2190808..de0dbd6 100644 --- a/src/i2c/src/i2c.rs +++ b/src/i2c/src/i2c.rs @@ -432,6 +432,7 @@ pub struct I2cMap { device_map: [u32; MAX_I2C_VDEV], } +#[derive(Debug, PartialEq)] pub(crate) struct DeviceConfig { adapter_no: u32, addr: Vec, @@ -459,6 +460,7 @@ impl DeviceConfig { } } +#[derive(Debug, PartialEq)] pub(crate) struct AdapterConfig { inner: Vec, } @@ -548,6 +550,18 @@ pub mod tests { use super::*; use std::convert::TryFrom; + impl DeviceConfig { + pub fn new_with(adapter_no: u32, addr: Vec) -> Self { + DeviceConfig { adapter_no, addr } + } + } + + impl AdapterConfig { + pub fn new_with(devices: Vec) -> Self { + AdapterConfig { inner: devices } + } + } + #[derive(Debug, Default)] pub struct DummyDevice { funcs_result: i32, diff --git a/src/i2c/src/main.rs b/src/i2c/src/main.rs index efa004a..0f1079a 100644 --- a/src/i2c/src/main.rs +++ b/src/i2c/src/main.rs @@ -21,6 +21,7 @@ use crate::i2c::DeviceConfig; use i2c::{AdapterConfig, I2cDevice, I2cMap, PhysDevice}; use vhu_i2c::VhostUserI2cBackend; +#[derive(PartialEq, Debug)] struct I2cConfiguration { socket_path: String, socket_count: usize, @@ -77,58 +78,19 @@ impl TryFrom for I2cConfiguration { } } -fn start_daemon( - backend: Arc>>, - listener: Listener, -) -> bool { - let mut daemon = VhostUserDaemon::new( - String::from("vhost-device-i2c-backend"), - backend.clone(), - GuestMemoryAtomic::new(GuestMemoryMmap::new()), - ) - .unwrap(); - - daemon.start(listener).unwrap(); - - match daemon.wait() { - Ok(()) => { - println!("Stopping cleanly."); - } - Err(vhost_user_backend::Error::HandleRequest(vhost_user::Error::PartialMessage)) => { - println!("vhost-user connection closed with partial message. If the VM is shutting down, this is expected behavior; otherwise, it might be a bug."); - } - Err(e) => { - println!("Error running daemon: {:?}", e); - } - } - - // No matter the result, we need to shut down the worker thread. - backend - .read() - .unwrap() - .exit_event - .write(1) - .expect("Shutting down worker thread"); - - false -} - -fn start_backend( - cmd_args: ArgMatches, - start_daemon: fn(Arc>>, Listener) -> bool, +fn start_backend( + config: I2cConfiguration, ) -> Result<(), String> { - let mut handles = Vec::new(); - - let i2c_config = I2cConfiguration::try_from(cmd_args)?; - // The same i2c_map structure instance is shared between all the guests let i2c_map = Arc::new( - I2cMap::::new(&i2c_config.devices) + I2cMap::::new(&config.devices) .map_err(|e| format!("Failed to create i2c_map ({})", e))?, ); - for i in 0..i2c_config.socket_count { - let socket = i2c_config.socket_path.to_owned() + &i.to_string(); + let mut handles = Vec::new(); + + for i in 0..config.socket_count { + let socket = config.socket_path.to_owned() + &i.to_string(); let i2c_map = i2c_map.clone(); let handle = spawn(move || loop { @@ -137,9 +99,36 @@ fn start_backend( )); let listener = Listener::new(socket.clone(), true).unwrap(); - if start_daemon(backend, listener) { - break; + let mut daemon = VhostUserDaemon::new( + String::from("vhost-device-i2c-backend"), + backend.clone(), + GuestMemoryAtomic::new(GuestMemoryMmap::new()), + ) + .unwrap(); + + daemon.start(listener).unwrap(); + + match daemon.wait() { + Ok(()) => { + println!("Stopping cleanly."); + } + Err(vhost_user_backend::Error::HandleRequest( + vhost_user::Error::PartialMessage, + )) => { + println!("vhost-user connection closed with partial message. If the VM is shutting down, this is expected behavior; otherwise, it might be a bug."); + } + Err(e) => { + println!("Error running daemon: {:?}", e); + } } + + // No matter the result, we need to shut down the worker thread. + backend + .read() + .unwrap() + .exit_event + .write(1) + .expect("Shutting down worker thread"); }); handles.push(handle); @@ -156,13 +145,13 @@ fn main() -> Result<(), String> { let yaml = load_yaml!("cli.yaml"); let cmd_args = App::from(yaml).get_matches(); - start_backend::(cmd_args, start_daemon) + let config = I2cConfiguration::try_from(cmd_args).unwrap(); + start_backend::(config) } #[cfg(test)] mod tests { use super::*; - use i2c::tests::DummyDevice; fn get_cmd_args(name: &str, devices: &str, count: u32) -> ArgMatches { let yaml = load_yaml!("cli.yaml"); @@ -185,34 +174,34 @@ mod tests { } } - fn mock_start_daemon( - _backend: Arc>>, - _listener: Listener, - ) -> bool { - true - } - #[test] - fn test_backend_single() { - let cmd_args = get_cmd_args("vi2c.sock_single", "1:4,2:32:21,5:5:23", 0); - assert!(start_backend::(cmd_args, mock_start_daemon).is_ok()); - } - - #[test] - fn test_backend_multiple() { - let cmd_args = get_cmd_args("vi2c.sock", "1:4,2:32:21,5:5:23", 5); - assert!(start_backend::(cmd_args, mock_start_daemon).is_ok()); - } - - #[test] - fn test_backend_failure() { + fn test_parse_failure() { let cmd_args = get_cmd_args("vi2c.sock_failure", "1:4d", 5); - assert!(start_backend::(cmd_args, mock_start_daemon).is_err()); + // TODO: Check against the actual error instead of `is_err`. + assert!(I2cConfiguration::try_from(cmd_args).is_err()); + + let cmd_args = get_cmd_args("vi2c.sock_duplicate", "1:4,2:32:21,5:4:23", 5); + // TODO: Check against the actual error instead of `is_err`. + assert!(I2cConfiguration::try_from(cmd_args).is_err()); } #[test] - fn test_backend_failure_duplicate_device4() { - let cmd_args = get_cmd_args("vi2c.sock_duplicate", "1:4,2:32:21,5:4:23", 5); - assert!(start_backend::(cmd_args, mock_start_daemon).is_err()); + fn test_parse_successful() { + let cmd_args = get_cmd_args("vi2c.sock_single", "1:4,2:32:21,5:5:23", 5); + let config = I2cConfiguration::try_from(cmd_args).unwrap(); + + let expected_devices = AdapterConfig::new_with(vec![ + DeviceConfig::new_with(1, vec![4]), + DeviceConfig::new_with(2, vec![32, 21]), + DeviceConfig::new_with(5, vec![5, 23]), + ]); + + let expected_config = I2cConfiguration { + socket_count: 5, + socket_path: String::from("vi2c.sock_single"), + devices: expected_devices, + }; + + assert_eq!(config, expected_config); } }