Implement translation from RateLimiterDescription to an actual RateLimiter object

Signed-off-by: Adrian Catangiu <[email protected]>

Author: acatangiu

api_server/src/request/sync/drive.rs

@@ -3,7 +3,7 @@ use std::result;
 use futures::sync::oneshot;
 use hyper::{Response, StatusCode};
 
-use super::rate_limiter::RateLimiter;
+use super::rate_limiter::RateLimiterDescription;
 use super::{DeviceState, GenerateResponse, SyncRequest};
 use http_service::{empty_response, json_fault_message, json_response};
 use request::ParsedRequest;
@@ -25,7 +25,7 @@ pub struct DriveDescription {
     pub is_root_device: bool,
     pub permissions: DrivePermissions,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub rate_limiter: Option<RateLimiter>,
+    pub rate_limiter: Option<RateLimiterDescription>,
 }
 
 impl DriveDescription {

api_server/src/request/sync/mod.rs

@@ -19,7 +19,8 @@ pub use self::boot_source::{BootSourceBody, BootSourceType, LocalImage};
 pub use self::drive::{DriveDescription, DriveError, DrivePermissions, PutDriveOutcome};
 pub use self::logger::{APILoggerDescription, APILoggerError, APILoggerLevel, PutLoggerOutcome};
 pub use self::net::NetworkInterfaceBody;
-pub use self::rate_limiter::RateLimiter;
+pub use self::rate_limiter::description_into_implementation as rate_limiter_description_into_implementation;
+pub use self::rate_limiter::RateLimiterDescription;
 
 // Unlike async requests, sync request have outcomes which implement this trait. The idea is for
 // each outcome to be a struct which is cheaply and quickly instantiated by the VMM thread, then

api_server/src/request/sync/net.rs

@@ -2,7 +2,7 @@ use std::result;
 
 use futures::sync::oneshot;
 
-use super::{DeviceState, RateLimiter, SyncRequest};
+use super::{DeviceState, RateLimiterDescription, SyncRequest};
 
 use net_util::MacAddr;
 use request::ParsedRequest;
@@ -17,9 +17,9 @@ pub struct NetworkInterfaceBody {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub guest_mac: Option<MacAddr>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub rx_rate_limiter: Option<RateLimiter>,
+    pub rx_rate_limiter: Option<RateLimiterDescription>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub tx_rate_limiter: Option<RateLimiter>,
+    pub tx_rate_limiter: Option<RateLimiterDescription>,
 }
 
 impl NetworkInterfaceBody {
@@ -74,8 +74,8 @@ mod tests {
             state: DeviceState::Attached,
             host_dev_name: String::from("bar"),
             guest_mac: Some(MacAddr::parse_str("12:34:56:78:9A:BC").unwrap()),
-            rx_rate_limiter: Some(RateLimiter::default()),
-            tx_rate_limiter: Some(RateLimiter::default()),
+            rx_rate_limiter: Some(RateLimiterDescription::default()),
+            tx_rate_limiter: Some(RateLimiterDescription::default()),
         };
 
         // This is the json encoding of the netif variable.

api_server/src/request/sync/rate_limiter.rs

@@ -1,3 +1,7 @@
+use std::io;
+
+use fc_util::ratelimiter::RateLimiter;
+
 // This struct represents the strongly typed equivalent of the json body for TokenBucket
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 pub struct TokenBucket {
@@ -16,11 +20,32 @@ impl Default for TokenBucket {
 
 // This struct represents the strongly typed equivalent of the json body for RateLimiter
 #[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)]
-pub struct RateLimiter {
+pub struct RateLimiterDescription {
     pub bandwidth: TokenBucket,
     pub ops: TokenBucket,
 }
 
+// TryFrom trait is sadly marked unstable, so make our own
+impl RateLimiterDescription {
+    fn into_implementation(&self) -> io::Result<RateLimiter> {
+        RateLimiter::new(
+            self.bandwidth.size,
+            self.bandwidth.refill_time,
+            self.ops.size,
+            self.ops.refill_time,
+        )
+    }
+}
+
+pub fn description_into_implementation(
+    rate_limiter_description: Option<&RateLimiterDescription>,
+) -> io::Result<Option<RateLimiter>> {
+    match rate_limiter_description {
+        Some(rld) => Ok(Some(rld.into_implementation()?)),
+        None => Ok(None),
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -38,10 +63,17 @@ mod tests {
 
     #[test]
     fn test_rate_limiter_default() {
-        let l = RateLimiter::default();
+        let l = RateLimiterDescription::default();
         assert_eq!(l.bandwidth.size, 0);
         assert_eq!(l.bandwidth.refill_time, 0);
         assert_eq!(l.ops.size, 0);
         assert_eq!(l.ops.refill_time, 0);
     }
+
+    #[test]
+    fn test_rate_limiter_into_impl() {
+        RateLimiterDescription::default()
+            .into_implementation()
+            .unwrap();
+    }
 }

vmm/Cargo.toml

@@ -11,7 +11,6 @@ scopeguard = "=0.3.3"
 api_server = { path = "../api_server" }
 data_model = { path = "../data_model" }
 devices = { path = "../devices" }
-fc_util = { path = "../fc_util"}
 kernel_loader = { path = "../kernel_loader" }
 kvm = { path = "../kvm" }
 kvm_sys = { path = "../kvm_sys" }

vmm/src/device_config/drive.rs

@@ -3,7 +3,7 @@ use std::path::PathBuf;
 
 use std::result;
 
-use api_server::request::sync::{DriveDescription, DriveError, RateLimiter};
+use api_server::request::sync::{DriveDescription, DriveError, RateLimiterDescription};
 
 type Result<T> = result::Result<T, DriveError>;
 
@@ -14,7 +14,7 @@ pub struct BlockDeviceConfig {
     pub path_on_host: PathBuf,
     pub is_root_device: bool,
     pub is_read_only: bool,
-    pub rate_limiter: Option<RateLimiter>,
+    pub rate_limiter: Option<RateLimiterDescription>,
 }
 
 // Wrapper for the collection that holds all the Block Devices Configs

vmm/src/device_config/net.rs

@@ -4,7 +4,7 @@ use std::rc::Rc;
 use std::result;
 
 use api_server::request::sync::{Error as SyncError, NetworkInterfaceBody,
-                                OkStatus as SyncOkStatus, RateLimiter};
+                                OkStatus as SyncOkStatus, RateLimiterDescription};
 use net_util::{MacAddr, Tap, TapError};
 
 pub struct NetworkInterfaceConfig {
@@ -19,8 +19,8 @@ pub struct NetworkInterfaceConfig {
     // and if so, we want to report the failure back to the API caller immediately. This is an
     // option, because the inner value will be moved to the actual virtio net device before boot.
     pub tap: Option<Tap>,
-    pub rx_rate_limiter: Option<RateLimiter>,
-    pub tx_rate_limiter: Option<RateLimiter>,
+    pub rx_rate_limiter: Option<RateLimiterDescription>,
+    pub tx_rate_limiter: Option<RateLimiterDescription>,
 }
 
 impl NetworkInterfaceConfig {
@@ -106,8 +106,8 @@ mod tests {
                 state: DeviceState::Attached,
                 host_dev_name: String::from("bar"),
                 guest_mac: Some(mac.clone()),
-                rx_rate_limiter: Some(RateLimiter::default()),
-                tx_rate_limiter: Some(RateLimiter::default()),
+                rx_rate_limiter: Some(RateLimiterDescription::default()),
+                tx_rate_limiter: Some(RateLimiterDescription::default()),
             };
             assert!(netif_configs.put(netif_body.clone()).is_ok());
             assert_eq!(netif_configs.if_list.len(), 1);

vmm/src/lib.rs

@@ -4,7 +4,6 @@ extern crate libc;
 extern crate api_server;
 extern crate data_model;
 extern crate devices;
-extern crate fc_util;
 extern crate kernel_loader;
 extern crate kvm;
 extern crate kvm_sys;
@@ -35,10 +34,12 @@ use api_server::request::instance_info::{InstanceInfo, InstanceState};
 use api_server::request::sync::boot_source::{PutBootSourceConfigError, PutBootSourceOutcome};
 use api_server::request::sync::machine_configuration::{PutMachineConfigurationError,
                                                        PutMachineConfigurationOutcome};
-use api_server::request::sync::{APILoggerDescription, BootSourceBody, DriveDescription,
+use api_server::request::sync::{rate_limiter_description_into_implementation,
+                                APILoggerDescription, BootSourceBody, DriveDescription,
                                 DriveError, Error as SyncError, GenerateResponse,
                                 NetworkInterfaceBody, OkStatus as SyncOkStatus, PutDriveOutcome,
                                 PutLoggerOutcome, SyncOutcomeSender, SyncRequest};
+
 use api_server::ApiRequest;
 use data_model::vm::MachineConfiguration;
 use device_config::*;
@@ -47,7 +48,6 @@ use device_manager::mmio::MMIODeviceManager;
 
 use devices::virtio;
 use devices::{DeviceEventT, EpollHandler};
-use fc_util::ratelimiter::RateLimiter;
 use kvm::*;
 use sys_util::{register_signal_handler, EventFd, GuestAddress, GuestMemory, Killable, Terminal};
 use vm_control::VmResponse;
@@ -565,15 +565,9 @@ impl Vmm {
                     .map_err(Error::RootDiskImage)?;
                 let epoll_config = epoll_context.allocate_virtio_block_tokens();
 
-                let rate_limiter = match drive_config.rate_limiter.as_ref() {
-                    Some(rl) => Some(RateLimiter::new(
-                        rl.bandwidth.size,
-                        rl.bandwidth.refill_time,
-                        rl.ops.size,
-                        rl.ops.refill_time,
-                    ).map_err(Error::RateLimiterNew)?),
-                    None => None,
-                };
+                let rate_limiter = rate_limiter_description_into_implementation(
+                    drive_config.rate_limiter.as_ref(),
+                ).map_err(Error::RateLimiterNew)?;
                 let block_box = Box::new(devices::virtio::Block::new(
                     root_image,
                     drive_config.is_read_only,
@@ -609,25 +603,12 @@ impl Vmm {
         for cfg in self.network_interface_configs.iter_mut() {
             let epoll_config = self.epoll_context.allocate_virtio_net_tokens();
 
-            // TODO: implement from to reduce code
-            let rx_rate_limiter = match cfg.rx_rate_limiter.as_ref() {
-                Some(rl) => Some(RateLimiter::new(
-                    rl.bandwidth.size,
-                    rl.bandwidth.refill_time,
-                    rl.ops.size,
-                    rl.ops.refill_time,
-                ).map_err(Error::RateLimiterNew)?),
-                None => None,
-            };
-            let tx_rate_limiter = match cfg.tx_rate_limiter.as_ref() {
-                Some(rl) => Some(RateLimiter::new(
-                    rl.bandwidth.size,
-                    rl.bandwidth.refill_time,
-                    rl.ops.size,
-                    rl.ops.refill_time,
-                ).map_err(Error::RateLimiterNew)?),
-                None => None,
-            };
+            let rx_rate_limiter = rate_limiter_description_into_implementation(
+                cfg.rx_rate_limiter.as_ref(),
+            ).map_err(Error::RateLimiterNew)?;
+            let tx_rate_limiter = rate_limiter_description_into_implementation(
+                cfg.tx_rate_limiter.as_ref(),
+            ).map_err(Error::RateLimiterNew)?;
 
             if let Some(tap) = cfg.take_tap() {
                 let net_box = Box::new(devices::virtio::Net::new_with_tap(