Skip to content

Commit

Permalink
config: Implementation of Builder Pattern #60
Browse files Browse the repository at this point in the history
Implemented Builder for VMMConfig (WIP).

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 as user input).

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

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

TODO: Documentation, Examples and Test cases.
Signed-off-by: Abhijit Gadgil <[email protected]>
  • Loading branch information
gabhijit committed Jan 20, 2021
1 parent 6b0b284 commit ce8a922
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 62 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
122 changes: 122 additions & 0 deletions src/vmm/src/config/builder.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
//! Config builder
use std::convert::TryFrom;

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

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

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

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

pub fn build(&self) -> Result<VMMConfig, ConversionError> {
// Check if there are any errors
match &self.inner {
Ok(vc) => {
// Empty kernel image path.
if vc.kernel_config.path.to_str().unwrap().is_empty() {
return Err(ConversionError::ParseKernel(
"Kernel Image Path is Empty.".to_string(),
));
}
}
Err(_) => {}
}

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) -> Result<VMMConfig, ConversionError>,
{
Builder {
inner: self.inner.and_then(func),
}
}
}
27 changes: 18 additions & 9 deletions src/vmm/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,14 @@ use arg_parser::CfgArgParser;

use super::{DEFAULT_HIGH_RAM_START, DEFAULT_KERNEL_CMDLINE};

use self::builder::Builder;

mod arg_parser;

mod builder;

/// Errors encountered converting the `*Config` objects.
#[derive(Debug, PartialEq)]
#[derive(Clone, Debug, PartialEq)]
pub enum ConversionError {
/// Failed to parse the string representation for the kernel.
ParseKernel(String),
Expand Down Expand Up @@ -46,6 +50,13 @@ impl ConversionError {
}
}

impl VMMConfig {
/// Builds a builder
pub fn builder() -> Builder {
Builder::new()
}
}

impl fmt::Display for ConversionError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
use ConversionError::*;
Expand All @@ -60,17 +71,15 @@ impl fmt::Display for ConversionError {
}

/// Guest memory configurations.
#[derive(Clone, Debug, Default, PartialEq)]
#[derive(Clone, Debug, PartialEq)]
pub struct MemoryConfig {
/// Guest memory size in MiB.
pub size_mib: u32,
}

impl Default for MemoryConfig {
fn default() -> Self {
MemoryConfig {
size_mib: 256 as u32,
}
MemoryConfig { size_mib: 256u32 }
}
}

Expand Down Expand Up @@ -101,7 +110,7 @@ pub struct VcpuConfig {

impl Default for VcpuConfig {
fn default() -> Self {
VcpuConfig { num: 1 as u8 }
VcpuConfig { num: 1u8 }
}
}

Expand All @@ -123,7 +132,7 @@ impl TryFrom<&str> for VcpuConfig {
}

/// Guest kernel configurations.
#[derive(Clone, Debug, Default, PartialEq)]
#[derive(Clone, Debug, PartialEq)]
pub struct KernelConfig {
/// Kernel command line.
pub cmdline: String,
Expand Down Expand Up @@ -177,7 +186,7 @@ impl TryFrom<&str> for KernelConfig {
}
}
/// Network device configuration.
#[derive(Clone, Debug, Default, PartialEq)]
#[derive(Clone, Debug, PartialEq)]
pub struct NetConfig {
/// Name of tap device.
pub tap_name: String,
Expand All @@ -203,7 +212,7 @@ impl TryFrom<&str> for NetConfig {
}

/// Block device configuration
#[derive(Clone, Debug, Default, PartialEq)]
#[derive(Clone, Debug, PartialEq)]
pub struct BlockConfig {
/// Path to the block device backend.
pub path: PathBuf,
Expand Down
4 changes: 3 additions & 1 deletion src/vmm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ use vm_vcpu::vm::{self, KvmVm, VmState};

mod boot;
mod config;
pub use config::*;

mod serial;

/// First address past 32 bits is where the MMIO gap ends.
Expand Down Expand Up @@ -184,7 +186,7 @@ impl TryFrom<VMMConfig> for VMM {
}

if !cmdline.as_str().is_empty() {
vmm.kernel_cfg.cmdline.push_str(" ");
vmm.kernel_cfg.cmdline.push(' ');
vmm.kernel_cfg.cmdline.push_str(cmdline.as_str());
}

Expand Down

0 comments on commit ce8a922

Please sign in to comment.