From 6f588d680957ea7db35798ad1830d921a6cc42ca Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 20 Jan 2022 10:30:20 +0530 Subject: [PATCH 1/2] i2c: Move to newer version of Clap Lets not use beta release anymore, since we have a stable version available now. The newer version doesn't support Yamls anymore, but rather another feature "derive", which is quite easy to use actually. Lets migrate to it. Some of the tests can't be done anymore, drop them. Signed-off-by: Viresh Kumar --- i2c/Cargo.toml | 2 +- i2c/src/cli.yaml | 37 ---------------- i2c/src/main.rs | 112 ++++++++++++++++++----------------------------- 3 files changed, 43 insertions(+), 108 deletions(-) delete mode 100644 i2c/src/cli.yaml diff --git a/i2c/Cargo.toml b/i2c/Cargo.toml index 1164740..3b96cd5 100644 --- a/i2c/Cargo.toml +++ b/i2c/Cargo.toml @@ -12,7 +12,7 @@ edition = "2018" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -clap = { version = "=3.0.0-beta.2", features = ["yaml"] } +clap = { version = ">=3.0", features = ["derive"] } env_logger = ">=0.9" libc = ">=0.2.95" log = ">=0.4.6" diff --git a/i2c/src/cli.yaml b/i2c/src/cli.yaml deleted file mode 100644 index 8a5933f..0000000 --- a/i2c/src/cli.yaml +++ /dev/null @@ -1,37 +0,0 @@ -name: vhost-device-i2c -version: "0.1.0" -author: "Viresh Kumar " -about: Virtio I2C backend daemon. - -settings: - - ArgRequiredElseHelp - -args: - # Connection to sockets - - socket_path: - short: s - long: socket-path - value_name: FILE - takes_value: true - about: Location of vhost-user Unix domain socket. This is suffixed by 0,1,2..socket_count-1. - - socket_count: - short: c - long: socket-count - value_name: INT - takes_value: true - about: Number of guests (sockets) to connect to. Default = 1. - # I2C device list on host - - devices: - short: l - long: device-list - value_name: PATH - takes_value: true - about: List of I2C bus and clients in format :[:][,:[:]] - -groups: - - required_args: - args: - - socket_path - args: - - devices - required: true diff --git a/i2c/src/main.rs b/i2c/src/main.rs index 81fd9c8..8218744 100644 --- a/i2c/src/main.rs +++ b/i2c/src/main.rs @@ -14,7 +14,7 @@ use std::num::ParseIntError; use std::sync::{Arc, RwLock}; use std::thread::spawn; -use clap::{load_yaml, App, ArgMatches}; +use clap::Parser; use thiserror::Error as ThisError; use vhost::{vhost_user, vhost_user::Listener}; use vhost_user_backend::VhostUserDaemon; @@ -28,8 +28,6 @@ type Result = std::result::Result; #[derive(Debug, PartialEq, ThisError)] /// Errors related to low level i2c helpers pub enum Error { - #[error("Invalid socket path")] - SocketPathInvalid, #[error("Invalid socket count: {0}")] SocketCountInvalid(usize), #[error("Invalid device list")] @@ -48,6 +46,23 @@ pub enum Error { FailedJoiningThreads, } +#[derive(Parser, Debug)] +#[clap(author, version, about, long_about = None)] +struct I2cArgs { + /// Location of vhost-user Unix domain socket. This is suffixed by 0,1,2..socket_count-1. + #[clap(short, long)] + socket_path: String, + + /// Number of guests (sockets) to connect to. + #[clap(short = 'c', long, default_value_t = 1)] + socket_count: usize, + + /// List of I2C bus and clients in format + /// :[:][,:[:]]. + #[clap(short = 'l', long)] + device_list: String, +} + #[derive(Debug, PartialEq)] struct DeviceConfig { adapter_no: u32, @@ -140,39 +155,25 @@ struct I2cConfiguration { devices: AdapterConfig, } -impl TryFrom for I2cConfiguration { +impl TryFrom for I2cConfiguration { type Error = Error; - fn try_from(cmd_args: ArgMatches) -> Result { - let socket_path = cmd_args - .value_of("socket_path") - .ok_or(Error::SocketPathInvalid)? - .to_string(); - - let socket_count = cmd_args - .value_of("socket_count") - .unwrap_or("1") - .parse::() - .map_err(Error::ParseFailure)?; - - if socket_count == 0 { + fn try_from(args: I2cArgs) -> Result { + if args.socket_count == 0 { return Err(Error::SocketCountInvalid(0)); } - let list = cmd_args - .value_of("devices") - .ok_or(Error::DeviceListInvalid)?; - let devices = AdapterConfig::try_from(list)?; + let devices = AdapterConfig::try_from(args.device_list.as_str())?; Ok(I2cConfiguration { - socket_path, - socket_count, + socket_path: args.socket_path, + socket_count: args.socket_count, devices, }) } } -fn start_backend(cmd_args: ArgMatches) -> Result<()> { - let config = I2cConfiguration::try_from(cmd_args).unwrap(); +fn start_backend(args: I2cArgs) -> Result<()> { + let config = I2cConfiguration::try_from(args).unwrap(); // The same i2c_map structure instance is shared between all the guests let i2c_map = Arc::new(I2cMap::::new(&config.devices).map_err(Error::I2cFailure)?); @@ -237,10 +238,7 @@ fn start_backend(cmd_args: ArgMatches) -> fn main() -> Result<()> { env_logger::init(); - let yaml = load_yaml!("cli.yaml"); - let cmd_args = App::from(yaml).get_matches(); - - start_backend::(cmd_args) + start_backend::(I2cArgs::parse()) } #[cfg(test)] @@ -260,19 +258,12 @@ mod tests { } } - fn get_cmd_args(name: Option<&str>, devices: &str, count: Option<&str>) -> ArgMatches { - let mut args = vec!["prog", "-l", devices]; - let yaml = load_yaml!("cli.yaml"); - let app = App::from(yaml); - - if let Some(name) = name { - args.extend_from_slice(&["-s", name]); + fn get_cmd_args(path: &str, devices: &str, count: usize) -> I2cArgs { + I2cArgs { + socket_path: path.to_string(), + socket_count: count, + device_list: devices.to_string(), } - - if let Some(count) = count { - args.extend_from_slice(&["-c", count]); - } - app.try_get_matches_from(args).unwrap() } #[test] @@ -296,45 +287,31 @@ mod tests { #[test] fn test_parse_failure() { - let socket_name = Some("vi2c.sock"); + let socket_name = "vi2c.sock"; // Invalid bus_addr - let cmd_args = get_cmd_args(socket_name, "1:4,3d:5", Some("5")); + let cmd_args = get_cmd_args(socket_name, "1:4,3d:5", 5); assert_eq!( I2cConfiguration::try_from(cmd_args).unwrap_err(), Error::ParseFailure("3d".parse::().unwrap_err()) ); // Invalid client address - let cmd_args = get_cmd_args(socket_name, "1:4d", Some("5")); + let cmd_args = get_cmd_args(socket_name, "1:4d", 5); assert_eq!( I2cConfiguration::try_from(cmd_args).unwrap_err(), Error::ParseFailure("4d".parse::().unwrap_err()) ); - // Invalid socket path - let cmd_args = get_cmd_args(None, "1:4d", Some("5")); - assert_eq!( - I2cConfiguration::try_from(cmd_args).unwrap_err(), - Error::SocketPathInvalid - ); - - // Invalid socket count - let cmd_args = get_cmd_args(socket_name, "1:4", Some("1d")); - assert_eq!( - I2cConfiguration::try_from(cmd_args).unwrap_err(), - Error::ParseFailure("1d".parse::().unwrap_err()) - ); - // Zero socket count - let cmd_args = get_cmd_args(socket_name, "1:4", Some("0")); + let cmd_args = get_cmd_args(socket_name, "1:4", 0); assert_eq!( I2cConfiguration::try_from(cmd_args).unwrap_err(), Error::SocketCountInvalid(0) ); // Duplicate client address: 4 - let cmd_args = get_cmd_args(socket_name, "1:4,2:32:21,5:4:23", Some("5")); + let cmd_args = get_cmd_args(socket_name, "1:4,2:32:21,5:4:23", 5); assert_eq!( I2cConfiguration::try_from(cmd_args).unwrap_err(), Error::ClientAddressDuplicate(4) @@ -343,14 +320,9 @@ mod tests { #[test] fn test_parse_successful() { - let socket_name = Some("vi2c.sock"); + let socket_name = "vi2c.sock"; - // Missing socket count, default (1) should be used. - let cmd_args = get_cmd_args(socket_name, "1:4,2:32:21,5:5:23", None); - let config = I2cConfiguration::try_from(cmd_args).unwrap(); - assert_eq!(config.socket_count, 1); - - let cmd_args = get_cmd_args(socket_name, "1:4,2:32:21,5:5:23", Some("5")); + let cmd_args = get_cmd_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![ @@ -361,7 +333,7 @@ mod tests { let expected_config = I2cConfiguration { socket_count: 5, - socket_path: String::from(socket_name.unwrap()), + socket_path: String::from(socket_name), devices: expected_devices, }; @@ -387,8 +359,8 @@ mod tests { #[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")); + let socket_name = "~/path/not/present/i2c"; + let cmd_args = get_cmd_args(socket_name, "1:4,3:5", 5); assert_eq!( start_backend::(cmd_args).unwrap_err(), From 33c40e5c064066c5d619188d697766e7a404090d Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 20 Jan 2022 14:49:15 +0530 Subject: [PATCH 2/2] Update cargo.lock and coverage Update cargo.lock and coverage to match the latest code. Signed-off-by: Viresh Kumar --- Cargo.lock | 65 ++++++++----------------------------- coverage_config_x86_64.json | 2 +- 2 files changed, 14 insertions(+), 53 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1181681..d590a37 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -48,9 +48,9 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" [[package]] name = "clap" -version = "3.0.0-beta.2" +version = "3.0.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4bd1061998a501ee7d4b6d449020df3266ca3124b941ec56cf2005c3779ca142" +checksum = "7a30c3bf9ff12dfe5dae53f0a96e0febcd18420d1c0e7fad77796d9d5c4b5375" dependencies = [ "atty", "bitflags", @@ -61,16 +61,13 @@ dependencies = [ "strsim", "termcolor", "textwrap", - "unicode-width", - "vec_map", - "yaml-rust", ] [[package]] name = "clap_derive" -version = "3.0.0-beta.5" +version = "3.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b15c6b4f786ffb6192ffe65a36855bc1fc2444bcd0945ae16748dcd6ed7d0d3" +checksum = "517358c28fcef6607bf6f76108e02afad7e82297d132a6b846dcc1fc3efcd153" dependencies = [ "heck", "proc-macro-error", @@ -100,12 +97,9 @@ checksum = "ab5ef0d4909ef3724cc8cce6ccc8572c5c817592e9285f5464f8e86f8bd3726e" [[package]] name = "heck" -version = "0.3.3" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6d621efb26863f0e9924c6ac577e8275e5e6b77455db64ffa6c65c904e9e132c" -dependencies = [ - "unicode-segmentation", -] +checksum = "2540771e65fc8cb83cd6e8a237f70c319bd5c29f78ed1084ba5d50eeac86f7f9" [[package]] name = "hermit-abi" @@ -144,12 +138,6 @@ version = "0.2.112" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1b03d17f364a3a042d5e5d46b053bbbf82c92c9430c592dd4c064dc6ee997125" -[[package]] -name = "linked-hash-map" -version = "0.5.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7fb9b38af92608140b86b693604b9ffcc5824240a484d1ecd4795bacb2fe88f3" - [[package]] name = "log" version = "0.4.14" @@ -167,9 +155,12 @@ checksum = "308cc39be01b73d0d18f82a0e7b2a3df85245f84af96fdddc5d202d27e47b86a" [[package]] name = "os_str_bytes" -version = "2.4.0" +version = "6.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "afb2e1c3ee07430c2cf76151675e583e0f19985fa6efae47d6848a3e2c824f85" +checksum = "8e22443d1643a904602595ba1cd8f7d896afe56d26712531c5ff73a15b2fbf64" +dependencies = [ + "memchr", +] [[package]] name = "proc-macro-error" @@ -258,12 +249,9 @@ dependencies = [ [[package]] name = "textwrap" -version = "0.12.1" +version = "0.14.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "203008d98caf094106cfaba70acfed15e18ed3ddb7d94e49baec153a2b462789" -dependencies = [ - "unicode-width", -] +checksum = "0066c8d12af8b5acd21e00547c3797fde4e8677254a7ee429176ccebbe93dd80" [[package]] name = "thiserror" @@ -285,30 +273,12 @@ dependencies = [ "syn", ] -[[package]] -name = "unicode-segmentation" -version = "1.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8895849a949e7845e06bd6dc1aa51731a103c42707010a5b591c0038fb73385b" - -[[package]] -name = "unicode-width" -version = "0.1.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3ed742d4ea2bd1176e236172c8429aaf54486e7ac098db29ffe6529e0ce50973" - [[package]] name = "unicode-xid" version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8ccb82d61f80a663efe1f787a51b16b5a51e3314d6ac365b08639f52387b33f3" -[[package]] -name = "vec_map" -version = "0.8.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f1bddf1187be692e79c5ffeab891132dfb0f236ed36a43c7ed39f1165ee20191" - [[package]] name = "version_check" version = "0.9.4" @@ -427,12 +397,3 @@ name = "winapi-x86_64-pc-windows-gnu" version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" - -[[package]] -name = "yaml-rust" -version = "0.4.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "56c1936c4cc7a1c9ab21a1ebb602eb942ba868cbd44a99cb7cdc5892335e1c85" -dependencies = [ - "linked-hash-map", -] diff --git a/coverage_config_x86_64.json b/coverage_config_x86_64.json index 425cd4f..25e5d8f 100644 --- a/coverage_config_x86_64.json +++ b/coverage_config_x86_64.json @@ -1,5 +1,5 @@ { - "coverage_score": 87.2, + "coverage_score": 85.7, "exclude_path": "", "crate_features": "" }