Skip to content

Commit

Permalink
Implementation of Builder pattern for Config
Browse files Browse the repository at this point in the history
Implemented `Builder` for `VMMConfig`. (Note: This is an early
implementation, more like prototype. But almost all functionality is
present.)

Removed `derive(Default)` from `MemoryConfig`, `KernelConfig` and
`VcpuConfig` and added sensible defaults (memory = 256M, vcpu=1) (derive
default will trivially initialize this to 0 and hence we are required to
take a memory coniguration from CLI launcher.

Brought these changes to `api` (`CLI`) to demonstrate a use.

Removed `required` from 'memory' and 'vcpu' config.

Changed `TryFrom<String>` to `TryFrom<&str>`. Believe this is better
because it's quite possible to get `&str` from `String` than other way
round and in fact often we'll get `&str` from CLI parsing etc. But the
other way round is painful.

Right now passes all 'existing' tests (`cargo test --all`, except one
which likely fails because the right kernel image is not available to
test.)

TODO: Documentation with examples for using the `Builder` and Test cases
for the `Builder`.

Signed-off-by: Abhijit Gadgil <[email protected]>
  • Loading branch information
gabhijit committed Jan 18, 2021
1 parent 0fe2344 commit 4653300
Show file tree
Hide file tree
Showing 5 changed files with 205 additions and 98 deletions.
62 changes: 10 additions & 52 deletions src/api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
//! CLI for the Reference VMM.
#![deny(missing_docs)]
use std::convert::TryFrom;
use std::result;

use clap::{App, Arg};
use vmm::{BlockConfig, KernelConfig, MemoryConfig, NetConfig, VMMConfig, VcpuConfig};
use vmm::VMMConfig;

/// Command line parser.
pub struct CLI;
Expand All @@ -26,17 +25,13 @@ impl CLI {
.arg(
Arg::with_name("memory")
.long("memory")
.required(true)
.takes_value(true)
.default_value("size_mib=256")
.help("Guest memory configuration.\n\tFormat: \"size_mib=<u32>\""),
)
.arg(
Arg::with_name("vcpu")
.long("vcpu")
.required(true)
.takes_value(true)
.default_value("num=1")
.help("vCPU configuration.\n\tFormat: \"num=<u8>\""),
)
.arg(
Expand Down Expand Up @@ -71,52 +66,14 @@ impl CLI {
format!("Invalid command line arguments: {}", e)
})?;

Ok(VMMConfig {
memory_config: MemoryConfig::try_from(
matches
.value_of("memory")
.expect("Missing memory configuration")
.to_string(),
)
.map_err(|e| {
eprintln!("{}", help_msg);
format!("{}", e)
})?,
kernel_config: KernelConfig::try_from(
matches
.value_of("kernel")
.expect("Missing kernel configuration")
.to_string(),
)
.map_err(|e| {
eprintln!("{}", help_msg);
format!("{}", e)
})?,
vcpu_config: VcpuConfig::try_from(
matches
.value_of("vcpu")
.expect("Missing vCPU configuration")
.to_string(),
)
.map_err(|e| {
eprintln!("{}", help_msg);
format!("{}", e)
})?,
block_config: match matches.value_of("block") {
Some(value) => Some(BlockConfig::try_from(value.to_string()).map_err(|e| {
eprintln!("{}", help_msg);
format!("{}", e)
})?),
None => None,
},
network_config: match matches.value_of("net") {
Some(value) => Some(NetConfig::try_from(value.to_string()).map_err(|e| {
eprintln!("{}", help_msg);
format!("{}", e)
})?),
None => None,
},
})
Ok(VMMConfig::builder()
.memory_config(matches.value_of("memory"))
.kernel_config(matches.value_of("kernel"))
.vcpu_config(matches.value_of("vcpu"))
.network_config(matches.value_of("network"))
.block_config(matches.value_of("block"))
.build()
.map_err(|e| format!("{:?}", e))?)
}
}

