i2c: Support for both bus name and bus number

When the adapter name is used as a parameter, some adapters
share the same name. For instance, both /dev/i2c-0 and
/dev/i2c-1 have the bus name "xxxx". This leads to only
the first found I2C adapter being opened.

To increase flexibility, accept both the I2C master's
name and number as parameters and parse accordingly.

Signed-off-by: Wei Liu <quic_wliu8@quicinc.com>

vhost-device-i2c/README.md

@@ -33,15 +33,27 @@ Examples section below.
 
 .. option:: -l, --device-list=I2C-DEVICES
 
-  I2c device list at the host OS in the format:
-      <bus-name>:<client_addr>[:<client_addr>],[<bus-name>:<client_addr>[:<client_addr>]]
+  I2c device list at the host OS can be in two different format, name and number:
 
+- format by name:
+      \<bus-name>:<client_addr>[:<client_addr>],[\<bus-name>:<client_addr>[:<client_addr>]]
+```
       Example: --device-list "i915 gmbus dpd:32:21,DPDDC-D:10:23"
-
+```
   Here,
       bus-name: is adatper's name. e.g. value of /sys/bus/i2c/devices/i2c-0/name.
       client_addr (decimal): address for client device, 32 == 0x20.
 
+- format by number:
+    \<bus>:<client_addr>[:<client_addr>],[\<bus>:<client_addr>[:<client_addr>]]
+
+```
+      Example: --device-list "2:32:21,3:10:23"
+```
+  Here,
+      bus (decimal): adatper bus number. e.g. 2 for /dev/i2c-2, 3 for /dev/i2c-3.
+      client_addr (decimal): address for client device, 32 == 0x20.
+
 ## Examples
 
 The daemon should be started first:
@@ -50,6 +62,8 @@ The daemon should be started first:
 
   host# vhost-device-i2c --socket-path=vi2c.sock --socket-count=1 --device-list "i915 gmbus dpd:32"
 
+  host# vhost-device-i2c --socket-path=vi2c.sock --socket-count=1 --device-list "0:32"
+
 The QEMU invocation needs to create a chardev socket the device can
 use to communicate as well as share the guests memory over a memfd.
 

vhost-device-i2c/src/i2c.rs

@@ -16,6 +16,7 @@ use thiserror::Error as ThisError;
 use vmm_sys_util::errno::Error as IoError;
 
 use super::AdapterConfig;
+use crate::AdapterIdentifier;
 
 // The type of the `req` parameter is different for the `musl` library. This will enable
 // successful build for other non-musl libraries.
