From 6f588d680957ea7db35798ad1830d921a6cc42ca Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 20 Jan 2022 10:30:20 +0530 Subject: [PATCH 1/4] 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/4] 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": "" } From 31489cc5b06128f77edcb1d2c00ff07d1c73f414 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 24 Jan 2022 04:06:48 +0000 Subject: [PATCH 3/4] build(deps): bump rust-vmm-ci from `d023262` to `d216a46` Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `d023262` to `d216a46`. - [Release notes](https://github.com/rust-vmm/rust-vmm-ci/releases) - [Commits](https://github.com/rust-vmm/rust-vmm-ci/compare/d023262164ef8766ac2b0be33de13ef52b8706a1...d216a46879096a4f1e83af3a7b6c90c26e1a688d) --- updated-dependencies: - dependency-name: rust-vmm-ci dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- rust-vmm-ci | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-vmm-ci b/rust-vmm-ci index d023262..d216a46 160000 --- a/rust-vmm-ci +++ b/rust-vmm-ci @@ -1 +1 @@ -Subproject commit d023262164ef8766ac2b0be33de13ef52b8706a1 +Subproject commit d216a46879096a4f1e83af3a7b6c90c26e1a688d From 3cc37dada445623a60ca05891d56c28f05eef6e1 Mon Sep 17 00:00:00 2001 From: Sergii Glushchenko Date: Tue, 25 Jan 2022 10:13:02 +0100 Subject: [PATCH 4/4] Adjust test coverage Signed-off-by: Sergii Glushchenko --- coverage_config_x86_64.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coverage_config_x86_64.json b/coverage_config_x86_64.json index 25e5d8f..58619aa 100644 --- a/coverage_config_x86_64.json +++ b/coverage_config_x86_64.json @@ -1,5 +1,5 @@ { - "coverage_score": 85.7, + "coverage_score": 85.5, "exclude_path": "", "crate_features": "" }