Skip to content

Commit 5585532

Browse files
committed
fix: open files with read modifier
The commit 7ee43cc ("fix: Support creating file with open_file_nonblock") did modify the file opening utility function by adding `open` option, but it also removed the `read` option from it. This causes an error during metrics and logs file initialization code if the the file is a FIFO and there are no readers already reading from it. This is because `open` returns `ENXIO` when opening a FIFO write-only as described in the man page: ``` ENXIO O_NONBLOCK | O_WRONLY is set, the named file is a FIFO, and no process has the FIFO open for reading. ``` Fix is just a partial revert of the part that changed the file opening logic by re-introducing same `open_file_nonblock` as it was before but with added `create` flag. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
1 parent 37593df commit 5585532

4 files changed

Lines changed: 13 additions & 11 deletions

File tree

src/vmm/src/device_manager/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ use crate::devices::virtio::device::VirtioDevice;
3535
use crate::devices::virtio::transport::mmio::{IrqTrigger, MmioTransport};
3636
use crate::resources::VmResources;
3737
use crate::snapshot::Persist;
38-
use crate::utils::open_file_write_nonblock;
38+
use crate::utils::open_file_nonblock;
3939
use crate::vstate::bus::BusError;
4040
use crate::vstate::memory::GuestMemoryMmap;
4141
use crate::{EmulateSerialInitError, EventManager, Vm};
@@ -125,7 +125,7 @@ impl DeviceManager {
125125
output: Option<&PathBuf>,
126126
) -> Result<Arc<Mutex<SerialDevice>>, std::io::Error> {
127127
let (serial_in, serial_out) = match output {
128-
Some(path) => (None, open_file_write_nonblock(path).map(SerialOut::File)?),
128+
Some(path) => (None, open_file_nonblock(path).map(SerialOut::File)?),
129129
None => {
130130
Self::set_stdout_nonblocking();
131131

src/vmm/src/logger/logging.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use serde::{Deserialize, Deserializer, Serialize};
1313
use utils::time::LocalTime;
1414

1515
use super::metrics::{IncMetric, METRICS};
16-
use crate::utils::open_file_write_nonblock;
16+
use crate::utils::open_file_nonblock;
1717

1818
/// Default level filter for logger matching the swagger specification
1919
/// (`src/firecracker/swagger/firecracker.yaml`).
@@ -62,7 +62,7 @@ impl Logger {
6262
);
6363

6464
if let Some(log_path) = config.log_path {
65-
let file = open_file_write_nonblock(&log_path).map_err(LoggerUpdateError)?;
65+
let file = open_file_nonblock(&log_path).map_err(LoggerUpdateError)?;
6666

6767
guard.target = Some(file);
6868
};

src/vmm/src/utils/mod.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,16 @@ pub const fn align_down(addr: u64, align: u64) -> u64 {
7676
addr & !(align - 1)
7777
}
7878

79-
/// Create and open a File for writing to it.
80-
/// In case we open a FIFO, in order to not block the instance if nobody is consuming the message
81-
/// that is flushed to it, we are opening it with `O_NONBLOCK` flag.
82-
/// In this case, writing to a pipe will start failing when reaching 64K of unconsumed content.
83-
pub fn open_file_write_nonblock(path: &Path) -> Result<File, std::io::Error> {
79+
/// Create and open a file for both reading and writing to it with a O_NONBLOCK flag.
80+
/// In case we open a FIFO, we need all READ, WRITE and O_NONBLOCK in order to not block the process
81+
/// if nobody is consuming the message. Otherwise opening the FIFO with only WRITE and O_NONBLOCK
82+
/// will fail with ENXIO if there is no reader already attached to it.
83+
/// NOTE: writing to a pipe will start failing when reaching 64K of unconsumed content.
84+
pub fn open_file_nonblock(path: &Path) -> Result<File, std::io::Error> {
8485
OpenOptions::new()
8586
.custom_flags(O_NONBLOCK)
8687
.create(true)
88+
.read(true)
8789
.write(true)
8890
.open(path)
8991
}

src/vmm/src/vmm_config/metrics.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use std::path::PathBuf;
77
use serde::{Deserialize, Serialize};
88

99
use crate::logger::{FcLineWriter, METRICS};
10-
use crate::utils::open_file_write_nonblock;
10+
use crate::utils::open_file_nonblock;
1111

1212
/// Strongly typed structure used to describe the metrics system.
1313
#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
@@ -26,7 +26,7 @@ pub enum MetricsConfigError {
2626
/// Configures the metrics as described in `metrics_cfg`.
2727
pub fn init_metrics(metrics_cfg: MetricsConfig) -> Result<(), MetricsConfigError> {
2828
let writer = FcLineWriter::new(
29-
open_file_write_nonblock(&metrics_cfg.metrics_path)
29+
open_file_nonblock(&metrics_cfg.metrics_path)
3030
.map_err(|err| MetricsConfigError::InitializationFailure(err.to_string()))?,
3131
);
3232
METRICS

0 commit comments

Comments
 (0)