@@ -305,8 +306,8 @@ pub(crate) struct I2cReq {
 /// mock implementation for the I2C driver so that we can test the I2C
 /// functionality without the need of a physical device.
 pub(crate) trait I2cDevice {
-    // Open the device specified by the adapter name.
-    fn open(adapter_name: &str) -> Result<Self>
+    // Open the device specified by the adapter identifier, number or name.
+    fn open(adapter_identifier: &AdapterIdentifier) -> Result<Self>
     where
         Self: Sized;
 
@@ -369,8 +370,11 @@ impl PhysDevice {
 }
 
 impl I2cDevice for PhysDevice {
-    fn open(adapter_name: &str) -> Result<Self> {
-        let adapter_no = PhysDevice::find_adapter(adapter_name)?;
+    fn open(adapter_identifier: &AdapterIdentifier) -> Result<Self> {
+        let adapter_no = match adapter_identifier {
+            AdapterIdentifier::Name(adapter_name) => PhysDevice::find_adapter(adapter_name)?,
+            AdapterIdentifier::Number(no) => *no,
+        };
         let device_path = format!("/dev/i2c-{}", adapter_no);
 
         Self::open_with(&device_path, adapter_no)
@@ -558,7 +562,7 @@ impl<D: I2cDevice> I2cMap<D> {
         let mut adapters: Vec<I2cAdapter<D>> = Vec::new();
 
         for (i, device_cfg) in device_config.inner.iter().enumerate() {
-            let device = D::open(&device_cfg.adapter_name)?;
+            let device = D::open(&device_cfg.adapter)?;
             let adapter = I2cAdapter::new(device)?;
 
             // Check that all addresses corresponding to the adapter are valid.
@@ -630,6 +634,12 @@ pub(crate) mod tests {
         adapter_no: u32,
     }
 
+    impl DummyDevice {
+        fn find_adapter(_name: &str) -> Result<u32> {
+            Ok(11)
+        }
+    }
+
     impl Default for DummyDevice {
         fn default() -> Self {
             Self {
@@ -643,16 +653,20 @@ pub(crate) mod tests {
     }
 
     impl I2cDevice for DummyDevice {
-        fn open(adapter_name: &str) -> Result<Self>
+        fn open(adapter_identifier: &AdapterIdentifier) -> Result<Self>
         where
             Self: Sized,
         {
-            Ok(DummyDevice {
-                adapter_no: adapter_name
-                    .parse::<u32>()
-                    .map_err(|_| Error::ParseFailure)?,
-                ..Default::default()
-            })
+            match adapter_identifier {
+                AdapterIdentifier::Name(adapter_name) => Ok(DummyDevice {
+                    adapter_no: DummyDevice::find_adapter(adapter_name)?,
+                    ..Default::default()
+                }),
+                AdapterIdentifier::Number(adapter_no) => Ok(DummyDevice {
+                    adapter_no: *adapter_no,
+                    ..Default::default()
+                }),
+            }
         }
 
         fn funcs(&mut self) -> Result<u64> {
@@ -730,9 +744,9 @@ pub(crate) mod tests {
     #[test]
     fn test_i2c_map() {
         let adapter_config = AdapterConfig::new_with(vec![
-            DeviceConfig::new_with(1, vec![4]),
-            DeviceConfig::new_with(2, vec![32, 21]),
-            DeviceConfig::new_with(5, vec![10, 23]),
+            DeviceConfig::new_with(AdapterIdentifier::Number(1), vec![4]),
+            DeviceConfig::new_with(AdapterIdentifier::Number(2), vec![32, 21]),
+            DeviceConfig::new_with(AdapterIdentifier::Number(5), vec![10, 23]),
         ]);
         let i2c_map: I2cMap<DummyDevice> = I2cMap::new(&adapter_config).unwrap();
 
@@ -750,7 +764,10 @@ pub(crate) mod tests {
 
     #[test]
     fn test_i2c_transfer() {
-        let adapter_config = AdapterConfig::new_with(vec![DeviceConfig::new_with(1, vec![3])]);
+        let adapter_config = AdapterConfig::new_with(vec![DeviceConfig::new_with(
+            AdapterIdentifier::Number(1),
+            vec![3],
+        )]);
         let mut i2c_map: I2cMap<DummyDevice> = I2cMap::new(&adapter_config).unwrap();
 
         i2c_map.adapters[0].smbus = false;
@@ -809,7 +826,10 @@ pub(crate) mod tests {
 
     #[test]
     fn test_smbus_transfer() {
-        let adapter_config = AdapterConfig::new_with(vec![DeviceConfig::new_with(1, vec![3])]);
+        let adapter_config = AdapterConfig::new_with(vec![DeviceConfig::new_with(
+            AdapterIdentifier::Number(1),
+            vec![3],
+        )]);
         let mut i2c_map: I2cMap<DummyDevice> = I2cMap::new(&adapter_config).unwrap();
 
         i2c_map.adapters[0].smbus = true;
@@ -915,7 +935,10 @@ pub(crate) mod tests {
 
     #[test]
     fn test_transfer_failure() {
-        let adapter_config = AdapterConfig::new_with(vec![DeviceConfig::new_with(1, vec![3])]);
+        let adapter_config = AdapterConfig::new_with(vec![DeviceConfig::new_with(
+            AdapterIdentifier::Number(1),
+            vec![3],
+        )]);
         let mut i2c_map: I2cMap<DummyDevice> = I2cMap::new(&adapter_config).unwrap();
 
         i2c_map.adapters[0].smbus = false;
@@ -936,7 +959,10 @@ pub(crate) mod tests {
 
     #[test]
     fn test_smbus_transfer_failure() {
-        let adapter_config = AdapterConfig::new_with(vec![DeviceConfig::new_with(1, vec![3])]);
+        let adapter_config = AdapterConfig::new_with(vec![DeviceConfig::new_with(
+            AdapterIdentifier::Number(1),
+            vec![3],
+        )]);
         let mut i2c_map: I2cMap<DummyDevice> = I2cMap::new(&adapter_config).unwrap();
         i2c_map.adapters[0].smbus = true;
 
@@ -1095,10 +1121,15 @@ pub(crate) mod tests {
     fn test_phys_device_failure() {
         // Open failure
         assert_eq!(
-            PhysDevice::open("555555").unwrap_err(),
+            PhysDevice::open(&AdapterIdentifier::Name("555555".to_string())).unwrap_err(),
             Error::AdapterNotFound
         );
 
+        assert_eq!(
+            PhysDevice::open(&AdapterIdentifier::Number(55555)).unwrap_err(),
+            Error::DeviceOpenFailed(55555)
+        );
+
         assert_eq!(
             PhysDevice::open_with("/dev/i2c-invalid-path", 0).unwrap_err(),
             Error::DeviceOpenFailed(0)

vhost-device-i2c/src/main.rs

@@ -8,6 +8,7 @@
 mod i2c;
 mod vhu_i2c;
 
+use core::fmt;
 use log::error;
 use std::num::ParseIntError;
 use std::path::PathBuf;
@@ -30,8 +31,10 @@ type Result<T> = std::result::Result<T, Error>;
 pub(crate) enum Error {
     #[error("Invalid socket count: {0}")]
     SocketCountInvalid(usize),
+    #[error("Failed while parsing adapter identifier")]
+    CoulodNotFindAdapterIdentifier,
     #[error("Duplicate adapter detected: {0}")]
-    AdapterDuplicate(String),
+    AdapterDuplicate(AdapterIdentifier),
     #[error("Invalid client address: {0}")]
     ClientAddressInvalid(u16),
     #[error("Duplicate client address detected: {0}")]
@@ -67,16 +70,38 @@ struct I2cArgs {
     device_list: String,
 }
 
+#[derive(Debug, PartialEq)]
+enum AdapterIdentifier {
+    Name(String),
+    Number(u32),
+}
+
+impl fmt::Display for AdapterIdentifier {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            AdapterIdentifier::Name(name) => write!(f, "adapter_name: {}", name),
+            AdapterIdentifier::Number(no) => write!(f, "adapter_no:: {}", no),
+        }
+    }
+}
+
 #[derive(Debug, PartialEq)]
 struct DeviceConfig {
-    adapter_name: String,
+    adapter: AdapterIdentifier,
     addr: Vec<u16>,
 }
 
 impl DeviceConfig {
-    fn new(name: &str) -> Result<Self> {
+    fn new_with_no(no: u32) -> Result<Self> {
         Ok(DeviceConfig {
-            adapter_name: name.trim().to_string(),
+            adapter: AdapterIdentifier::Number(no),
+            addr: Vec::new(),
+        })
+    }
+
+    fn new_with_name(name: &str) -> Result<Self> {
+        Ok(DeviceConfig {
+            adapter: AdapterIdentifier::Name(name.trim().to_string()),
             addr: Vec::new(),
         })
     }
@@ -104,11 +129,10 @@ impl AdapterConfig {
     fn new() -> Self {
         Self { inner: Vec::new() }
     }
-
-    fn contains_adapter(&self, adapter_name: &str) -> bool {
+    fn contains_adapter(&self, adapter: &DeviceConfig) -> bool {
         self.inner
             .iter()
-            .any(|elem| elem.adapter_name == adapter_name)
+            .any(|elem| elem.adapter == adapter.adapter)
     }
 
     fn contains_addr(&self, addr: u16) -> bool {
@@ -116,8 +140,8 @@ impl AdapterConfig {
     }
 
     fn push(&mut self, device: DeviceConfig) -> Result<()> {
-        if self.contains_adapter(&device.adapter_name) {
-            return Err(Error::AdapterDuplicate(device.adapter_name));
+        if self.contains_adapter(&device) {
+            return Err(Error::AdapterDuplicate(device.adapter));
         }
 
         for addr in device.addr.iter() {
@@ -135,12 +159,16 @@ impl TryFrom<&str> for AdapterConfig {
     type Error = Error;
 
     fn try_from(list: &str) -> Result<Self> {
-        let busses: Vec<&str> = list.split(',').collect();
+        let adapter_identifiers: Vec<&str> = list.split(',').collect();
         let mut devices = AdapterConfig::new();
 
-        for businfo in busses.iter() {
-            let list: Vec<&str> = businfo.split(':').collect();
-            let mut adapter = DeviceConfig::new(list[0])?;
+        for identifier_info in adapter_identifiers.iter() {
+            let list: Vec<&str> = identifier_info.split(':').collect();
+            let identifier = list.first().ok_or(Error::CoulodNotFindAdapterIdentifier)?;
+            let mut adapter = match identifier.parse::<u32>() {
+                Ok(no) => DeviceConfig::new_with_no(no)?,
+                Err(_) => DeviceConfig::new_with_name(identifier)?,
+            };
 
             for device_str in list[1..].iter() {
                 let addr = device_str.parse::<u16>().map_err(Error::ParseFailure)?;
@@ -291,9 +319,9 @@ mod tests {
     use crate::i2c::tests::DummyDevice;
 
     impl DeviceConfig {
-        pub fn new_with(adapter_no: u32, addr: Vec<u16>) -> Self {
+        pub fn new_with(adaper_id: AdapterIdentifier, addr: Vec<u16>) -> Self {
             DeviceConfig {
-                adapter_name: adapter_no.to_string(),
+                adapter: adaper_id,
                 addr,
             }
         }
@@ -317,7 +345,26 @@ mod tests {
 
     #[test]
     fn test_device_config() {
-        let mut config = DeviceConfig::new_with(5, Vec::new());
+        let id_name = AdapterIdentifier::Name("i915 gmbus dpd".to_string());
+        let mut config = DeviceConfig::new_with(id_name, Vec::new());
+        assert_eq!(
+            config,
+            DeviceConfig {
+                adapter: AdapterIdentifier::Name("i915 gmbus dpd".to_string()),
+                addr: Vec::new()
+            }
+        );
+
+        let id_no = AdapterIdentifier::Number(11);
+        config = DeviceConfig::new_with(id_no, Vec::new());
+        assert_eq!(
+            config,
+            DeviceConfig {
+                adapter: AdapterIdentifier::Number(11),
+                addr: Vec::new()
+            }
+        );
+
         let invalid_addr = (MAX_I2C_VDEV + 1) as u16;
 
         config.push(5).unwrap();
@@ -379,14 +426,14 @@ mod tests {
         let config = I2cConfiguration::try_from(cmd_args).unwrap();
         Listener::new(config.socket_path, true).unwrap();
 
-        // Valid configuration
+        // Valid configuration with number as identifier
         let cmd_args = I2cArgs::from_args(socket_name, "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]),
+            DeviceConfig::new_with(AdapterIdentifier::Number(1), vec![4]),
+            DeviceConfig::new_with(AdapterIdentifier::Number(2), vec![32, 21]),
+            DeviceConfig::new_with(AdapterIdentifier::Number(5), vec![5, 23]),
         ]);
 
         let expected_config = I2cConfiguration {
@@ -397,6 +444,38 @@ mod tests {
 
         assert_eq!(config, expected_config);
 
+        // Valid configuration with name as identifier
+        let cmd_args = I2cArgs::from_args(socket_name, "bus1:4,bus2:32:21,bus5:5:23", 5);
+        let config = I2cConfiguration::try_from(cmd_args).unwrap();
+        let expected_devices = AdapterConfig::new_with(vec![
+            DeviceConfig::new_with(AdapterIdentifier::Name("bus1".to_string()), vec![4]),
+            DeviceConfig::new_with(AdapterIdentifier::Name("bus2".to_string()), vec![32, 21]),
+            DeviceConfig::new_with(AdapterIdentifier::Name("bus5".to_string()), vec![5, 23]),
+        ]);
+        let expected_config = I2cConfiguration {
+            socket_count: 5,
+            socket_path: socket_name.into(),
+            devices: expected_devices,
+        };
+
+        assert_eq!(config, expected_config);
+
+        //Valid configuration with mixing name and number identifier
+        let cmd_args = I2cArgs::from_args(socket_name, "123asd:4,11:32:21,23:5:23", 5);
+        let config = I2cConfiguration::try_from(cmd_args).unwrap();
+        let expected_devices = AdapterConfig::new_with(vec![
+            DeviceConfig::new_with(AdapterIdentifier::Name("123asd".to_string()), vec![4]),
+            DeviceConfig::new_with(AdapterIdentifier::Number(11), vec![32, 21]),
+            DeviceConfig::new_with(AdapterIdentifier::Number(23), vec![5, 23]),
+        ]);
+        let expected_config = I2cConfiguration {
+            socket_count: 5,
+            socket_path: socket_name.into(),
+            devices: expected_devices,
+        };
+
+        assert_eq!(config, expected_config);
+
         // Socket paths are what we expect them to be.
         assert_eq!(
             config.generate_socket_paths(),
@@ -414,14 +493,25 @@ mod tests {
     fn test_i2c_map_duplicate_device4() {
         let mut config = AdapterConfig::new();
 
-        config.push(DeviceConfig::new_with(1, vec![4])).unwrap();
         config
-            .push(DeviceConfig::new_with(2, vec![32, 21]))
+            .push(DeviceConfig::new_with(
+                AdapterIdentifier::Number(1),
+                vec![4],
+            ))
+            .unwrap();
+        config
+            .push(DeviceConfig::new_with(
+                AdapterIdentifier::Number(2),
+                vec![32, 21],
+            ))
             .unwrap();
 
         assert_matches!(
             config
-                .push(DeviceConfig::new_with(5, vec![4, 23]))
+                .push(DeviceConfig::new_with(
+                    AdapterIdentifier::Number(5),
+                    vec![4, 23]
+                ))
                 .unwrap_err(),
             Error::ClientAddressDuplicate(4)
         );
@@ -431,16 +521,49 @@ mod tests {
     fn test_duplicated_adapter_no() {
         let mut config = AdapterConfig::new();
 
-        config.push(DeviceConfig::new_with(1, vec![4])).unwrap();
         config
-            .push(DeviceConfig::new_with(5, vec![10, 23]))
+            .push(DeviceConfig::new_with(
+                AdapterIdentifier::Number(1),
+                vec![4],
+            ))
+            .unwrap();
+        config
+            .push(DeviceConfig::new_with(
+                AdapterIdentifier::Number(5),
+                vec![10, 23],
+            ))
             .unwrap();
 
         assert_matches!(
             config
-                .push(DeviceConfig::new_with(1, vec![32, 21]))
+                .push(DeviceConfig::new_with(AdapterIdentifier::Number(1), vec![32, 21]))
                 .unwrap_err(),
-            Error::AdapterDuplicate(n) if n == "1"
+            Error::AdapterDuplicate(n) if n == AdapterIdentifier::Number(1)
+        );
+    }
+
+    #[test]
+    fn test_duplicated_adapter_name() {
+        let mut config = AdapterConfig::new();
+
+        config
+            .push(DeviceConfig::new_with(
+                AdapterIdentifier::Name("bus1".to_string()),
+                vec![4],
+            ))
+            .unwrap();
+        config
+            .push(DeviceConfig::new_with(
+                AdapterIdentifier::Name("bus5".to_string()),
+                vec![10, 23],
+            ))
+            .unwrap();
+
+        assert_matches!(
+            config
+                .push(DeviceConfig::new_with(AdapterIdentifier::Name("bus5".to_string()), vec![32, 21]))
+                .unwrap_err(),
+            Error::AdapterDuplicate(n) if n == AdapterIdentifier::Name("bus5".to_string())
         );
     }