Move data types used by the vm and api_server in a different crate

Cargo.toml

@@ -7,7 +7,6 @@ authors = ["Amazon firecracker team <[email protected]>"]
 clap = "=2.27.1"
 
 api_server = { path = "api_server" }
-data_model = { path = "data_model" }
 jailer = { path = "jailer" }
 logger = { path = "logger"}
 seccomp_sys = { path = "seccomp_sys" }

api_server/Cargo.toml

@@ -10,8 +10,8 @@ serde = "=1.0.27"
 serde_derive = "=1.0.27"
 serde_json = "=1.0.9"
 tokio-core = "=0.1.12"
-tokio-uds = "=0.1.7"
 tokio-io = "=0.1.5"
+tokio-uds = "=0.1.7"
 
 data_model = { path = "../data_model" }
 fc_util = { path = "../fc_util" }

api_server/src/http_service.rs

@@ -7,16 +7,18 @@ use std::sync::{Arc, RwLock};
 
 use futures::future::{self, Either};
 use futures::{Future, Stream};
-
 use hyper::{self, Chunk, Headers, Method, StatusCode};
 use serde_json;
 use tokio_core::reactor::Handle;
 
 use super::{ActionMap, ActionMapValue};
+use data_model::device_config::{DriveConfig, NetworkInterfaceConfig};
+use data_model::vm::boot_source::BootSource;
+use data_model::vm::LoggerDescription;
 use data_model::vm::MachineConfiguration;
 use logger::{Metric, METRICS};
 use request::instance_info::InstanceInfo;
