devices: serial: parse options using serde_keyvalue crate

Use our new serde_keyvalue crate to annotate the SerialParameters
structure and allow us to create it from a key-values string. Add tests
to help ensure parsing doesn't break in the future.

The existing arguments can be parsed identically by this new code, so
replace the old serial options.

BUG=b:218223240
TEST=cargo test -p devices serial_device::tests::params_from_key_values
TEST=cargo test parse_serial

Change-Id: I4898a45399b69b87a44f80d3a214daf081b06173
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3439670
Reviewed-by: Anton Romanov <romanton@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
This commit is contained in:
Alexandre Courbot 2022-02-03 19:39:30 +09:00 committed by Chromeos LUCI
parent aa043e8c00
commit da20cf1d78
4 changed files with 157 additions and 112 deletions

View file

@ -55,6 +55,7 @@ remain = "*"
resources = { path = "../resources" }
serde = { version = "1", features = [ "derive" ] }
serde_json = "1"
serde_keyvalue = { path = "../serde_keyvalue" }
smallvec = "1.6.1"
sync = { path = "../common/sync" }
system_api = { version = "*", optional = true }

View file

@ -8,7 +8,6 @@ use std::fs::{File, OpenOptions};
use std::io::{self, stdin, stdout, ErrorKind};
use std::os::unix::net::UnixDatagram;
use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::thread;
use std::time::Duration;
@ -18,6 +17,7 @@ use base::{
};
use hypervisor::ProtectionType;
use remain::sorted;
use serde::Deserialize;
use thiserror::Error as ThisError;
#[sorted]
@ -54,15 +54,23 @@ pub trait SerialDevice {
}
/// Enum for possible type of serial devices
#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum SerialType {
File,
Stdout,
Sink,
Syslog,
#[serde(rename = "unix")]
UnixSocket,
}
impl Default for SerialType {
fn default() -> Self {
Self::Sink
}
}
impl Display for SerialType {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let s = match &self {
@ -77,27 +85,20 @@ impl Display for SerialType {
}
}
impl FromStr for SerialType {
type Err = Error;
fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
match s {
"file" | "File" => Ok(SerialType::File),
"stdout" | "Stdout" => Ok(SerialType::Stdout),
"sink" | "Sink" => Ok(SerialType::Sink),
"syslog" | "Syslog" => Ok(SerialType::Syslog),
"unix" | "UnixSocket" => Ok(SerialType::UnixSocket),
_ => Err(Error::InvalidSerialType(s.to_string())),
}
}
}
/// Serial device hardware types
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum SerialHardware {
Serial, // Standard PC-style (8250/16550 compatible) UART
VirtioConsole, // virtio-console device
}
impl Default for SerialHardware {
fn default() -> Self {
Self::Serial
}
}
impl Display for SerialHardware {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let s = match &self {
@ -109,17 +110,6 @@ impl Display for SerialHardware {
}
}
impl FromStr for SerialHardware {
type Err = Error;
fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
match s {
"serial" => Ok(SerialHardware::Serial),
"virtio-console" => Ok(SerialHardware::VirtioConsole),
_ => Err(Error::InvalidSerialHardware(s.to_string())),
}
}
}
struct WriteSocket {
sock: UnixDatagram,
buf: String,
@ -187,13 +177,19 @@ impl io::Write for WriteSocket {
}
}
/// Holds the parameters for a serial device
#[derive(Clone, Debug)]
fn serial_parameters_default_num() -> u8 {
1
}
#[derive(Clone, Debug, Default, PartialEq, Deserialize)]
#[serde(deny_unknown_fields, default)]
pub struct SerialParameters {
#[serde(rename = "type")]
pub type_: SerialType,
pub hardware: SerialHardware,
pub path: Option<PathBuf>,
pub input: Option<PathBuf>,
#[serde(default = "serial_parameters_default_num")]
pub num: u8,
pub console: bool,
pub earlycon: bool,
@ -348,3 +344,116 @@ impl SerialParameters {
Ok(T::new(protected_vm, evt, input, output, keep_rds.to_vec()))
}
}
#[cfg(test)]
mod tests {
use super::*;
use serde_keyvalue::*;
fn from_serial_arg(options: &str) -> Result<SerialParameters, ParseError> {
from_key_values(options)
}
#[test]
fn params_from_key_values() {
// Defaults
let params = from_serial_arg("").unwrap();
assert_eq!(
params,
SerialParameters {
type_: SerialType::Sink,
hardware: SerialHardware::Serial,
path: None,
input: None,
num: 1,
console: false,
earlycon: false,
stdin: false,
}
);
// type parameter
let params = from_serial_arg("type=file").unwrap();
assert_eq!(params.type_, SerialType::File);
let params = from_serial_arg("type=stdout").unwrap();
assert_eq!(params.type_, SerialType::Stdout);
let params = from_serial_arg("type=sink").unwrap();
assert_eq!(params.type_, SerialType::Sink);
let params = from_serial_arg("type=syslog").unwrap();
assert_eq!(params.type_, SerialType::Syslog);
let params = from_serial_arg("type=unix").unwrap();
assert_eq!(params.type_, SerialType::UnixSocket);
let params = from_serial_arg("type=foobar");
assert!(params.is_err());
// hardware parameter
let params = from_serial_arg("hardware=serial").unwrap();
assert_eq!(params.hardware, SerialHardware::Serial);
let params = from_serial_arg("hardware=virtio-console").unwrap();
assert_eq!(params.hardware, SerialHardware::VirtioConsole);
let params = from_serial_arg("hardware=foobar");
assert!(params.is_err());
// path parameter
let params = from_serial_arg("path=/test/path").unwrap();
assert_eq!(params.path, Some("/test/path".into()));
let params = from_serial_arg("path");
assert!(params.is_err());
// input parameter
let params = from_serial_arg("input=/path/to/input").unwrap();
assert_eq!(params.input, Some("/path/to/input".into()));
let params = from_serial_arg("input");
assert!(params.is_err());
// console parameter
let params = from_serial_arg("console").unwrap();
assert!(params.console);
let params = from_serial_arg("console=true").unwrap();
assert!(params.console);
let params = from_serial_arg("console=false").unwrap();
assert!(!params.console);
let params = from_serial_arg("console=foobar");
assert!(params.is_err());
// earlycon parameter
let params = from_serial_arg("earlycon").unwrap();
assert!(params.earlycon);
let params = from_serial_arg("earlycon=true").unwrap();
assert!(params.earlycon);
let params = from_serial_arg("earlycon=false").unwrap();
assert!(!params.earlycon);
let params = from_serial_arg("earlycon=foobar");
assert!(params.is_err());
// stdin parameter
let params = from_serial_arg("stdin").unwrap();
assert!(params.stdin);
let params = from_serial_arg("stdin=true").unwrap();
assert!(params.stdin);
let params = from_serial_arg("stdin=false").unwrap();
assert!(!params.stdin);
let params = from_serial_arg("stdin=foobar");
assert!(params.is_err());
// all together
let params = from_serial_arg("type=stdout,path=/some/path,hardware=virtio-console,num=5,earlycon,console,stdin,input=/some/input").unwrap();
assert_eq!(
params,
SerialParameters {
type_: SerialType::Stdout,
hardware: SerialHardware::VirtioConsole,
path: Some("/some/path".into()),
input: Some("/some/input".into()),
num: 5,
console: true,
earlycon: true,
stdin: true,
}
);
// invalid field
let params = from_serial_arg("type=stdout,foo=bar");
assert!(params.is_err());
}
}

