pxcloud/vhost-device
5
0
Fork 0
mirror of https://github.com/rust-vmm/vhost-device.git synced 2025-12-27 15:45:44 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

scmi: Refactor device specification and creation

Browse Source 
Making the device configuration polymorphic requires the device struct
to exist before the device parameters are checked and assigned to the
struct fields.  Which means wrapping the struct fields by Option
unnecessarily or introducing other data confusion.

Let's extract the device configuration from traits to plain functions
in order to keep the device struct's unencumbered.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
This commit is contained in:
Milan Zamazal 2023-07-31 16:17:10 +02:00 committed by Alex Bennée
parent 73c536df2e
commit ca8f181bcd
4 changed files with 199 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

254
crates/scmi/src/devices.rs
View File

@ -1,42 +1,147 @@
// SPDX-FileCopyrightText: Red Hat, Inc.
// SPDX-License-Identifier: Apache-2.0
use log::debug;
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::fmt::Write;
use std::process::exit;
use crate::{
scmi::{
DeviceResult, MessageId, MessageValue, MessageValues, ProtocolId, ScmiDevice,
ScmiDeviceError, MAX_SIMPLE_STRING_LENGTH, SENSOR_AXIS_DESCRIPTION_GET, SENSOR_CONFIG_GET,
SENSOR_CONFIG_SET, SENSOR_CONTINUOUS_UPDATE_NOTIFY, SENSOR_DESCRIPTION_GET,
SENSOR_PROTOCOL_ID, SENSOR_READING_GET, SENSOR_UNIT_METERS_PER_SECOND_SQUARED,
},
DeviceProperties,
use itertools::Itertools;
use log::debug;
use thiserror::Error as ThisError;
use crate::scmi::{
DeviceResult, MessageId, MessageValue, MessageValues, ProtocolId, ScmiDevice, ScmiDeviceError,
MAX_SIMPLE_STRING_LENGTH, SENSOR_AXIS_DESCRIPTION_GET, SENSOR_CONFIG_GET, SENSOR_CONFIG_SET,
SENSOR_CONTINUOUS_UPDATE_NOTIFY, SENSOR_DESCRIPTION_GET, SENSOR_PROTOCOL_ID,
SENSOR_READING_GET, SENSOR_UNIT_METERS_PER_SECOND_SQUARED,
};
enum ExitCodes {
Help = 1,
}
type DeviceSpecification = fn() -> Box<dyn ScmiDevice>;
type NameDeviceMapping = HashMap<String, DeviceSpecification>;
#[derive(Debug, PartialEq, Eq, ThisError)]
pub enum DeviceError {
#[error("Invalid device parameter: {0}")]
InvalidProperty(String),
#[error("Missing device parameters: {}", .0.join(", "))]
MissingDeviceProperties(Vec<String>),
#[error("Unexpected device parameters: {}", .0.join(", "))]
UnexpectedDeviceProperties(Vec<String>),
}
// [(NAME, [(PROPERTY, VALUE), ...]), ...]
pub type DeviceDescription = Vec<(String, DeviceProperties)>;
type PropertyPairs = Vec<(String, String)>;
#[derive(Debug, Eq, PartialEq, Hash)]
pub struct DeviceProperties(PropertyPairs);
impl DeviceProperties {
pub(crate) fn new(properties: PropertyPairs) -> Self {
Self(properties)
}
fn get(&self, name: &str) -> Option<&str> {
self.0
.iter()
.find(|(n, _)| n == name)
.map(|(_, v)| v.as_str())
}
fn names(&self) -> HashSet<&str> {
self.0.iter().map(|(n, _)| -> &str { n.as_str() }).collect()
}
fn extra<'a>(&'a self, allowed: &[&'a str]) -> HashSet<&str> {
let allowed_set: HashSet<&str> = HashSet::from_iter(allowed.iter().copied());
self.names().difference(&allowed_set).copied().collect()
}
fn missing<'a>(&'a self, required: &[&'a str]) -> HashSet<&str> {
let required_set: HashSet<&str> = HashSet::from_iter(required.iter().copied());
required_set.difference(&self.names()).copied().collect()
}
fn check(&self, required: &[&str], optional: &[&str]) -> Result<(), DeviceError> {
let missing = self.missing(required);
if !missing.is_empty() {
return Err(DeviceError::MissingDeviceProperties(
missing
.iter()
.sorted()
.map(|s| (*s).to_owned())
.collect::<Vec<String>>(),
));
}
let mut all_allowed = Vec::from(required);
all_allowed.extend(optional.iter());
let extra = self.extra(&all_allowed);
if !extra.is_empty() {
return Err(DeviceError::UnexpectedDeviceProperties(
extra
.iter()
.sorted()
.map(|s| (*s).to_owned())
.collect::<Vec<String>>(),
));
}
Ok(())
}
}
type MaybeDevice = Result<Box<dyn ScmiDevice>, DeviceError>;
type DeviceConstructor = fn(&DeviceProperties) -> MaybeDevice;
pub struct DeviceSpecification {
pub(crate) constructor: DeviceConstructor,
short_help: String,
long_help: String,
parameters_help: Vec<String>,
}
impl DeviceSpecification {
fn new(
constructor: DeviceConstructor,
short_help: &str,
long_help: &str,
parameters_help: &[&str],
) -> Self {
Self {
constructor,
short_help: short_help.to_owned(),
long_help: long_help.to_owned(),
parameters_help: parameters_help
.iter()
.map(|s| String::from(*s))
.collect::<Vec<String>>(),
}
}
}
type NameDeviceMapping = HashMap<&'static str, DeviceSpecification>;
pub fn available_devices() -> NameDeviceMapping {
let mut devices: NameDeviceMapping = HashMap::new();
devices.insert("fake".to_owned(), FakeSensor::new);
devices.insert(
"fake",
DeviceSpecification::new(
FakeSensor::new,
"fake accelerometer",
"A simple 3-axes sensor providing fake pre-defined values.",
&["name: an optional name of the sensor, max. 15 characters"],
),
);
devices
}
fn devices_help() -> String {
let mut help = String::new();
writeln!(help, "Available devices:").unwrap();
for (name, constructor) in available_devices().iter() {
let device = constructor();
let short_help = device.short_help();
let long_help = device.long_help();
let parameters_help = device.parameters_help();
for (name, specification) in available_devices().iter() {
let short_help = &specification.short_help;
let long_help = &specification.long_help;
let parameters_help = &specification.parameters_help;
writeln!(help, "\n- {name}: {short_help}").unwrap();
for line in long_help.lines() {
writeln!(help, " {line}").unwrap();
@ -57,7 +162,7 @@ fn devices_help() -> String {
help
}
pub(crate) fn print_devices_help() {
pub fn print_devices_help() {
let help = devices_help();
println!("{}", help);
exit(ExitCodes::Help as i32);
@ -71,9 +176,10 @@ pub struct Sensor {
}
impl Sensor {
const fn new(default_name: String) -> Self {
fn new(properties: &DeviceProperties, default_name: &str) -> Self {
let name = properties.get("name").unwrap_or(default_name);
Self {
name: default_name,
name: name.to_owned(),
enabled: false,
}
}
@ -83,42 +189,18 @@ trait SensorT: Send {
fn sensor(&self) -> &Sensor;
fn sensor_mut(&mut self) -> &mut Sensor;
fn short_help(&self) -> String {
"sensor".to_owned()
}
fn long_help(&self) -> String {
"".to_owned()
}
fn parameters_help(&self) -> Vec<String> {
vec!["name: an optional name of the sensor, max. 15 characters".to_owned()]
}
fn protocol(&self) -> ProtocolId {
SENSOR_PROTOCOL_ID
}
fn invalid_property(&self, name: &String) -> Result<(), String> {
Result::Err(format!("Invalid device option: {name}"))
fn invalid_property(&self, name: &str) -> Result<(), DeviceError> {
Result::Err(DeviceError::InvalidProperty(name.to_owned()))
}
fn process_property(&mut self, name: &String, _value: &str) -> Result<(), String> {
fn process_property(&mut self, name: &str, _value: &str) -> Result<(), DeviceError> {
self.invalid_property(name)
}
fn configure(&mut self, properties: &DeviceProperties) -> Result<(), String> {
for (name, value) in properties {
if name == "name" {
// TODO: Check for duplicate names
self.sensor_mut().name = String::from(value);
} else {
self.process_property(name, value)?;
}
}
Ok(())
}
fn number_of_axes(&self) -> u32 {
1
}
@ -229,22 +311,6 @@ trait SensorT: Send {
struct SensorDevice(Box<dyn SensorT>);
impl ScmiDevice for SensorDevice {
fn configure(&mut self, properties: &DeviceProperties) -> Result<(), String> {
self.0.configure(properties)
}
fn short_help(&self) -> String {
self.0.short_help()
}
fn long_help(&self) -> String {
self.0.long_help()
}
fn parameters_help(&self) -> Vec<String> {
self.0.parameters_help()
}
fn protocol(&self) -> ProtocolId {
self.0.protocol()
}
@ -270,14 +336,6 @@ impl SensorT for FakeSensor {
&mut self.sensor
}
fn short_help(&self) -> String {
"fake accelerometer".to_owned()
}
fn long_help(&self) -> String {
"A simple 3-axes sensor providing fake pre-defined values.".to_owned()
}
fn number_of_axes(&self) -> u32 {
3
}
@ -309,16 +367,19 @@ impl SensorT for FakeSensor {
impl FakeSensor {
#[allow(clippy::new_ret_no_self)]
pub fn new() -> Box<dyn ScmiDevice> {
let sensor = Sensor::new("fake".to_owned());
pub fn new(properties: &DeviceProperties) -> MaybeDevice {
properties.check(&[], &["name"])?;
let sensor = Sensor::new(properties, "fake");
let fake_sensor = Self { sensor, value: 0 };
let sensor_device = SensorDevice(Box::new(fake_sensor));
Box::new(sensor_device)
Ok(Box::new(sensor_device))
}
}
#[cfg(test)]
mod tests {
use std::assert_eq;
use super::*;
#[test]
@ -337,4 +398,47 @@ mod tests {
assert!(help.contains("Parameters:\n"), "parameter label missing");
assert!(help.contains("- name:"), "parameter `name' missing");
}
fn device_properties() -> DeviceProperties {
DeviceProperties::new(vec![
("foo".to_owned(), "val1".to_owned()),
("def".to_owned(), "val2".to_owned()),
("bar".to_owned(), "val3".to_owned()),
])
}
#[test]
fn test_device_properties() {
let properties = device_properties();
assert_eq!(properties.get("bar"), Some("val3"));
assert_eq!(properties.get("baz"), None);
assert_eq!(properties.names(), HashSet::from(["foo", "def", "bar"]));
let expected = ["abc", "def", "ghi"];
let missing = properties.missing(&expected);
assert_eq!(missing, HashSet::from(["abc", "ghi"]));
let extra = properties.extra(&expected);
assert_eq!(extra, HashSet::from(["foo", "bar"]));
}
#[test]
fn test_check_device_properties() {
let properties = device_properties();
let result = properties.check(&["abc", "def", "ghi"], &["foo", "baz"]);
assert_eq!(
result,
Err(DeviceError::MissingDeviceProperties(vec![
"abc".to_owned(),
"ghi".to_owned()
]))
);
let result = properties.check(&["def"], &["foo", "baz"]);
assert_eq!(
result,
Err(DeviceError::UnexpectedDeviceProperties(vec![
"bar".to_owned()
]))
);
let result = properties.check(&["def"], &["foo", "bar"]);
assert_eq!(result, Ok(()));
}
}

18
crates/scmi/src/main.rs
View File

@ -6,6 +6,8 @@ mod devices;
mod scmi;
mod vhu_scmi;
use devices::{DeviceDescription, DeviceProperties};
use std::{
process::exit,
sync::{Arc, RwLock},
@ -38,10 +40,6 @@ struct ScmiArgs {
device: Vec<String>,
}
// [(NAME, [(PROPERTY, VALUE), ...]), ...]
type DeviceDescription = Vec<(String, DeviceProperties)>;
type DeviceProperties = Vec<(String, String)>;
pub struct VuScmiConfig {
socket_path: String,
devices: DeviceDescription,
@ -57,7 +55,7 @@ impl TryFrom<ScmiArgs> for VuScmiConfig {
for d in device_iterator {
let mut split = d.split(',');
let name = split.next().unwrap().to_owned();
let mut properties: DeviceProperties = vec![];
let mut properties = vec![];
for s in split {
if let Some((key, value)) = s.split('=').collect_tuple() {
properties.push((key.to_owned(), value.to_owned()));
@ -65,7 +63,7 @@ impl TryFrom<ScmiArgs> for VuScmiConfig {
return Result::Err(format!("Invalid device {name} property format: {s}"));
}
}
devices.push((name, properties));
devices.push((name, DeviceProperties::new(properties)));
}
Ok(Self {
socket_path,
@ -146,17 +144,17 @@ mod tests {
let config = VuScmiConfig::try_from(args).unwrap();
assert_eq!(config.socket_path, path);
let devices = vec![
("dummy".to_owned(), vec![]),
("dummy".to_owned(), DeviceProperties::new(vec![])),
(
"fake".to_owned(),
vec![
DeviceProperties::new(vec![
("name".to_owned(), "foo".to_owned()),
("prop".to_owned(), "value".to_owned()),
],
]),
),
(
"fake".to_owned(),
vec![("name".to_owned(), "bar".to_owned())],
DeviceProperties::new(vec![("name".to_owned(), "bar".to_owned())]),
),
];
assert_eq!(config.devices, devices);

13
crates/scmi/src/scmi.rs
View File

@ -11,8 +11,6 @@ use itertools::Itertools;
use log::{debug, error, info};
use thiserror::Error as ThisError;
use crate::DeviceProperties;
pub type MessageHeader = u32;
pub const MAX_SIMPLE_STRING_LENGTH: usize = 16; // incl. NULL terminator
@ -480,10 +478,6 @@ pub enum ScmiDeviceError {
}
pub trait ScmiDevice: Send {
fn short_help(&self) -> String;
fn long_help(&self) -> String;
fn parameters_help(&self) -> Vec<String>;
fn configure(&mut self, properties: &DeviceProperties) -> Result<(), String>;
fn protocol(&self) -> ProtocolId;
fn handle(
&mut self,
@ -722,7 +716,7 @@ impl ScmiHandler {
#[cfg(test)]
mod tests {
use crate::devices::FakeSensor;
use crate::devices::{DeviceProperties, FakeSensor};
use super::*;
@ -866,9 +860,8 @@ mod tests {
fn make_handler() -> ScmiHandler {
let mut handler = ScmiHandler::new();
for i in 0..2 {
let properties = vec![("name".to_owned(), format!("fake{i}"))];
let mut fake_sensor = FakeSensor::new();
fake_sensor.configure(&properties).unwrap();
let properties = DeviceProperties::new(vec![("name".to_owned(), format!("fake{i}"))]);
let fake_sensor = FakeSensor::new(&properties).unwrap();
handler.register_device(fake_sensor);
}
handler

21
crates/scmi/src/vhu_scmi.rs
View File

@ -20,9 +20,7 @@ use vm_memory::{
use vmm_sys_util::epoll::EventSet;
use vmm_sys_util::eventfd::{EventFd, EFD_NONBLOCK};
use crate::devices::available_devices;
use crate::devices::print_devices_help;
use crate::scmi::ScmiDevice;
use crate::devices::{available_devices, print_devices_help, DeviceError};
use crate::scmi::{MessageHeader, ScmiHandler, ScmiRequest};
use crate::VuScmiConfig;
@ -45,7 +43,7 @@ pub enum VuScmiError {
#[error("Descriptor write failed")]
DescriptorWriteFailed,
#[error("Error when configuring device {0}: {1}")]
DeviceConfigurationError(String, String),
DeviceConfigurationError(String, DeviceError),
#[error("Failed to create new EventFd")]
EventFdFailed,
#[error("Failed to handle event, didn't match EPOLLIN")]
@ -103,17 +101,16 @@ impl VuScmiBackend {
if name == "help" {
print_devices_help();
}
match device_mapping.get(name) {
Some(constructor) => {
let mut device: Box<dyn ScmiDevice> = constructor();
if let Err(message) = device.configure(properties) {
match device_mapping.get(name.as_str()) {
Some(specification) => match (specification.constructor)(properties) {
Ok(device) => handler.register_device(device),
Err(error) => {
return Result::Err(VuScmiError::DeviceConfigurationError(
name.clone(),
message,
));
error,
))
}
handler.register_device(device);
}
},
None => return Result::Err(VuScmiError::UnknownDeviceRequested(name.clone())),
};
}