[i2c] separate parsing of strings from device

This separation is needed so we can easily write unit tests for parsing
without needing to create full blown objects. (as a side note, before
this is possible, we also need to get rid of errno, and replace it with
custom Errors, so that we can also write the much needed negative
tests).

This separation is achieved through creating configuration structures
that can be either programatically initialized, or initialized through
parsing the command line parameters.

This commit is still WIP because we also need to make sure that
configuration objects can only be created valid (to reduce some risks
for future extensions where parameters might be passed some other way
rather than yaml). Also, we need to move the check for uniquness of
device addresses in the DeviceConfig instead of the `I2cMap`.

Signed-off-by: Andreea Florescu <fandree@amazon.com>

src/i2c/src/i2c.rs

@@ -416,7 +416,7 @@ impl<D: I2cDevice> I2cAdapter<D> {
 }
 
 /// I2C map and helpers
-const MAX_I2C_VDEV: usize = 1 << 7;
+pub(crate) const MAX_I2C_VDEV: usize = 1 << 7;
 const I2C_INVALID_ADAPTER: u32 = 0xFFFFFFFF;
 
 pub struct I2cMap<D: I2cDevice> {
@@ -424,46 +424,46 @@ pub struct I2cMap<D: I2cDevice> {
     device_map: [u32; MAX_I2C_VDEV],
 }
 
+pub(crate) struct DeviceConfig {
+    pub(crate) adapter_no: u32,
+    pub(crate) addr: Vec<usize>,
+}
+
+pub(crate) struct I2cConfiguration {
+    pub(crate) socket_path: String,
+    pub(crate) socket_count: usize,
+    pub(crate) devices: Vec<DeviceConfig>,
+}
+
 impl<D: I2cDevice> I2cMap<D> {
-    pub fn new(list: &str) -> Result<Self>
+    pub(crate) fn new(list: &Vec<DeviceConfig>) -> Result<Self>
     where
         Self: Sized,
     {
         let mut device_map: [u32; MAX_I2C_VDEV] = [I2C_INVALID_ADAPTER; MAX_I2C_VDEV];
         let mut adapters: Vec<I2cAdapter<D>> = Vec::new();
-        let busses: Vec<&str> = list.split(',').collect();
 
-        for (i, businfo) in busses.iter().enumerate() {
-            let list: Vec<&str> = businfo.split(':').collect();
-            let adapter_no = list[0].parse::<u32>().map_err(|_| Error::new(EINVAL))?;
-            let mut adapter = I2cAdapter::new(adapter_no)?;
-            let devices = &list[1..];
+        for (i, device_cfg) in list.iter().enumerate() {
+            let adapter = I2cAdapter::new(device_cfg.adapter_no)?;
 
-            for device in devices {
-                let device = device.parse::<usize>().map_err(|_| Error::new(EINVAL))?;
-
-                if device > MAX_I2C_VDEV {
-                    println!("Invalid device address {}", device);
-                    return Err(Error::new(EADDRNOTAVAIL));
-                }
-
-                if device_map[device] != I2C_INVALID_ADAPTER {
+            for addr in &device_cfg.addr {
+                if device_map[*addr as usize] != I2C_INVALID_ADAPTER {
                     println!(
                         "Client address {} is already used by {}",
-                        device,
-                        adapters[device_map[device] as usize].adapter_no()
+                        *addr,
+                        adapters[device_map[*addr as usize] as usize].adapter_no()
                     );
                     return Err(Error::new(EADDRINUSE));
                 }
 
-                adapter.set_device_addr(device)?;
-                device_map[device] = i as u32;
+                // Calling this to check that the `addr` is valid.
+                adapter.set_device_addr(*addr)?;
+                device_map[*addr as usize] = i as u32;
             }
 
             println!(
-                "Added I2C master with bus id: {:x} for devices: {:?}",
+                "Added I2C master with bus id: {:x} for devices",
                 adapter.adapter_no(),
-                devices
             );
 
             adapters.push(adapter);

src/i2c/src/main.rs

@@ -8,6 +8,8 @@
 mod i2c;
 mod vhu_i2c;
 
+use std::convert::TryFrom;
+use std::num::ParseIntError;
 use std::sync::{Arc, RwLock};
 use std::thread::spawn;
 
@@ -16,11 +18,63 @@ use vhost::{vhost_user, vhost_user::Listener};
 use vhost_user_backend::VhostUserDaemon;
 use vm_memory::{GuestMemoryAtomic, GuestMemoryMmap};
 
-use i2c::{I2cDevice, I2cMap, PhysDevice};
-use std::convert::TryFrom;
-use std::num::ParseIntError;
+use i2c::{DeviceConfig, I2cConfiguration, I2cDevice, I2cMap, PhysDevice, MAX_I2C_VDEV};
 use vhu_i2c::VhostUserI2cBackend;
 
+impl TryFrom<ArgMatches> for I2cConfiguration {
+    type Error = String;
+
+    fn try_from(cmd_args: ArgMatches) -> Result<Self, Self::Error> {
+        let socket_path = cmd_args
+            .value_of("socket_path")
+            .ok_or("Invalid socket path")?
+            .to_string();
+
+        let socket_count = cmd_args
+            .value_of("socket_count")
+            .unwrap_or("1")
+            .parse::<usize>()
+            .map_err(|_| "Invalid socket_count")?;
+
+        let list = cmd_args.value_of("devices").ok_or("Invalid devices list")?;
+        let busses: Vec<&str> = list.split(',').collect();
+
+        let mut devices = Vec::new();
+
+        for businfo in busses.iter() {
+            let list: Vec<&str> = businfo.split(':').collect();
+            let bus_addr = list[0].parse::<u32>().map_err(|_| "Invalid bus address")?;
+            let bus_devices = list[1..]
+                .iter()
+                .map(|str| str.parse::<usize>())
+                .collect::<Result<Vec<usize>, ParseIntError>>()
+                .map_err(|_| "Invalid device")?;
+
+            // Check if any of the devices has a size > the maximum allowed one.
+            if bus_devices
+                .iter()
+                .filter(|addr| **addr > MAX_I2C_VDEV)
+                .count()
+                > 0
+            {
+                // TODO: if needed we can show which one is actually not respecting the max size.
+                return Err("Invalid addr.".to_string());
+            }
+
+            devices.push(DeviceConfig {
+                adapter_no: bus_addr,
+                addr: bus_devices,
+            })
+        }
+
+        Ok(I2cConfiguration {
+            socket_path,
+            socket_count,
+            devices,
+        })
+    }
+}
+
 fn start_daemon<D: 'static + I2cDevice + Send + Sync>(
     backend: Arc<RwLock<VhostUserI2cBackend<D>>>,
     listener: Listener,
@@ -63,24 +117,16 @@ fn start_backend<D: I2cDevice + Sync + Send + 'static>(
 ) -> Result<(), String> {
     let mut handles = Vec::new();
 
-    let path = cmd_args
-        .value_of("socket_path")
-        .ok_or("Invalid socket path")?;
-
-    let count = cmd_args
-        .value_of("socket_count")
-        .unwrap_or("1")
-        .parse::<u32>()
-        .map_err(|_| "Invalid socket_count")?;
-
-    let list = cmd_args.value_of("devices").ok_or("Invalid devices list")?;
+    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::<D>::new(list).map_err(|e| format!("Failed to create i2c_map ({})", e))?);
+    let i2c_map = Arc::new(
+        I2cMap::<D>::new(&i2c_config.devices)
+            .map_err(|e| format!("Failed to create i2c_map ({})", e))?,
+    );
 
-    for i in 0..count {
-        let socket = path.to_owned() + &i.to_string();
+    for i in 0..i2c_config.socket_count {
+        let socket = i2c_config.socket_path.to_owned() + &i.to_string();
         let i2c_map = i2c_map.clone();
 
         let handle = spawn(move || loop {