From 02b388d6cc30d6bbd11e6ca9d5f5b018ad6638bb Mon Sep 17 00:00:00 2001 From: chenjiayi Date: Wed, 8 Nov 2023 02:11:59 +0800 Subject: [PATCH 047/103] 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) } } -- 2.33.0