Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ and this project adheres to

### Changed

- [#4028](https://github.com/firecracker-microvm/firecracker/pull/4028):
Firecracker now creates the log and metrics files if they do not exist,
simplifying the launch of Firecracker by removing a manual step.

### Deprecated

### Removed
Expand Down
3 changes: 0 additions & 3 deletions docs/getting-started.md
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,6 @@ sudo iptables -t nat -A POSTROUTING -o "$HOST_IFACE" -j MASQUERADE
API_SOCKET="/tmp/firecracker.socket"
LOGFILE="./firecracker.log"

# Create log file
touch $LOGFILE

# Set log file
sudo curl -X PUT --unix-socket "${API_SOCKET}" \
--data "{
Expand Down
11 changes: 2 additions & 9 deletions src/vmm/src/device_manager/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

use std::convert::Infallible;
use std::fmt::Debug;
use std::os::unix::prelude::OpenOptionsExt;
use std::path::PathBuf;
use std::sync::{Arc, Mutex};

Expand Down Expand Up @@ -36,6 +35,7 @@ use crate::devices::virtio::device::VirtioDevice;
use crate::devices::virtio::transport::mmio::{IrqTrigger, MmioTransport};
use crate::resources::VmResources;
use crate::snapshot::Persist;
use crate::utils::open_file_write_nonblock;
use crate::vstate::bus::BusError;
use crate::vstate::memory::GuestMemoryMmap;
use crate::{EmulateSerialInitError, EventManager, Vm};
Expand Down Expand Up @@ -127,14 +127,7 @@ impl DeviceManager {
output: Option<&PathBuf>,
) -> Result<Arc<Mutex<SerialDevice>>, std::io::Error> {
let (serial_in, serial_out) = match output {
Some(ref path) => (
None,
std::fs::OpenOptions::new()
.custom_flags(libc::O_NONBLOCK)
.write(true)
.open(path)
.map(SerialOut::File)?,
),
Some(path) => (None, open_file_write_nonblock(path).map(SerialOut::File)?),
None => {
Self::set_stdout_nonblocking();

Expand Down
8 changes: 2 additions & 6 deletions src/vmm/src/logger/logging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

use std::fmt::Debug;
use std::io::Write;
use std::os::unix::fs::OpenOptionsExt;
use std::path::PathBuf;
use std::str::FromStr;
use std::sync::{Mutex, OnceLock};
Expand All @@ -14,6 +13,7 @@ use serde::{Deserialize, Deserializer, Serialize};
use utils::time::LocalTime;

use super::metrics::{IncMetric, METRICS};
use crate::utils::open_file_write_nonblock;

/// Default level filter for logger matching the swagger specification
/// (`src/firecracker/swagger/firecracker.yaml`).
Expand Down Expand Up @@ -62,11 +62,7 @@ impl Logger {
);

if let Some(log_path) = config.log_path {
let file = std::fs::OpenOptions::new()
.custom_flags(libc::O_NONBLOCK)
.write(true)
.open(log_path)
.map_err(LoggerUpdateError)?;
let file = open_file_write_nonblock(&log_path).map_err(LoggerUpdateError)?;

guard.target = Some(file);
};
Expand Down
18 changes: 18 additions & 0 deletions src/vmm/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ pub mod signal;
/// Module with state machine
pub mod sm;

use std::fs::{File, OpenOptions};
use std::num::Wrapping;
use std::os::unix::fs::OpenOptionsExt;
use std::path::Path;
use std::result::Result;

use libc::O_NONBLOCK;

/// How many bits to left-shift by to convert MiB to bytes
const MIB_TO_BYTES_SHIFT: usize = 20;
Expand Down Expand Up @@ -64,3 +70,15 @@ pub const fn align_down(addr: u64, align: u64) -> u64 {
debug_assert!(align != 0);
addr & !(align - 1)
}

/// Create and open a File for writing to it.
/// In case we open a FIFO, in order to not block the instance if nobody is consuming the message
/// that is flushed to it, we are opening it with `O_NONBLOCK` flag.
/// In this case, writing to a pipe will start failing when reaching 64K of unconsumed content.
pub fn open_file_write_nonblock(path: &Path) -> Result<File, std::io::Error> {
OpenOptions::new()
.custom_flags(O_NONBLOCK)
.create(true)
.write(true)
.open(path)
}
10 changes: 2 additions & 8 deletions src/vmm/src/vmm_config/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use std::path::PathBuf;

use serde::{Deserialize, Serialize};

use super::open_file_nonblock;
use crate::logger::{FcLineWriter, METRICS};
use crate::utils::open_file_write_nonblock;

/// Strongly typed structure used to describe the metrics system.
#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
Expand All @@ -26,7 +26,7 @@ pub enum MetricsConfigError {
/// Configures the metrics as described in `metrics_cfg`.
pub fn init_metrics(metrics_cfg: MetricsConfig) -> Result<(), MetricsConfigError> {
let writer = FcLineWriter::new(
open_file_nonblock(&metrics_cfg.metrics_path)
open_file_write_nonblock(&metrics_cfg.metrics_path)
.map_err(|err| MetricsConfigError::InitializationFailure(err.to_string()))?,
);
METRICS
Expand All @@ -42,12 +42,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"),
};
init_metrics(desc).unwrap_err();

// Initializing metrics with valid pipe is ok.
let metrics_file = TempFile::new().unwrap();
let desc = MetricsConfig {
Expand Down
16 changes: 0 additions & 16 deletions src/vmm/src/vmm_config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,8 @@
// SPDX-License-Identifier: Apache-2.0

use std::convert::{From, TryInto};
use std::fs::{File, OpenOptions};
use std::io;
use std::os::unix::fs::OpenOptionsExt;
use std::path::Path;

use libc::O_NONBLOCK;
use serde::{Deserialize, Serialize};

use crate::rate_limiter::{BucketUpdate, RateLimiter, TokenBucket};
Expand Down Expand Up @@ -167,18 +163,6 @@ impl RateLimiterConfig {
}
}

/// Create and opens a File for writing to it.
/// In case we open a FIFO, in order to not block the instance if nobody is consuming the message
/// that is flushed to the two pipes, we are opening it with `O_NONBLOCK` flag.
/// In this case, writing to a pipe will start failing when reaching 64K of unconsumed content.
fn open_file_nonblock(path: &Path) -> Result<File, std::io::Error> {
OpenOptions::new()
.custom_flags(O_NONBLOCK)
.read(true)
.write(true)
.open(path)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
21 changes: 0 additions & 21 deletions tests/integration_tests/functional/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,27 +71,6 @@ def check_log_message_format(log_str, instance_id, level, show_level, show_origi
assert tag_level_no <= configured_level_no


def test_log_config_failure(uvm_plain):
"""
Check passing invalid FIFOs is detected and reported as an error.
"""
microvm = uvm_plain
microvm.spawn(log_file=None)
microvm.basic_config()

# only works if log level is Debug
microvm.time_api_requests = False

expected_msg = re.escape("No such file or directory (os error 2)")
with pytest.raises(RuntimeError, match=expected_msg):
microvm.api.logger.put(
log_path="invalid log file",
level="Info",
show_level=True,
show_log_origin=True,
)


def test_api_requests_logs(uvm_plain):
"""
Test that API requests are logged.
Expand Down