View file

@ -54,6 +54,9 @@ use thiserror::Error;
#[sorted]
#[derive(Error, Debug)]
pub enum Error {
/// Free error for use with the `serde_keyvalue` crate parser.
#[error("failed to parse key-value arguments: {0}")]
ConfigParserError(String),
/// The argument was required.
#[error("expected argument: {0}")]
ExpectedArgument(String),

View file

@ -32,7 +32,7 @@ use crosvm::{
SharedDir, TouchDeviceOption, VfioCommand, VhostUserFsOption, VhostUserOption,
VhostUserWlOption, VhostVsockDeviceParameter, VvuOption, DISK_ID_LEN,
};
use devices::serial_device::{SerialHardware, SerialParameters, SerialType};
use devices::serial_device::{SerialHardware, SerialParameters};
#[cfg(feature = "audio_cras")]
use devices::virtio::snd::cras_backend::Error as CrasSndError;
#[cfg(feature = "audio_cras")]
@ -61,6 +61,7 @@ use disk::{
create_composite_disk, create_disk_file, create_zero_filler, ImagePartitionType, PartitionInfo,
};
use hypervisor::ProtectionType;
use serde_keyvalue::from_key_values;
use uuid::Uuid;
use vm_control::{
client::{
@ -690,88 +691,19 @@ fn parse_ac97_options(s: &str) -> argument::Result<Ac97Parameters> {
}
fn parse_serial_options(s: &str) -> argument::Result<SerialParameters> {
let mut serial_setting = SerialParameters {
type_: SerialType::Sink,
hardware: SerialHardware::Serial,
path: None,
input: None,
num: 1,
console: false,
earlycon: false,
stdin: false,
};
let serial_setting: SerialParameters =
from_key_values(s).map_err(|e| argument::Error::ConfigParserError(e.to_string()))?;
let opts = s
.split(',')
.map(|frag| frag.splitn(2, '='))
.map(|mut kv| (kv.next().unwrap_or(""), kv.next().unwrap_or("")));
for (k, v) in opts {
match k {
"hardware" => {
serial_setting.hardware = v
.parse::<SerialHardware>()
.map_err(|e| argument::Error::UnknownArgument(format!("{}", e)))?
}
"type" => {
serial_setting.type_ = v
.parse::<SerialType>()
.map_err(|e| argument::Error::UnknownArgument(format!("{}", e)))?
}
"num" => {
let num = v.parse::<u8>().map_err(|e| {
argument::Error::Syntax(format!("serial device number is not parsable: {}", e))
})?;
if num < 1 {
return Err(argument::Error::InvalidValue {
value: num.to_string(),
expected: String::from("Serial port num must be at least 1"),
});
}
serial_setting.num = num;
}
"console" => {
serial_setting.console = v.parse::<bool>().map_err(|e| {
argument::Error::Syntax(format!(
"serial device console is not parseable: {}",
e
))
})?
}
"earlycon" => {
serial_setting.earlycon = v.parse::<bool>().map_err(|e| {
argument::Error::Syntax(format!(
"serial device earlycon is not parseable: {}",
e,
))
})?
}
"stdin" => {
serial_setting.stdin = v.parse::<bool>().map_err(|e| {
argument::Error::Syntax(format!("serial device stdin is not parseable: {}", e))
})?;
if serial_setting.stdin && serial_setting.input.is_some() {
return Err(argument::Error::TooManyArguments(
"Cannot specify both stdin and input options".to_string(),
));
}
}
"path" => serial_setting.path = Some(PathBuf::from(v)),
"input" => {
if serial_setting.stdin {
return Err(argument::Error::TooManyArguments(
"Cannot specify both stdin and input options".to_string(),
));
}
serial_setting.input = Some(PathBuf::from(v));
}
_ => {
return Err(argument::Error::UnknownArgument(format!(
"serial parameter {}",
k
)));
}
}
if serial_setting.stdin && serial_setting.input.is_some() {
return Err(argument::Error::TooManyArguments(
"Cannot specify both stdin and input options".to_string(),
));
}
if serial_setting.num < 1 {
return Err(argument::Error::InvalidValue {
value: serial_setting.num.to_string(),
expected: String::from("Serial port num must be at least 1"),
});
}
if serial_setting.hardware == SerialHardware::Serial && serial_setting.num > 4 {