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 <sebastien.boeuf@intel.com>

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> {
+fn restore_api_command(socket: &mut UnixStream, config: &str) -> Result<(), Error> {
-    let restore_config = vmm::api::VmRestoreConfig {
+    let restore_config = vmm::config::RestoreConfig::parse(config).map_err(Error::Restore)?;
-        source_url: String::from(url),
-    };
 
     simple_api_command(
         socket,
@@ -445,7 +444,7 @@ fn main() {
                 .arg(
                     Arg::with_name("restore_config")
                         .index(1)
-                        .help("<source_url>"),
+                        .help(vmm::config::RestoreConfig::SYNTAX),
                 ),
         );
 

src/main.rs

@@ -234,10 +234,7 @@ fn create_app<'a, 'b>(
         .arg(
             Arg::with_name("restore")
                 .long("restore")
-                .help(
+                .help(config::RestoreConfig::SYNTAX)
-                    "Restore from a VM snapshot. \
-                    Should be a valid URL (e.g file:///foo/bar or tcp://192.168.1.10/foo)",
-                )
                 .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 {
+            Arc::new(match config::RestoreConfig::parse(restore_params) {
-                source_url: restore_url.to_string(),
+                Ok(config) => config,
+                Err(e) => {
+                    println!("{}", e);
+                    process::exit(1);
+                }
             }),
         )
         .expect("Could not restore the VM");

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
+                        // Deserialize into a RestoreConfig
-                        let vm_restore_data: VmRestoreConfig =
+                        let vm_restore_data: RestoreConfig =
                             match serde_json::from_slice(body.raw())
                                 .map_err(HttpError::SerdeJsonDeserialize)
                             {

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<VmSnapshotConfig>, Sender<ApiResponse>),
 
     /// Restore from a VM snapshot
-    VmRestore(Arc<VmRestoreConfig>, Sender<ApiResponse>),
+    VmRestore(Arc<RestoreConfig>, Sender<ApiResponse>),
 }
 
 pub fn vm_create(
@@ -357,7 +350,7 @@ pub fn vm_snapshot(
 pub fn vm_restore(
     api_evt: EventFd,
     api_sender: Sender<ApiRequest>,
-    data: Arc<VmRestoreConfig>,
+    data: Arc<RestoreConfig>,
 ) -> ApiResult<()> {
     let (response_sender, response_receiver) = channel();
 

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>\" \
+        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<Self> {
+        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)]

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);
 

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<T> = result::Result<T, Error>;