From a784d581af03e6c34f1ad68e5f1288b4bdecfe0b Mon Sep 17 00:00:00 2001
From: Jonathan Woollett-Light <jcawl@amazon.co.uk>
Date: Sun, 13 Aug 2023 20:29:15 +0100
Subject: [PATCH] fix: Support creating file with `open_file_nonblock`

Updates the `OpenOptions` with `.create(true)` to support creating a
file when if it doesn't exist. This also brings the functionality
inline with the documentation which notes
`Create and opens a File for writing to it.`. This improves usability
simplifying setting up the logger.

Signed-off-by: Jonathan Woollett-Light <jcawl@amazon.co.uk>
---
 docs/getting-started.md                       |  3 ---
 src/vmm/src/vmm_config/logger.rs              |  9 ---------
 src/vmm/src/vmm_config/metrics.rs             |  6 ------
 src/vmm/src/vmm_config/mod.rs                 |  1 +
 .../functional/test_logging.py                | 20 -------------------
 5 files changed, 1 insertion(+), 38 deletions(-)

diff --git a/docs/getting-started.md b/docs/getting-started.md
index e0b4a021a46..a746615ad64 100644
--- a/docs/getting-started.md
+++ b/docs/getting-started.md
@@ -134,9 +134,6 @@ sudo iptables -I FORWARD 1 -i tap0 -o eth0 -j ACCEPT
 API_SOCKET="/tmp/firecracker.socket"
 LOGFILE="./firecracker.log"
 
-# Create log file
-touch $LOGFILE
-
 # Set log file
 curl -X PUT --unix-socket "${API_SOCKET}" \
     --data "{
diff --git a/src/vmm/src/vmm_config/logger.rs b/src/vmm/src/vmm_config/logger.rs
index 4c108c14bf7..f773fbbb590 100644
--- a/src/vmm/src/vmm_config/logger.rs
+++ b/src/vmm/src/vmm_config/logger.rs
@@ -152,15 +152,6 @@ mod tests {
     fn test_init_logger() {
         let default_instance_info = InstanceInfo::default();
 
-        // Error case: initializing logger with invalid pipe returns error.
-        let desc = LoggerConfig {
-            log_path: PathBuf::from("not_found_file_log"),
-            level: LoggerLevel::Debug,
-            show_level: false,
-            show_log_origin: false,
-        };
-        assert!(init_logger(desc, &default_instance_info).is_err());
-
         // Initializing logger with valid pipe is ok.
         let log_file = TempFile::new().unwrap();
         let desc = LoggerConfig {
diff --git a/src/vmm/src/vmm_config/metrics.rs b/src/vmm/src/vmm_config/metrics.rs
index 4ba6d21089f..c7314873e6a 100644
--- a/src/vmm/src/vmm_config/metrics.rs
+++ b/src/vmm/src/vmm_config/metrics.rs
@@ -43,12 +43,6 @@ mod tests {
 
     #[test]
     fn test_init_metrics() {
-        // Error case: initializing metrics with invalid pipe returns error.
-        let desc = MetricsConfig {
-            metrics_path: PathBuf::from("not_found_file_metrics"),
-        };
-        assert!(init_metrics(desc).is_err());
-
         // Initializing metrics with valid pipe is ok.
         let metrics_file = TempFile::new().unwrap();
         let desc = MetricsConfig {
diff --git a/src/vmm/src/vmm_config/mod.rs b/src/vmm/src/vmm_config/mod.rs
index 74e9b78baea..79c8311046f 100644
--- a/src/vmm/src/vmm_config/mod.rs
+++ b/src/vmm/src/vmm_config/mod.rs
@@ -172,6 +172,7 @@ impl RateLimiterConfig {
 fn open_file_nonblock(path: &Path) -> Result<File, std::io::Error> {
     OpenOptions::new()
         .custom_flags(O_NONBLOCK)
+        .create(true)
         .read(true)
         .write(true)
         .open(path)
diff --git a/tests/integration_tests/functional/test_logging.py b/tests/integration_tests/functional/test_logging.py
index 520c44361f0..2f11958127b 100644
--- a/tests/integration_tests/functional/test_logging.py
+++ b/tests/integration_tests/functional/test_logging.py
@@ -112,26 +112,6 @@ def test_error_logs(test_microvm_with_api):
     _test_log_config(microvm=test_microvm_with_api, log_level="Error")
 
 
-def test_log_config_failure(test_microvm_with_api):
-    """
-    Check passing invalid FIFOs is detected and reported as an error.
-    """
-    microvm = test_microvm_with_api
-    microvm.spawn(create_logger=False)
-    microvm.basic_config()
-
-    response = microvm.logger.put(
-        log_path="invalid log fifo",
-        level="Info",
-        show_level=True,
-        show_log_origin=True,
-    )
-    # only works if log level is Debug
-    microvm.time_api_requests = False
-    assert microvm.api_session.is_status_bad_request(response.status_code)
-    assert response.json()["fault_message"]
-
-
 def test_api_requests_logs(test_microvm_with_api):
     """
     Test that API requests are logged.