From a4d8e3269f4f89e5f24635d922bbd93afb75ac36 Mon Sep 17 00:00:00 2001 From: Diana Popa Date: Thu, 3 May 2018 06:25:05 -0500 Subject: [PATCH 1/5] api_server: adding support for specifying log file... through an API call. Signed-off-by: Diana Popa --- api/swagger/firecracker-beta.yaml | 42 ++++++++++++++++ api_server/src/http_service.rs | 24 +++++++++- api_server/src/request/mod.rs | 4 +- api_server/src/request/sync/logger.rs | 69 +++++++++++++++++++++++++++ api_server/src/request/sync/mod.rs | 3 ++ vmm/src/lib.rs | 43 +++++++++++------ vmm/src/logger_config.rs | 42 ++++++++++++++++ 7 files changed, 209 insertions(+), 18 deletions(-) create mode 100644 api_server/src/request/sync/logger.rs create mode 100644 vmm/src/logger_config.rs diff --git a/api/swagger/firecracker-beta.yaml b/api/swagger/firecracker-beta.yaml index 766cd852a23..5705f5dffdb 100644 --- a/api/swagger/firecracker-beta.yaml +++ b/api/swagger/firecracker-beta.yaml @@ -143,6 +143,29 @@ paths: schema: $ref: "#/definitions/Error" + /logger: + put: + summary: Initializes the logging system by specifying a log path on the host. + operationId: putLogger + parameters: + - name: body + in: body + description: Logging system description + required: true + schema: + $ref: "#/definitions/Logger" + responses: + 201: + description: Logger created. + 400: + description: Logger cannot be initialized due to bad input. + schema: + $ref: "#/definitions/Error" + default: + description: Internal server error. + schema: + $ref: "#/definitions/Error" + /machine-config: get: summary: Get the machine configuration of the VM. @@ -348,6 +371,25 @@ definitions: type: string description: Host level path to the kernel image used to boot the guest + Logger: + type: object + description: + Describes the configuration option for the logger intitialization. + properties: + path: + type: string + description: The path on the host for the log file. + level: + type: string + description: Set the level. + enum: [Error, Warning, Info, Debug] + show_level: + type: boolean + description: Whether or not to output the level in the logs. + show_log_origin: + type: boolean + description: Whether or not to include the file path and line number of the log's origin. + MachineConfiguration: type: object description: diff --git a/api_server/src/http_service.rs b/api_server/src/http_service.rs index d684c1fa478..5966a36484f 100644 --- a/api_server/src/http_service.rs +++ b/api_server/src/http_service.rs @@ -65,7 +65,7 @@ enum Error<'a> { Generic(StatusCode, String), // The HTTP method & request path combination is not valid. InvalidPathMethod(&'a str, Method), - // An error occured when deserializing the json body of a request. + // An error occurred when deserializing the json body of a request. SerdeJson(serde_json::Error), } @@ -154,6 +154,25 @@ fn parse_boot_source_req<'a>( } } +// Turns a GET/PUT /boot-source HTTP request into a ParsedRequest +fn parse_logger_req<'a>( + path_tokens: &Vec<&str>, + path: &'a str, + method: Method, + body: &Chunk, +) -> Result<'a, ParsedRequest> { + match path_tokens[1..].len() { + 0 if method == Method::Get => Ok(ParsedRequest::Dummy), + + 0 if method == Method::Put => Ok(serde_json::from_slice::( + body, + ).map_err(Error::SerdeJson)? + .into_parsed_request() + .map_err(|s| Error::Generic(StatusCode::BadRequest, s))?), + _ => Err(Error::InvalidPathMethod(path, method)), + } +} + // Turns a GET/PUT /drives HTTP request into a ParsedRequest fn parse_drives_req<'a>( path_tokens: &Vec<&str>, @@ -265,6 +284,8 @@ fn parse_request<'a>( method, path, str::from_utf8(body.as_ref()).unwrap() + // when time will come, we could better do + // serde_json::from_slice(&body).unwrap() ) ); } @@ -297,6 +318,7 @@ fn parse_request<'a>( "actions" => parse_actions_req(&path_tokens, path, method, &id_from_path, body, action_map), "boot-source" => parse_boot_source_req(&path_tokens, path, method, body), "drives" => parse_drives_req(&path_tokens, path, method, &id_from_path, body), + "logger" => parse_logger_req(&path_tokens, path, method, body), "machine-config" => parse_machine_config_req(&path_tokens, path, method, body), "network-interfaces" => parse_netif_req(&path_tokens, path, method, &id_from_path, body), "vsocks" => parse_vsocks_req(&path_tokens, path, method, &id_from_path, body), diff --git a/api_server/src/request/mod.rs b/api_server/src/request/mod.rs index 80a1f2d4a99..90979ba9a98 100644 --- a/api_server/src/request/mod.rs +++ b/api_server/src/request/mod.rs @@ -6,8 +6,8 @@ use std::result; use hyper::Method; pub use self::async::{AsyncOutcome, AsyncOutcomeReceiver, AsyncOutcomeSender, AsyncRequest, AsyncRequestBody}; -pub use self::sync::{BootSourceBody, DriveDescription, NetworkInterfaceBody, SyncOutcomeReceiver, - SyncOutcomeSender, SyncRequest, VsockJsonBody}; +pub use self::sync::{APILoggerDescription, BootSourceBody, DriveDescription, NetworkInterfaceBody, + SyncOutcomeReceiver, SyncOutcomeSender, SyncRequest, VsockJsonBody}; pub mod instance_info; diff --git a/api_server/src/request/sync/logger.rs b/api_server/src/request/sync/logger.rs new file mode 100644 index 00000000000..f3561295042 --- /dev/null +++ b/api_server/src/request/sync/logger.rs @@ -0,0 +1,69 @@ +use std::result; + +use futures::sync::oneshot; +use hyper::{Response, StatusCode}; + +use http_service::{empty_response, json_fault_message, json_response}; +use request::{ParsedRequest, SyncRequest}; +use request::sync::GenerateResponse; + +#[derive(Debug, Deserialize, Serialize)] +pub enum APILoggerLevel { + Error, + Warning, + Info, + Debug, +} + +#[derive(Debug, Deserialize, Serialize)] +pub struct APILoggerDescription { + pub path: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub level: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub show_level: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub show_log_origin: Option, +} + +#[derive(Debug)] +pub enum APILoggerError { + InitializationFailure(String), +} + +impl GenerateResponse for APILoggerError { + fn generate_response(&self) -> Response { + use self::APILoggerError::*; + match *self { + InitializationFailure(ref e) => json_response( + StatusCode::BadRequest, + json_fault_message(format!{"Cannot initialize logging system! {}", e}), + ), + } + } +} + +pub enum PutLoggerOutcome { + Initialized, + Error(APILoggerError), +} + +impl GenerateResponse for PutLoggerOutcome { + fn generate_response(&self) -> Response { + use self::PutLoggerOutcome::*; + match *self { + Initialized => empty_response(StatusCode::Created), + Error(ref e) => e.generate_response(), + } + } +} + +impl APILoggerDescription { + pub fn into_parsed_request(self) -> result::Result { + let (sender, receiver) = oneshot::channel(); + Ok(ParsedRequest::Sync( + SyncRequest::PutLogger(self, sender), + receiver, + )) + } +} diff --git a/api_server/src/request/sync/mod.rs b/api_server/src/request/sync/mod.rs index 946bdb97283..d1ca0b3a784 100644 --- a/api_server/src/request/sync/mod.rs +++ b/api_server/src/request/sync/mod.rs @@ -10,12 +10,14 @@ use net_util::TapError; pub mod boot_source; mod drive; +mod logger; pub mod machine_configuration; mod net; mod vsock; pub use self::drive::{DriveDescription, DriveError, DrivePermissions, PutDriveOutcome}; pub use self::boot_source::{BootSourceBody, BootSourceType, LocalImage}; +pub use self::logger::{APILoggerDescription, APILoggerError, APILoggerLevel, PutLoggerOutcome}; pub use self::net::NetworkInterfaceBody; pub use self::vsock::VsockJsonBody; @@ -56,6 +58,7 @@ pub enum SyncRequest { GetMachineConfiguration(SyncOutcomeSender), PutBootSource(BootSourceBody, SyncOutcomeSender), PutDrive(DriveDescription, SyncOutcomeSender), + PutLogger(APILoggerDescription, SyncOutcomeSender), PutMachineConfiguration(MachineConfiguration, SyncOutcomeSender), PutNetworkInterface(NetworkInterfaceBody, SyncOutcomeSender), PutVsock(VsockJsonBody, SyncOutcomeSender), diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs index ccb8c0af566..d8ce41c3a88 100644 --- a/vmm/src/lib.rs +++ b/vmm/src/lib.rs @@ -17,6 +17,7 @@ extern crate x86_64; pub mod device_config; pub mod device_manager; pub mod kernel_cmdline; +mod logger_config; mod vm_control; mod vstate; @@ -34,7 +35,7 @@ use api_server::request::async::{AsyncOutcome, AsyncRequest}; use api_server::request::instance_info::{InstanceInfo, InstanceState}; use api_server::request::sync::{DriveError, Error as SyncError, GenerateResponse, NetworkInterfaceBody, OkStatus as SyncOkStatus, PutDriveOutcome, - SyncRequest, VsockJsonBody}; + PutLoggerOutcome, SyncRequest, VsockJsonBody}; use api_server::request::sync::boot_source::{PutBootSourceConfigError, PutBootSourceOutcome}; use api_server::request::sync::machine_configuration::{PutMachineConfigurationError, PutMachineConfigurationOutcome}; @@ -911,20 +912,6 @@ impl Vmm { .map_err(|_| ()) .expect("one-shot channel closed"); } - SyncRequest::PutDrive(drive_description, sender) => { - match self.put_block_device(BlockDeviceConfig::from(drive_description)) { - Ok(_) => - // doing expect() to crash this thread if the other thread crashed - sender.send(Box::new(PutDriveOutcome::Created)) - .map_err(|_| ()) - .expect("one-shot channel closed"), - Err(e) => - // doing expect() to crash this thread if the other thread crashed - sender.send(Box::new(e)) - .map_err(|_| ()) - .expect("one-shot channel closed"), - } - } SyncRequest::PutBootSource(boot_source_body, sender) => { // check that the kernel path exists and it is valid let box_response: Box = match boot_source_body @@ -970,6 +957,32 @@ impl Vmm { .map_err(|_| ()) .expect("one-shot channel closed"); } + SyncRequest::PutDrive(drive_description, sender) => { + match self.put_block_device(BlockDeviceConfig::from(drive_description)) { + Ok(_) => + // doing expect() to crash this thread if the other thread crashed + sender.send(Box::new(PutDriveOutcome::Created)) + .map_err(|_| ()) + .expect("one-shot channel closed"), + Err(e) => + // doing expect() to crash this thread if the other thread crashed + sender.send(Box::new(e)) + .map_err(|_| ()) + .expect("one-shot channel closed"), + } + } + SyncRequest::PutLogger(logger_description, sender) => { + match logger_config::init_logger(logger_description) { + Ok(_) => sender + .send(Box::new(PutLoggerOutcome::Initialized)) + .map_err(|_| ()) + .expect("one-shot channel closed"), + Err(e) => sender + .send(Box::new(e)) + .map_err(|_| ()) + .expect("one-shot channel closed"), + } + } SyncRequest::PutMachineConfiguration(machine_config_body, sender) => { let boxed_response = match self.put_virtual_machine_configuration( machine_config_body.vcpu_count, diff --git a/vmm/src/logger_config.rs b/vmm/src/logger_config.rs new file mode 100644 index 00000000000..4c7f7275eb6 --- /dev/null +++ b/vmm/src/logger_config.rs @@ -0,0 +1,42 @@ +use std::result; +use api_server::request::sync::{APILoggerDescription, APILoggerError, APILoggerLevel}; +use logger::{Level, Logger}; + +type Result = result::Result; + +pub fn init_logger(api_logger: APILoggerDescription) -> Result<()> { + //there are 3 things we need to get out: the level, whether to show it and whether to show the origin of the log + let mut logger = Logger::new(); + let level = from_api_level(api_logger.level); + + if let Some(val) = level { + logger.set_level(val); + } + + if let Some(val) = api_logger.show_log_origin { + logger.set_include_origin(val, val); + } + + if let Some(val) = api_logger.show_level { + logger.set_include_level(val); + } + + if let Err(ref e) = logger.init(Some(api_logger.path)) { + return Err(APILoggerError::InitializationFailure(e.to_string())); + } else { + Ok(()) + } +} + +fn from_api_level(api_level: Option) -> Option { + if let Some(val) = api_level { + match val { + APILoggerLevel::Error => Some(Level::Error), + APILoggerLevel::Warning => Some(Level::Warn), + APILoggerLevel::Info => Some(Level::Info), + APILoggerLevel::Debug => Some(Level::Debug), + } + } else { + None + } +} From 66536df530bab54a5e93803d430d25ca7d258fba Mon Sep 17 00:00:00 2001 From: Diana Popa Date: Thu, 3 May 2018 09:10:16 -0500 Subject: [PATCH 2/5] logger: ensuring thread safety for logger's initialization * removed static variable for keeping status * used AtomicSize for safely sharing state between threads * appended relevant error on top of the low level one * removed Poison status error as it stopped us from being able to try reinitialization Signed-off-by: Diana Popa --- logger/src/error.rs | 23 +++++++++------ logger/src/lib.rs | 71 +++++++++++++++++++-------------------------- 2 files changed, 44 insertions(+), 50 deletions(-) diff --git a/logger/src/error.rs b/logger/src/error.rs index 061f6b38e77..b79912c002e 100644 --- a/logger/src/error.rs +++ b/logger/src/error.rs @@ -10,8 +10,6 @@ pub enum LoggerError { NeverInitialized(String), /// The logger does not allow reinitialization. AlreadyInitialized, - /// Initialization has previously failed and can not be retried. - Poisoned(String), /// Creating log file fails. CreateLogFile(std::io::Error), /// Writing to log file fails. @@ -25,13 +23,20 @@ pub enum LoggerError { impl fmt::Display for LoggerError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let printable = match *self { - LoggerError::NeverInitialized(ref e) => e, - LoggerError::AlreadyInitialized => "Reinitialization of logger not allowed", - LoggerError::Poisoned(ref e) => e, - LoggerError::CreateLogFile(ref e) => e.description(), - LoggerError::FileLogWrite(ref e) => e.description(), - LoggerError::FileLogFlush(ref e) => e.description(), - LoggerError::FileLogLock(ref e) => e, + LoggerError::NeverInitialized(ref e) => format!("{}", e), + LoggerError::AlreadyInitialized => { + format!("{}", "Reinitialization of logger not allowed.") + } + LoggerError::CreateLogFile(ref e) => { + format!("Failed to create log file. Error: {}", e.description()) + } + LoggerError::FileLogWrite(ref e) => { + format!("Failed to write to log file. Error: {}", e.description()) + } + LoggerError::FileLogFlush(ref e) => { + format!("Failed to flush log file. Error: {}", e.description()) + } + LoggerError::FileLogLock(ref e) => format!("{}", e), }; write!(f, "{}", printable) } diff --git a/logger/src/lib.rs b/logger/src/lib.rs index 5661840a90f..b0011f38439 100644 --- a/logger/src/lib.rs +++ b/logger/src/lib.rs @@ -32,7 +32,9 @@ mod writers; use error::LoggerError; use log::{set_boxed_logger, set_max_level, Log, Metadata, Record}; use std::result; -use std::sync::{Arc, Once, ONCE_INIT}; +use std::sync::Arc; +use std::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT}; + use writers::*; /// Output sources for the log subsystem. @@ -65,10 +67,10 @@ pub const DEFAULT_LEVEL: Level = Level::Warn; /// Synchronization primitives used to run a one-time global initialization. /// const UNINITIALIZED: usize = 0; -const INITIALIZED: usize = 1; +const INITIALIZING: usize = 1; +const INITIALIZED: usize = 2; -static INIT: Once = ONCE_INIT; -static mut INIT_RES: Result = Ok(UNINITIALIZED); +static STATE: AtomicUsize = ATOMIC_USIZE_INIT; /// Logger representing the logging subsystem. /// @@ -248,47 +250,34 @@ impl Logger { /// } /// ``` pub fn init(mut self, log_path: Option) -> Result<()> { - unsafe { - if INIT_RES.is_err() { - INIT_RES = Err(LoggerError::Poisoned(format!( - "{}", - INIT_RES.as_ref().err().unwrap() - ))); - return Err(LoggerError::Poisoned(format!( - "{}", - INIT_RES.as_ref().err().unwrap() - ))); - } else if INIT_RES.is_ok() && INIT_RES.as_ref().unwrap() != &UNINITIALIZED { - return Err(LoggerError::AlreadyInitialized); - } + // if the logger was already initialized, return error + if STATE.compare_and_swap(UNINITIALIZED, INITIALIZING, Ordering::SeqCst) != UNINITIALIZED { + return Err(LoggerError::AlreadyInitialized); + } - INIT.call_once(|| { - if let Some(path) = log_path.as_ref() { - match FileLogWriter::new(path) { - Ok(t) => { - self.file = Some(Arc::new(t)); - self.level_info.writer = Destination::File; - } - Err(ref e) => { - INIT_RES = Err(LoggerError::NeverInitialized(format!("{}", e))); - } - }; + // otherwise try initialization + if let Some(path) = log_path.as_ref() { + match FileLogWriter::new(path) { + Ok(t) => { + self.file = Some(Arc::new(t)); + self.level_info.writer = Destination::File; } - set_max_level(self.level_info.code.to_level_filter()); - - if let Err(e) = set_boxed_logger(Box::new(self)) { - INIT_RES = Err(LoggerError::NeverInitialized(format!("{}", e))) + Err(ref e) => { + STATE.store(UNINITIALIZED, Ordering::SeqCst); + return Err(LoggerError::NeverInitialized(format!("{}", e))); } - }); - if INIT_RES.is_err() { - return Err(LoggerError::NeverInitialized(format!( - "{}", - INIT_RES.as_ref().err().unwrap() - ))); - } else { - INIT_RES = Ok(INITIALIZED); - } + }; + } + + // if code reaches here, we are all good + set_max_level(self.level_info.code.to_level_filter()); + + if let Err(e) = set_boxed_logger(Box::new(self)) { + STATE.store(UNINITIALIZED, Ordering::SeqCst); + return Err(LoggerError::NeverInitialized(format!("{}", e))); } + + STATE.store(INITIALIZED, Ordering::SeqCst); Ok(()) } } From c44371f01e2829feab0312beda7a0342a1b3236c Mon Sep 17 00:00:00 2001 From: Diana Popa Date: Thu, 3 May 2018 10:11:09 -0500 Subject: [PATCH 3/5] logger: update unit tests * call asserts on the last code in the test_init * exercise Display for the LoggerError Signed-off-by: Diana Popa --- logger/src/error.rs | 40 +++++++++++++++++++++++++++++++++++----- logger/src/lib.rs | 34 +++++++++++++++++++++++----------- 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/logger/src/error.rs b/logger/src/error.rs index b79912c002e..85a8b718b19 100644 --- a/logger/src/error.rs +++ b/logger/src/error.rs @@ -55,30 +55,60 @@ mod tests { LoggerError::NeverInitialized(String::from("Bad Log Path Provided")) ).contains("NeverInitialized") ); - assert!(format!("{:?}", LoggerError::AlreadyInitialized).contains("AlreadyInitialized")); - assert!( + assert_eq!( format!( - "{:?}", - LoggerError::Poisoned(String::from("Never Initialized")) - ).contains("Poisoned") + "{}", + LoggerError::NeverInitialized(String::from("Bad Log Path Provided")) + ), + "Bad Log Path Provided" + ); + + assert!(format!("{:?}", LoggerError::AlreadyInitialized).contains("AlreadyInitialized")); + assert_eq!( + format!("{}", LoggerError::AlreadyInitialized), + "Reinitialization of logger not allowed." ); + assert!( format!( "{:?}", LoggerError::FileLogWrite(std::io::Error::new(ErrorKind::Interrupted, "write")) ).contains("FileLogWrite") ); + assert_eq!( + format!( + "{}", + LoggerError::FileLogWrite(std::io::Error::new(ErrorKind::Interrupted, "write")) + ), + "Failed to write to log file. Error: write" + ); + assert!( format!( "{:?}", LoggerError::FileLogFlush(std::io::Error::new(ErrorKind::Interrupted, "flush")) ).contains("FileLogFlush") ); + assert_eq!( + format!( + "{}", + LoggerError::FileLogFlush(std::io::Error::new(ErrorKind::Interrupted, "flush")) + ), + "Failed to flush log file. Error: flush" + ); + assert!( format!( "{:?}", LoggerError::FileLogLock(String::from("File log lock")) ).contains("FileLogLock") ); + assert_eq!( + format!( + "{}", + LoggerError::FileLogLock(String::from("File log lock")) + ), + "File log lock" + ); } } diff --git a/logger/src/lib.rs b/logger/src/lib.rs index b0011f38439..50c8ffe4ae3 100644 --- a/logger/src/lib.rs +++ b/logger/src/lib.rs @@ -410,20 +410,21 @@ mod tests { ("[ERROR", "lib.rs", "error"), ], ); - remove_file("tmp.log").unwrap(); let l = Logger::new(); - unsafe { - if let Err(e) = set_boxed_logger(Box::new(l)) { - INIT_RES = Err(LoggerError::NeverInitialized(format!("{}", e))); - } - assert!(format!("{:?}", INIT_RES).contains("NeverInitialized")); - } + STATE.store(UNINITIALIZED, Ordering::SeqCst); + assert_eq!( + format!("{:?}", l.init(Some(String::from("tmp.log"))).err().unwrap()), + "NeverInitialized(\"attempted to set a logger after \ + the logging system was already initialized\")" + ); + + remove_file("tmp.log").unwrap(); + + // exercise the case when there is an error in opening file let l = Logger::new(); - assert!(l.init(None).is_err()); - unsafe { - assert!(format!("{:?}", INIT_RES).contains("Poisoned")); - } + STATE.store(UNINITIALIZED, Ordering::SeqCst); + assert!(l.init(Some(String::from(""))).is_err()); let mut l = Logger::new(); l.set_include_level(true); @@ -431,10 +432,21 @@ mod tests { let error_metadata = MetadataBuilder::new().level(Level::Error).build(); let log_record = log::Record::builder().metadata(error_metadata).build(); Logger::log(&l, &log_record); + + assert_eq!(l.show_level, true); + assert_eq!(l.show_file_path, false); + assert_eq!(l.show_line_numbers, false); + let mut l = Logger::new(); l.set_include_level(false); l.set_include_origin(true, true); + let error_metadata = MetadataBuilder::new().level(Level::Info).build(); + let log_record = log::Record::builder().metadata(error_metadata).build(); Logger::log(&l, &log_record); + + assert_eq!(l.show_level, false); + assert_eq!(l.show_file_path, true); + assert_eq!(l.show_line_numbers, true); } #[test] From 8fc25c5f9a2271eaf393c2481926d88d880acd60 Mon Sep 17 00:00:00 2001 From: Diana Popa Date: Thu, 3 May 2018 10:48:54 -0500 Subject: [PATCH 4/5] api_server: added unit tests for the logger description Signed-off-by: Diana Popa --- api_server/src/request/sync/logger.rs | 62 ++++++++++++++++++++++++++- api_server/src/request/sync/mod.rs | 4 ++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/api_server/src/request/sync/logger.rs b/api_server/src/request/sync/logger.rs index f3561295042..b33031849af 100644 --- a/api_server/src/request/sync/logger.rs +++ b/api_server/src/request/sync/logger.rs @@ -7,7 +7,7 @@ use http_service::{empty_response, json_fault_message, json_response}; use request::{ParsedRequest, SyncRequest}; use request::sync::GenerateResponse; -#[derive(Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub enum APILoggerLevel { Error, Warning, @@ -15,7 +15,7 @@ pub enum APILoggerLevel { Debug, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub struct APILoggerDescription { pub path: String, #[serde(skip_serializing_if = "Option::is_none")] @@ -67,3 +67,61 @@ impl APILoggerDescription { )) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_generate_response_logger_error() { + assert_eq!( + APILoggerError::InitializationFailure("Could not initialize log system".to_string()) + .generate_response() + .status(), + StatusCode::BadRequest + ); + assert!( + format!( + "{:?}", + APILoggerError::InitializationFailure( + "Could not initialize log system".to_string() + ) + ).contains("InitializationFailure") + ); + } + + #[test] + fn test_generate_response_put_logger_outcome() { + assert_eq!( + PutLoggerOutcome::Initialized.generate_response().status(), + StatusCode::Created + ); + assert_eq!( + PutLoggerOutcome::Error(APILoggerError::InitializationFailure( + "Could not initialize log system".to_string() + )).generate_response() + .status(), + StatusCode::BadRequest + ); + } + + #[test] + fn test_into_parsed_request() { + let desc = APILoggerDescription { + source_type: APILoggerSourceType::LocalPath, + source: String::from(""), + level: None, + show_level: None, + show_log_origin: None, + }; + format!("{:?}", desc); + assert!(&desc.clone().into_parsed_request().is_ok()); + let (sender, receiver) = oneshot::channel(); + assert!(&desc.clone() + .into_parsed_request() + .eq(&Ok(ParsedRequest::Sync( + SyncRequest::PutLogger(desc, sender), + receiver + )))); + } +} diff --git a/api_server/src/request/sync/mod.rs b/api_server/src/request/sync/mod.rs index d1ca0b3a784..6bf22d950db 100644 --- a/api_server/src/request/sync/mod.rs +++ b/api_server/src/request/sync/mod.rs @@ -140,6 +140,10 @@ mod tests { &SyncRequest::PutDrive(ref ddesc, _), &SyncRequest::PutDrive(ref other_ddesc, _), ) => ddesc == other_ddesc, + ( + &SyncRequest::PutLogger(ref logdesc, _), + &SyncRequest::PutLogger(ref other_logdesc, _), + ) => logdesc == other_logdesc, ( &SyncRequest::PutMachineConfiguration(ref mcb, _), &SyncRequest::PutMachineConfiguration(ref other_mcb, _), From 4c6bd25fc7902d66f50c29bc7fb77d9afb883e9b Mon Sep 17 00:00:00 2001 From: Diana Popa Date: Fri, 4 May 2018 06:44:37 -0500 Subject: [PATCH 5/5] vmm: unit testing the logger configuration... inside the vmm as an intermediate step in the logger setup triggered by the API. Signed-off-by: Diana Popa --- api_server/src/request/sync/logger.rs | 3 +- vmm/src/api_logger_config.rs | 130 ++++++++++++++++++++++++++ vmm/src/lib.rs | 4 +- 3 files changed, 133 insertions(+), 4 deletions(-) create mode 100644 vmm/src/api_logger_config.rs diff --git a/api_server/src/request/sync/logger.rs b/api_server/src/request/sync/logger.rs index b33031849af..68ba3ccd5ed 100644 --- a/api_server/src/request/sync/logger.rs +++ b/api_server/src/request/sync/logger.rs @@ -108,8 +108,7 @@ mod tests { #[test] fn test_into_parsed_request() { let desc = APILoggerDescription { - source_type: APILoggerSourceType::LocalPath, - source: String::from(""), + path: String::from(""), level: None, show_level: None, show_log_origin: None, diff --git a/vmm/src/api_logger_config.rs b/vmm/src/api_logger_config.rs new file mode 100644 index 00000000000..881a6d99e51 --- /dev/null +++ b/vmm/src/api_logger_config.rs @@ -0,0 +1,130 @@ +use std::result; + +use api_server::request::sync::{APILoggerDescription, APILoggerError, APILoggerLevel}; +use logger::{Level, Logger}; + +type Result = result::Result; + +pub fn init_logger(api_logger: APILoggerDescription) -> Result<()> { + //there are 3 things we need to get out: the level, whether to show it and whether to show the origin of the log + let mut logger = Logger::new(); + let level = from_api_level(api_logger.level); + + if let Some(val) = level { + logger.set_level(val); + } + + if let Some(val) = api_logger.show_log_origin { + logger.set_include_origin(val, val); + } + + if let Some(val) = api_logger.show_level { + logger.set_include_level(val); + } + + if let Err(ref e) = logger.init(Some(api_logger.path)) { + return Err(APILoggerError::InitializationFailure(e.to_string())); + } else { + Ok(()) + } +} + +fn from_api_level(api_level: Option) -> Option { + if let Some(val) = api_level { + match val { + APILoggerLevel::Error => Some(Level::Error), + APILoggerLevel::Warning => Some(Level::Warn), + APILoggerLevel::Info => Some(Level::Info), + APILoggerLevel::Debug => Some(Level::Debug), + } + } else { + None + } +} + +#[cfg(test)] +mod tests { + use super::*; + use api_server::request::sync::{APILoggerDescription, APILoggerLevel}; + use std::io::{BufRead, BufReader}; + use std::fs::{self, File}; + + fn validate_logs( + log_path: &str, + expected: &[(&'static str, &'static str, &'static str)], + ) -> bool { + let f = File::open(log_path).unwrap(); + let mut reader = BufReader::new(f); + let mut res = true; + let mut line = String::new(); + for tuple in expected { + line.clear(); + reader.read_line(&mut line).unwrap(); + res &= line.contains(&tuple.0); + res &= line.contains(&tuple.1); + res &= line.contains(&tuple.2); + } + res + } + + #[test] + fn test_init_logger_from_api() { + let desc = APILoggerDescription { + path: String::from(""), + level: None, + show_level: None, + show_log_origin: None, + }; + assert!(init_logger(desc).is_err()); + + let filename = "tmp.log"; + let desc = APILoggerDescription { + path: String::from(filename), + level: Some(APILoggerLevel::Warning), + show_level: Some(true), + show_log_origin: Some(true), + }; + let res = init_logger(desc).is_ok(); + + if !res { + let _x = fs::remove_file(filename); + } + + assert!(res); + + info!("info"); + warn!("warning"); + error!("error"); + + // info should not be outputted + let res = validate_logs( + filename, + &[ + ("[WARN", "logger_config.rs", "warn"), + ("[ERROR", "logger_config.rs", "error"), + ], + ); + let _x = fs::remove_file(filename); + assert!(res); + } + + #[test] + fn test_from_api_level() { + assert_eq!( + from_api_level(Some(APILoggerLevel::Error)), + Some(Level::Error) + ); + assert_eq!( + from_api_level(Some(APILoggerLevel::Warning)), + Some(Level::Warn) + ); + assert_eq!( + from_api_level(Some(APILoggerLevel::Info)), + Some(Level::Info) + ); + assert_eq!( + from_api_level(Some(APILoggerLevel::Debug)), + Some(Level::Debug) + ); + } +} diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs index d8ce41c3a88..d7db757f7e4 100644 --- a/vmm/src/lib.rs +++ b/vmm/src/lib.rs @@ -17,7 +17,7 @@ extern crate x86_64; pub mod device_config; pub mod device_manager; pub mod kernel_cmdline; -mod logger_config; +mod api_logger_config; mod vm_control; mod vstate; @@ -972,7 +972,7 @@ impl Vmm { } } SyncRequest::PutLogger(logger_description, sender) => { - match logger_config::init_logger(logger_description) { + match api_logger_config::init_logger(logger_description) { Ok(_) => sender .send(Box::new(PutLoggerOutcome::Initialized)) .map_err(|_| ())