From 8870ddf640e4e989b4058e7d2e4fc4750a2d10e0 Mon Sep 17 00:00:00 2001 From: Tom Kirchner Date: Wed, 16 Oct 2019 17:28:21 -0700 Subject: [PATCH] block-party: Use a full snafu error rather than an io::Error shim --- workspaces/Cargo.lock | 3 + workspaces/growpart/src/diskpart/error.rs | 4 +- workspaces/updater/block-party/Cargo.toml | 3 + workspaces/updater/block-party/src/lib.rs | 171 +++++++++++++++------- workspaces/updater/signpost/src/error.rs | 8 +- 5 files changed, 133 insertions(+), 56 deletions(-) diff --git a/workspaces/Cargo.lock b/workspaces/Cargo.lock index 723070aa3cc..d7a6c59f5df 100644 --- a/workspaces/Cargo.lock +++ b/workspaces/Cargo.lock @@ -400,6 +400,9 @@ dependencies = [ [[package]] name = "block-party" version = "0.1.0" +dependencies = [ + "snafu 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", +] [[package]] name = "bork" diff --git a/workspaces/growpart/src/diskpart/error.rs b/workspaces/growpart/src/diskpart/error.rs index f8315a13673..fb0d7be6059 100644 --- a/workspaces/growpart/src/diskpart/error.rs +++ b/workspaces/growpart/src/diskpart/error.rs @@ -12,13 +12,13 @@ pub enum Error { #[snafu(display("Failed to find block device for '{}': {}", path.display(), source))] FindBlockDevice { path: std::path::PathBuf, - source: std::io::Error, + source: block_party::Error, }, #[snafu(display("Failed to find disk for '{}': {}", path.display(), source))] FindDisk { path: std::path::PathBuf, - source: std::io::Error, + source: block_party::Error, }, #[snafu(display("Expected partition for '{}'", path.display()))] diff --git a/workspaces/updater/block-party/Cargo.toml b/workspaces/updater/block-party/Cargo.toml index 7233f68f0c5..5b91d63e16b 100644 --- a/workspaces/updater/block-party/Cargo.toml +++ b/workspaces/updater/block-party/Cargo.toml @@ -4,3 +4,6 @@ version = "0.1.0" authors = ["iliana destroyer of worlds "] edition = "2018" publish = false + +[dependencies] +snafu = "0.5.0" diff --git a/workspaces/updater/block-party/src/lib.rs b/workspaces/updater/block-party/src/lib.rs index f59fca5c56e..a9c61ca30f3 100644 --- a/workspaces/updater/block-party/src/lib.rs +++ b/workspaces/updater/block-party/src/lib.rs @@ -9,14 +9,101 @@ #![deny(missing_docs, rust_2018_idioms)] #![warn(clippy::pedantic)] +use snafu::{ensure, OptionExt, ResultExt}; use std::ffi::OsString; use std::fmt; use std::fs; -use std::io::{Error, ErrorKind, Result}; +use std::io; use std::os::linux::fs::MetadataExt; use std::path::{Path, PathBuf}; use std::str::FromStr; +mod error { + use snafu::Snafu; + use std::io; + use std::path::PathBuf; + + #[derive(Debug, Snafu)] + #[snafu(visibility = "pub(super)")] + /// The error type for this library. + pub enum Error { + #[snafu(display("Target of {} ends in `..`", path.display()))] + /// The target of a link ends in `..` + LinkTargetFileName { + /// Contains the invalid link path. + path: PathBuf + }, + + #[snafu(display("Cannot parse {} as major/minor numbers: {}", path.display(), source))] + /// Can't parse the given path as major/minor numbers. + MajorMinorParseInt { + /// Contains the path we failed to parse as major/minor. + path: PathBuf, + /// The source error describing the parse failure. + source: std::num::ParseIntError, + }, + + #[snafu(display( + "Cannot parse {} as major/minor numbers: invalid number of colons", + path.display()) + )] + /// Can't parse the given string as major/minor numbers because it has an invalid number + /// of colons. + MajorMinorLen { + /// Contains the path which in turn contains an invalid major/minor string. + path: PathBuf + }, + + #[snafu(display("Unable to read device name through link at {}: {} ", path.display(), source))] + /// Unable to read device name through the given link. + SysPathLinkRead { + /// Contains the path we failed to read. + path: PathBuf, + /// The source error describing the read failure. + source: io::Error + }, + + #[snafu(display("Unable to read filesystem metadata of {}: {} ", path.display(), source))] + /// Unable to read filesystem metadata of a given path. + PathMetadata { + /// Contains the path for which we failed to read metadata. + path: PathBuf, + /// The source error describing the read failure. + source: io::Error + }, + + #[snafu(display("Unable to read file {}: {} ", path.display(), source))] + /// Unable to read a given file. + FileRead { + /// Contains the path we failed to read. + path: PathBuf, + /// The source error describing the read failure. + source: io::Error + }, + + #[snafu(display("Unable to list directory {}: {} ", path.display(), source))] + /// Unable to list a given directory. + ListDirectory { + /// Contains the directory we failed to list. + path: PathBuf, + /// The source error describing the list failure. + source: io::Error + }, + + #[snafu(display("Unable to read directory entry in {}: {} ", path.display(), source))] + /// Unable to read a listed directory entry. + ReadDirectoryEntry { + /// Contains the directory with an entry we failed to read. + path: PathBuf, + /// The source error describing the read failure. + source: io::Error + }, + } +} +pub use error::Error; +/// Convenience alias pointing to our Error type. +pub type Result = std::result::Result; + /// Get the path in `/sys/dev/block` for a major/minor number. fn sys_path(major: u64, minor: u64) -> PathBuf { PathBuf::from("/sys/dev/block").join(format!("{}:{}", major, minor)) @@ -34,10 +121,10 @@ impl BlockDevice { /// Creates a `BlockDevice` for a major/minor number. pub fn from_major_minor(major: u64, minor: u64) -> Result { let path = sys_path(major, minor); - let link_target = fs::read_link(&path)?; + let link_target = fs::read_link(&path).context(error::SysPathLinkRead { path })?; let device_name = link_target .file_name() - .ok_or_else(|| ErrorShim::LinkTargetFileName(&link_target))? + .context(error::LinkTargetFileName { path: &link_target })? .to_owned(); Ok(Self { @@ -49,7 +136,8 @@ impl BlockDevice { /// Creates a `BlockDevice` from a path residing on a block device. pub fn from_device_path>(path: P) -> Result { - let metadata = fs::metadata(&path)?; + let path = path.as_ref(); + let metadata = fs::metadata(path).context(error::PathMetadata { path })?; let major = metadata.st_dev() >> 8; let minor = metadata.st_dev() & 0xff; Ok(Self::from_major_minor(major, minor)?) @@ -57,7 +145,8 @@ impl BlockDevice { /// Creates a `BlockDevice` from a special block device node. pub fn from_device_node>(path: P) -> Result { - let metadata = fs::metadata(&path)?; + let path = path.as_ref(); + let metadata = fs::metadata(&path).context(error::PathMetadata { path })?; let major = metadata.st_rdev() >> 8; let minor = metadata.st_rdev() & 0xff; Ok(Self::from_major_minor(major, minor)?) @@ -65,16 +154,17 @@ impl BlockDevice { /// Creates a `BlockDevice` from the major:minor string from the file at `path`. fn from_major_minor_in_file>(path: P) -> Result { - let s = fs::read_to_string(path.as_ref())?; + let path = path.as_ref(); + let s = fs::read_to_string(path).context(error::FileRead { path })?; let parts = s .trim() .splitn(2, ':') .map(u64::from_str) .collect::, _>>() - .map_err(|err| ErrorShim::MajorMinorParseInt(path.as_ref(), err))?; - if parts.len() != 2 { - Err(ErrorShim::MajorMinorLen(path.as_ref()))?; - } + .context(error::MajorMinorParseInt { path })?; + + ensure!(parts.len() == 2, error::MajorMinorLen { path }); + Self::from_major_minor(parts[0], parts[1]) } @@ -93,8 +183,10 @@ impl BlockDevice { //#[allow(clippy::identity_conversion)] // https://github.com/rust-lang/rust-clippy/issues/4133 pub fn disk(&self) -> Result> { // Globbing for /sys/block/*/{self.device_name}/dev - for entry in fs::read_dir("/sys/block")? { - let entry = entry?; + for entry in + fs::read_dir("/sys/block").context(error::ListDirectory { path: "/sys/block" })? + { + let entry = entry.context(error::ReadDirectoryEntry { path: "/sys/block" })?; if entry.path().join(&self.device_name).exists() { return Self::from_major_minor_in_file(entry.path().join("dev")).map(Some); } @@ -111,15 +203,19 @@ impl BlockDevice { pub fn partition(&self, part_num: u32) -> Result> { let sys_path = self.sys_path(); // Globbing for /sys/dev/block/{major}:{minor}/*/partition - for entry in fs::read_dir(&sys_path)? { - let entry = entry?; + for entry in fs::read_dir(&sys_path).context(error::ListDirectory { path: &sys_path })? { + let entry = entry.context(error::ReadDirectoryEntry { path: &sys_path })?; if entry.path().is_dir() { let partition_path = entry.path().join("partition"); let partition_str = match fs::read_to_string(&partition_path) { Ok(s) => s, Err(err) => match err.kind() { - ErrorKind::NotFound => continue, - _ => return Err(err), + io::ErrorKind::NotFound => continue, + _ => { + return Err(err).context(error::FileRead { + path: partition_path, + }) + } }, }; if partition_str.trim() == part_num.to_string() { @@ -135,7 +231,10 @@ impl BlockDevice { /// For example, given a dm-verity device, this iterator would return the data device and the /// hash device. pub fn lower_devices(&self) -> Result { - fs::read_dir(&self.sys_path().join("slaves")).map(|iter| LowerIter { iter }) + let path = self.sys_path().join("slaves"); + fs::read_dir(&path) + .context(error::ListDirectory { path: &path }) + .map(move |iter| LowerIter { path, iter }) } } @@ -155,6 +254,7 @@ impl PartialEq for BlockDevice { /// /// This struct is created by [`BlockDevice::lower_devices`]. pub struct LowerIter { + path: PathBuf, iter: fs::ReadDir, } @@ -164,38 +264,9 @@ impl Iterator for LowerIter { fn next(&mut self) -> Option> { self.iter .next() - .map(|entry| BlockDevice::from_major_minor_in_file(entry?.path().join("dev"))) - } -} - -enum ErrorShim<'a> { - LinkTargetFileName(&'a Path), - MajorMinorParseInt(&'a Path, std::num::ParseIntError), - MajorMinorLen(&'a Path), -} - -impl<'a> From> for Error { - fn from(err: ErrorShim<'_>) -> Self { - match err { - ErrorShim::LinkTargetFileName(path) => Self::new( - ErrorKind::InvalidData, - format!("target of {} ends in `..`", path.display()), - ), - ErrorShim::MajorMinorParseInt(path, err) => Self::new( - ErrorKind::InvalidData, - format!( - "cannot parse {} as major/minor numbers: {}", - path.display(), - err - ), - ), - ErrorShim::MajorMinorLen(path) => Self::new( - ErrorKind::InvalidData, - format!( - "cannot parse {} as major/minor numbers: invalid number of colons", - path.display() - ), - ), - } + .map(|entry| { + let entry = entry.context(error::ReadDirectoryEntry { path: &self.path })?; + BlockDevice::from_major_minor_in_file(entry.path().join("dev")) + }) } } diff --git a/workspaces/updater/signpost/src/error.rs b/workspaces/updater/signpost/src/error.rs index 7a5d58609aa..b38e16eebe3 100644 --- a/workspaces/updater/signpost/src/error.rs +++ b/workspaces/updater/signpost/src/error.rs @@ -19,19 +19,19 @@ pub enum Error { #[snafu(display("Failed to get block device from path {}: {}", device.display(), source))] BlockDeviceFromPath { device: PathBuf, - source: std::io::Error, + source: block_party::Error, }, #[snafu(display("Failed to get disk from partition {}: {}", device.display(), source))] DiskFromPartition { device: PathBuf, - source: std::io::Error, + source: block_party::Error, }, #[snafu(display("Failed to get partition on disk {}: {}", device.display(), source))] PartitionFromDisk { device: PathBuf, - source: std::io::Error, + source: block_party::Error, }, #[snafu(display("Failed to find GPT on device {}: {}", device.display(), source))] @@ -74,7 +74,7 @@ pub enum Error { #[snafu(display("Failed to get lower devices for {}: {}", root.display(), source))] RootLowerDevices { root: PathBuf, - source: std::io::Error, + source: block_party::Error, }, #[snafu(display("Block device {} is not a partition", device.display()))]