mirrors/crosvm
1
0
Fork 0
mirror of https://chromium.googlesource.com/crosvm/crosvm synced 2024-11-25 05:03:05 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

devices: fw_cfg: make --fw-cfg name required

Browse source 
Also add a config test with a valid --fw-cfg with path, and check the
values returned by the valid config parse tests.

BUG=b:283990685
TEST=cargo test parse_fw_cfg

Change-Id: I80ccefa9f449e6fa4bdd56d55cf519e15a213d32
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4795352
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
This commit is contained in:
Daniel Verkamp 2023-08-18 15:42:22 -07:00 committed by crosvm LUCI
parent b1c644ec34
commit 37cea3d21e
2 changed files with 41 additions and 49 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

53
devices/src/fw_cfg.rs
View file

@ -68,7 +68,7 @@ pub type Result<T> = std::result::Result<T, Error>;
#[derive(Clone, Debug, Deserialize, Serialize, FromKeyValues, PartialEq, Eq)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
pub struct FwCfgParameters {
pub name: Option<String>,
pub name: String,
pub string: Option<String>,
pub path: Option<PathBuf>,
}
@ -152,20 +152,18 @@ impl FwCfgDevice {
};
for param in fw_cfg_parameters {
if let Some(_name) = &param.name {
let data: Vec<u8> = if let Some(string) = &param.string {
string.as_bytes().to_vec()
} else if let Ok(bytes) = fs::read(param.clone().path.unwrap()) {
bytes
} else {
return Err(Error::FileOpen(param.path.unwrap()));
};
let data: Vec<u8> = if let Some(string) = &param.string {
string.as_bytes().to_vec()
} else if let Ok(bytes) = fs::read(param.clone().path.unwrap()) {
bytes
} else {
return Err(Error::FileOpen(param.path.unwrap()));
};
// The file added from the command line will be a generic item. QEMU does not
// give users the option to specify whether the user-specified blob is
// arch-specific, so we won't either.
device.add_file(&param.name.unwrap(), data, FwCfgItemType::GenericItem)?
}
// The file added from the command line will be a generic item. QEMU does not
// give users the option to specify whether the user-specified blob is
// arch-specific, so we won't either.
device.add_file(&param.name, data, FwCfgItemType::GenericItem)?
}
device.add_bytes(FW_CFG_SIGNATURE.to_vec(), FwCfgItemType::Signature);
@ -404,11 +402,7 @@ mod tests {
];
fn default_params() -> Vec<FwCfgParameters> {
vec![FwCfgParameters {
name: None,
string: None,
path: None,
}]
Vec::new()
}
fn get_contents() -> [Vec<u8>; 6] {
@ -442,7 +436,7 @@ mod tests {
Ok(device)
}
fn from_serial_arg(options: &str) -> std::result::Result<FwCfgParameters, ParseError> {
fn from_fw_cfg_arg(options: &str) -> std::result::Result<FwCfgParameters, ParseError> {
from_key_values(options)
}
@ -584,32 +578,23 @@ mod tests {
#[test]
// Attempt to build FwCfgParams from key value pairs
fn params_from_key_values() {
let params = from_serial_arg("").unwrap();
assert_eq!(
params,
FwCfgParameters {
name: None,
string: None,
path: None,
}
);
let params = from_serial_arg("name=foo,path=/path/to/input").unwrap();
from_fw_cfg_arg("").expect_err("parsing empty string should fail");
let params = from_fw_cfg_arg("name=foo,path=/path/to/input").unwrap();
assert_eq!(
params,
FwCfgParameters {
name: Some("foo".into()),
name: "foo".into(),
path: Some("/path/to/input".into()),
string: None,
}
);
let params = from_serial_arg("name=bar,string=testdata").unwrap();
let params = from_fw_cfg_arg("name=bar,string=testdata").unwrap();
assert_eq!(
params,
FwCfgParameters {
name: Some("bar".into()),
name: "bar".into(),
string: Some("testdata".into()),
path: None,
}

37
src/crosvm/config.rs
View file

@ -427,13 +427,7 @@ pub fn parse_mmio_address_range(s: &str) -> Result<Vec<AddressRange>, String> {
}
pub fn validate_fw_cfg_parameters(params: &FwCfgParameters) -> Result<(), String> {
if params.name.is_none() && (params.string.is_some() || params.path.is_some()) {
return Err("Must give name of data to --fw-cfg".to_string());
}
if params.string.is_some() && params.path.is_some()
|| params.name.is_some() && params.string.is_none() && params.path.is_none()
{
if !(params.string.is_some() ^ params.path.is_some()) {
return Err("Provide exactly one of string or path args to --fw-cfg".to_string());
}
@ -1813,24 +1807,37 @@ mod tests {
}
#[test]
fn parse_fw_cfg_valid_no_params() {
assert!(TryInto::<Config>::try_into(
crate::crosvm::cmdline::RunCommand::from_args(&[], &["--fw-cfg", "", "/dev/null"],)
.unwrap()
fn parse_fw_cfg_valid_path() {
let cfg = TryInto::<Config>::try_into(
crate::crosvm::cmdline::RunCommand::from_args(
&[],
&["--fw-cfg", "name=bar,path=data.bin", "/dev/null"],
)
.unwrap(),
)
.is_ok());
.unwrap();
assert_eq!(cfg.fw_cfg_parameters.len(), 1);
assert_eq!(cfg.fw_cfg_parameters[0].name, "bar".to_string());
assert_eq!(cfg.fw_cfg_parameters[0].string, None);
assert_eq!(cfg.fw_cfg_parameters[0].path, Some("data.bin".into()));
}
#[test]
fn parse_fw_cfg_valid_string() {
assert!(TryInto::<Config>::try_into(
let cfg = TryInto::<Config>::try_into(
crate::crosvm::cmdline::RunCommand::from_args(
&[],
&["--fw-cfg", "name=bar,string=foo", "/dev/null"],
)
.unwrap()
.unwrap(),
)
.is_ok());
.unwrap();
assert_eq!(cfg.fw_cfg_parameters.len(), 1);
assert_eq!(cfg.fw_cfg_parameters[0].name, "bar".to_string());
assert_eq!(cfg.fw_cfg_parameters[0].string, Some("foo".to_string()));
assert_eq!(cfg.fw_cfg_parameters[0].path, None);
}
#[test]