Expand All @@ -126,6 +83,7 @@ mod tests {

use std::path::PathBuf;
use vmm::DEFAULT_KERNEL_CMDLINE;
use vmm::{KernelConfig, MemoryConfig, VcpuConfig};

#[test]
fn test_launch() {
Expand Down
6 changes: 3 additions & 3 deletions src/vmm/src/config/arg_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub(super) struct CfgArgParser {
}

impl CfgArgParser {
pub(super) fn new(input: String) -> Self {
pub(super) fn new(input: &str) -> Self {
let args = input
.split(',')
.filter(|tok| !tok.is_empty())
Expand Down Expand Up @@ -89,7 +89,7 @@ mod tests {

#[test]
fn test_cfg_arg_parse() -> Result<(), CfgArgParseError> {
let input_params = "path=/path,string=HelloWorld,int=123,u8=1".to_string();
let input_params = "path=/path,string=HelloWorld,int=123,u8=1";
let mut arg_parser = CfgArgParser::new(input_params);

// No parameter was consumed yet
Expand Down Expand Up @@ -119,7 +119,7 @@ mod tests {
// All params were consumed:
assert!(arg_parser.all_consumed().is_ok());

let input_params = "path=".to_string();
let input_params = "path=";
assert!(CfgArgParser::new(input_params)
.value_of::<String>("path")?
.is_none());
Expand Down
110 changes: 110 additions & 0 deletions src/vmm/src/config/builder.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
//! Config builder
use std::convert::TryFrom;

use super::{
BlockConfig, ConfigResult, ConversionError, KernelConfig, MemoryConfig, NetConfig, VMMConfig,
VcpuConfig,
};

/// Builder structure for VMMConfig
#[derive(Debug)]
pub struct Builder {
inner: ConfigResult<VMMConfig>,
}

impl Default for Builder {
fn default() -> Self {
Self::new()
}
}

impl Builder {
pub fn new() -> Self {
Builder {
inner: Ok(VMMConfig::default()),
}
}

pub fn build(&self) -> ConfigResult<VMMConfig> {
self.inner.clone()
}

pub fn memory_config<T>(self, memory: Option<T>) -> Self
where
MemoryConfig: TryFrom<T>,
<MemoryConfig as TryFrom<T>>::Error: Into<ConversionError>,
{
match memory {
Some(m) => self.and_then(move |mut config| {
config.memory_config = TryFrom::try_from(m).map_err(Into::into)?;
Ok(config)
}),
None => self,
}
}

pub fn vcpu_config<T>(self, vcpu: Option<T>) -> Self
where
VcpuConfig: TryFrom<T>,
<VcpuConfig as TryFrom<T>>::Error: Into<ConversionError>,
{
match vcpu {
Some(v) => self.and_then(move |mut config| {
config.vcpu_config = TryFrom::try_from(v).map_err(Into::into)?;
Ok(config)
}),
None => self,
}
}

pub fn kernel_config<T>(self, kernel: Option<T>) -> Self
where
KernelConfig: TryFrom<T>,
<KernelConfig as TryFrom<T>>::Error: Into<ConversionError>,
{
match kernel {
Some(k) => self.and_then(move |mut config| {
config.kernel_config = TryFrom::try_from(k).map_err(Into::into)?;
Ok(config)
}),
None => self,
}
}

pub fn network_config<T>(self, network: Option<T>) -> Self
where
NetConfig: TryFrom<T>,
<NetConfig as TryFrom<T>>::Error: Into<ConversionError>,
{
match network {
Some(n) => self.and_then(move |mut config| {
config.network_config = Some(TryFrom::try_from(n).map_err(Into::into)?);
Ok(config)
}),
None => self,
}
}

pub fn block_config<T>(self, block: Option<T>) -> Self
where
BlockConfig: TryFrom<T>,
<BlockConfig as TryFrom<T>>::Error: Into<ConversionError>,
{
match block {
Some(b) => self.and_then(move |mut config| {
config.block_config = Some(TryFrom::try_from(b).map_err(Into::into)?);
Ok(config)
}),
None => self,
}
}

fn and_then<F>(self, func: F) -> Self
where
F: FnOnce(VMMConfig) -> ConfigResult<VMMConfig>,
{
Builder {
inner: self.inner.and_then(func),
}
}
}
Loading

0 comments on commit 4653300

Please sign in to comment.