-use request::{self, ApiRequest, AsyncOutcome, AsyncRequestBody, IntoParsedRequest, ParsedRequest};
+use request::{ApiRequest, AsyncOutcome, AsyncRequestBody, IntoParsedRequest, ParsedRequest};
 use sys_util::EventFd;
 
 fn build_response_base<B: Into<hyper::Body>>(
@@ -171,12 +173,12 @@ fn parse_boot_source_req<'a>(
 
         0 if method == Method::Put => {
             METRICS.put_api_requests.boot_source_count.inc();
-            Ok(serde_json::from_slice::<request::BootSourceBody>(body)
+            Ok(serde_json::from_slice::<BootSource>(body)
                 .map_err(|e| {
                     METRICS.put_api_requests.boot_source_fails.inc();
                     Error::SerdeJson(e)
                 })?
-                .into_parsed_request()
+                .into_parsed_request(Method::Put, None)
                 .map_err(|s| {
                     METRICS.put_api_requests.boot_source_fails.inc();
                     Error::Generic(StatusCode::BadRequest, s)
@@ -202,12 +204,14 @@ fn parse_drives_req<'a>(
         1 if method == Method::Put => {
             METRICS.put_api_requests.drive_count.inc();
 
-            Ok(serde_json::from_slice::<request::DriveDescription>(body)
+            let unwrapped_id = id_from_path.ok_or(Error::InvalidID)?;
+
+            Ok(serde_json::from_slice::<DriveConfig>(body)
                 .map_err(|e| {
                     METRICS.put_api_requests.drive_fails.inc();
                     Error::SerdeJson(e)
                 })?
-                .into_parsed_request(id_from_path.unwrap())
+                .into_parsed_request(method, Some(unwrapped_id))
                 .map_err(|s| {
                     METRICS.put_api_requests.drive_fails.inc();
                     Error::Generic(StatusCode::BadRequest, s)
@@ -229,18 +233,16 @@ fn parse_logger_req<'a>(
 
         0 if method == Method::Put => {
             METRICS.put_api_requests.logger_count.inc();
-            Ok(
-                serde_json::from_slice::<request::APILoggerDescription>(body)
-                    .map_err(|e| {
-                        METRICS.put_api_requests.logger_fails.inc();
-                        Error::SerdeJson(e)
-                    })?
-                    .into_parsed_request()
-                    .map_err(|s| {
-                        METRICS.put_api_requests.logger_fails.inc();
-                        Error::Generic(StatusCode::BadRequest, s)
-                    })?,
-            )
+            Ok(serde_json::from_slice::<LoggerDescription>(body)
+                .map_err(|e| {
+                    METRICS.put_api_requests.logger_fails.inc();
+                    Error::SerdeJson(e)
+                })?
+                .into_parsed_request(method, None)
+                .map_err(|s| {
+                    METRICS.put_api_requests.logger_fails.inc();
+                    Error::Generic(StatusCode::BadRequest, s)
+                })?)
         }
         _ => Err(Error::InvalidPathMethod(path, method)),
     }
@@ -263,7 +265,7 @@ fn parse_machine_config_req<'a>(
                 cpu_template: None,
             };
             Ok(empty_machine_config
-                .into_parsed_request(method)
+                .into_parsed_request(method, None)
                 .map_err(|s| {
                     METRICS.get_api_requests.machine_cfg_fails.inc();
                     Error::Generic(StatusCode::BadRequest, s)
@@ -277,7 +279,7 @@ fn parse_machine_config_req<'a>(
                     METRICS.put_api_requests.machine_cfg_fails.inc();
                     Error::SerdeJson(e)
                 })?
-                .into_parsed_request(method)
+                .into_parsed_request(method, None)
                 .map_err(|s| {
                     METRICS.put_api_requests.machine_cfg_fails.inc();
                     Error::Generic(StatusCode::BadRequest, s)
@@ -304,18 +306,16 @@ fn parse_netif_req<'a>(
             let unwrapped_id = id_from_path.ok_or(Error::InvalidID)?;
             METRICS.put_api_requests.network_count.inc();
 
-            Ok(
-                serde_json::from_slice::<request::NetworkInterfaceBody>(body)
-                    .map_err(|e| {
-                        METRICS.put_api_requests.network_fails.inc();
-                        Error::SerdeJson(e)
-                    })?
-                    .into_parsed_request(unwrapped_id)
-                    .map_err(|s| {
-                        METRICS.put_api_requests.network_fails.inc();
-                        Error::Generic(StatusCode::BadRequest, s)
-                    })?,
-            )
+            Ok(serde_json::from_slice::<NetworkInterfaceConfig>(body)
+                .map_err(|e| {
+                    METRICS.put_api_requests.network_fails.inc();
+                    Error::SerdeJson(e)
+                })?
+                .into_parsed_request(method, Some(unwrapped_id))
+                .map_err(|s| {
+                    METRICS.put_api_requests.network_fails.inc();
+                    Error::Generic(StatusCode::BadRequest, s)
+                })?)
         }
         _ => Err(Error::InvalidPathMethod(path, method)),
     }
@@ -671,10 +671,9 @@ mod tests {
     use futures::sync::oneshot;
     use hyper::header::{ContentType, Headers};
     use hyper::Body;
-    use net_util::MacAddr;
     use request::async::AsyncRequest;
-    use request::sync::{DeviceState, DriveDescription, DrivePermissions, NetworkInterfaceBody,
-                        SyncRequest};
+    use request::sync::SyncRequest;
+    use std::result;
 
     fn body_to_string(body: hyper::Body) -> String {
         let ret = body.fold(Vec::new(), |mut acc, chunk| {
@@ -934,9 +933,9 @@ mod tests {
 
         // PUT
         // Falling back to json deserialization for constructing the "correct" request because not
-        // all of BootSourceBody's members are accessible. Rather than making them all public just
+        // all of BootSource's members are accessible. Rather than making them all public just
         // for the purpose of unit tests, it's preferable to trust the deserialization.
-        let res_bsb = serde_json::from_slice::<request::BootSourceBody>(&body);
+        let res_bsb = serde_json::from_slice::<BootSource>(&body);
         match res_bsb {
             Ok(boot_source_body) => {
                 match parse_boot_source_req(&path_tokens, &path, Method::Put, &body) {
@@ -1000,16 +999,11 @@ mod tests {
         }
 
         // PUT
-        let drive_desc = DriveDescription {
-            drive_id: String::from("bar"),
-            path_on_host: String::from("/foo/bar"),
-            state: DeviceState::Attached,
-            is_root_device: true,
-            permissions: DrivePermissions::ro,
-            rate_limiter: None,
-        };
+        let result: result::Result<DriveConfig, serde_json::Error> = serde_json::from_str(json);
+        assert!(result.is_ok());
+        let drive_desc = result.unwrap();
 
-        match drive_desc.into_parsed_request("bar") {
+        match drive_desc.into_parsed_request(Method::Put, Some("bar")) {
             Ok(pr) => match parse_drives_req(
                 &"/foo/bar"[1..].split_terminator('/').collect(),
                 &"/foo/bar",
@@ -1066,7 +1060,7 @@ mod tests {
             cpu_template: Some(CpuFeaturesTemplate::T2),
         };
 
-        match mcb.into_parsed_request(Method::Put) {
+        match mcb.into_parsed_request(Method::Put, None) {
             Ok(pr) => match parse_machine_config_req(&path_tokens, &path, Method::Put, &body) {
                 Ok(pr_mcb) => assert!(pr.eq(&pr_mcb)),
                 _ => assert!(false),
@@ -1130,16 +1124,14 @@ mod tests {
         }
 
         // PUT
-        let netif = NetworkInterfaceBody {
-            iface_id: String::from("bar"),
-            state: DeviceState::Attached,
-            host_dev_name: String::from("foo"),
-            guest_mac: Some(MacAddr::parse_str("12:34:56:78:9a:BC").unwrap()),
-            rx_rate_limiter: None,
-            tx_rate_limiter: None,
-        };
-
-        match netif.into_parsed_request("bar") {
+        let result: result::Result<NetworkInterfaceConfig, serde_json::Error> =
+            serde_json::from_str(json);
+        assert!(result.is_ok());
+
+        match result
+            .unwrap()
+            .into_parsed_request(Method::Put, Some("bar"))
+        {
             Ok(pr) => match parse_netif_req(
                 &"/foo/bar"[1..].split_terminator('/').collect(),
                 &"/foo/bar",

api_server/src/lib.rs

@@ -86,9 +86,7 @@ impl ApiServer {
         let mut core = Core::new().map_err(Error::Io)?;
         let handle = Rc::new(core.handle());
 
-        let listener = if data_model::FIRECRACKER_IS_JAILED
-            .load(std::sync::atomic::Ordering::Relaxed)
-        {
+        let listener = if jailer::FIRECRACKER_IS_JAILED.load(std::sync::atomic::Ordering::Relaxed) {
             // This is a UnixListener of the tokio_uds variety. Using fd inherited from the jailer.
             UnixListener::from_listener(
                 unsafe { std::os::unix::net::UnixListener::from_raw_fd(jailer::LISTENER_FD) },
@@ -135,3 +133,34 @@ impl ApiServer {
         self.efd.try_clone().map_err(Error::Eventfd)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use request::instance_info::InstanceState;
+    use std::sync::mpsc::channel;
+
+    #[test]
+    fn test_debug_traits_enums() {
+        assert!(
+            format!("{:?}", Error::Io(std::io::Error::from_raw_os_error(22)))
+                .contains("Io(Os { code: 22")
+        );
+        assert_eq!(
+            format!("{:?}", Error::Eventfd(sys_util::Error::new(0))),
+            "Eventfd(Error(0))"
+        );
+    }
+
+    #[test]
+    fn test_api_server_init() {
+        let (to_vmm, _from_api) = channel();
+
+        let shared_info = Arc::new(RwLock::new(InstanceInfo {
+            state: InstanceState::Uninitialized,
+        }));
+        let server = ApiServer::new(shared_info.clone(), to_vmm, 0);
+        assert!(server.is_ok());
+        assert!(server.unwrap().get_event_fd_clone().is_ok());
+    }
+}

api_server/src/request/async/mod.rs

@@ -13,7 +13,7 @@ pub enum AsyncOutcome {
     Error(String),
 }
 
-// The halves of a request/reponse channel associated with each async request.
+// The halves of a request/response channel associated with each async request.
 pub type AsyncOutcomeSender = oneshot::Sender<AsyncOutcome>;
 pub type AsyncOutcomeReceiver = oneshot::Receiver<AsyncOutcome>;
 
@@ -124,6 +124,80 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_ser_deser() {
+        let j = "\"Drive\"";
+        let result: Result<DeviceType, serde_json::Error> = serde_json::from_str(j);
+        assert!(result.is_ok());
+        let result = serde_json::to_string(&result.unwrap());
+        assert!(result.is_ok());
+        assert_eq!(
+            result.unwrap(),
+            String::from(j).replace("\n", "").replace(" ", "")
+        );
+
+        let j = "{
+                    \"device_type\": \"Drive\",
+                    \"device_resource_id\": \"dummy\",
+                    \"force\": true\
+                    }";
+        let result: InstanceDeviceDetachAction = serde_json::from_str(j).unwrap();
+        assert_eq!(
+            format!("{:?}", result),
+            "InstanceDeviceDetachAction { device_type: Drive, device_resource_id: \"dummy\", force: true }"
+        );
+
+        let result = serde_json::to_string(&result);
+        assert!(result.is_ok());
+        assert_eq!(
+            result.unwrap(),
+            String::from(j).replace("\n", "").replace(" ", "")
+        );
+
+        let j = "\"InstanceStart\"";
+        let result: Result<AsyncActionType, serde_json::Error> = serde_json::from_str(j);
+        assert!(result.is_ok());
+        let result = serde_json::to_string(&result.unwrap());
+        assert!(result.is_ok());
+        assert_eq!(
+            result.unwrap(),
+            String::from(j).replace("\n", "").replace(" ", "")
+        );
+
+        let j = "\"InstanceHalt\"";
+        let result: Result<AsyncActionType, serde_json::Error> = serde_json::from_str(j);
+        assert!(result.is_ok());
+        let result = serde_json::to_string(&result.unwrap());
+        assert!(result.is_ok());
+        assert_eq!(
+            result.unwrap(),
+            String::from(j).replace("\n", "").replace(" ", "")
+        );
+
+        let j = "{
+                \"action_id\": \"dummy\",
+                \"action_type\": \"InstanceStart\",
+                \"instance_device_detach_action\": {\
+                    \"device_type\": \"Drive\",
+                    \"device_resource_id\": \"dummy\",
+                    \"force\": true},
+                \"timestamp\": 1522850095\
+                }";
+        let result: AsyncRequestBody = serde_json::from_str(j).unwrap();
+        assert_eq!(
+            format!("{:?}", result),
+            "AsyncRequestBody { action_id: \"dummy\", action_type: InstanceStart, \
+             instance_device_detach_action: Some(InstanceDeviceDetachAction { device_type: Drive, \
+             device_resource_id: \"dummy\", force: true }), timestamp: Some(1522850095) }"
+        );
+        let result = serde_json::to_string(&result);
+        assert!(result.is_ok());
+        assert_eq!(
+            result.unwrap(),
+            String::from(j).replace("\n", "").replace(" ", "")
+        );
+    }
+
     #[test]
     fn test_to_parsed_request() {
         let jsons = vec![
@@ -197,4 +271,30 @@ mod tests {
 
         assert_eq!(async_body.timestamp, Some(1522850096));
     }
+
+    #[test]
+    fn test_debug_traits_enums() {
+        assert_eq!(format!("{:?}", AsyncOutcome::Ok(0)), "Ok(0)");
+        assert_eq!(
+            format!("{:?}", AsyncOutcome::Error(String::from("SomeError"))),
+            "Error(\"SomeError\")"
+        );
+        let (sender, _receiver) = oneshot::channel();
+        assert!(
+            format!("{:?}", AsyncRequest::StartInstance(sender)).contains("StartInstance(Sender")
+        );
+        let (sender, _receiver) = oneshot::channel();
+        assert!(
+            format!("{:?}", AsyncRequest::StopInstance(sender)).contains("StopInstance(Sender")
+        );
+        assert_eq!(format!("{:?}", DeviceType::Drive), "Drive");
+        assert_eq!(
+            format!("{:?}", AsyncActionType::InstanceStart),
+            "InstanceStart"
+        );
+        assert_eq!(
+            format!("{:?}", AsyncActionType::InstanceHalt),
+            "InstanceHalt"
+        );
+    }
 }