From 3f6d938f2d037e606aa34451b5f91f6c2286713e Mon Sep 17 00:00:00 2001 From: chenjiayi Date: Tue, 7 Nov 2023 16:24:59 +0800 Subject: [PATCH 1/8] fix(docs): fix incorrect script directory path after place moving Automatically recognize the directory path that contains the install_devmaster.sh script. --- .../install_devmaster.sh" | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git "a/docs/use/devmaster\346\233\277\344\273\243udev\350\277\220\350\241\214/install_devmaster.sh" "b/docs/use/devmaster\346\233\277\344\273\243udev\350\277\220\350\241\214/install_devmaster.sh" index 9edef609..d9bed1ab 100644 --- "a/docs/use/devmaster\346\233\277\344\273\243udev\350\277\220\350\241\214/install_devmaster.sh" +++ "b/docs/use/devmaster\346\233\277\344\273\243udev\350\277\220\350\241\214/install_devmaster.sh" @@ -7,7 +7,7 @@ target_dir=${pwd}/target/${mode} services=(devctl-trigger.service devmaster.service devmaster-simu-udev.service) tools=(ata_id) -run_with_devmaster=tools/run_with_devmaster +run_with_devmaster=${0%/*} service_dir=${run_with_devmaster}/service lib_rules_dir=${run_with_devmaster}/rules.d -- Gitee From 1854092105d86caf418013e4aa7f5ca477b1cc9b Mon Sep 17 00:00:00 2001 From: chenjiayi Date: Wed, 8 Nov 2023 02:09:09 +0800 Subject: [PATCH 2/8] fix(basic): complete feature dependencies uuid depends on random. random depends on io. Also add get_errno method for basic Error. --- libs/basic/Cargo.toml | 4 ++-- libs/basic/src/error.rs | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/libs/basic/Cargo.toml b/libs/basic/Cargo.toml index f7c84157..654c1a3b 100644 --- a/libs/basic/Cargo.toml +++ b/libs/basic/Cargo.toml @@ -111,9 +111,9 @@ string = [] sysfs = ["nix/dir"] unistd = ["nix/user"] unit_name = [] -uuid = ["bitflags"] +uuid = ["bitflags", "random"] murmurhash2 = [] strbuf = [] argv = [] exec_util = [] -random = [] +random = ["io"] diff --git a/libs/basic/src/error.rs b/libs/basic/src/error.rs index aead8ce7..d138f477 100644 --- a/libs/basic/src/error.rs +++ b/libs/basic/src/error.rs @@ -70,6 +70,42 @@ pub enum Error { Other { msg: String }, } +impl Error { + /// Translate the basic error to error number. + pub fn get_errno(&self) -> i32 { + match self { + Self::Syscall { + syscall: _, + ret: _, + errno, + } => *errno, + Error::Io { source } => source.raw_os_error().unwrap_or_default(), + Error::Caps { what: _ } => nix::errno::Errno::EINVAL as i32, + Error::Nix { source } => *source as i32, + Error::Var { source } => { + (match source { + std::env::VarError::NotPresent => nix::errno::Errno::ENOENT, + std::env::VarError::NotUnicode(_) => nix::errno::Errno::EINVAL, + }) as i32 + } + Error::Proc { source } => match source { + procfs::ProcError::Incomplete(_) => nix::errno::Errno::EINVAL as i32, + procfs::ProcError::PermissionDenied(_) => nix::errno::Errno::EPERM as i32, + procfs::ProcError::NotFound(_) => nix::errno::Errno::ENOENT as i32, + procfs::ProcError::Io(_, _) => nix::errno::Errno::EIO as i32, + procfs::ProcError::Other(_) => nix::errno::Errno::EINVAL as i32, + procfs::ProcError::InternalError(_) => nix::errno::Errno::EINVAL as i32, + }, + Error::NulError { source: _ } => nix::errno::Errno::EINVAL as i32, + Error::Parse { source: _ } => nix::errno::Errno::EINVAL as i32, + Error::ParseNamingScheme { what: _ } => nix::errno::Errno::EINVAL as i32, + Error::NotExisted { what: _ } => nix::errno::Errno::ENOENT as i32, + Error::Invalid { what: _ } => nix::errno::Errno::EINVAL as i32, + Error::Other { msg: _ } => nix::errno::Errno::EINVAL as i32, + } + } +} + #[allow(unused_macros)] macro_rules! errfrom { ($($st:ty),* => $variant:ident) => ( -- Gitee From 6ea342419c89d2b06daca296262b9655c5f28c07 Mon Sep 17 00:00:00 2001 From: chenjiayi Date: Wed, 8 Nov 2023 02:10:37 +0800 Subject: [PATCH 3/8] fix(device): replenish feature dependency of uuid on basic crate uuid is used on trigger_with_uuid method. --- libs/device/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/device/Cargo.toml b/libs/device/Cargo.toml index 6af6a404..ecb9955c 100644 --- a/libs/device/Cargo.toml +++ b/libs/device/Cargo.toml @@ -12,6 +12,7 @@ basic = { path = "../basic", default-features = false, features = [ "fs", "fd", "murmurhash2", + "uuid", ] } event = { path = "../event" } log = { path = "../log" } -- Gitee From a4294bc30e46988b6df9d95f02a01ff10ffd218c Mon Sep 17 00:00:00 2001 From: chenjiayi Date: Wed, 8 Nov 2023 02:11:59 +0800 Subject: [PATCH 4/8] test(device): refactor and increase test line coverage --- exts/devmaster/src/lib/rules/exec_mgr.rs | 13 +- exts/devmaster/src/lib/utils/commons.rs | 9 +- libs/device/src/device.rs | 909 ++++++++++++++++------- libs/device/src/device_monitor.rs | 4 +- libs/device/src/error.rs | 30 +- libs/device/src/utils.rs | 2 + 6 files changed, 651 insertions(+), 316 deletions(-) diff --git a/exts/devmaster/src/lib/rules/exec_mgr.rs b/exts/devmaster/src/lib/rules/exec_mgr.rs index bb8ed012..66c1a06c 100644 --- a/exts/devmaster/src/lib/rules/exec_mgr.rs +++ b/exts/devmaster/src/lib/rules/exec_mgr.rs @@ -150,13 +150,7 @@ impl ExecuteManager { // copy all tags to cloned device for tag in &device.borrow().tag_iter() { - device_db_clone - .borrow() - .add_tag(tag, false) - .map_err(|e| Error::RulesExecuteError { - msg: format!("failed to add tag ({})", e), - errno: e.get_errno(), - })?; + device_db_clone.borrow().add_tag(tag, false); } // add property to cloned device @@ -1539,10 +1533,7 @@ impl ExecuteManager { if token.read().unwrap().as_ref().unwrap().op == OperatorType::Remove { device.borrow().remove_tag(&value); } else { - execute_err!( - token.read().unwrap().as_ref().unwrap(), - device.borrow().add_tag(&value, true) - )?; + device.borrow().add_tag(&value, true); } Ok(true) diff --git a/exts/devmaster/src/lib/utils/commons.rs b/exts/devmaster/src/lib/utils/commons.rs index 5b465710..1162a9ef 100644 --- a/exts/devmaster/src/lib/utils/commons.rs +++ b/exts/devmaster/src/lib/utils/commons.rs @@ -541,14 +541,7 @@ pub(crate) fn initialize_device_usec( .map_or(0, |v| v.as_secs()) }); - dev_new - .borrow() - .set_usec_initialized(timestamp) - .map_err(|e| { - log::error!("failed to set initialization timestamp: {}", e); - e - }) - .context(DeviceSnafu)?; + dev_new.borrow().set_usec_initialized(timestamp); Ok(()) } diff --git a/libs/device/src/device.rs b/libs/device/src/device.rs index d6a4dc07..6673d823 100644 --- a/libs/device/src/device.rs +++ b/libs/device/src/device.rs @@ -41,10 +41,12 @@ use std::path::Path; use std::rc::Rc; use std::result::Result; +/// default directory to contain runtime temporary files +pub const DEFAULT_BASE_DIR: &str = "/run/devmaster"; /// database directory path -pub const DB_BASE_DIR: &str = "/run/devmaster/data/"; +pub const DB_BASE_DIR: &str = "data"; /// tags directory path -pub const TAGS_BASE_DIR: &str = "/run/devmaster/tags/"; +pub const TAGS_BASE_DIR: &str = "tags"; /// Device #[derive(Debug, Clone)] @@ -147,6 +149,9 @@ pub struct Device { pub sealed: RefCell, /// persist device db during switching root from initrd pub db_persist: RefCell, + + /// the base directory path to contain runtime temporary files + pub base_path: RefCell, } impl Default for Device { @@ -206,9 +211,15 @@ impl Device { children: RefCell::new(HashMap::new()), children_enumerated: RefCell::new(false), sysattrs_cached: RefCell::new(false), + base_path: RefCell::new(DEFAULT_BASE_DIR.to_string()), } } + /// change db prefix + pub fn set_base_path(&self, prefix: &str) { + self.base_path.replace(prefix.to_string()); + } + /// create Device from buffer pub fn from_nulstr(nulstr: &[u8]) -> Result { let device = Device::new(); @@ -297,8 +308,9 @@ impl Device { Ok(device) } - /// create a Device instance from path - /// path falls into two kinds: devname (/dev/...) and syspath (/sys/devices/...) + /// Create a Device instance from path. + /// + /// The path falls into two kinds: devname (/dev/...) and syspath (/sys/devices/...) pub fn from_path(path: &str) -> Result { if path.starts_with("/dev") { return Device::from_devname(path); @@ -726,9 +738,9 @@ impl Device { }; if !filename.is_empty() { - self.set_subsystem(&filename)?; + self.set_subsystem(&filename); } else if self.devpath.borrow().starts_with("/module/") { - self.set_subsystem("module")?; + self.set_subsystem("module"); } else if self.devpath.borrow().contains("/drivers/") || self.devpath.borrow().contains("/drivers") { @@ -736,7 +748,7 @@ impl Device { } else if self.devpath.borrow().starts_with("/class/") || self.devpath.borrow().starts_with("/bus/") { - self.set_subsystem("subsystem")?; + self.set_subsystem("subsystem"); } else { self.subsystem_set.replace(true); } @@ -828,10 +840,7 @@ impl Device { }; // if the device has no driver, clear it from internal property - self.set_driver(&driver).map_err(|e| Error::Nix { - msg: format!("get_driver failed: {}", e), - source: e.get_errno(), - })?; + self.set_driver(&driver); } if self.driver.borrow().is_empty() { @@ -846,12 +855,7 @@ impl Device { /// get device name pub fn get_devname(&self) -> Result { - match self.read_uevent_file() { - Ok(_) => {} - Err(e) => { - return Err(e); - } - } + self.read_uevent_file()?; if self.devname.borrow().is_empty() { return Err(Error::Nix { @@ -1364,9 +1368,9 @@ impl Device { } /// add tag to the device object - pub fn add_tag(&self, tag: &str, both: bool) -> Result<(), Error> { + pub fn add_tag(&self, tag: &str, both: bool) { if tag.trim().is_empty() { - return Ok(()); + return; } self.all_tags.borrow_mut().insert(tag.trim().to_string()); @@ -1377,16 +1381,13 @@ impl Device { .insert(tag.trim().to_string()); } self.property_tags_outdated.replace(true); - Ok(()) } /// add a set of tags, separated by ':' - pub fn add_tags(&self, tags: &str, both: bool) -> Result<(), Error> { + pub fn add_tags(&self, tags: &str, both: bool) { for tag in tags.split(':') { - self.add_tag(tag, both)?; + self.add_tag(tag, both); } - - Ok(()) } /// remove specific tag @@ -1420,8 +1421,22 @@ impl Device { Ok(*self.devlink_priority.borrow()) } - /// get the device id - /// device id is used to identify database file in /run/devmaster/data/ + /// Get the device id. + /// + /// Device id is used to identify database file in /run/devmaster/data/. + /// + /// The format is like: + /// + /// character device: c: + /// + /// block device: b: + /// + /// network interface: n + /// + /// drivers: +drivers:: + /// + /// other subsystems: +: + /// pub fn get_device_id(&self) -> Result { if self.device_id.borrow().is_empty() { let subsystem = self.get_subsystem().map_err(|e| Error::Nix { @@ -1535,25 +1550,25 @@ impl Device { } /// set the initialized timestamp - pub fn set_usec_initialized(&self, time: u64) -> Result<(), Error> { - self.add_property_internal("USEC_INITIALIZED", &time.to_string())?; + pub fn set_usec_initialized(&self, time: u64) { + self.add_property_internal("USEC_INITIALIZED", &time.to_string()) + .unwrap(); self.usec_initialized.replace(time); - Ok(()) + } + + #[inline] + fn cleanup(db: &str, tmp_file: &str) { + let _ = unlink(db); + let _ = unlink(tmp_file); } /// update device database pub fn update_db(&self) -> Result<(), Error> { - #[inline] - fn cleanup(db: &str, tmp_file: &str) { - let _ = unlink(db); - let _ = unlink(tmp_file); - } - let has_info = self.has_info(); let id = self.get_device_id()?; - let db_path = format!("{}{}", DB_BASE_DIR, id); + let db_path = format!("{}/{}/{}", self.base_path.borrow(), DB_BASE_DIR, id); if !has_info && *self.devnum.borrow() == 0 && *self.ifindex.borrow() == 0 { unlink(db_path.as_str()).map_err(|e| Error::Nix { @@ -1564,14 +1579,19 @@ impl Device { return Ok(()); } - create_dir_all(DB_BASE_DIR).map_err(|e| Error::Nix { - msg: "update_db failed: can't create db directory".to_string(), - source: e - .raw_os_error() - .map_or_else(|| nix::Error::EIO, nix::Error::from_i32), + create_dir_all(&format!("{}/{}", self.base_path.borrow(), DB_BASE_DIR)).map_err(|e| { + Error::Nix { + msg: "update_db failed: can't create db directory".to_string(), + source: e + .raw_os_error() + .map_or_else(|| nix::Error::EIO, nix::Error::from_i32), + } })?; - if let Err(e) = chmod(DB_BASE_DIR, 0o750) { + if let Err(e) = chmod( + &format!("{}/{}", self.base_path.borrow(), DB_BASE_DIR), + 0o750, + ) { log::error!("Failed to set permission for /run/devmaster/data/: {}", e); } @@ -1586,6 +1606,21 @@ impl Device { } })?; + self.atomic_create_db(&mut file, tmp_file.as_str(), db_path.as_str()) + .map_err(|e| { + Self::cleanup(&db_path, &tmp_file); + e + })?; + + Ok(()) + } + + fn atomic_create_db( + &self, + file: &mut File, + tmp_file: &str, + db_path: &str, + ) -> Result<(), Error> { fchmod( file.as_raw_fd(), if *self.db_persist.borrow() { @@ -1594,135 +1629,72 @@ impl Device { Mode::from_bits(0o640).unwrap() }, ) - .map_err(|e| { - cleanup(&db_path, &tmp_file); - Error::Nix { - msg: "update_db failed: can't change the mode of temporary file".to_string(), - source: e, - } + .context(Nix { + msg: "update_db failed: can't change the mode of temporary file".to_string(), })?; - if has_info { + if self.has_info() { if *self.devnum.borrow() > 0 { for link in self.devlinks.borrow().iter() { file.write(format!("S:{}\n", link.strip_prefix("/dev/").unwrap()).as_bytes()) - .map_err(|e| { - cleanup(&db_path, &tmp_file); - Error::Nix { - msg: format!( - "update_db failed: can't write devlink '{}' to db", - link - ), - source: e - .raw_os_error() - .map(nix::Error::from_i32) - .unwrap_or(nix::Error::EIO), - } + .context(Io { + msg: format!("update_db failed: can't write devlink '{}' to db", link), })?; } if *self.devlink_priority.borrow() != 0 { file.write(format!("L:{}\n", self.devlink_priority.borrow()).as_bytes()) - .map_err(|e| { - cleanup(&db_path, &tmp_file); - Error::Nix { - msg: format!( - "update_db failed: can't write devlink priority '{}' to db", - *self.devlink_priority.borrow() - ), - source: e - .raw_os_error() - .map(nix::Error::from_i32) - .unwrap_or(nix::Error::EIO), - } + .context(Io { + msg: format!( + "update_db failed: can't write devlink priority '{}' to db", + *self.devlink_priority.borrow() + ), })?; } } if *self.usec_initialized.borrow() > 0 { file.write(format!("I:{}\n", self.usec_initialized.borrow()).as_bytes()) - .map_err(|e| { - cleanup(&db_path, &tmp_file); - Error::Nix { - msg: format!( - "update_db failed: can't write initial usec '{}' to db", - *self.usec_initialized.borrow() - ), - source: e - .raw_os_error() - .map(nix::Error::from_i32) - .unwrap_or(nix::Error::EIO), - } + .context(Io { + msg: format!( + "update_db failed: can't write initial usec '{}' to db", + *self.usec_initialized.borrow() + ), })?; } for (k, v) in self.properties_db.borrow().iter() { file.write(format!("E:{}={}\n", k, v).as_bytes()) - .map_err(|e| { - cleanup(&db_path, &tmp_file); - Error::Nix { - msg: format!( - "update_db failed: can't write property '{}'='{}' to db", - k, v - ), - source: e - .raw_os_error() - .map(nix::Error::from_i32) - .unwrap_or(nix::Error::EIO), - } + .context(Io { + msg: format!( + "update_db failed: can't write property '{}'='{}' to db", + k, v + ), })?; } for tag in self.all_tags.borrow().iter() { - file.write(format!("G:{}\n", tag).as_bytes()).map_err(|e| { - cleanup(&db_path, &tmp_file); - Error::Nix { - msg: "update_db failed: can't write tag '{}' to db".to_string(), - source: e - .raw_os_error() - .map(nix::Error::from_i32) - .unwrap_or(nix::Error::EIO), - } + file.write(format!("G:{}\n", tag).as_bytes()).context(Io { + msg: "update_db failed: can't write tag '{}' to db".to_string(), })?; } for tag in self.current_tags.borrow().iter() { - file.write(format!("Q:{}\n", tag).as_bytes()).map_err(|e| { - cleanup(&db_path, &tmp_file); - Error::Nix { - msg: format!( - "update_db failed: failed to write current tag '{}' to db", - tag - ), - source: e - .raw_os_error() - .map(nix::Error::from_i32) - .unwrap_or(nix::Error::EIO), - } + file.write(format!("Q:{}\n", tag).as_bytes()).context(Io { + msg: format!( + "update_db failed: failed to write current tag '{}' to db", + tag + ), })?; } } - file.flush().map_err(|e| { - cleanup(&db_path, &tmp_file); - Error::Nix { - msg: "update_db failed: can't flush db".to_string(), - source: e - .raw_os_error() - .map(nix::Error::from_i32) - .unwrap_or(nix::Error::EIO), - } + file.flush().context(Io { + msg: "update_db failed: can't flush db".to_string(), })?; - rename(&tmp_file, &db_path).map_err(|e| { - cleanup(&db_path, &tmp_file); - Error::Nix { - msg: "update_db failed: can't rename temporary file".to_string(), - source: e - .raw_os_error() - .map(nix::Error::from_i32) - .unwrap_or(nix::Error::EIO), - } + rename(tmp_file, &db_path).context(Io { + msg: "update_db failed: can't rename temporary file".to_string(), })?; Ok(()) @@ -1732,7 +1704,13 @@ impl Device { pub fn update_tag(&self, tag: &str, add: bool) -> Result<(), Error> { let id = self.get_device_id()?; - let tag_path = format!("{}{}/{}", TAGS_BASE_DIR, tag, id); + let tag_path = format!( + "{}/{}/{}/{}", + self.base_path.borrow(), + TAGS_BASE_DIR, + tag, + id + ); if add { touch_file(&tag_path, true, Some(0o444), None, None).map_err(|e| Error::Nix { @@ -1740,14 +1718,24 @@ impl Device { source: nix::Error::EINVAL, })?; - if let Err(e) = chmod(TAGS_BASE_DIR, 0o750) { - log::error!("Failed to set permission for {}: {}", TAGS_BASE_DIR, e); + if let Err(e) = chmod( + &format!("{}/{}", self.base_path.borrow(), TAGS_BASE_DIR), + 0o750, + ) { + log::error!( + "Failed to set permission for {}: {}", + format!("{}/{}", self.base_path.borrow(), TAGS_BASE_DIR), + e + ); } - if let Err(e) = chmod(&format!("{}{}", TAGS_BASE_DIR, tag), 0o750) { + if let Err(e) = chmod( + &format!("{}/{}/{}", self.base_path.borrow(), TAGS_BASE_DIR, tag), + 0o750, + ) { log::error!( "Failed to set permission for {}: {}", - format!("{}{}", TAGS_BASE_DIR, tag), + format!("{}/{}/{}", self.base_path.borrow(), TAGS_BASE_DIR, tag), e ); } @@ -1794,7 +1782,7 @@ impl Device { source: e.get_errno(), })?; - let path = format!("{}{}", DB_BASE_DIR, id); + let path = format!("{}/{}/{}", self.base_path.borrow(), DB_BASE_DIR, id); self.read_db_internal_filename(&path) .map_err(|e| Error::Nix { @@ -1917,15 +1905,8 @@ impl Device { // refuse going down into /sys/fs/cgroup/ or similar places // where things are not arranged as kobjects in kernel - match path.as_os_str().to_str() { - Some(s) => s.to_string(), - None => { - return Err(Error::Nix { - msg: format!("set_syspath failed: '{:?}' can not change to string", path), - source: Errno::EINVAL, - }); - } - } + /* The path is validated before, thus can directly be unwrapped from os str. */ + path.as_os_str().to_str().unwrap().to_string() } else { if !path.starts_with("/sys/") { return Err(Error::Nix { @@ -1937,15 +1918,8 @@ impl Device { path.to_string() }; - let devpath = match p.strip_prefix("/sys") { - Some(p) => p, - None => { - return Err(Error::Nix { - msg: format!("set_syspath failed: '{}' does not start with /sys", p), - source: Errno::EINVAL, - }); - } - }; + /* The syspath is already validated to start with /sys. */ + let devpath = p.strip_prefix("/sys").unwrap(); if !devpath.starts_with('/') { return Err(Error::Nix { @@ -1957,15 +1931,9 @@ impl Device { }); } - match self.add_property_internal("DEVPATH", devpath) { - Ok(_) => {} - Err(e) => { - return Err(Error::Nix { - msg: format!("set_syspath failed: {}", e), - source: Errno::ENODEV, - }) - } - } + /* The key 'DEVPATH' is not empty, definitely be ok. */ + self.add_property_internal("DEVPATH", devpath).unwrap(); + self.devpath.replace(devpath.to_string()); self.syspath.replace(p); @@ -2074,11 +2042,10 @@ impl Device { } /// set subsystem - pub fn set_subsystem(&self, subsystem: &str) -> Result<(), Error> { - self.add_property_internal("SUBSYSTEM", subsystem)?; + pub fn set_subsystem(&self, subsystem: &str) { + self.add_property_internal("SUBSYSTEM", subsystem).unwrap(); self.subsystem_set.replace(true); self.subsystem.replace(subsystem.to_string()); - Ok(()) } /// set drivers subsystem @@ -2104,7 +2071,7 @@ impl Device { }); } - self.set_subsystem("drivers")?; + self.set_subsystem("drivers"); self.driver_subsystem.replace(subsystem); Ok(()) @@ -2120,24 +2087,18 @@ impl Device { let mut file = match fs::OpenOptions::new().read(true).open(uevent_file) { Ok(f) => f, - Err(e) => match e.raw_os_error() { - Some(n) => { - if [libc::EACCES, libc::ENODEV, libc::ENXIO, libc::ENOENT].contains(&n) { - // the uevent file may be write-only, or the device may be already removed or the device has no uevent file - return Ok(()); - } - return Err(Error::Nix { - msg: "read_uevent_file failed: can't open uevent file".to_string(), - source: Errno::from_i32(n), - }); - } - None => { - return Err(Error::Nix { - msg: "read_uevent_file failed: can't open uevent file".to_string(), - source: Errno::EINVAL, - }); + Err(e) => { + let n = e.raw_os_error().unwrap_or_default(); + + if [libc::EACCES, libc::ENODEV, libc::ENXIO, libc::ENOENT].contains(&n) { + // the uevent file may be write-only, or the device may be already removed or the device has no uevent file + return Ok(()); } - }, + return Err(Error::Nix { + msg: "read_uevent_file failed: can't open uevent file".to_string(), + source: Errno::from_i32(n), + }); + } }; let mut buf = String::new(); @@ -2177,15 +2138,14 @@ impl Device { } /// set devtype - pub fn set_devtype(&self, devtype: &str) -> Result<(), Error> { - self.add_property_internal("DEVTYPE", devtype)?; + pub fn set_devtype(&self, devtype: &str) { + self.add_property_internal("DEVTYPE", devtype).unwrap(); self.devtype.replace(devtype.to_string()); - Ok(()) } /// set ifindex pub fn set_ifindex(&self, ifindex: &str) -> Result<(), Error> { - self.add_property_internal("IFINDEX", ifindex)?; + self.add_property_internal("IFINDEX", ifindex).unwrap(); self.ifindex.replace(match ifindex.parse::() { Ok(idx) => idx, Err(e) => { @@ -2199,16 +2159,15 @@ impl Device { } /// set devname - pub fn set_devname(&self, devname: &str) -> Result<(), Error> { + pub fn set_devname(&self, devname: &str) { let devname = if devname.starts_with('/') { devname.to_string() } else { format!("/dev/{}", devname) }; - self.add_property_internal("DEVNAME", &devname)?; + self.add_property_internal("DEVNAME", &devname).unwrap(); self.devname.replace(devname); - Ok(()) } /// set devmode @@ -2223,7 +2182,7 @@ impl Device { self.devmode.replace(m); - self.add_property_internal("DEVMODE", devmode)?; + self.add_property_internal("DEVMODE", devmode).unwrap(); Ok(()) } @@ -2251,7 +2210,7 @@ impl Device { self.devgid.replace(Some(Gid::from_raw(gid))); - self.add_property_internal("DEVGID", devgid)?; + self.add_property_internal("DEVGID", devgid).unwrap(); Ok(()) } @@ -2277,8 +2236,8 @@ impl Device { } }; - self.add_property_internal("MAJOR", major)?; - self.add_property_internal("MINOR", minor)?; + self.add_property_internal("MAJOR", major).unwrap(); + self.add_property_internal("MINOR", minor).unwrap(); self.devnum.replace(makedev(major_num, minor_num)); Ok(()) @@ -2286,7 +2245,7 @@ impl Device { /// set diskseq pub fn set_diskseq(&self, diskseq: &str) -> Result<(), Error> { - self.add_property_internal("DISKSEQ", diskseq)?; + self.add_property_internal("DISKSEQ", diskseq).unwrap(); let diskseq_num: u64 = match diskseq.parse() { Ok(n) => n, @@ -2304,10 +2263,10 @@ impl Device { } /// set action - pub fn set_action(&self, action: DeviceAction) -> Result<(), Error> { - self.add_property_internal("ACTION", &action.to_string())?; + pub fn set_action(&self, action: DeviceAction) { + self.add_property_internal("ACTION", &action.to_string()) + .unwrap(); self.action.replace(action); - Ok(()) } /// set action from string @@ -2325,7 +2284,9 @@ impl Device { } }; - self.set_action(action) + self.set_action(action); + + Ok(()) } /// set seqnum from string @@ -2343,22 +2304,22 @@ impl Device { } }; - self.set_seqnum(seqnum) + self.set_seqnum(seqnum); + Ok(()) } /// set seqnum - pub fn set_seqnum(&self, seqnum: u64) -> Result<(), Error> { - self.add_property_internal("SEQNUM", &seqnum.to_string())?; + pub fn set_seqnum(&self, seqnum: u64) { + self.add_property_internal("SEQNUM", &seqnum.to_string()) + .unwrap(); self.seqnum.replace(seqnum); - Ok(()) } /// set driver - pub fn set_driver(&self, driver: &str) -> Result<(), Error> { - self.add_property_internal("DRIVER", driver)?; + pub fn set_driver(&self, driver: &str) { + self.add_property_internal("DRIVER", driver).unwrap(); self.driver_set.replace(true); self.driver.replace(driver.to_string()); - Ok(()) } /// cache sysattr value @@ -2502,11 +2463,7 @@ impl Device { }; if !devlinks.is_empty() { - self.add_property_internal("DEVLINKS", &devlinks) - .map_err(|e| Error::Nix { - msg: format!("properties_prepare failed: {}", e), - source: e.get_errno(), - })?; + self.add_property_internal("DEVLINKS", &devlinks).unwrap(); self.property_devlinks_outdated.replace(false); } @@ -2521,11 +2478,7 @@ impl Device { if !all_tags.is_empty() { all_tags.push(':'); - self.add_property_internal("TAGS", &all_tags) - .map_err(|e| Error::Nix { - msg: format!("properties_prepare failed: {}", e), - source: e.get_errno(), - })?; + self.add_property_internal("TAGS", &all_tags).unwrap(); } let mut current_tags: String = { @@ -2536,10 +2489,7 @@ impl Device { if !current_tags.is_empty() { current_tags.push(':'); self.add_property_internal("CURRENT_TAGS", ¤t_tags) - .map_err(|e| Error::Nix { - msg: format!("properties_prepare failed: {}", e), - source: e.get_errno(), - })?; + .unwrap(); } self.property_tags_outdated.replace(false); @@ -2552,29 +2502,19 @@ impl Device { pub fn read_db_internal_filename(&self, filename: &str) -> Result<(), Error> { let mut file = match fs::OpenOptions::new().read(true).open(filename) { Ok(f) => f, - Err(e) => match e.raw_os_error() { - Some(n) => { - if n == libc::ENOENT { - return Ok(()); - } - return Err(Error::Nix { - msg: format!( - "read_db_internal_filename failed: can't open db '{}': {}", - filename, e - ), - source: Errno::from_i32(n), - }); - } - None => { - return Err(Error::Nix { - msg: format!( - "read_db_internal_filename failed: can't open db '{}': {}", - filename, e - ), - source: Errno::EINVAL, - }); + Err(e) => { + let n = e.raw_os_error().unwrap_or_default(); + if n == libc::ENOENT { + return Ok(()); } - }, + return Err(Error::Nix { + msg: format!( + "read_db_internal_filename failed: can't open db '{}': {}", + filename, e + ), + source: Errno::from_i32(n), + }); + } }; let mut buf = String::new(); @@ -2613,17 +2553,10 @@ impl Device { pub fn handle_db_line(&self, key: &str, value: &str) -> Result<(), Error> { match key { "G" | "Q" => { - self.add_tag(value, key == "Q").map_err(|e| Error::Nix { - msg: format!("handle_db_line failed: failed to add_tag: {}", e), - source: e.get_errno(), - })?; + self.add_tag(value, key == "Q"); } "S" => { - self.add_devlink(&format!("/dev/{}", value)) - .map_err(|e| Error::Nix { - msg: format!("handle_db_line failed: failed to add_devlink: {}", e), - source: e.get_errno(), - })?; + self.add_devlink(&format!("/dev/{}", value)).unwrap(); } "E" => { let tokens: Vec<_> = value.split('=').collect(); @@ -2653,10 +2586,7 @@ impl Device { source: Errno::EINVAL, })?; - self.set_usec_initialized(time).map_err(|e| Error::Nix { - msg: format!("handle_db_line failed: {}", e), - source: Errno::EINVAL, - })?; + self.set_usec_initialized(time); } "L" => { let priority = value.parse::().map_err(|e| Error::Nix { @@ -2712,10 +2642,7 @@ impl Device { source: e.get_errno(), })?; - device.set_subsystem(&subsystem).map_err(|e| Error::Nix { - msg: format!("shallow_clone failed: {}", e), - source: e.get_errno(), - })?; + device.set_subsystem(&subsystem); if subsystem == "drivers" { device @@ -2751,11 +2678,11 @@ impl Device { match key { "DEVPATH" => self.set_syspath(&format!("/sys{}", value), false)?, "ACTION" => self.set_action_from_string(value)?, - "SUBSYSTEM" => self.set_subsystem(value)?, - "DEVTYPE" => self.set_devtype(value)?, - "DEVNAME" => self.set_devname(value)?, + "SUBSYSTEM" => self.set_subsystem(value), + "DEVTYPE" => self.set_devtype(value), + "DEVNAME" => self.set_devname(value), "SEQNUM" => self.set_seqnum_from_string(value)?, - "DRIVER" => self.set_driver(value)?, + "DRIVER" => self.set_driver(value), "IFINDEX" => self.set_ifindex(value)?, "USEC_INITIALIZED" => { self.set_usec_initialized(value.parse::().map_err(|e| Error::Nix { @@ -2764,14 +2691,14 @@ impl Device { value, e ), source: Errno::EINVAL, - })?)? + })?); } "DEVMODE" => self.set_devmode(value)?, "DEVUID" => self.set_devuid(value)?, "DEVGID" => self.set_devgid(value)?, "DISKSEQ" => self.set_diskseq(value)?, "DEVLINKS" => self.add_devlinks(value)?, - "TAGS" | "CURRENT_TAGS" => self.add_tags(value, key == "CURRENT_TAGS")?, + "TAGS" | "CURRENT_TAGS" => self.add_tags(value, key == "CURRENT_TAGS"), _ => self.add_property_internal(key, value)?, } @@ -3090,7 +3017,7 @@ impl Device { log::debug!( "failed to read db of '{}': {}", self.get_device_id() - .unwrap_or_else(|_| self.devpath.borrow().clone()), + .unwrap_or(self.devpath.borrow().clone()), e ) } @@ -3106,7 +3033,7 @@ impl Device { log::error!( "failed to read db of '{}': {}", self.get_device_id() - .unwrap_or_else(|_| self.devpath.borrow().clone()), + .unwrap_or(self.devpath.borrow().clone()), e ) } @@ -3122,7 +3049,7 @@ impl Device { log::debug!( "failed to read db of '{}': {}", self.get_device_id() - .unwrap_or_else(|_| self.devpath.borrow().clone()), + .unwrap_or(self.devpath.borrow().clone()), e ) } @@ -3138,7 +3065,7 @@ impl Device { log::debug!( "failed to prepare properties of '{}': {}", self.get_device_id() - .unwrap_or_else(|_| self.devpath.borrow().clone()), + .unwrap_or(self.devpath.borrow().clone()), e ) } @@ -3154,7 +3081,7 @@ impl Device { log::debug!( "failed to enumerate children of '{}': {}", self.get_device_id() - .unwrap_or_else(|_| self.devpath.borrow().clone()), + .unwrap_or(self.devpath.borrow().clone()), e ) } @@ -3170,8 +3097,7 @@ impl Device { if let Err(e) = self.read_all_sysattrs() { log::debug!( "{}: failed to read all sysattrs: {}", - self.get_sysname() - .unwrap_or_else(|_| self.devpath.borrow().clone()), + self.get_sysname().unwrap_or(self.devpath.borrow().clone()), e ); } @@ -3183,15 +3109,30 @@ impl Device { } } +impl PartialEq for Device { + fn eq(&self, other: &Self) -> bool { + self.get_syspath().unwrap_or_default() == other.get_syspath().unwrap_or_default() + } +} + #[cfg(test)] mod tests { + use std::panic::catch_unwind; + use crate::{ device::*, device_enumerator::{DeviceEnumerationType, DeviceEnumerator}, utils::LoopDev, }; + use basic::IN_SET; use libc::S_IFBLK; + fn compare(dev1: &Device, dev2: &Device) -> bool { + let syspath_1 = dev1.get_syspath().unwrap(); + let syspath_2 = dev2.get_syspath().unwrap(); + syspath_1 == syspath_2 + } + /// test a single device fn test_device_one(device: &mut Device) { let syspath = device.get_syspath().unwrap(); @@ -3257,7 +3198,14 @@ mod tests { } if device.get_is_initialized().unwrap() { - // test get_usec_since_initialized: todo + match device.get_usec_initialized() { + Ok(usec) => { + assert!(usec > 0); + } + Err(e) => { + assert_eq!(e.get_errno(), nix::Error::ENODATA); + } + } } match device.get_property_value("ID_NET_DRIVER") { @@ -3266,6 +3214,8 @@ mod tests { assert_eq!(e.get_errno(), Errno::ENOENT); } } + + let _ = device.get_parent_with_subsystem_devtype("usb", Some("usb_interface")); } } Err(e) => { @@ -3309,6 +3259,9 @@ mod tests { assert!(basic::error::errno_is_privilege(e.get_errno())); } } + + let dev2 = Device::from_path(&device.get_syspath().unwrap()).unwrap(); + assert!(compare(&device_new, &dev2)); } Err(e) => { assert!( @@ -3399,6 +3352,82 @@ mod tests { } } + if let Err(e) = device.get_diskseq() { + assert_eq!(e.get_errno(), errno::Errno::ENOENT); + } + + if let Err(e) = device.get_seqnum() { + assert_eq!(e.get_errno(), errno::Errno::ENOENT); + } + + if let Err(e) = device.get_trigger_uuid() { + assert_eq!(e.get_errno(), nix::errno::Errno::ENOENT); + } + + match device.get_devname() { + Ok(devname) => { + let st = nix::sys::stat::stat(devname.as_str()).unwrap(); + let uid = st.st_uid; + let gid = st.st_gid; + let mode = st.st_mode; + + match device.get_devnode_uid() { + Ok(dev_uid) => { + assert_eq!(uid, dev_uid.as_raw()); + } + Err(e) => { + assert!(IN_SET!(e.get_errno(), nix::errno::Errno::ENOENT)); + } + } + + match device.get_devnode_gid() { + Ok(dev_gid) => { + assert_eq!(gid, dev_gid.as_raw()); + } + Err(e) => { + assert!(IN_SET!(e.get_errno(), nix::errno::Errno::ENOENT)); + } + } + + match device.get_devnode_mode() { + Ok(dev_mode) => { + assert_eq!(mode & 0o777, dev_mode); + } + Err(e) => { + assert!(IN_SET!(e.get_errno(), nix::errno::Errno::ENOENT)); + } + } + } + Err(e) => { + assert!(IN_SET!(e.get_errno(), nix::errno::Errno::ENOENT)); + } + } + + if let Err(e) = device.get_action() { + assert_eq!(e.get_errno(), nix::errno::Errno::ENOENT); + } + + let _shadow = device.shallow_clone().unwrap(); + let _db = device.clone_with_db().unwrap(); + + /* Test set and get devlink priority. */ + device.set_devlink_priority(10); + assert_eq!(10, device.get_devlink_priority().unwrap()); + + /* Test add devlinks */ + device.add_devlinks("/dev/test /dev/test1").unwrap(); + + assert_eq!( + device.add_devlink("/root/test").unwrap_err().get_errno(), + nix::Error::EINVAL + ); + assert_eq!( + device.add_devlink("/dev").unwrap_err().get_errno(), + nix::Error::EINVAL + ); + + device.update_db().unwrap(); + /* Test enumerating child devices */ for (subdir, child) in &device.child_iter() { let canoicalized_path = @@ -3414,7 +3443,51 @@ mod tests { let p = format!("{}/{}", syspath, sysattr); let path = Path::new(&p); assert!(path.exists()); + + let st = nix::sys::stat::stat(path).unwrap(); + if st.st_mode & S_IWUSR == 0 { + assert!(device + .set_sysattr_value(sysattr.as_str(), Some("")) + .is_err()); + } + + let _value = device.get_sysattr_value(sysattr.as_str()); + } + + /* Test iterators */ + for tag in &device.tag_iter() { + assert!(device.has_tag(tag.as_str()).unwrap()); + } + + for tag in &device.current_tag_iter() { + assert!(device.has_current_tag(tag.as_str()).unwrap()); } + + for devlink in &device.devlink_iter() { + assert!(device.has_devlink(devlink.as_str())); + } + + for (k, v) in &device.property_iter() { + assert_eq!(&device.get_property_value(k.as_str()).unwrap(), v); + } + + device.cleanup_devlinks(); + device.cleanup_tags(); + + let db = device.get_device_id().unwrap(); + let _ = unlink(format!("/tmp/devmaster/data/{}", db).as_str()); + + /* Test open device node. */ + let _ = device.open(OFlag::O_RDONLY); + let _ = device.open(OFlag::O_WRONLY); + let _ = device.open(OFlag::O_RDWR); + let _ = device.open(OFlag::O_EXCL); + + /* Cover exceptional code branches. */ + let _ = device.tag_iter(); + let _ = device.current_tag_iter(); + let _ = device.devlink_iter(); + let _ = device.property_iter(); } #[test] @@ -3422,6 +3495,7 @@ mod tests { let mut enumerator = DeviceEnumerator::new(); enumerator.set_enumerator_type(DeviceEnumerationType::All); for device in enumerator.iter() { + device.borrow().set_base_path("/tmp/devmaster"); test_device_one(&mut device.as_ref().borrow_mut()); } } @@ -3494,12 +3568,15 @@ mod tests { fn test_device_tag_iterator() { #[inline] fn inner_test(dev: &mut Device) -> Result<(), Error> { - dev.add_tag("test_tag", true).unwrap(); + dev.add_tag("test_tag", true); + + let mut all_tags = HashSet::new(); for tag in &dev.tag_iter() { - assert_eq!(tag, "test_tag"); + all_tags.insert(tag.clone()); } + assert!(all_tags.contains("test_tag")); Ok(()) } @@ -3596,14 +3673,14 @@ mod tests { #[inline] fn inner_test(dev: &mut Device) -> Result<(), Error> { dev.add_devlinks("test1 test2")?; - dev.add_tags("tag1:tag2", true)?; + dev.add_tags("tag1:tag2", true); dev.add_property("key", "value")?; dev.set_devlink_priority(10); - dev.set_usec_initialized(1000)?; + dev.set_usec_initialized(1000); dev.update_db()?; - let db_path = format!("{}{}", DB_BASE_DIR, dev.get_device_id()?); + let db_path = format!("/tmp/devmaster/{}/{}", DB_BASE_DIR, dev.get_device_id()?); unlink(db_path.as_str()).unwrap(); @@ -3619,9 +3696,14 @@ mod tests { fn test_update_tag() { #[inline] fn inner_test(dev: &mut Device) -> Result<(), Error> { + dev.set_usec_initialized(1000); + dev.add_tags("test_update_tag:test_update_tag2", true); + dev.remove_tag("test_update_tag"); + dev.update_db()?; dev.update_tag("test_update_tag", true)?; + dev.update_tag("test_update_tag2", true)?; let tag_path = format!( - "/run/devmaster/tags/test_update_tag/{}", + "/tmp/devmaster/tags/test_update_tag/{}", dev.get_device_id()? ); assert!(Path::new(tag_path.as_str()).exists()); @@ -3629,6 +3711,15 @@ mod tests { dev.update_tag("test_update_tag", false)?; assert!(!Path::new(tag_path.as_str()).exists()); + let _ = dev.get_usec_initialized().unwrap(); + + assert!(dev.has_tag("test_update_tag").unwrap()); + assert!(dev.has_tag("test_update_tag2").unwrap()); + assert!(!dev.has_current_tag("test_update_tag").unwrap()); + assert!(dev.has_current_tag("test_update_tag2").unwrap()); + + dev.cleanup_tags(); + Ok(()) } @@ -3636,4 +3727,250 @@ mod tests { assert!(e.is_errno(nix::Error::EACCES) || e.is_errno(nix::Error::EBUSY)); } } + + #[test] + fn test_read_all_sysattrs() { + #[inline] + fn inner_test(dev: &mut Device) -> Result<(), Error> { + dev.read_all_sysattrs().unwrap(); + + for sysattr in &dev.sysattr_iter() { + if let Err(e) = dev.get_sysattr_value(sysattr) { + assert!(!IN_SET!(e.get_errno(), Errno::EPERM, Errno::EINVAL)); + } + } + + Ok(()) + } + + if let Err(e) = LoopDev::inner_process("/tmp/test_read_all_sysattrs", 1024 * 10, inner_test) + { + assert!(e.is_errno(nix::Error::EACCES) || e.is_errno(nix::Error::EBUSY)); + } + } + + #[test] + fn test_enumerate_children() { + #[inline] + fn inner_test(dev: &mut Device) -> Result<(), Error> { + dev.enumerate_children().unwrap(); + + for _ in &dev.child_iter() {} + + Ok(()) + } + + if let Err(e) = + LoopDev::inner_process("/tmp/test_enumerate_children", 1024 * 10, inner_test) + { + assert!(e.is_errno(nix::Error::EACCES) || e.is_errno(nix::Error::EBUSY)); + } + } + + #[test] + fn test_shallow_clone() { + #[inline] + fn inner_test(dev: &mut Device) -> Result<(), Error> { + let s1 = dev.get_syspath().unwrap(); + + let dev_clone = dev.shallow_clone().unwrap(); + + assert_eq!(s1, dev_clone.get_syspath().unwrap()); + + Ok(()) + } + + if let Err(e) = LoopDev::inner_process("/tmp/test_shallow_clone", 1024 * 10, inner_test) { + assert!(e.is_errno(nix::Error::EACCES) || e.is_errno(nix::Error::EBUSY)); + } + } + + #[test] + fn test_add_devlink() { + #[inline] + fn inner_test(dev: &mut Device) -> Result<(), Error> { + let s1 = dev.get_syspath().unwrap(); + + let dev_clone = dev.shallow_clone().unwrap(); + + assert_eq!(s1, dev_clone.get_syspath().unwrap()); + + Ok(()) + } + + if let Err(e) = LoopDev::inner_process("/tmp/test_add_devlink", 1024 * 10, inner_test) { + assert!(e.is_errno(nix::Error::EACCES) || e.is_errno(nix::Error::EBUSY)); + } + } + + #[test] + fn test_from() { + #[inline] + fn inner_test(dev: &mut Device) -> Result<(), Error> { + let syspath = dev.get_syspath().unwrap(); + let devnum = dev.get_devnum().unwrap(); + let id = dev.get_device_id().unwrap(); + let (nulstr, _) = dev.get_properties_nulstr().unwrap(); + let devname = dev.get_devname().unwrap(); + + let dev_new = Device::from_syspath(&syspath, true).unwrap(); + assert_eq!(dev, &dev_new); + + let dev_new = Device::from_device_id(&id).unwrap(); + assert_eq!(dev, &dev_new); + + let dev_new = Device::from_nulstr(nulstr.as_slice()).unwrap(); + assert_eq!(dev, &dev_new); + + let dev_new = Device::from_devnum('b', devnum).unwrap(); + assert_eq!(dev, &dev_new); + + let dev_new = Device::from_devname(&devname).unwrap(); + assert_eq!(dev, &dev_new); + + let dev_new_1 = Device::from_path(&syspath).unwrap(); + let dev_new_2 = Device::from_path(&devname).unwrap(); + assert_eq!(dev_new_1, dev_new_2); + + assert_eq!( + Device::from_devname(&syspath).unwrap_err().get_errno(), + Errno::EINVAL + ); + + Ok(()) + } + + if let Err(e) = LoopDev::inner_process("/tmp/test_from", 1024 * 10, inner_test) { + assert!(e.is_errno(nix::Error::EACCES) || e.is_errno(nix::Error::EBUSY)); + } + } + + #[test] + fn test_set_syspath_error() { + let device = Device::new(); + + assert!(device.set_syspath("", true).is_err()); + assert!(device.set_syspath(".././///../.", true).is_err()); + assert!(device.set_syspath("/not/exist", true).is_err()); + assert!(device.set_syspath("/dev/hello", true).is_err()); + assert!(device.set_syspath("/sys/devices/none", true).is_err()); + assert!(device.set_syspath("/sys/none", true).is_err()); + assert_eq!( + device.set_syspath("/sys/", true).unwrap_err().get_errno(), + nix::Error::ENODEV + ); + + assert_eq!( + device + .set_syspath("/dev/hello", false) + .unwrap_err() + .get_errno(), + nix::Error::EINVAL + ); + assert!(device.set_syspath("/sys/", false).is_ok()); + assert!(device.set_syspath("/sys", false).is_err()); + } + + #[test] + fn test_from_ifindex_error() { + assert!(Device::from_ifindex(10000).is_err()); + } + + #[test] + fn test_set_seqnum_from_string() { + let device = Device::new(); + device.set_seqnum_from_string("1000").unwrap(); + + assert!(device.set_seqnum_from_string("xxxx").is_err()); + } + + #[test] + fn test_set_db_persist() { + let device = Device::new(); + device.set_db_persist(); + } + + #[test] + fn test_from_db() { + /* Legal db content. */ + { + let content = "S:disk/by-path/pci-0000:00:10.0-scsi-0:0:0:0-part1 +I:1698916066 +E:ID_PART_ENTRY_OFFSET=2048 +G:devmaster +Q:devmaster +V:100 +"; + touch_file("/tmp/tmp_db", false, None, None, None).unwrap(); + let mut f = File::options().write(true).open("/tmp/tmp_db").unwrap(); + f.write(content.as_bytes()).unwrap(); + let device = Device::new(); + device.read_db_internal_filename("/tmp/tmp_db").unwrap(); + } + + /* Strange db entry would be ignored. */ + { + let content = "error +"; + let mut f = File::options().write(true).open("/tmp/tmp_db").unwrap(); + f.write(content.as_bytes()).unwrap(); + let device = Device::new(); + device.read_db_internal_filename("/tmp/tmp_db").unwrap(); + } + + /* Illegal db entry value would throw error. */ + { + let content = "I:invalid +"; + let mut f = File::options().write(true).open("/tmp/tmp_db").unwrap(); + f.write(content.as_bytes()).unwrap(); + let device = Device::new(); + assert!(device.read_db_internal_filename("/tmp/tmp_db").is_err()); + } + + /* DB shoud be readable. */ + { + touch_file("/tmp/tmp_db_writeonly", false, Some(0o222), None, None).unwrap(); + let device = Device::new(); + assert!(device.read_db_internal_filename("/tmp/tmp_db").is_err()); + } + + /* Test different kinds of illegal db entry. */ + { + let device = Device::new(); + assert!(device + .amend_key_value("USEC_INITIALIZED", "invalid") + .is_err()); + assert!(device.handle_db_line("E", "ID_TEST==invalid").is_err()); + assert!(device.handle_db_line("E", "=invalid").is_err()); + assert!(device.handle_db_line("I", "invalid").is_err()); + assert!(device.handle_db_line("L", "invalid").is_err()); + assert!(device.handle_db_line("W", "").is_ok()); + assert!(device.handle_db_line("V", "invalid").is_err()); + } + + unlink("/tmp/tmp_db").unwrap(); + } + + #[test] + fn test_set_is_initialized() { + let device = Device::from_subsystem_sysname("net", "lo").unwrap(); + device.set_is_initialized(); + device + .trigger_with_uuid(DeviceAction::Change, false) + .unwrap(); + device + .trigger_with_uuid(DeviceAction::Change, true) + .unwrap(); + device.trigger(DeviceAction::Change).unwrap(); + } + + #[test] + fn test_get_usec_since_initialized() { + assert!(catch_unwind(|| { + let dev = Device::new(); + dev.get_usec_since_initialized().unwrap(); + }) + .is_err()); + } } diff --git a/libs/device/src/device_monitor.rs b/libs/device/src/device_monitor.rs index 0c9bcba1..729a7dcb 100644 --- a/libs/device/src/device_monitor.rs +++ b/libs/device/src/device_monitor.rs @@ -299,8 +299,8 @@ mod tests { spawn(|| { let device = Device::from_devname("/dev/sda").unwrap(); device.set_action_from_string("change").unwrap(); - device.set_subsystem("block").unwrap(); - device.set_seqnum(1000).unwrap(); + device.set_subsystem("block"); + device.set_seqnum(1000); let broadcaster = DeviceMonitor::new(MonitorNetlinkGroup::None, None); broadcaster.send_device(&device, None).unwrap(); diff --git a/libs/device/src/error.rs b/libs/device/src/error.rs index 90f67d9d..40589c21 100644 --- a/libs/device/src/error.rs +++ b/libs/device/src/error.rs @@ -28,16 +28,31 @@ pub enum Error { /// errno indicates the error kind source: nix::Error, }, + + #[snafu(context, display("IO error: {}", msg))] + Io { + /// message + msg: String, + source: std::io::Error, + }, + + #[snafu(context, display("Basic error: {}", msg))] + Basic { msg: String, source: basic::Error }, } impl Error { /// extract the errno from error pub fn get_errno(&self) -> Errno { match self { - Error::Nix { + Self::Nix { msg: _, source: errno, } => *errno, + Self::Io { + msg: _, + source: errno, + } => Errno::from_i32(errno.raw_os_error().unwrap_or_default()), + Self::Basic { msg: _, source } => Errno::from_i32(source.get_errno()), } } } @@ -56,17 +71,14 @@ macro_rules! err_wrapper { impl Error { /// check whether the device error belongs to specific errno pub fn is_errno(&self, errno: nix::Error) -> bool { - match self { - Self::Nix { msg: _, source } => *source == errno, - } + self.get_errno() == errno } /// check whether the device error indicates the device is absent pub fn is_absent(&self) -> bool { - match self { - Self::Nix { msg: _, source } => { - matches!(source, Errno::ENODEV | Errno::ENXIO | Errno::ENOENT) - } - } + matches!( + self.get_errno(), + Errno::ENODEV | Errno::ENXIO | Errno::ENOENT + ) } } diff --git a/libs/device/src/utils.rs b/libs/device/src/utils.rs index 9d768b38..df750edf 100644 --- a/libs/device/src/utils.rs +++ b/libs/device/src/utils.rs @@ -188,6 +188,8 @@ impl LoopDev { source: nix::Error::EINVAL, })?)?; + dev.set_base_path("/tmp/devmaster"); + f(&mut dev) } } -- Gitee From 8e70281b5b00c8f1d3a950abf46c4068fb57b660 Mon Sep 17 00:00:00 2001 From: chenjiayi Date: Wed, 8 Nov 2023 05:17:04 +0800 Subject: [PATCH 5/8] fix(basic): use feature to control procfs compilation in get_errno --- libs/basic/src/error.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/basic/src/error.rs b/libs/basic/src/error.rs index d138f477..89ac5a3e 100644 --- a/libs/basic/src/error.rs +++ b/libs/basic/src/error.rs @@ -88,6 +88,7 @@ impl Error { std::env::VarError::NotUnicode(_) => nix::errno::Errno::EINVAL, }) as i32 } + #[cfg(feature = "process")] Error::Proc { source } => match source { procfs::ProcError::Incomplete(_) => nix::errno::Errno::EINVAL as i32, procfs::ProcError::PermissionDenied(_) => nix::errno::Errno::EPERM as i32, -- Gitee From 891d921492b436c1eaa9a2092aa1f3783f278479 Mon Sep 17 00:00:00 2001 From: chenjiayi Date: Wed, 8 Nov 2023 05:18:36 +0800 Subject: [PATCH 6/8] test(device): compress closures and add some unit cases Some unnecessary closures may decrease line coverage, thus compress them. Also drop Clone trait implementation for Device as it is not necessary. --- .../src/bin/devctl/subcmds/devctl_info.rs | 4 +- exts/devmaster/src/lib/builtin/hwdb.rs | 12 +- exts/devmaster/src/lib/framework/devmaster.rs | 1 - exts/devmaster/src/lib/framework/job_queue.rs | 2 - .../src/lib/framework/uevent_monitor.rs | 1 - libs/device/examples/device_from.rs | 4 +- libs/device/src/device.rs | 689 +++++++----------- libs/device/src/device_enumerator.rs | 10 +- libs/device/src/device_monitor.rs | 2 +- libs/device/src/error.rs | 65 +- 10 files changed, 324 insertions(+), 466 deletions(-) diff --git a/exts/devmaster/src/bin/devctl/subcmds/devctl_info.rs b/exts/devmaster/src/bin/devctl/subcmds/devctl_info.rs index 71c9254d..d4fcb6c4 100644 --- a/exts/devmaster/src/bin/devctl/subcmds/devctl_info.rs +++ b/exts/devmaster/src/bin/devctl/subcmds/devctl_info.rs @@ -286,7 +286,7 @@ fn print_device_chain(device: Device) -> Result<()> { while let Ok(parent) = child.get_parent() { print_all_attributes(&parent.borrow(), true)?; - child = parent.borrow().clone(); + child = parent.borrow().shallow_clone().unwrap(); } Ok(()) @@ -489,7 +489,7 @@ fn export_devices() -> Result<()> { } for device in e.iter() { - print_record(device.borrow().clone(), ""); + print_record(device.borrow().shallow_clone().unwrap(), ""); } Ok(()) diff --git a/exts/devmaster/src/lib/builtin/hwdb.rs b/exts/devmaster/src/lib/builtin/hwdb.rs index 63308e37..0b797874 100644 --- a/exts/devmaster/src/lib/builtin/hwdb.rs +++ b/exts/devmaster/src/lib/builtin/hwdb.rs @@ -123,7 +123,7 @@ impl Hwdb { let mut last = false; let mut src_dev = match srcdev { Some(d) => d, - None => dev.borrow_mut().clone(), + None => dev.borrow_mut().shallow_clone().unwrap(), }; loop { @@ -131,7 +131,7 @@ impl Hwdb { Ok(str_subsystem) => str_subsystem, Err(_) => { src_dev = match src_dev.get_parent() { - Ok(d) => d.borrow_mut().clone(), + Ok(d) => d.borrow_mut().shallow_clone().unwrap(), Err(_) => break, }; continue; @@ -142,7 +142,7 @@ impl Hwdb { if let Some(str_subsystem) = subsystem { if &dsubsys != str_subsystem { src_dev = match src_dev.get_parent() { - Ok(d) => d.borrow_mut().clone(), + Ok(d) => d.borrow_mut().shallow_clone().unwrap(), Err(_) => break, }; continue; @@ -170,7 +170,7 @@ impl Hwdb { if modalias.is_empty() { src_dev = match src_dev.get_parent() { - Ok(d) => d.borrow_mut().clone(), + Ok(d) => d.borrow_mut().shallow_clone().unwrap(), Err(_) => break, }; continue; @@ -189,7 +189,7 @@ impl Hwdb { } src_dev = match src_dev.get_parent() { - Ok(d) => d.borrow_mut().clone(), + Ok(d) => d.borrow_mut().shallow_clone().unwrap(), Err(_) => break, }; } @@ -258,7 +258,7 @@ impl Builtin for Hwdb { Ok(srcdev) => Some(srcdev), Err(e) => { return Err(Error::Other { - msg: format!("Failed to create sd_device object '{:?}'", dev), + msg: format!("Failed to create device object '{}'", device_id), errno: e.get_errno(), }); } diff --git a/exts/devmaster/src/lib/framework/devmaster.rs b/exts/devmaster/src/lib/framework/devmaster.rs index 370f4e84..94b93ca0 100644 --- a/exts/devmaster/src/lib/framework/devmaster.rs +++ b/exts/devmaster/src/lib/framework/devmaster.rs @@ -24,7 +24,6 @@ use std::{ }; /// encapsulate all submanagers -#[derive(Debug)] pub struct Devmaster { /// reference to events pub(crate) events: Rc, diff --git a/exts/devmaster/src/lib/framework/job_queue.rs b/exts/devmaster/src/lib/framework/job_queue.rs index e3e6e90b..7c6d6a8d 100644 --- a/exts/devmaster/src/lib/framework/job_queue.rs +++ b/exts/devmaster/src/lib/framework/job_queue.rs @@ -51,7 +51,6 @@ impl Display for JobState { } /// device job -#[derive(Debug)] pub struct DeviceJob { /// internal device pub device: Device, @@ -132,7 +131,6 @@ impl PartialEq for DeviceJob { } /// job queue -#[derive(Debug)] pub struct JobQueue { /// internal container of jobs pub(crate) jobs: RefCell>>, diff --git a/exts/devmaster/src/lib/framework/uevent_monitor.rs b/exts/devmaster/src/lib/framework/uevent_monitor.rs index fe7d19c3..809071d1 100644 --- a/exts/devmaster/src/lib/framework/uevent_monitor.rs +++ b/exts/devmaster/src/lib/framework/uevent_monitor.rs @@ -22,7 +22,6 @@ use std::os::unix::io::RawFd; use std::rc::Rc; /// uevent monitor -#[derive(Debug)] pub struct UeventMonitor { /// receive uevent from netlink socket device_monitor: DeviceMonitor, diff --git a/libs/device/examples/device_from.rs b/libs/device/examples/device_from.rs index 29e24bbc..146543ee 100644 --- a/libs/device/examples/device_from.rs +++ b/libs/device/examples/device_from.rs @@ -30,11 +30,11 @@ fn main() { let dev = Device::from_subsystem_sysname("drivers", "usb:hub").unwrap(); println!("{}", dev.get_sysname().unwrap()); println!("{}", dev.get_subsystem().unwrap()); - println!("{:?}", dev); + println!("{}", dev.get_device_id().unwrap()); } { let dev = Device::from_ifindex(2).unwrap(); - println!("{:?}", dev); + println!("{}", dev.get_device_id().unwrap()); } } diff --git a/libs/device/src/device.rs b/libs/device/src/device.rs index 6673d823..284cd6e9 100644 --- a/libs/device/src/device.rs +++ b/libs/device/src/device.rs @@ -12,7 +12,6 @@ //! struct Device //! -use crate::err_wrapper; use crate::utils::readlink_value; use crate::{error::*, DeviceAction}; use basic::fs_util::{chmod, open_temporary, touch_file}; @@ -49,7 +48,7 @@ pub const DB_BASE_DIR: &str = "data"; pub const TAGS_BASE_DIR: &str = "tags"; /// Device -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct Device { /// inotify handler pub watch_handle: RefCell, @@ -355,33 +354,16 @@ impl Device { let buf_trans: &[u8] = unsafe { std::slice::from_raw_parts(buf.as_ptr() as *const _, 16) }; - let ifname = String::from_utf8(buf_trans.to_vec()).map_err(|e| Error::Nix { - msg: format!("from_ifindex failed: from_utf8 {:?} ({})", buf_trans, e), - source: Errno::EINVAL, + let ifname = String::from_utf8(buf_trans.to_vec()).context(FromUtf8 { + msg: format!("invalid utf-8 string {:?}", buf_trans), })?; let syspath = format!("/sys/class/net/{}", ifname.trim_matches(char::from(0))); - let dev = Self::from_syspath(&syspath, true).map_err(|e| Error::Nix { - msg: format!("from_ifindex failed: {}", e), - source: e.get_errno(), - })?; + let dev = Self::from_syspath(&syspath, true)?; - let i = match dev.get_ifindex() { - Ok(i) => i, - Err(e) => { - if e.get_errno() == Errno::ENOENT { - return Err(Error::Nix { - msg: format!("from_ifindex failed: {}", e), - source: Errno::ENXIO, - }); - } - - return Err(Error::Nix { - msg: format!("from_ifindex failed: {}", e), - source: e.get_errno(), - }); - } - }; + let i = dev + .get_ifindex() + .map_err(|e| e.replace_errno(Errno::ENOENT, Errno::ENXIO))?; if i != ifindex { return Err(Error::Nix { @@ -393,8 +375,9 @@ impl Device { Ok(dev) } - /// create a Device instance from subsystem and sysname - /// if subsystem is 'drivers', sysname should be like 'xxx:yyy' + /// Create a Device instance from subsystem and sysname. + /// + /// If subsystem is 'drivers', sysname should be like 'xxx:yyy' pub fn from_subsystem_sysname(subsystem: &str, sysname: &str) -> Result { let sysname = sysname.replace('/', "!"); if subsystem == "subsystem" { @@ -506,10 +489,12 @@ impl Device { }) } - /// set sysattr value + /// Set sysattr value. + /// + /// If the sysattr is not 'uevent', the value will be cached. pub fn set_sysattr_value(&self, sysattr: &str, value: Option<&str>) -> Result<(), Error> { if value.is_none() { - self.remove_cached_sysattr_value(sysattr)?; + self.remove_cached_sysattr_value(sysattr); return Ok(()); } @@ -529,7 +514,7 @@ impl Device { }; if let Err(e) = file.write(value.unwrap().as_bytes()) { - self.remove_cached_sysattr_value(sysattr)?; + self.remove_cached_sysattr_value(sysattr); return Err(Error::Nix { msg: format!( "set_sysattr_value failed: can't write sysattr '{}'", @@ -559,28 +544,18 @@ impl Device { match id.chars().next() { Some('b') | Some('c') => { - let devnum = parse_devnum(&id[1..]).map_err(|_| Error::Nix { + let devnum = parse_devnum(&id[1..]).context(Basic { msg: format!("from_device_id failed: parse_devnum '{}' failed", id), - source: Errno::EINVAL, })?; - return Device::from_devnum(id.chars().next().unwrap(), devnum).map_err(|e| { - Error::Nix { - msg: format!("from_device_id failed: {}", e), - source: e.get_errno(), - } - }); + Device::from_devnum(id.chars().next().unwrap(), devnum) } Some('n') => { - let ifindex = parse_ifindex(&id[1..]).map_err(|_| Error::Nix { + let ifindex = parse_ifindex(&id[1..]).context(Basic { msg: format!("from_device_id failed: parse_ifindex '{}' failed", id), - source: Errno::EINVAL, })?; - Device::from_ifindex(ifindex).map_err(|e| Error::Nix { - msg: format!("from_device_id failed: {}", e), - source: e.get_errno(), - }) + Device::from_ifindex(ifindex) } Some('+') => { let sep = match id.find(':') { @@ -604,10 +579,7 @@ impl Device { let subsystem = id[1..sep].to_string(); let sysname = id[sep + 1..].to_string(); - Device::from_subsystem_sysname(&subsystem, &sysname).map_err(|e| Error::Nix { - msg: format!("from_device_id failed: {}", e), - source: e.get_errno(), - }) + Device::from_subsystem_sysname(&subsystem, &sysname) } _ => Err(Error::Nix { msg: format!("from_device_id failed: invalid id '{}'", id), @@ -648,7 +620,7 @@ impl Device { /// get the sysname of the device pub fn get_sysname(&self) -> Result { if self.sysname.borrow().is_empty() { - err_wrapper!(self.set_sysname_and_sysnum(), "get_sysname")?; + self.set_sysname_and_sysnum()?; } Ok(self.sysname.borrow().clone()) @@ -729,10 +701,7 @@ impl Device { // e.g. /sys/devices/pci0000:00/0000:00:10.0/host2/target2:0:1/2:0:1:0/block/sda/subsystem -> ../../../../../../../../class/block // get `block` let filename = if Path::exists(Path::new(subsystem_path)) { - readlink_value(subsystem_path).map_err(|e| Error::Nix { - msg: format!("get_subsystem failed: {}", e), - source: e.get_errno(), - })? + readlink_value(subsystem_path)? } else { "".to_string() }; @@ -766,10 +735,7 @@ impl Device { /// get the ifindex of device pub fn get_ifindex(&self) -> Result { - self.read_uevent_file().map_err(|e| Error::Nix { - msg: format!("get_ifindex failed: {}", e), - source: e.get_errno(), - })?; + self.read_uevent_file()?; if *self.ifindex.borrow() == 0 { return Err(Error::Nix { @@ -802,12 +768,7 @@ impl Device { /// get devnum pub fn get_devnum(&self) -> Result { - match self.read_uevent_file() { - Ok(_) => {} - Err(e) => { - return Err(e); - } - } + self.read_uevent_file()?; if major(*self.devnum.borrow()) == 0 { return Err(Error::Nix { @@ -870,10 +831,7 @@ impl Device { /// get device sysnum pub fn get_sysnum(&self) -> Result { if self.sysname.borrow().is_empty() { - self.set_sysname_and_sysnum().map_err(|e| Error::Nix { - msg: format!("get_sysnum failed: {}", e), - source: e.get_errno(), - })?; + self.set_sysname_and_sysnum()?; } if self.sysnum.borrow().is_empty() { @@ -915,10 +873,7 @@ impl Device { /// get device diskseq pub fn get_diskseq(&self) -> Result { - self.read_uevent_file().map_err(|e| Error::Nix { - msg: format!("get_diskseq failed: {}", e), - source: e.get_errno(), - })?; + self.read_uevent_file()?; if *self.diskseq.borrow() == 0 { return Err(Error::Nix { @@ -935,7 +890,6 @@ impl Device { /// get is initialized pub fn get_is_initialized(&self) -> Result { - // match self.read_db match self.read_db() { Ok(_) => {} Err(e) => { @@ -979,30 +933,21 @@ impl Device { /// check whether the device has the tag pub fn has_tag(&self, tag: &str) -> Result { - self.read_db().map_err(|e| Error::Nix { - msg: format!("has_tag failed: {}", e), - source: e.get_errno(), - })?; + self.read_db()?; Ok(self.all_tags.borrow().contains(tag)) } /// check whether the device has the current tag pub fn has_current_tag(&self, tag: &str) -> Result { - self.read_db().map_err(|e| Error::Nix { - msg: format!("has_tag failed: {}", e), - source: e.get_errno(), - })?; + self.read_db()?; Ok(self.current_tags.borrow().contains(tag)) } /// get the value of specific device property pub fn get_property_value(&self, key: &str) -> Result { - self.properties_prepare().map_err(|e| Error::Nix { - msg: format!("get_property_value failed: {}", e), - source: e.get_errno(), - })?; + self.properties_prepare()?; match self.properties.borrow().get(key) { Some(v) => Ok(v.clone()), @@ -1083,27 +1028,22 @@ impl Device { let mut file = std::fs::OpenOptions::new() .read(true) .open(&sysattr_path) - .map_err(|e| Error::Nix { + .context(Io { msg: format!( - "get_sysattr_value failed: can't open sysattr '{}': {}", - sysattr, e + "get_sysattr_value failed: can't open sysattr '{}'", + sysattr ), - source: Errno::from_i32(e.raw_os_error().unwrap_or_default()), })?; let mut value = String::new(); - file.read_to_string(&mut value).map_err(|e| Error::Nix { - msg: format!( - "get_sysattr_value failed: can't read sysattr '{}': {}", - sysattr, e - ), - source: Errno::from_i32(e.raw_os_error().unwrap_or_default()), + file.read_to_string(&mut value).context(Io { + msg: format!("get_sysattr_value failed: can't read sysattr '{}'", sysattr), })?; value.trim_end().to_string() } } Err(e) => { - self.remove_cached_sysattr_value(sysattr).unwrap(); + self.remove_cached_sysattr_value(sysattr); return Err(Error::Nix { msg: format!("get_sysattr_value failed: can't lstat '{}'", sysattr_path), source: e, @@ -1111,11 +1051,7 @@ impl Device { } }; - self.cache_sysattr_value(sysattr, &value) - .map_err(|e| Error::Nix { - msg: format!("get_sysattr_value failed: {}", e), - source: e.get_errno(), - })?; + self.cache_sysattr_value(sysattr, &value)?; Ok(value) } @@ -1133,15 +1069,9 @@ impl Device { let s = format!("{}", action); - let id = match randomize() { - Ok(id) => id, - Err(e) => { - return Err(Error::Nix { - msg: "Failed to randomize".to_string(), - source: e, - }) - } - }; + let id = randomize().context(Nix { + msg: "Failed to randomize".to_string(), + })?; let j = s + " " + &id.to_string(); @@ -1152,33 +1082,12 @@ impl Device { /// open device pub fn open(&self, oflags: OFlag) -> Result { - let devname = self.get_devname().map_err(|e| { - if e.get_errno() == Errno::ENOENT { - Error::Nix { - msg: format!("open failed: {}", e), - source: Errno::ENOEXEC, - } - } else { - Error::Nix { - msg: format!("open failed: {}", e), - source: e.get_errno(), - } - } - })?; - - let devnum = self.get_devnum().map_err(|e| { - if e.get_errno() == Errno::ENOENT { - Error::Nix { - msg: format!("open failed: {}", e), - source: Errno::ENOEXEC, - } - } else { - Error::Nix { - msg: format!("open failed: {}", e), - source: e.get_errno(), - } - } - })?; + let devname = self + .get_devname() + .map_err(|e| e.replace_errno(Errno::ENOENT, Errno::ENOEXEC))?; + let devnum = self + .get_devnum() + .map_err(|e| e.replace_errno(Errno::ENOENT, Errno::ENOEXEC))?; let subsystem = match self.get_subsystem() { Ok(s) => s, @@ -1212,19 +1121,13 @@ impl Device { } }; - let stat = match nix::sys::stat::fstat(file.as_raw_fd()) { - Ok(s) => s, - Err(e) => { - return Err(Error::Nix { - msg: format!( - "open failed: can't fstat fd {} for '{}'", - file.as_raw_fd(), - devname - ), - source: e, - }) - } - }; + let stat = nix::sys::stat::fstat(file.as_raw_fd()).context(Nix { + msg: format!( + "open failed: can't fstat fd {} for '{}'", + file.as_raw_fd(), + devname + ), + })?; if stat.st_rdev != devnum { return Err(Error::Nix { @@ -1265,18 +1168,11 @@ impl Device { let mut diskseq: u64 = 0; - if self.get_is_initialized().map_err(|e| Error::Nix { - msg: format!("open failed: {}", e), - source: e.get_errno(), - })? { + if self.get_is_initialized()? { match self.get_property_value("ID_IGNORE_DISKSEQ") { Ok(value) => { - if !value.parse::().map_err(|e| Error::Nix { - msg: format!( - "open failed: failed to parse value '{}' to boolean: {}", - value, e - ), - source: Errno::EINVAL, + if !value.parse::().context(ParseBool { + msg: format!("invalid value '{}'", value), })? { match self.get_diskseq() { Ok(n) => diskseq = n, @@ -1302,28 +1198,16 @@ impl Device { } } - let file2 = - basic::fd_util::fd_reopen(file.as_raw_fd(), oflags).map_err(|e| Error::Nix { - msg: format!("open failed: {}", e), - source: match e { - basic::Error::Nix { source } => source, - _ => Errno::EINVAL, - }, - })?; + let file2 = basic::fd_util::fd_reopen(file.as_raw_fd(), oflags).context(Basic { + msg: format!("failed to open {}", file.as_raw_fd()), + })?; if diskseq == 0 { return Ok(file2); } - let q = basic::fd_util::fd_get_diskseq(file2.as_raw_fd()).map_err(|e| Error::Nix { - msg: format!( - "open failed: failed to get diskseq on fd {}", - file2.as_raw_fd() - ), - source: match e { - basic::Error::Nix { source } => source, - _ => Errno::EINVAL, - }, + let q = basic::fd_util::fd_get_diskseq(file2.as_raw_fd()).context(Basic { + msg: format!("failed to get diskseq on fd {}", file2.as_raw_fd()), })?; if q != diskseq { @@ -1352,18 +1236,9 @@ impl Device { /// shadow clone a device object and import properties from db pub fn clone_with_db(&self) -> Result { - let device = self.shallow_clone().map_err(|e| Error::Nix { - msg: format!("clone_with_db failed: {}", e), - source: e.get_errno(), - })?; - - device.read_db().map_err(|e| Error::Nix { - msg: format!("clone_with_db failed: {}", e), - source: e.get_errno(), - })?; - + let device = self.shallow_clone()?; + device.read_db()?; device.sealed.replace(true); - Ok(device) } @@ -1427,22 +1302,14 @@ impl Device { /// /// The format is like: /// - /// character device: c: - /// - /// block device: b: - /// - /// network interface: n - /// - /// drivers: +drivers:: - /// - /// other subsystems: +: - /// + /// - character device: c: + /// - block device: b: + /// - network interface: n + /// - drivers: +drivers:: + /// - other subsystems: +: pub fn get_device_id(&self) -> Result { if self.device_id.borrow().is_empty() { - let subsystem = self.get_subsystem().map_err(|e| Error::Nix { - msg: format!("get_device_id failed: {}", e), - source: e.get_errno(), - })?; + let subsystem = self.get_subsystem()?; let id: String; if let Ok(devnum) = self.get_devnum() { @@ -1455,10 +1322,7 @@ impl Device { } else if let Ok(ifindex) = self.get_ifindex() { id = format!("n{}", ifindex); } else { - let sysname = self.get_sysname().map_err(|e| Error::Nix { - msg: format!("get_device_id failed: {}", e), - source: e.get_errno(), - })?; + let sysname = self.get_sysname()?; if subsystem == "drivers" { id = format!("+drivers:{}:{}", self.driver_subsystem.borrow(), sysname); @@ -1571,21 +1435,14 @@ impl Device { let db_path = format!("{}/{}/{}", self.base_path.borrow(), DB_BASE_DIR, id); if !has_info && *self.devnum.borrow() == 0 && *self.ifindex.borrow() == 0 { - unlink(db_path.as_str()).map_err(|e| Error::Nix { + unlink(db_path.as_str()).context(Nix { msg: format!("update_db failed: can't unlink db '{}'", db_path), - source: e, })?; - return Ok(()); } - create_dir_all(&format!("{}/{}", self.base_path.borrow(), DB_BASE_DIR)).map_err(|e| { - Error::Nix { - msg: "update_db failed: can't create db directory".to_string(), - source: e - .raw_os_error() - .map_or_else(|| nix::Error::EIO, nix::Error::from_i32), - } + create_dir_all(&format!("{}/{}", self.base_path.borrow(), DB_BASE_DIR)).context(Io { + msg: "failed to create db directory".to_string(), })?; if let Err(e) = chmod( @@ -1595,22 +1452,14 @@ impl Device { log::error!("Failed to set permission for /run/devmaster/data/: {}", e); } - let (mut file, tmp_file) = open_temporary(&db_path).map_err(|e| { - let errno = match e { - basic::error::Error::Nix { source } => source, - _ => nix::Error::EINVAL, - }; - Error::Nix { - msg: "update_db failed: can't open temporary file".to_string(), - source: errno, - } + let (mut file, tmp_file) = open_temporary(&db_path).context(Basic { + msg: "can't open temporary file".to_string(), })?; - self.atomic_create_db(&mut file, tmp_file.as_str(), db_path.as_str()) - .map_err(|e| { - Self::cleanup(&db_path, &tmp_file); - e - })?; + if let Err(e) = self.atomic_create_db(&mut file, tmp_file.as_str(), db_path.as_str()) { + Self::cleanup(&db_path, &tmp_file); + return Err(e); + } Ok(()) } @@ -1713,9 +1562,8 @@ impl Device { ); if add { - touch_file(&tag_path, true, Some(0o444), None, None).map_err(|e| Error::Nix { - msg: format!("tag_persist failed: can't touch file '{}': {}", tag_path, e), - source: nix::Error::EINVAL, + touch_file(&tag_path, true, Some(0o444), None, None).context(Basic { + msg: format!("can't touch file '{}'", tag_path), })?; if let Err(e) = chmod( @@ -1765,10 +1613,7 @@ impl Device { /// read database pub fn read_db(&self) -> Result<(), Error> { - self.read_db_internal(false).map_err(|e| Error::Nix { - msg: format!("read_db failed: {}", e), - source: e.get_errno(), - }) + self.read_db_internal(false) } /// read database internally @@ -1777,18 +1622,11 @@ impl Device { return Ok(()); } - let id = self.get_device_id().map_err(|e| Error::Nix { - msg: format!("read_db_internal failed: {}", e), - source: e.get_errno(), - })?; + let id = self.get_device_id()?; let path = format!("{}/{}/{}", self.base_path.borrow(), DB_BASE_DIR, id); self.read_db_internal_filename(&path) - .map_err(|e| Error::Nix { - msg: format!("read_db_internal failed: {}", e), - source: e.get_errno(), - }) } /// get properties nulstr, if it is out of date, update it @@ -1836,10 +1674,7 @@ impl Device { } // verify subsystem - let subsystem_ret = device.get_subsystem().map_err(|e| Error::Nix { - msg: format!("from_mode_and_devnum failed: {}", e), - source: e.get_errno(), - })?; + let subsystem_ret = device.get_subsystem()?; if (subsystem_ret == "block") != ((mode & S_IFMT) == S_IFBLK) { return Err(Error::Nix { msg: "from_mode_and_devnum failed: inconsistent subsystem".to_string(), @@ -1942,19 +1777,9 @@ impl Device { /// set the sysname and sysnum of device object pub fn set_sysname_and_sysnum(&self) -> Result<(), Error> { - let sysname = match self.devpath.borrow().rfind('/') { - Some(i) => String::from(&self.devpath.borrow()[i + 1..]), - None => { - return Err(Error::Nix { - msg: format!( - "set_sysname_and_sysnum failed: invalid devpath '{}'", - self.devpath.borrow() - ), - source: Errno::EINVAL, - }); - } - }; - + /* The devpath is validated to begin with '/' when setting syspath. */ + let idx = self.devpath.borrow().rfind('/').unwrap(); + let sysname = String::from(&self.devpath.borrow()[idx + 1..]); let sysname = sysname.replace('!', "/"); let mut ridx = sysname.len(); @@ -2146,15 +1971,10 @@ impl Device { /// set ifindex pub fn set_ifindex(&self, ifindex: &str) -> Result<(), Error> { self.add_property_internal("IFINDEX", ifindex).unwrap(); - self.ifindex.replace(match ifindex.parse::() { - Ok(idx) => idx, - Err(e) => { - return Err(Error::Nix { - msg: format!("set_ifindex failed: {}", e), - source: Errno::EINVAL, - }); - } - }); + self.ifindex + .replace(ifindex.parse::().context(ParseInt { + msg: format!("invalid integer '{}'", ifindex), + })?); Ok(()) } @@ -2172,12 +1992,8 @@ impl Device { /// set devmode pub fn set_devmode(&self, devmode: &str) -> Result<(), Error> { - let m = Some(mode_t::from_str_radix(devmode, 8).map_err(|e| Error::Nix { - msg: format!( - "set_devmode failed: can't change '{}' to mode: {}", - devmode, e - ), - source: Errno::EINVAL, + let m = Some(mode_t::from_str_radix(devmode, 8).context(ParseInt { + msg: format!("invalid octal mode '{}'", devmode), })?); self.devmode.replace(m); @@ -2189,9 +2005,8 @@ impl Device { /// set device uid pub fn set_devuid(&self, devuid: &str) -> Result<(), Error> { - let uid = devuid.parse::().map_err(|e| Error::Nix { - msg: format!("set_devuid failed: can't change '{}' to uid: {}", devuid, e), - source: Errno::EINVAL, + let uid = devuid.parse::().context(ParseInt { + msg: format!("invalid uid '{}'", devuid), })?; self.devuid.replace(Some(Uid::from_raw(uid))); @@ -2203,9 +2018,8 @@ impl Device { /// set device gid pub fn set_devgid(&self, devgid: &str) -> Result<(), Error> { - let gid = devgid.parse::().map_err(|e| Error::Nix { - msg: format!("set_devgid failed: can't change '{}' to gid: {}", devgid, e), - source: Errno::EINVAL, + let gid = devgid.parse::().context(ParseInt { + msg: format!("invalid gid '{}'", devgid), })?; self.devgid.replace(Some(Gid::from_raw(gid))); @@ -2217,15 +2031,10 @@ impl Device { /// set devnum pub fn set_devnum(&self, major: &str, minor: &str) -> Result<(), Error> { - let major_num: u64 = match major.parse() { - Ok(n) => n, - Err(e) => { - return Err(Error::Nix { - msg: format!("set_devnum failed: invalid major number '{}': {}", major, e), - source: Errno::EINVAL, - }); - } - }; + let major_num: u64 = major.parse().context(ParseInt { + msg: format!("invalid major number '{}'", major), + })?; + let minor_num: u64 = match minor.parse() { Ok(n) => n, Err(e) => { @@ -2247,15 +2056,9 @@ impl Device { pub fn set_diskseq(&self, diskseq: &str) -> Result<(), Error> { self.add_property_internal("DISKSEQ", diskseq).unwrap(); - let diskseq_num: u64 = match diskseq.parse() { - Ok(n) => n, - Err(e) => { - return Err(Error::Nix { - msg: format!("set_diskseq failed: invalid diskseq '{}': {}", diskseq, e), - source: Errno::EINVAL, - }); - } - }; + let diskseq_num: u64 = diskseq.parse().context(ParseInt { + msg: format!("invalid diskseq '{}'", diskseq), + })?; self.diskseq.replace(diskseq_num); @@ -2271,18 +2074,7 @@ impl Device { /// set action from string pub fn set_action_from_string(&self, action_s: &str) -> Result<(), Error> { - let action = match action_s.parse::() { - Ok(a) => a, - Err(_) => { - return Err(Error::Nix { - msg: format!( - "set_action_from_string failed: invalid action '{}'", - action_s - ), - source: Errno::EINVAL, - }); - } - }; + let action = action_s.parse::()?; self.set_action(action); @@ -2291,19 +2083,9 @@ impl Device { /// set seqnum from string pub fn set_seqnum_from_string(&self, seqnum_s: &str) -> Result<(), Error> { - let seqnum: u64 = match seqnum_s.parse() { - Ok(n) => n, - Err(_) => { - return Err(Error::Nix { - msg: format!( - "set_seqnum_from_string failed: invalid seqnum '{}'", - seqnum_s - ), - source: Errno::EINVAL, - }); - } - }; - + let seqnum: u64 = seqnum_s.parse().context(ParseInt { + msg: format!("invalid seqnum '{}'", seqnum_s), + })?; self.set_seqnum(seqnum); Ok(()) } @@ -2325,7 +2107,7 @@ impl Device { /// cache sysattr value pub fn cache_sysattr_value(&self, sysattr: &str, value: &str) -> Result<(), Error> { if value.is_empty() { - self.remove_cached_sysattr_value(sysattr)?; + self.remove_cached_sysattr_value(sysattr); } else { self.sysattr_values .borrow_mut() @@ -2336,34 +2118,27 @@ impl Device { } /// remove cached sysattr value - pub fn remove_cached_sysattr_value(&self, sysattr: &str) -> Result<(), Error> { + pub fn remove_cached_sysattr_value(&self, sysattr: &str) { self.sysattr_values.borrow_mut().remove(sysattr); - - Ok(()) } /// get cached sysattr value pub fn get_cached_sysattr_value(&self, sysattr: &str) -> Result { if !self.sysattr_values.borrow().contains_key(sysattr) { return Err(Error::Nix { - msg: format!( - "get_cached_sysattr_value failed: no cached sysattr '{}'", - sysattr - ), + msg: format!("no cached sysattr '{}'", sysattr), source: Errno::ESTALE, }); } - match self.sysattr_values.borrow().get(sysattr) { - Some(value) => Ok(value.clone()), - None => Err(Error::Nix { - msg: format!( - "get_cached_sysattr_value failed: non-existing sysattr '{}'", - sysattr - ), + self.sysattr_values + .borrow() + .get(sysattr) + .cloned() + .ok_or(Error::Nix { + msg: format!("non-existing sysattr '{}'", sysattr), source: Errno::ENOENT, - }), - } + }) } /// new from child @@ -2378,8 +2153,7 @@ impl Device { Some(p) => { if p == Path::new("/sys") { return Err(Error::Nix { - msg: "new_from_child failed: no available parent device until /sys" - .to_string(), + msg: "no available parent device".to_string(), source: Errno::ENODEV, }); } @@ -2387,7 +2161,7 @@ impl Device { let path = p .to_str() .ok_or(Error::Nix { - msg: format!("new_from_child failed: invalid path '{:?}'", p), + msg: format!("invalid path '{:?}'", p), source: Errno::ENODEV, })? .to_string(); @@ -2444,15 +2218,9 @@ impl Device { /// 3. if self devlinks are outdated, add to internal property /// 4. if self tags are outdated ,add to internal property pub fn properties_prepare(&self) -> Result<(), Error> { - self.read_uevent_file().map_err(|e| Error::Nix { - msg: format!("properties_prepare failed: {}", e), - source: e.get_errno(), - })?; + self.read_uevent_file()?; - self.read_db().map_err(|e| Error::Nix { - msg: format!("properties_prepare failed: {}", e), - source: e.get_errno(), - })?; + self.read_db()?; let property_devlinks_outdated = *self.property_devlinks_outdated.borrow(); if property_devlinks_outdated { @@ -2518,15 +2286,8 @@ impl Device { }; let mut buf = String::new(); - file.read_to_string(&mut buf).map_err(|e| Error::Nix { - msg: format!( - "read_db_internal_filename failed: can't read db '{}': {}", - filename, e - ), - source: e - .raw_os_error() - .map(nix::Error::from_i32) - .unwrap_or(nix::Error::EIO), + file.read_to_string(&mut buf).context(Io { + msg: format!("can't read db '{}'", filename), })?; self.is_initialized.replace(true); @@ -2540,10 +2301,7 @@ impl Device { let key = &line[0..1]; let value = &line[2..]; - self.handle_db_line(key, value).map_err(|e| Error::Nix { - msg: format!("read_db_internal_filename failed: {}", e), - source: e.get_errno(), - })?; + self.handle_db_line(key, value)?; } Ok(()) @@ -2572,29 +2330,18 @@ impl Device { let (k, v) = (tokens[0], tokens[1]); - self.add_property_internal(k, v).map_err(|e| Error::Nix { - msg: format!("handle_db_line failed: {}", e), - source: e.get_errno(), - })?; + self.add_property_internal(k, v)?; } "I" => { - let time = value.parse::().map_err(|e| Error::Nix { - msg: format!( - "handle_db_line failed: invalid initialized time '{}': {}", - value, e - ), - source: Errno::EINVAL, + let time = value.parse::().context(ParseInt { + msg: format!("invalid usec integer '{}'", value), })?; self.set_usec_initialized(time); } "L" => { - let priority = value.parse::().map_err(|e| Error::Nix { - msg: format!( - "handle_db_line failed: failed to parse devlink priority '{}': {}", - value, e - ), - source: Errno::EINVAL, + let priority = value.parse::().context(ParseInt { + msg: format!("invalid link priority integer '{}'", value), })?; self.devlink_priority.replace(priority); @@ -2603,12 +2350,8 @@ impl Device { log::debug!("watch handle in database is deprecated."); } "V" => { - let version = value.parse::().map_err(|e| Error::Nix { - msg: format!( - "handle_db_line failed: failed to parse database version '{}': {}", - value, e - ), - source: Errno::EINVAL, + let version = value.parse::().context(ParseInt { + msg: format!("invalid db version integer '{}'", value), })?; self.database_version.replace(version); @@ -2625,22 +2368,11 @@ impl Device { pub fn shallow_clone(&self) -> Result { let device = Self::default(); - let syspath = self.get_syspath().map_err(|e| Error::Nix { - msg: format!("shallow_clone failed: {}", e), - source: e.get_errno(), - })?; + let syspath = self.get_syspath()?; - device - .set_syspath(&syspath, false) - .map_err(|e| Error::Nix { - msg: format!("shallow_clone failed: {}", e), - source: e.get_errno(), - })?; + device.set_syspath(&syspath, false)?; - let subsystem = self.get_subsystem().map_err(|e| Error::Nix { - msg: format!("shallow_clone failed: {}", e), - source: e.get_errno(), - })?; + let subsystem = self.get_subsystem()?; device.set_subsystem(&subsystem); @@ -2651,24 +2383,15 @@ impl Device { } if let Ok(ifindex) = self.get_property_value("IFINDEX") { - device.set_ifindex(&ifindex).map_err(|e| Error::Nix { - msg: format!("shallow_clone failed: failed to set_ifindex ({})", e), - source: e.get_errno(), - })?; + device.set_ifindex(&ifindex)?; } if let Ok(major) = self.get_property_value("MAJOR") { let minor = self.get_property_value("MINOR")?; - device.set_devnum(&major, &minor).map_err(|e| Error::Nix { - msg: format!("shallow_clone failed: {}", e), - source: e.get_errno(), - })?; + device.set_devnum(&major, &minor)?; } - device.read_uevent_file().map_err(|e| Error::Nix { - msg: format!("shallow_clone failed: {}", e), - source: e.get_errno(), - })?; + device.read_uevent_file()?; Ok(device) } @@ -2685,12 +2408,8 @@ impl Device { "DRIVER" => self.set_driver(value), "IFINDEX" => self.set_ifindex(value)?, "USEC_INITIALIZED" => { - self.set_usec_initialized(value.parse::().map_err(|e| Error::Nix { - msg: format!( - "amend_key_value failed: failed to parse initialized time '{}': {}", - value, e - ), - source: Errno::EINVAL, + self.set_usec_initialized(value.parse::().context(ParseInt { + msg: format!("invalid usec integer '{}'", value), })?); } "DEVMODE" => self.set_devmode(value)?, @@ -2814,9 +2533,8 @@ impl Device { format!("{}/{}", syspath, subdir) }; - std::fs::read_dir(&dir).map_err(|e| Error::Nix { + std::fs::read_dir(&dir).context(Io { msg: format!("Failed to read directory '{}'", &dir), - source: nix::Error::from_i32(e.raw_os_error().unwrap_or_default()), }) } @@ -3017,7 +2735,7 @@ impl Device { log::debug!( "failed to read db of '{}': {}", self.get_device_id() - .unwrap_or(self.devpath.borrow().clone()), + .unwrap_or_else(|_| self.devpath.borrow().clone()), e ) } @@ -3033,7 +2751,7 @@ impl Device { log::error!( "failed to read db of '{}': {}", self.get_device_id() - .unwrap_or(self.devpath.borrow().clone()), + .unwrap_or_else(|_| self.devpath.borrow().clone()), e ) } @@ -3049,7 +2767,7 @@ impl Device { log::debug!( "failed to read db of '{}': {}", self.get_device_id() - .unwrap_or(self.devpath.borrow().clone()), + .unwrap_or_else(|_| self.devpath.borrow().clone()), e ) } @@ -3065,7 +2783,7 @@ impl Device { log::debug!( "failed to prepare properties of '{}': {}", self.get_device_id() - .unwrap_or(self.devpath.borrow().clone()), + .unwrap_or_else(|_| self.devpath.borrow().clone()), e ) } @@ -3081,7 +2799,7 @@ impl Device { log::debug!( "failed to enumerate children of '{}': {}", self.get_device_id() - .unwrap_or(self.devpath.borrow().clone()), + .unwrap_or_else(|_| self.devpath.borrow().clone()), e ) } @@ -3097,7 +2815,8 @@ impl Device { if let Err(e) = self.read_all_sysattrs() { log::debug!( "{}: failed to read all sysattrs: {}", - self.get_sysname().unwrap_or(self.devpath.borrow().clone()), + self.get_sysname() + .unwrap_or_else(|_| self.devpath.borrow().clone()), e ); } @@ -3117,6 +2836,7 @@ impl PartialEq for Device { #[cfg(test)] mod tests { + use std::fs::OpenOptions; use std::panic::catch_unwind; use crate::{ @@ -3426,6 +3146,11 @@ mod tests { nix::Error::EINVAL ); + /* Test other add_* methods. */ + device.add_property("A", "AA").unwrap(); + device.add_property("B", "BB").unwrap(); + device.add_tags("A:B:C", true); + device.update_db().unwrap(); /* Test enumerating child devices */ @@ -3902,8 +3627,8 @@ Q:devmaster V:100 "; touch_file("/tmp/tmp_db", false, None, None, None).unwrap(); - let mut f = File::options().write(true).open("/tmp/tmp_db").unwrap(); - f.write(content.as_bytes()).unwrap(); + let mut f = OpenOptions::new().write(true).open("/tmp/tmp_db").unwrap(); + f.write_all(content.as_bytes()).unwrap(); let device = Device::new(); device.read_db_internal_filename("/tmp/tmp_db").unwrap(); } @@ -3912,8 +3637,8 @@ V:100 { let content = "error "; - let mut f = File::options().write(true).open("/tmp/tmp_db").unwrap(); - f.write(content.as_bytes()).unwrap(); + let mut f = OpenOptions::new().write(true).open("/tmp/tmp_db").unwrap(); + f.write_all(content.as_bytes()).unwrap(); let device = Device::new(); device.read_db_internal_filename("/tmp/tmp_db").unwrap(); } @@ -3922,13 +3647,13 @@ V:100 { let content = "I:invalid "; - let mut f = File::options().write(true).open("/tmp/tmp_db").unwrap(); - f.write(content.as_bytes()).unwrap(); + let mut f = OpenOptions::new().write(true).open("/tmp/tmp_db").unwrap(); + f.write_all(content.as_bytes()).unwrap(); let device = Device::new(); assert!(device.read_db_internal_filename("/tmp/tmp_db").is_err()); } - /* DB shoud be readable. */ + /* DB should be readable. */ { touch_file("/tmp/tmp_db_writeonly", false, Some(0o222), None, None).unwrap(); let device = Device::new(); @@ -3973,4 +3698,98 @@ V:100 }) .is_err()); } + + #[test] + fn test_set() { + let device = Device::from_subsystem_sysname("net", "lo").unwrap(); + device.set_devuid("1").unwrap(); + device.set_devgid("1").unwrap(); + device.set_devmode("666").unwrap(); + device.set_diskseq("1").unwrap(); + device.set_action_from_string("change").unwrap(); + device.set_sysattr_value("ifalias", Some("test")).unwrap(); + + assert_eq!(&device.get_property_value("DEVUID").unwrap(), "1"); + assert_eq!(&device.get_property_value("DEVGID").unwrap(), "1"); + assert_eq!(&device.get_property_value("DEVMODE").unwrap(), "666"); + assert_eq!(&device.get_property_value("DISKSEQ").unwrap(), "1"); + assert_eq!(&device.get_property_value("ACTION").unwrap(), "change"); + assert_eq!(&device.get_cached_sysattr_value("ifalias").unwrap(), "test"); + + assert!(device.set_devuid("invalid").is_err()); + assert!(device.set_devgid("invalid").is_err()); + assert!(device.set_devmode("invalid").is_err()); + assert!(device.set_diskseq("invalid").is_err()); + assert!(device.set_action_from_string("invalid").is_err()); + assert!(device.set_sysattr_value("nonexist", Some("test")).is_err()); + + assert!(device.set_sysattr_value("nonexist", None).is_ok()); + assert!(device.set_sysattr_value("ifalias", None).is_ok()); + } + + #[test] + fn test_from_device_id() { + assert!(Device::from_device_id("invalid").is_err()); + assert!(Device::from_device_id("b").is_err()); + assert!(Device::from_device_id("+drivers").is_err()); + assert!(Device::from_device_id("+drivers:").is_err()); + assert!(Device::from_device_id("+drivers::usb").is_err()); + + let dev = Device::from_device_id("+drivers:usb:usb").unwrap(); + println!("{}", dev.get_device_id().unwrap()); + + let dev = Device::from_syspath("/sys/bus/usb/drivers/usb", true).unwrap(); + println!("{}", dev.get_device_id().unwrap()); + + let _ = unlink("/tmp/devmaster/data/+drivers:usb:usb"); + dev.set_base_path("/tmp/devmaster"); + assert!(dev.update_db().is_err()); + dev.add_property("hello", "world").unwrap(); + dev.update_db().unwrap(); + assert!(Path::new("/tmp/devmaster/data/+drivers:usb:usb").exists()); + } + + #[test] + fn test_get_err() { + let device = Device::new(); + assert!(device.get_syspath().is_err()); + assert!(device.get_devpath().is_err()); + assert!(device.get_parent().is_err()); + assert!(device.get_devtype().is_err()); + assert!(!device.get_is_initialized().unwrap()); + } + + #[test] + fn test_cleanup() { + let _ = touch_file("/tmp/devmaster/a", false, None, None, None); + let _ = touch_file("/tmp/devmaster/b", false, None, None, None); + Device::cleanup("/tmp/devmaster/a", "/tmp/devmaster/b"); + } + + #[test] + fn test_fmt() { + let device = Device::from_subsystem_sysname("net", "lo").unwrap(); + println!("{:?}", device); + } + + #[test] + fn test_set_syspath_no_verify() { + let device = Device::new(); + device.set_syspath("/sys/test", false).unwrap(); + + assert!(device.set_sysname_and_sysnum().is_ok()); + } + + #[test] + fn test_partial_eq_trait() { + let dev1 = Device::from_syspath("/sys/class/net/lo", true).unwrap(); + let dev2 = Device::from_subsystem_sysname("net", "lo").unwrap(); + + assert!(dev1 == dev2); + } + + #[test] + fn test_from_devnum_err() { + assert!(Device::from_devnum('x', 100).is_err()); + } } diff --git a/libs/device/src/device_enumerator.rs b/libs/device/src/device_enumerator.rs index 96cac2d9..55f8681d 100644 --- a/libs/device/src/device_enumerator.rs +++ b/libs/device/src/device_enumerator.rs @@ -12,7 +12,7 @@ //! enumerate /sys to collect devices //! -use crate::{device::Device, err_wrapper, error::Error, utils::*}; +use crate::{device::Device, error::Error, utils::*}; use bitflags::bitflags; use fnmatch_sys::fnmatch; use nix::errno::Errno; @@ -45,7 +45,7 @@ impl Default for MatchInitializedType { } /// enumerate devices or subsystems under /sys -#[derive(Debug, Default)] +#[derive(Default)] pub struct DeviceEnumerator { /// enumerator type pub(crate) etype: RefCell, @@ -289,7 +289,7 @@ impl DeviceEnumerator { /// add match parent pub fn add_match_parent_incremental(&mut self, parent: &Device) -> Result<(), Error> { - let syspath = err_wrapper!(parent.get_syspath(), "add_match_parent_incremental")?; + let syspath = parent.get_syspath()?; self.match_parent.borrow_mut().insert(syspath); self.scan_up_to_date.replace(false); Ok(()) @@ -364,7 +364,7 @@ impl DeviceEnumerator { // remove already sorted devices from the hashmap (self.devices_by_syspath) // avoid get repeated devices from the hashmap later for device in devices.iter().skip(m) { - let syspath = err_wrapper!(device.borrow().get_syspath(), "sort_devices")?; + let syspath = device.borrow().get_syspath()?; self.devices_by_syspath.borrow_mut().remove(&syspath); } @@ -400,7 +400,7 @@ impl DeviceEnumerator { /// add device pub(crate) fn add_device(&self, device: Rc>) -> Result { - let syspath = err_wrapper!(device.borrow().get_syspath(), "add_device")?; + let syspath = device.borrow().get_syspath()?; match self.devices_by_syspath.borrow_mut().insert(syspath, device) { Some(_) => { diff --git a/libs/device/src/device_monitor.rs b/libs/device/src/device_monitor.rs index 729a7dcb..db51c40f 100644 --- a/libs/device/src/device_monitor.rs +++ b/libs/device/src/device_monitor.rs @@ -250,7 +250,7 @@ mod tests { /// fn dispatch(&self, e: &Events) -> i32 { let device = self.device_monitor.receive_device().unwrap(); - println!("{:?}", device); + println!("{}", device.get_device_id().unwrap()); e.set_exit(); 0 } diff --git a/libs/device/src/error.rs b/libs/device/src/error.rs index 40589c21..10410301 100644 --- a/libs/device/src/error.rs +++ b/libs/device/src/error.rs @@ -38,6 +38,24 @@ pub enum Error { #[snafu(context, display("Basic error: {}", msg))] Basic { msg: String, source: basic::Error }, + + #[snafu(context, display("Failed to parse boolean: {}", msg))] + ParseBool { + msg: String, + source: std::str::ParseBoolError, + }, + + #[snafu(context, display("Failed to parse integer: {}", msg))] + ParseInt { + msg: String, + source: std::num::ParseIntError, + }, + + #[snafu(context, display("Failed to parse utf-8: {}", msg))] + FromUtf8 { + msg: String, + source: std::string::FromUtf8Error, + }, } impl Error { @@ -53,21 +71,13 @@ impl Error { source: errno, } => Errno::from_i32(errno.raw_os_error().unwrap_or_default()), Self::Basic { msg: _, source } => Errno::from_i32(source.get_errno()), + Self::ParseBool { msg: _, source: _ } => nix::Error::EINVAL, + Self::ParseInt { msg: _, source: _ } => nix::Error::EINVAL, + Self::FromUtf8 { msg: _, source: _ } => nix::Error::EINVAL, } } } -/// append current function and inherit the errno -#[macro_export] -macro_rules! err_wrapper { - ($e:expr, $s:expr) => { - $e.map_err(|e| Error::Nix { - msg: format!("$s failed: {}", e), - source: e.get_errno(), - }) - }; -} - impl Error { /// check whether the device error belongs to specific errno pub fn is_errno(&self, errno: nix::Error) -> bool { @@ -81,4 +91,37 @@ impl Error { Errno::ENODEV | Errno::ENXIO | Errno::ENOENT ) } + + pub(crate) fn replace_errno(self, from: Errno, to: Errno) -> Self { + let n = self.get_errno(); + + if n == from { + Self::Nix { + msg: self.to_string(), + source: to, + } + } else { + self + } + } +} + + +#[cfg(test)] +mod test { + use super::*; + use nix::errno::Errno; + + #[test] + fn test_replace_errno() { + let e = Error::Nix { + msg: "test".to_string(), + source: Errno::ENOENT, + }; + + assert_eq!( + Errno::ENOEXEC, + e.replace_errno(Errno::ENOENT, Errno::ENOEXEC).get_errno(), + ); + } } -- Gitee From 9edb0233d9fed295c4c14ec06edee8d5f68420f9 Mon Sep 17 00:00:00 2001 From: chenjiayi Date: Wed, 8 Nov 2023 14:28:35 +0800 Subject: [PATCH 7/8] fix(devmaster): drop unnecessary debug trait implementations The debug trait derivations will break the compilation. --- .../src/lib/framework/control_manager.rs | 1 - .../src/lib/framework/garbage_collect.rs | 2 -- .../src/lib/framework/worker_manager.rs | 2 -- libs/device/src/error.rs | 16 +++------------- 4 files changed, 3 insertions(+), 18 deletions(-) diff --git a/exts/devmaster/src/lib/framework/control_manager.rs b/exts/devmaster/src/lib/framework/control_manager.rs index 75fae984..342c6a0c 100644 --- a/exts/devmaster/src/lib/framework/control_manager.rs +++ b/exts/devmaster/src/lib/framework/control_manager.rs @@ -30,7 +30,6 @@ use std::{ pub const CONTROL_MANAGER_LISTEN_ADDR: &str = "/run/devmaster/control"; /// control manager -#[derive(Debug)] pub struct ControlManager { /// listener for devctl messages listener: RefCell, diff --git a/exts/devmaster/src/lib/framework/garbage_collect.rs b/exts/devmaster/src/lib/framework/garbage_collect.rs index 7a12ede8..c154f430 100644 --- a/exts/devmaster/src/lib/framework/garbage_collect.rs +++ b/exts/devmaster/src/lib/framework/garbage_collect.rs @@ -25,7 +25,6 @@ use std::{ /// max time interval for idle worker const WORKER_MAX_IDLE_INTERVAL: u64 = 3; -#[derive(Debug)] pub(crate) struct GarbageCollect { devmaster: Weak>, @@ -128,7 +127,6 @@ impl Source for GarbageCollect { } /// kill idle workers -#[derive(Debug)] pub(crate) struct IdleWorkerKiller { /// time interval pub(crate) time: u64, diff --git a/exts/devmaster/src/lib/framework/worker_manager.rs b/exts/devmaster/src/lib/framework/worker_manager.rs index c7c1d45f..870f6779 100644 --- a/exts/devmaster/src/lib/framework/worker_manager.rs +++ b/exts/devmaster/src/lib/framework/worker_manager.rs @@ -47,7 +47,6 @@ pub(crate) enum WorkerMessage { } /// worker manager -#[derive(Debug)] pub struct WorkerManager { /// max number of workers pub(crate) workers_capacity: u32, @@ -65,7 +64,6 @@ pub struct WorkerManager { } /// worker -#[derive(Debug)] pub struct Worker { /// worker unique id id: u32, diff --git a/libs/device/src/error.rs b/libs/device/src/error.rs index 10410301..2cea6f4b 100644 --- a/libs/device/src/error.rs +++ b/libs/device/src/error.rs @@ -19,22 +19,13 @@ use snafu::prelude::Snafu; #[derive(Debug, Snafu)] #[snafu(visibility(pub))] #[non_exhaustive] +#[allow(missing_docs)] pub enum Error { - /// other error #[snafu(context, display("Device error: {}", msg))] - Nix { - /// message - msg: String, - /// errno indicates the error kind - source: nix::Error, - }, + Nix { msg: String, source: nix::Error }, #[snafu(context, display("IO error: {}", msg))] - Io { - /// message - msg: String, - source: std::io::Error, - }, + Io { msg: String, source: std::io::Error }, #[snafu(context, display("Basic error: {}", msg))] Basic { msg: String, source: basic::Error }, @@ -106,7 +97,6 @@ impl Error { } } - #[cfg(test)] mod test { use super::*; -- Gitee From db90ea666e0bc7bfc6915689abc3f5dc98d0e30b Mon Sep 17 00:00:00 2001 From: chenjiayi Date: Wed, 8 Nov 2023 16:55:29 +0800 Subject: [PATCH 8/8] fix(device): fix UT privilege error in ci The user in ci has limited privileges, which will prevent actions like writing some device sysattrs. --- libs/device/src/device.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/libs/device/src/device.rs b/libs/device/src/device.rs index 284cd6e9..4f5779ce 100644 --- a/libs/device/src/device.rs +++ b/libs/device/src/device.rs @@ -3626,7 +3626,7 @@ G:devmaster Q:devmaster V:100 "; - touch_file("/tmp/tmp_db", false, None, None, None).unwrap(); + touch_file("/tmp/tmp_db", false, Some(0o777), None, None).unwrap(); let mut f = OpenOptions::new().write(true).open("/tmp/tmp_db").unwrap(); f.write_all(content.as_bytes()).unwrap(); let device = Device::new(); @@ -3675,19 +3675,22 @@ V:100 } unlink("/tmp/tmp_db").unwrap(); + unlink("/tmp/tmp_db_writeonly").unwrap(); } #[test] fn test_set_is_initialized() { let device = Device::from_subsystem_sysname("net", "lo").unwrap(); device.set_is_initialized(); - device + if device .trigger_with_uuid(DeviceAction::Change, false) - .unwrap(); - device - .trigger_with_uuid(DeviceAction::Change, true) - .unwrap(); - device.trigger(DeviceAction::Change).unwrap(); + .is_ok() + { + device + .trigger_with_uuid(DeviceAction::Change, true) + .unwrap(); + device.trigger(DeviceAction::Change).unwrap(); + } } #[test] @@ -3707,14 +3710,16 @@ V:100 device.set_devmode("666").unwrap(); device.set_diskseq("1").unwrap(); device.set_action_from_string("change").unwrap(); - device.set_sysattr_value("ifalias", Some("test")).unwrap(); + + if device.set_sysattr_value("ifalias", Some("test")).is_ok() { + assert_eq!(&device.get_cached_sysattr_value("ifalias").unwrap(), "test"); + } assert_eq!(&device.get_property_value("DEVUID").unwrap(), "1"); assert_eq!(&device.get_property_value("DEVGID").unwrap(), "1"); assert_eq!(&device.get_property_value("DEVMODE").unwrap(), "666"); assert_eq!(&device.get_property_value("DISKSEQ").unwrap(), "1"); assert_eq!(&device.get_property_value("ACTION").unwrap(), "change"); - assert_eq!(&device.get_cached_sysattr_value("ifalias").unwrap(), "test"); assert!(device.set_devuid("invalid").is_err()); assert!(device.set_devgid("invalid").is_err()); -- Gitee