From a517ca23a0bcf9362543664dfd151d98ae494a92 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Tue, 7 Apr 2020 14:50:19 +0200 Subject: [PATCH] vmm: Move restore parameters into common RestoreConfig structure The goal here is to move the restore parameters into a dedicated structure that can be reused from the entire codebase, making the addition or removal of a parameter easier. Signed-off-by: Sebastien Boeuf --- src/bin/ch-remote.rs | 9 ++++----- src/main.rs | 15 ++++++++------- vmm/src/api/http_endpoint.rs | 6 +++--- vmm/src/api/mod.rs | 13 +++---------- vmm/src/config.rs | 31 +++++++++++++++++++++++++++++++ vmm/src/lib.rs | 15 +++++++++++---- vmm/src/vm.rs | 3 +++ 7 files changed, 63 insertions(+), 29 deletions(-) diff --git a/src/bin/ch-remote.rs b/src/bin/ch-remote.rs index 9d61ca998..3a7204ff1 100644 --- a/src/bin/ch-remote.rs +++ b/src/bin/ch-remote.rs @@ -26,6 +26,7 @@ enum Error { AddDiskConfig(vmm::config::Error), AddPmemConfig(vmm::config::Error), AddNetConfig(vmm::config::Error), + Restore(vmm::config::Error), } #[derive(Clone, Copy, Debug)] @@ -262,10 +263,8 @@ fn snapshot_api_command(socket: &mut UnixStream, url: &str) -> Result<(), Error> ) } -fn restore_api_command(socket: &mut UnixStream, url: &str) -> Result<(), Error> { - let restore_config = vmm::api::VmRestoreConfig { - source_url: String::from(url), - }; +fn restore_api_command(socket: &mut UnixStream, config: &str) -> Result<(), Error> { + let restore_config = vmm::config::RestoreConfig::parse(config).map_err(Error::Restore)?; simple_api_command( socket, @@ -445,7 +444,7 @@ fn main() { .arg( Arg::with_name("restore_config") .index(1) - .help(""), + .help(vmm::config::RestoreConfig::SYNTAX), ), ); diff --git a/src/main.rs b/src/main.rs index a16aca3a1..3b685b3a4 100755 --- a/src/main.rs +++ b/src/main.rs @@ -234,10 +234,7 @@ fn create_app<'a, 'b>( .arg( Arg::with_name("restore") .long("restore") - .help( - "Restore from a VM snapshot. \ - Should be a valid URL (e.g file:///foo/bar or tcp://192.168.1.10/foo)", - ) + .help(config::RestoreConfig::SYNTAX) .takes_value(true) .min_values(1) .group("vmm-config"), @@ -344,12 +341,16 @@ fn start_vmm(cmd_arguments: ArgMatches) { ) .expect("Could not create the VM"); vmm::api::vm_boot(api_evt.try_clone().unwrap(), sender).expect("Could not boot the VM"); - } else if let Some(restore_url) = cmd_arguments.value_of("restore") { + } else if let Some(restore_params) = cmd_arguments.value_of("restore") { vmm::api::vm_restore( api_evt.try_clone().unwrap(), api_request_sender, - Arc::new(vmm::api::VmRestoreConfig { - source_url: restore_url.to_string(), + Arc::new(match config::RestoreConfig::parse(restore_params) { + Ok(config) => config, + Err(e) => { + println!("{}", e); + process::exit(1); + } }), ) .expect("Could not restore the VM"); diff --git a/vmm/src/api/http_endpoint.rs b/vmm/src/api/http_endpoint.rs index 0884c82f5..ef95771c9 100644 --- a/vmm/src/api/http_endpoint.rs +++ b/vmm/src/api/http_endpoint.rs @@ -8,7 +8,7 @@ use crate::api::{ vm_add_device, vm_add_disk, vm_add_net, vm_add_pmem, vm_boot, vm_create, vm_delete, vm_info, vm_pause, vm_reboot, vm_remove_device, vm_resize, vm_restore, vm_resume, vm_shutdown, vm_snapshot, vmm_ping, vmm_shutdown, ApiError, ApiRequest, ApiResult, DeviceConfig, DiskConfig, - NetConfig, PmemConfig, VmAction, VmConfig, VmRemoveDeviceData, VmResizeData, VmRestoreConfig, + NetConfig, PmemConfig, RestoreConfig, VmAction, VmConfig, VmRemoveDeviceData, VmResizeData, VmSnapshotConfig, }; use micro_http::{Body, Method, Request, Response, StatusCode, Version}; @@ -252,8 +252,8 @@ impl EndpointHandler for VmRestore { Method::Put => { match &req.body { Some(body) => { - // Deserialize into a VmRestoreConfig - let vm_restore_data: VmRestoreConfig = + // Deserialize into a RestoreConfig + let vm_restore_data: RestoreConfig = match serde_json::from_slice(body.raw()) .map_err(HttpError::SerdeJsonDeserialize) { diff --git a/vmm/src/api/mod.rs b/vmm/src/api/mod.rs index ca5add44f..571e78a49 100644 --- a/vmm/src/api/mod.rs +++ b/vmm/src/api/mod.rs @@ -37,7 +37,7 @@ pub use self::http::start_http_thread; pub mod http; pub mod http_endpoint; -use crate::config::{DeviceConfig, DiskConfig, NetConfig, PmemConfig, VmConfig}; +use crate::config::{DeviceConfig, DiskConfig, NetConfig, PmemConfig, RestoreConfig, VmConfig}; use crate::vm::{Error as VmError, VmState}; use std::io; use std::sync::mpsc::{channel, RecvError, SendError, Sender}; @@ -158,13 +158,6 @@ pub struct VmSnapshotConfig { pub destination_url: String, } -#[derive(Clone, Deserialize, Serialize)] -pub struct VmRestoreConfig { - /// The snapshot restore source URL. - /// This is where the VMM is going to get its snapshot to restore itself from. - pub source_url: String, -} - pub enum ApiResponsePayload { /// No data is sent on the channel. Empty, @@ -247,7 +240,7 @@ pub enum ApiRequest { VmSnapshot(Arc, Sender), /// Restore from a VM snapshot - VmRestore(Arc, Sender), + VmRestore(Arc, Sender), } pub fn vm_create( @@ -357,7 +350,7 @@ pub fn vm_snapshot( pub fn vm_restore( api_evt: EventFd, api_sender: Sender, - data: Arc, + data: Arc, ) -> ApiResult<()> { let (response_sender, response_receiver) = channel(); diff --git a/vmm/src/config.rs b/vmm/src/config.rs index 91eaed41a..a50005c2b 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -46,6 +46,8 @@ pub enum Error { ParseVsockSockMissing, /// Missing vsock cid parameter. ParseVsockCidMissing, + /// Missing restore source_url parameter. + ParseRestoreSourceUrlMissing, /// Error parsing CPU options ParseCpus(OptionParserError), /// Error parsing memory options @@ -72,6 +74,8 @@ pub enum Error { ParseDevicePathMissing, /// Failed to parse vsock parameters ParseVsock(OptionParserError), + /// Failed to parse restore parameters + ParseRestore(OptionParserError), } impl fmt::Display for Error { @@ -115,6 +119,10 @@ impl fmt::Display for Error { ParseNetwork(o) => write!(f, "Error parsing --net: {}", o), ParseDisk(o) => write!(f, "Error parsing --disk: {}", o), ParseRNG(o) => write!(f, "Error parsing --rng: {}", o), + ParseRestore(o) => write!(f, "Error parsing --restore: {}", o), + ParseRestoreSourceUrlMissing => { + write!(f, "Error parsing --restore: source_url missing") + } } } } @@ -1106,6 +1114,29 @@ impl VsockConfig { } } +#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)] +pub struct RestoreConfig { + pub source_url: PathBuf, +} + +impl RestoreConfig { + pub const SYNTAX: &'static str = "Restore from a VM snapshot. \ + Restore parameters \"source_url=\" \ + source_url should be a valid URL (e.g file:///foo/bar or tcp://192.168.1.10/foo)"; + pub fn parse(restore: &str) -> Result { + let mut parser = OptionParser::new(); + parser.add("source_url"); + parser.parse(restore).map_err(Error::ParseRestore)?; + + let source_url = parser + .get("source_url") + .map(PathBuf::from) + .ok_or(Error::ParseRestoreSourceUrlMissing)?; + + Ok(RestoreConfig { source_url }) + } +} + #[derive(Clone, Debug, PartialEq, Deserialize, Serialize)] pub struct VmConfig { #[serde(default)] diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs index f729daa5d..9a6426581 100644 --- a/vmm/src/lib.rs +++ b/vmm/src/lib.rs @@ -18,7 +18,7 @@ extern crate url; extern crate vmm_sys_util; use crate::api::{ApiError, ApiRequest, ApiResponse, ApiResponsePayload, VmInfo, VmmPingResponse}; -use crate::config::{DeviceConfig, DiskConfig, NetConfig, PmemConfig, VmConfig}; +use crate::config::{DeviceConfig, DiskConfig, NetConfig, PmemConfig, RestoreConfig, VmConfig}; use crate::migration::{recv_vm_snapshot, vm_config_from_snapshot}; use crate::seccomp_filters::{get_seccomp_filter, Thread}; use crate::vm::{Error as VmError, Vm, VmState}; @@ -305,11 +305,18 @@ impl Vmm { } } - fn vm_restore(&mut self, source_url: &str) -> result::Result<(), VmError> { + fn vm_restore(&mut self, restore_cfg: RestoreConfig) -> result::Result<(), VmError> { if self.vm.is_some() || self.vm_config.is_some() { return Err(VmError::VmAlreadyCreated); } + let source_url = restore_cfg.source_url.as_path().to_str(); + if source_url.is_none() { + return Err(VmError::RestoreSourceUrlPathToStr); + } + // Safe to unwrap as we checked it was Some(&str). + let source_url = source_url.unwrap(); + let vm_snapshot = recv_vm_snapshot(source_url).map_err(VmError::Restore)?; let vm_config = vm_config_from_snapshot(&vm_snapshot).map_err(VmError::Restore)?; @@ -644,9 +651,9 @@ impl Vmm { sender.send(response).map_err(Error::ApiResponseSend)?; } - ApiRequest::VmRestore(snapshot_data, sender) => { + ApiRequest::VmRestore(restore_data, sender) => { let response = self - .vm_restore(&snapshot_data.source_url) + .vm_restore(restore_data.as_ref().clone()) .map_err(ApiError::VmRestore) .map(|_| ApiResponsePayload::Empty); diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 86d14701b..7e0555a5f 100755 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -188,6 +188,9 @@ pub enum Error { /// Cannot send VM snapshot SnapshotSend(MigratableError), + + /// Cannot convert source URL from Path into &str + RestoreSourceUrlPathToStr, } pub type Result = result::Result;