[i2c] Unify Error types

The crate uses lots of ways to report errors, unify them and use a
separate Error enum type for each file. This will also help us improve
the test code later on.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
This commit is contained in:
Viresh Kumar 2021-10-26 13:49:48 +05:30
parent e62e27aead
commit ca5e7e02dd
4 changed files with 183 additions and 141 deletions

View File

@ -16,6 +16,7 @@ clap = { version = "=3.0.0-beta.2", features = ["yaml"] }
epoll = "4.3"
libc = ">=0.2.95"
log = ">=0.4.6"
thiserror = "1.0"
vhost = { version = "0.2", features = ["vhost-user-slave"] }
vhost-user-backend = { git = "https://github.com/rust-vmm/vhost-user-backend", rev = "bd6b53348f06055abcb2b7254168d716b742f383" }
virtio-bindings = ">=0.1"

View File

@ -9,8 +9,9 @@ use log::info;
use std::fs::{File, OpenOptions};
use std::os::unix::io::AsRawFd;
use libc::{c_ulong, ioctl, EINVAL};
use vmm_sys_util::errno::{errno_result, Error, Result};
use libc::{c_ulong, ioctl};
use thiserror::Error as ThisError;
use vmm_sys_util::errno::Error as IoError;
use super::AdapterConfig;
@ -21,6 +22,27 @@ type IoctlRequest = libc::c_int;
#[cfg(not(target_env = "musl"))]
type IoctlRequest = c_ulong;
type Result<T> = std::result::Result<T, Error>;
#[derive(Copy, Clone, Debug, PartialEq, ThisError)]
/// Errors related to low level i2c helpers
pub enum Error {
#[error("Incorrect message length for {0} operation: {1}")]
MessageLengthInvalid(&'static str, usize),
#[error("Invalid SMBUS command: {0}")]
SMBusCommandInvalid(u32),
#[error("Invalid SMBus transfer, request-count: {0}, req[0].len: {1}, req[1].len: {2}")]
SMBusTransferInvalid(usize, u16, u16),
#[error("Failed to open adapter at /dev/i2c-{0}")]
DeviceOpenFailed(u32),
#[error("Ioctl command failed for {0} operation: {1}")]
IoctlFailure(&'static str, IoError),
#[error("Invalid Adapter function: {0:x}")]
AdapterFunctionInvalid(u64),
#[error("Invalid Client Address")]
ClientAddressInvalid,
}
/// Linux I2C/SMBUS definitions
/// IOCTL commands
@ -148,11 +170,7 @@ impl I2cSmbusIoctlData {
if (reqs[0].flags & I2C_M_RD) != 0 {
// Special Read requests, reqs[0].len can be 0 or 1 only.
if reqs[0].len > 1 {
println!(
"Incorrect message length for read operation: {}",
reqs[0].len
);
return Err(Error::new(EINVAL));
return Err(Error::MessageLengthInvalid("read", reqs[0].len as usize));
}
read_write = I2C_SMBUS_READ;
} else {
@ -174,11 +192,7 @@ impl I2cSmbusIoctlData {
I2C_SMBUS_WORD_DATA
}
_ => {
println!(
"Message length not supported for write operation: {}",
reqs[0].len
);
return Err(Error::new(EINVAL));
return Err(Error::MessageLengthInvalid("write", reqs[0].len as usize));
}
}
}
@ -196,11 +210,11 @@ impl I2cSmbusIoctlData {
|| (reqs[0].len != 1)
|| (reqs[1].len > 2)
{
println!(
"Expecting a valid read smbus transfer: {:?}",
(reqs.len(), reqs[0].len, reqs[1].len)
);
return Err(Error::new(EINVAL));
return Err(Error::SMBusTransferInvalid(
reqs.len(),
reqs[0].len,
reqs[1].len,
));
}
read_write = I2C_SMBUS_READ;
@ -212,8 +226,11 @@ impl I2cSmbusIoctlData {
}
_ => {
println!("Invalid number of messages for smbus xfer: {}", reqs.len());
return Err(Error::new(EINVAL));
return Err(Error::SMBusTransferInvalid(
reqs.len(),
reqs[0].len,
reqs[1].len,
));
}
}
@ -247,16 +264,16 @@ pub trait I2cDevice {
Self: Sized;
// Corresponds to the I2C_FUNCS ioctl call.
fn funcs(&mut self, func: u64) -> i32;
fn funcs(&mut self, func: u64) -> Result<()>;
// Corresponds to the I2C_RDWR ioctl call.
fn rdwr(&self, data: &I2cRdwrIoctlData) -> i32;
fn rdwr(&self, data: &I2cRdwrIoctlData) -> Result<()>;
// Corresponds to the I2C_SMBUS ioctl call.
fn smbus(&self, data: &I2cSmbusIoctlData) -> i32;
fn smbus(&self, data: &I2cSmbusIoctlData) -> Result<()>;
// Corresponds to the I2C_SLAVE ioctl call.
fn slave(&self, addr: u64) -> i32;
fn slave(&self, addr: u64) -> Result<()>;
// Returns the adapter number corresponding to this device.
fn adapter_no(&self) -> u32;
@ -276,25 +293,50 @@ impl I2cDevice for PhysDevice {
file: OpenOptions::new()
.read(true)
.write(true)
.open(device_path)?,
.open(device_path)
.map_err(|_| Error::DeviceOpenFailed(adapter_no))?,
adapter_no,
})
}
fn funcs(&mut self, func: u64) -> i32 {
unsafe { ioctl(self.file.as_raw_fd(), I2C_FUNCS, &func) }
fn funcs(&mut self, func: u64) -> Result<()> {
let ret = unsafe { ioctl(self.file.as_raw_fd(), I2C_FUNCS, &func) };
if ret == -1 {
Err(Error::IoctlFailure("funcs", IoError::last()))
} else {
Ok(())
}
}
fn rdwr(&self, data: &I2cRdwrIoctlData) -> i32 {
unsafe { ioctl(self.file.as_raw_fd(), I2C_RDWR, data) }
fn rdwr(&self, data: &I2cRdwrIoctlData) -> Result<()> {
let ret = unsafe { ioctl(self.file.as_raw_fd(), I2C_RDWR, data) };
if ret == -1 {
Err(Error::IoctlFailure("rdwr", IoError::last()))
} else {
Ok(())
}
}
fn smbus(&self, data: &I2cSmbusIoctlData) -> i32 {
unsafe { ioctl(self.file.as_raw_fd(), I2C_SMBUS, data) }
fn smbus(&self, data: &I2cSmbusIoctlData) -> Result<()> {
let ret = unsafe { ioctl(self.file.as_raw_fd(), I2C_SMBUS, data) };
if ret == -1 {
Err(Error::IoctlFailure("smbus", IoError::last()))
} else {
Ok(())
}
}
fn slave(&self, addr: u64) -> i32 {
unsafe { ioctl(self.file.as_raw_fd(), I2C_SLAVE, addr as c_ulong) }
fn slave(&self, addr: u64) -> Result<()> {
let ret = unsafe { ioctl(self.file.as_raw_fd(), I2C_SLAVE, addr as c_ulong) };
if ret == -1 {
Err(Error::IoctlFailure("slave", IoError::last()))
} else {
Ok(())
}
}
fn adapter_no(&self) -> u32 {
@ -312,11 +354,8 @@ impl<D: I2cDevice> I2cAdapter<D> {
// Creates a new adapter corresponding to `device`.
fn new(mut device: D) -> Result<I2cAdapter<D>> {
let func: u64 = I2C_FUNC_SMBUS_ALL;
let ret = device.funcs(func);
if ret == -1 {
println!("Failed to get I2C function");
return errno_result();
}
device.funcs(func)?;
let smbus;
if (func & I2C_FUNC_I2C) != 0 {
@ -324,8 +363,7 @@ impl<D: I2cDevice> I2cAdapter<D> {
} else if (func & I2C_FUNC_SMBUS_ALL) != 0 {
smbus = true;
} else {
println!("Invalid functionality {:x}", func);
return Err(Error::new(EINVAL));
return Err(Error::AdapterFunctionInvalid(func));
}
Ok(I2cAdapter {
@ -339,7 +377,6 @@ impl<D: I2cDevice> I2cAdapter<D> {
fn i2c_transfer(&self, reqs: &mut [I2cReq]) -> Result<()> {
let mut msgs: Vec<I2cMsg> = Vec::with_capacity(reqs.len());
let len = reqs.len();
let addr = reqs[0].addr;
for req in reqs {
msgs.push(I2cMsg {
@ -355,27 +392,14 @@ impl<D: I2cDevice> I2cAdapter<D> {
nmsgs: len as u32,
};
let ret = self.device.rdwr(&data);
if ret == -1 {
println!("Failed to transfer i2c data to device addr to {:x}", addr);
errno_result()
} else {
Ok(())
}
self.device.rdwr(&data)
}
/// Perform I2C_SMBUS transfer
fn smbus_transfer(&self, reqs: &mut [I2cReq]) -> Result<()> {
let smbus_data = I2cSmbusIoctlData::new(reqs)?;
let ret = self.device.smbus(&smbus_data);
if ret == -1 {
println!(
"Failed to transfer smbus data to device addr to {:x}",
reqs[0].addr
);
return errno_result();
}
self.device.smbus(&smbus_data)?;
if smbus_data.read_write == I2C_SMBUS_READ {
unsafe {
@ -388,8 +412,7 @@ impl<D: I2cDevice> I2cAdapter<D> {
}
_ => {
println!("Invalid SMBUS command: {}", smbus_data.size);
return Err(Error::new(EINVAL));
return Err(Error::SMBusCommandInvalid(smbus_data.size));
}
}
}
@ -407,14 +430,7 @@ impl<D: I2cDevice> I2cAdapter<D> {
/// Sets device's address for an I2C adapter.
fn set_device_addr(&self, addr: usize) -> Result<()> {
let ret = self.device.slave(addr as u64);
if ret == -1 {
println!("Failed to set device addr to {:x}", addr);
errno_result()
} else {
Ok(())
}
self.device.slave(addr as u64)
}
fn transfer(&self, reqs: &mut [I2cReq]) -> Result<()> {
@ -475,7 +491,7 @@ impl<D: I2cDevice> I2cMap<D> {
// This can happen a lot while scanning the bus, don't print any errors.
if index == I2C_INVALID_ADAPTER {
return Err(Error::new(EINVAL));
return Err(Error::ClientAddressInvalid);
}
// get the corresponding adapter based on the device config.
@ -492,12 +508,12 @@ pub mod tests {
use super::*;
use std::convert::TryFrom;
#[derive(Debug, Default)]
#[derive(Debug)]
pub struct DummyDevice {
funcs_result: i32,
rdwr_result: i32,
smbus_result: i32,
slave_result: i32,
funcs_result: Result<()>,
rdwr_result: Result<()>,
smbus_result: Result<()>,
slave_result: Result<()>,
adapter_no: u32,
}
@ -508,23 +524,26 @@ pub mod tests {
{
Ok(DummyDevice {
adapter_no,
..Default::default()
funcs_result: Ok(()),
rdwr_result: Ok(()),
smbus_result: Ok(()),
slave_result: Ok(()),
})
}
fn funcs(&mut self, _func: u64) -> i32 {
fn funcs(&mut self, _func: u64) -> Result<()> {
self.funcs_result
}
fn rdwr(&self, _data: &I2cRdwrIoctlData) -> i32 {
fn rdwr(&self, _data: &I2cRdwrIoctlData) -> Result<()> {
self.rdwr_result
}
fn smbus(&self, _data: &I2cSmbusIoctlData) -> i32 {
fn smbus(&self, _data: &I2cSmbusIoctlData) -> Result<()> {
self.smbus_result
}
fn slave(&self, _addr: u64) -> i32 {
fn slave(&self, _addr: u64) -> Result<()> {
self.slave_result
}
@ -533,16 +552,6 @@ pub mod tests {
}
}
#[test]
fn test_i2c_map_duplicate_device4() {
assert!(AdapterConfig::try_from("1:4,2:32:21,5:4:23").is_err());
}
#[test]
fn test_duplicated_adapter_no() {
assert!(AdapterConfig::try_from("1:4,1:32:21,5:10:23").is_err());
}
#[test]
fn test_i2c_map() {
let adapter_config = AdapterConfig::try_from("1:4,2:32:21,5:10:23").unwrap();

View File

@ -10,10 +10,12 @@ mod vhu_i2c;
use log::{info, warn};
use std::convert::TryFrom;
use std::num::ParseIntError;
use std::sync::{Arc, RwLock};
use std::thread::spawn;
use clap::{load_yaml, App, ArgMatches};
use thiserror::Error as ThisError;
use vhost::{vhost_user, vhost_user::Listener};
use vhost_user_backend::VhostUserDaemon;
use vm_memory::{GuestMemoryAtomic, GuestMemoryMmap};
@ -21,6 +23,31 @@ use vm_memory::{GuestMemoryAtomic, GuestMemoryMmap};
use i2c::{I2cDevice, I2cMap, PhysDevice, MAX_I2C_VDEV};
use vhu_i2c::VhostUserI2cBackend;
type Result<T> = std::result::Result<T, Error>;
#[derive(Debug, PartialEq, ThisError)]
/// Errors related to low level i2c helpers
pub enum Error {
#[error("Invalid socket path")]
SocketPathInvalid,
#[error("Invalid socket count: {0}")]
SocketCountInvalid(usize),
#[error("Invalid device list")]
DeviceListInvalid,
#[error("Duplicate adapter detected: {0}")]
AdapterDuplicate(u32),
#[error("Invalid client address: {0}")]
ClientAddressInvalid(u16),
#[error("Duplicate client address detected: {0}")]
ClientAddressDuplicate(u16),
#[error("Low level I2c failure: {0:?}")]
I2cFailure(i2c::Error),
#[error("Failed while parsing to integer: {0:?}")]
ParseFailure(ParseIntError),
#[error("Failed to join threads")]
FailedJoiningThreads,
}
#[derive(Debug, PartialEq)]
struct DeviceConfig {
adapter_no: u32,
@ -35,13 +62,13 @@ impl DeviceConfig {
}
}
fn push(&mut self, addr: u16) -> std::result::Result<(), String> {
fn push(&mut self, addr: u16) -> Result<()> {
if addr as usize > MAX_I2C_VDEV {
return Err(format!("Invalid address: {} (> maximum allowed)", addr));
return Err(Error::ClientAddressInvalid(addr));
}
if self.addr.contains(&addr) {
return Err(format!("Address already in use: {}", addr));
return Err(Error::ClientAddressDuplicate(addr));
}
self.addr.push(addr);
@ -67,14 +94,14 @@ impl AdapterConfig {
self.inner.iter().any(|elem| elem.addr.contains(&addr))
}
fn push(&mut self, device: DeviceConfig) -> std::result::Result<(), String> {
fn push(&mut self, device: DeviceConfig) -> Result<()> {
if self.contains_adapter_no(device.adapter_no) {
return Err("Duplicated adapter number".to_string());
return Err(Error::AdapterDuplicate(device.adapter_no));
}
for addr in device.addr.iter() {
if self.contains_addr(*addr) {
return Err(format!("Address already in use: {}", addr));
return Err(Error::ClientAddressDuplicate(*addr));
}
}
@ -84,21 +111,19 @@ impl AdapterConfig {
}
impl TryFrom<&str> for AdapterConfig {
type Error = String;
type Error = Error;
fn try_from(list: &str) -> Result<Self, Self::Error> {
fn try_from(list: &str) -> Result<Self> {
let busses: Vec<&str> = list.split(',').collect();
let mut devices = AdapterConfig::new();
for businfo in busses.iter() {
let list: Vec<&str> = businfo.split(':').collect();
let bus_addr = list[0].parse::<u32>().map_err(|_| "Invalid bus address")?;
let bus_addr = list[0].parse::<u32>().map_err(Error::ParseFailure)?;
let mut adapter = DeviceConfig::new(bus_addr);
for device_str in list[1..].iter() {
let addr = device_str
.parse::<u16>()
.map_err(|_| "Invalid device addr")?;
let addr = device_str.parse::<u16>().map_err(Error::ParseFailure)?;
adapter.push(addr)?;
}
@ -116,25 +141,27 @@ struct I2cConfiguration {
}
impl TryFrom<ArgMatches> for I2cConfiguration {
type Error = String;
type Error = Error;
fn try_from(cmd_args: ArgMatches) -> Result<Self, Self::Error> {
fn try_from(cmd_args: ArgMatches) -> Result<Self> {
let socket_path = cmd_args
.value_of("socket_path")
.ok_or("Invalid socket path")?
.ok_or(Error::SocketPathInvalid)?
.to_string();
let socket_count = cmd_args
.value_of("socket_count")
.unwrap_or("1")
.parse::<usize>()
.map_err(|_| "Invalid socket_count")?;
.map_err(Error::ParseFailure)?;
if socket_count == 0 {
return Err("Socket count can't be 0".to_string());
return Err(Error::SocketCountInvalid(0));
}
let list = cmd_args.value_of("devices").ok_or("Invalid devices list")?;
let list = cmd_args
.value_of("devices")
.ok_or(Error::DeviceListInvalid)?;
let devices = AdapterConfig::try_from(list)?;
Ok(I2cConfiguration {
socket_path,
@ -144,14 +171,9 @@ impl TryFrom<ArgMatches> for I2cConfiguration {
}
}
fn start_backend<D: 'static + I2cDevice + Send + Sync>(
config: I2cConfiguration,
) -> Result<(), String> {
fn start_backend<D: 'static + I2cDevice + Send + Sync>(config: I2cConfiguration) -> Result<()> {
// The same i2c_map structure instance is shared between all the guests
let i2c_map = Arc::new(
I2cMap::<D>::new(&config.devices)
.map_err(|e| format!("Failed to create i2c_map ({})", e))?,
);
let i2c_map = Arc::new(I2cMap::<D>::new(&config.devices).map_err(Error::I2cFailure)?);
let mut handles = Vec::new();
@ -201,13 +223,13 @@ fn start_backend<D: 'static + I2cDevice + Send + Sync>(
}
for handle in handles {
handle.join().map_err(|_| "Failed to join threads")?;
handle.join().map_err(|_| Error::FailedJoiningThreads)?;
}
Ok(())
}
fn main() -> Result<(), String> {
fn main() -> Result<()> {
let yaml = load_yaml!("cli.yaml");
let cmd_args = App::from(yaml).get_matches();
@ -252,21 +274,21 @@ mod tests {
let cmd_args = get_cmd_args(socket_name, "1:4d", Some(5));
assert_eq!(
I2cConfiguration::try_from(cmd_args).unwrap_err(),
"Invalid device addr"
Error::ParseFailure("4d".parse::<u32>().unwrap_err())
);
// Invalid socket count
let cmd_args = get_cmd_args(socket_name, "1:4", Some(0));
assert_eq!(
I2cConfiguration::try_from(cmd_args).unwrap_err(),
"Socket count can't be 0"
Error::SocketCountInvalid(0)
);
// Duplicate client address: 4
let cmd_args = get_cmd_args(socket_name, "1:4,2:32:21,5:4:23", Some(5));
assert_eq!(
I2cConfiguration::try_from(cmd_args).unwrap_err(),
"Address already in use: 4"
Error::ClientAddressDuplicate(4)
);
}
@ -296,4 +318,20 @@ mod tests {
assert_eq!(config, expected_config);
}
#[test]
fn test_i2c_map_duplicate_device4() {
assert_eq!(
AdapterConfig::try_from("1:4,2:32:21,5:4:23").unwrap_err(),
Error::ClientAddressDuplicate(4)
);
}
#[test]
fn test_duplicated_adapter_no() {
assert_eq!(
AdapterConfig::try_from("1:4,1:32:21,5:10:23").unwrap_err(),
Error::AdapterDuplicate(1)
);
}
}

View File

@ -8,8 +8,9 @@
use log::warn;
use std::mem::size_of;
use std::sync::Arc;
use std::{convert, error, fmt, io};
use std::{convert, io};
use thiserror::Error as ThisError;
use vhost::vhost_user::message::{VhostUserProtocolFeatures, VhostUserVirtioFeatures};
use vhost_user_backend::{VhostUserBackendMut, VringRwLock, VringT};
use virtio_bindings::bindings::virtio_net::{VIRTIO_F_NOTIFY_ON_EMPTY, VIRTIO_F_VERSION_1};
@ -27,36 +28,29 @@ const NUM_QUEUES: usize = 1;
type Result<T> = std::result::Result<T, Error>;
type VhostUserBackendResult<T> = std::result::Result<T, std::io::Error>;
#[derive(Debug)]
#[derive(Debug, ThisError)]
/// Errors related to vhost-device-i2c daemon.
pub enum Error {
/// Failed to handle event other than input event.
#[error("Failed to handle event, didn't match EPOLLIN")]
HandleEventNotEpollIn,
/// Failed to handle unknown event.
HandleEventUnknownEvent,
/// Guest gave us a write only descriptor that protocol says to read from.
#[error("Failed to handle unknown event")]
HandleEventUnknown,
#[error("Received unexpected write only descriptor")]
UnexpectedWriteOnlyDescriptor,
/// Guest gave us a readable descriptor that protocol says to only write to.
UnexpectedReadDescriptor,
/// Invalid descriptor count
#[error("Received unexpected readable descriptor")]
UnexpectedReadableDescriptor,
#[error("Invalid descriptor count")]
UnexpectedDescriptorCount,
/// Invalid descriptor
#[error("Invalid descriptor size")]
UnexpectedDescriptorSize,
/// Descriptor not found
#[error("Descriptor not found")]
DescriptorNotFound,
/// Descriptor read failed
#[error("Descriptor read failed")]
DescriptorReadFailed,
/// Descriptor write failed
#[error("Descriptor write failed")]
DescriptorWriteFailed,
/// Descriptor send failed
DescriptorSendFailed,
}
impl error::Error for Error {}
impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "vhost-device-i2c error: {:?}", self)
}
#[error("Failed to send notification")]
NotificationFailed,
}
impl convert::From<Error> for io::Error {
@ -143,7 +137,7 @@ impl<D: I2cDevice> VhostUserI2cBackend<D> {
let desc_in_hdr = descriptors[2];
if !desc_in_hdr.is_write_only() {
return Err(Error::UnexpectedReadDescriptor);
return Err(Error::UnexpectedReadableDescriptor);
}
if desc_in_hdr.len() as usize != size_of::<u8>() {
@ -217,7 +211,7 @@ impl<D: I2cDevice> VhostUserI2cBackend<D> {
// Send notification once all the requests are processed
vring
.signal_used_queue()
.map_err(|_| Error::DescriptorSendFailed)?;
.map_err(|_| Error::NotificationFailed)?;
Ok(true)
}
}
@ -294,7 +288,7 @@ impl<D: 'static + I2cDevice + Sync + Send> VhostUserBackendMut<VringRwLock, ()>
_ => {
warn!("unhandled device_event: {}", device_event);
return Err(Error::HandleEventUnknownEvent.into());
return Err(Error::HandleEventUnknown.into());
}
}
Ok(false)