diff --git a/aarch64/src/lib.rs b/aarch64/src/lib.rs index 032efd27f1..37f807b9dd 100644 --- a/aarch64/src/lib.rs +++ b/aarch64/src/lib.rs @@ -271,7 +271,7 @@ impl arch::LinuxArch for AArch64 { system_allocator: &mut SystemAllocator, serial_parameters: &BTreeMap<(SerialHardware, u8), SerialParameters>, serial_jail: Option, - (bat_type, bat_jail): (&Option, Option), + (bat_type, bat_jail): (Option, Option), mut vm: V, ramoops_region: Option, devs: Vec<(Box, Option)>, diff --git a/arch/src/lib.rs b/arch/src/lib.rs index 42d0e0a2ad..ff8ff22746 100644 --- a/arch/src/lib.rs +++ b/arch/src/lib.rs @@ -250,7 +250,7 @@ pub trait LinuxArch { system_allocator: &mut SystemAllocator, serial_parameters: &BTreeMap<(SerialHardware, u8), SerialParameters>, serial_jail: Option, - battery: (&Option, Option), + battery: (Option, Option), vm: V, ramoops_region: Option, devices: Vec<(Box, Option)>, diff --git a/src/crosvm/cmdline.rs b/src/crosvm/cmdline.rs index 545d8fb86d..29cc0df080 100644 --- a/src/crosvm/cmdline.rs +++ b/src/crosvm/cmdline.rs @@ -44,7 +44,6 @@ use devices::SerialParameters; use devices::StubPciParameters; use hypervisor::ProtectionType; use resources::AddressRange; -use vm_control::BatteryType; #[cfg(feature = "gpu")] use super::sys::config::parse_gpu_options; @@ -55,7 +54,6 @@ use super::sys::GpuRenderServerParameters; use crate::crosvm::config::numbered_disk_option; #[cfg(feature = "audio")] use crate::crosvm::config::parse_ac97_options; -use crate::crosvm::config::parse_battery_options; use crate::crosvm::config::parse_bus_id_addr; use crate::crosvm::config::parse_cpu_affinity; use crate::crosvm::config::parse_cpu_capacity; @@ -76,6 +74,7 @@ use crate::crosvm::config::parse_serial_options; use crate::crosvm::config::parse_stub_pci_parameters; #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] use crate::crosvm::config::parse_userspace_msr_options; +use crate::crosvm::config::BatteryConfig; #[cfg(feature = "plugin")] use crate::crosvm::config::BindMount; #[cfg(feature = "direct")] @@ -471,13 +470,13 @@ pub struct RunCommand { #[argh(option, arg_name = "PATH")] /// path for balloon controller socket. pub balloon_control: Option, - #[argh(option, from_str_fn(parse_battery_options))] + #[argh(option)] /// comma separated key=value pairs for setting up battery /// device /// Possible key values: /// type=goldfish - type of battery emulation, defaults to /// goldfish - pub battery: Option, + pub battery: Option, #[argh(option)] /// path to BIOS/firmware ROM pub bios: Option, @@ -1761,7 +1760,7 @@ impl TryFrom for super::config::Config { cfg.pvm_fw = Some(p); } - cfg.battery_type = cmd.battery; + cfg.battery_config = cmd.battery; #[cfg(all(target_arch = "x86_64", feature = "gdb"))] { diff --git a/src/crosvm/config.rs b/src/crosvm/config.rs index af68e34483..b4ca6fe4e8 100644 --- a/src/crosvm/config.rs +++ b/src/crosvm/config.rs @@ -43,6 +43,7 @@ use hypervisor::ProtectionType; use resources::AddressRange; use serde::Deserialize; use serde::Serialize; +use serde_keyvalue::FromKeyValues; use uuid::Uuid; use vm_control::BatteryType; #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] @@ -893,30 +894,11 @@ pub fn invalid_value_err, S: ToString>(value: T, expected: S) -> S format!("invalid value {}: {}", value.as_ref(), expected.to_string()) } -pub fn parse_battery_options(s: &str) -> Result { - let mut battery_type: BatteryType = Default::default(); - - let opts = s - .split(',') - .map(|frag| frag.split('=')) - .map(|mut kv| (kv.next().unwrap_or(""), kv.next().unwrap_or(""))); - - for (k, v) in opts { - match k { - "type" => match v.parse::() { - Ok(type_) => battery_type = type_, - Err(e) => { - return Err(invalid_value_err(v, e)); - } - }, - "" => {} - _ => { - return Err(format!("battery parameter {}", k)); - } - } - } - - Ok(battery_type) +#[derive(Debug, Serialize, Deserialize, FromKeyValues)] +#[serde(deny_unknown_fields, rename_all = "kebab-case")] +pub struct BatteryConfig { + #[serde(rename = "type", default)] + pub type_: BatteryType, } pub fn parse_cpu_capacity(s: &str) -> Result, String> { @@ -1215,7 +1197,7 @@ pub struct Config { pub balloon: bool, pub balloon_bias: i64, pub balloon_control: Option, - pub battery_type: Option, + pub battery_config: Option, #[cfg(windows)] pub block_control_tube: Vec, #[cfg(windows)] @@ -1406,7 +1388,7 @@ impl Default for Config { balloon: true, balloon_bias: 0, balloon_control: None, - battery_type: None, + battery_config: None, #[cfg(windows)] block_control_tube: Vec::new(), #[cfg(windows)] @@ -2038,23 +2020,25 @@ mod tests { } #[test] - fn parse_battery_vaild() { - parse_battery_options("type=goldfish").expect("parse should have succeded"); + fn parse_battery_valid() { + let bat_config: BatteryConfig = from_key_values("type=goldfish").unwrap(); + assert_eq!(bat_config.type_, BatteryType::Goldfish); } #[test] - fn parse_battery_vaild_no_type() { - parse_battery_options("").expect("parse should have succeded"); + fn parse_battery_valid_no_type() { + let bat_config: BatteryConfig = from_key_values("").unwrap(); + assert_eq!(bat_config.type_, BatteryType::Goldfish); } #[test] - fn parse_battery_invaild_parameter() { - parse_battery_options("tyep=goldfish").expect_err("parse should have failed"); + fn parse_battery_invalid_parameter() { + from_key_values::("tyep=goldfish").expect_err("parse should have failed"); } #[test] - fn parse_battery_invaild_type_value() { - parse_battery_options("type=xxx").expect_err("parse should have failed"); + fn parse_battery_invalid_type_value() { + from_key_values::("type=xxx").expect_err("parse should have failed"); } #[test] diff --git a/src/crosvm/sys/unix.rs b/src/crosvm/sys/unix.rs index 5edbc99377..483d19634b 100644 --- a/src/crosvm/sys/unix.rs +++ b/src/crosvm/sys/unix.rs @@ -1399,7 +1399,7 @@ where control_tubes.push(TaggedControlTube::VmIrq(ioapic_host_tube)); } - let battery = if cfg.battery_type.is_some() { + let battery = if cfg.battery_config.is_some() { #[cfg_attr(not(feature = "power-monitor-powerd"), allow(clippy::manual_map))] let jail = match simple_jail(&cfg.jail_config, "battery")? { #[cfg_attr(not(feature = "power-monitor-powerd"), allow(unused_mut))] @@ -1425,9 +1425,9 @@ where } None => None, }; - (&cfg.battery_type, jail) + (cfg.battery_config.as_ref().map(|c| c.type_), jail) } else { - (&cfg.battery_type, None) + (cfg.battery_config.as_ref().map(|c| c.type_), None) }; let map_request: Arc>> = Arc::new(Mutex::new(None)); diff --git a/vm_control/src/lib.rs b/vm_control/src/lib.rs index 0b9c20c4fc..851872dfb9 100644 --- a/vm_control/src/lib.rs +++ b/vm_control/src/lib.rs @@ -591,6 +591,7 @@ impl Display for BatControlResult { } #[derive(Serialize, Deserialize, Copy, Clone, Debug, PartialEq)] +#[serde(rename_all = "kebab-case")] pub enum BatteryType { Goldfish, } diff --git a/x86_64/src/lib.rs b/x86_64/src/lib.rs index 3bc4958f0e..f766103dbe 100644 --- a/x86_64/src/lib.rs +++ b/x86_64/src/lib.rs @@ -549,7 +549,7 @@ impl arch::LinuxArch for X8664arch { system_allocator: &mut SystemAllocator, serial_parameters: &BTreeMap<(SerialHardware, u8), SerialParameters>, serial_jail: Option, - battery: (&Option, Option), + battery: (Option, Option), mut vm: V, ramoops_region: Option, devs: Vec<(Box, Option)>, @@ -1575,7 +1575,7 @@ impl X8664arch { #[cfg(feature = "direct")] direct_gpe: &[u32], irq_chip: &mut dyn IrqChip, sci_irq: u32, - battery: (&Option, Option), + battery: (Option, Option), #[cfg_attr(windows, allow(unused_variables))] mmio_bus: &devices::Bus, max_bus: u8, resume_notify_devices: &mut Vec>>, diff --git a/x86_64/src/test_integration.rs b/x86_64/src/test_integration.rs index 2bcd17efc4..33286fdde3 100644 --- a/x86_64/src/test_integration.rs +++ b/x86_64/src/test_integration.rs @@ -200,7 +200,7 @@ where &[], // direct_gpe &mut irq_chip, X86_64_SCI_IRQ, - (&None, None), + (None, None), &mmio_bus, max_bus, &mut resume_notify_devices,