diff --git a/devices/Cargo.toml b/devices/Cargo.toml index 3338a6a23a..7c8c456d74 100644 --- a/devices/Cargo.toml +++ b/devices/Cargo.toml @@ -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 } diff --git a/devices/src/serial_device.rs b/devices/src/serial_device.rs index 795e05509f..8e5c6419ab 100644 --- a/devices/src/serial_device.rs +++ b/devices/src/serial_device.rs @@ -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 { - 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 { - 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, pub input: Option, + #[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 { + 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()); + } +} diff --git a/src/argument.rs b/src/argument.rs index 42211fdd55..e659391018 100644 --- a/src/argument.rs +++ b/src/argument.rs @@ -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), diff --git a/src/main.rs b/src/main.rs index d28a54be1c..7f3366fd3b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -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 { } fn parse_serial_options(s: &str) -> argument::Result { - 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::() - .map_err(|e| argument::Error::UnknownArgument(format!("{}", e)))? - } - "type" => { - serial_setting.type_ = v - .parse::() - .map_err(|e| argument::Error::UnknownArgument(format!("{}", e)))? - } - "num" => { - let num = v.parse::().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::().map_err(|e| { - argument::Error::Syntax(format!( - "serial device console is not parseable: {}", - e - )) - })? - } - "earlycon" => { - serial_setting.earlycon = v.parse::().map_err(|e| { - argument::Error::Syntax(format!( - "serial device earlycon is not parseable: {}", - e, - )) - })? - } - "stdin" => { - serial_setting.stdin = v.parse::().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 {