Skip to content

[Backport 1.14] Save fifos - #5707

Merged
ShadowCurse merged 3 commits into
firecracker-microvm:firecracker-v1.14from
ShadowCurse:save_fifos_1_14
Feb 24, 2026
Merged

[Backport 1.14] Save fifos#5707
ShadowCurse merged 3 commits into
firecracker-microvm:firecracker-v1.14from
ShadowCurse:save_fifos_1_14

Conversation

@ShadowCurse

@ShadowCurse ShadowCurse commented Feb 20, 2026

Copy link
Copy Markdown
Contributor

Changes

Backport of #5698

Reason

Fix

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.

PR Checklist

  • I have read and understand CONTRIBUTING.md.
  • I have run tools/devtool checkbuild --all to verify that the PR passes
    build checks on all supported architectures.
  • I have run tools/devtool checkstyle to verify that the PR passes the
    automated style checks.
  • I have described what is done in these changes, why they are needed, and
    how they are solving the problem in a clear and encompassing way.
  • I have updated any relevant documentation (both in code and in the docs)
    in the PR.
  • I have mentioned all user-facing changes in CHANGELOG.md.
  • If a specific issue led to this PR, this PR closes the issue.
  • When making API changes, I have followed the
    Runbook for Firecracker API changes.
  • I have tested all new and changed functionalities in unit tests and/or
    integration tests.
  • I have linked an issue to every new TODO.
  • This functionality cannot be added in rust-vmm.

@ShadowCurse ShadowCurse self-assigned this Feb 20, 2026
@ShadowCurse ShadowCurse added the Status: Awaiting review Indicates that a pull request is ready to be reviewed label Feb 20, 2026
@ShadowCurse ShadowCurse added the Type: Fix Indicates a fix to existing code label Feb 20, 2026
ilstam added a commit to ilstam/firecracker that referenced this pull request Feb 20, 2026
@ilstam
CHANGELOG: 1.15.0: Mention diff snapshot memory corruption bugfix fir…
1167489 
…ecracker-microvm#5707

Update v1.15.0 CHANGELOG mentioning the fix for firecracker-microvm#5707:
firecracker-microvm#5705

Signed-off-by: Ilias Stamatis <ilstam@amazon.com>
ilstam added a commit to ilstam/firecracker that referenced this pull request Feb 20, 2026
@ilstam
CHANGELOG: 1.15.0: Mention diff snapshot memory corruption bugfix
3671ee0 
Update v1.15.0 CHANGELOG mentioning the fix for firecracker-microvm#5707:
firecracker-microvm#5705

Signed-off-by: Ilias Stamatis <ilstam@amazon.com>
ilstam added a commit to ilstam/firecracker that referenced this pull request Feb 20, 2026
@ilstam
CHANGELOG: 1.15.0: Mention diff snapshot memory corruption bugfix
1e26eab 
Update v1.15.0 CHANGELOG mentioning the fix for firecracker-microvm#5707:
firecracker-microvm#5705

Signed-off-by: Ilias Stamatis <ilstam@amazon.com>
ilstam added a commit to ilstam/firecracker that referenced this pull request Feb 20, 2026
@ilstam
CHANGELOG: 1.15.0: Mention diff snapshot memory corruption bugfix
9d6d7bc 
Update v1.15.0 CHANGELOG mentioning the fix for firecracker-microvm#5707:
firecracker-microvm#5705

Signed-off-by: Ilias Stamatis <ilstam@amazon.com>
@codecov

codecov Bot commented Feb 20, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.37%. Comparing base (37593df) to head (4f86587).
⚠️ Report is 3 commits behind head on firecracker-v1.14.

Files with missing lines Patch % Lines
src/vmm/src/device_manager/mod.rs 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@                Coverage Diff                 @@
##           firecracker-v1.14    #5707   +/-   ##
==================================================
  Coverage              83.37%   83.37%           
==================================================
  Files                    277      277           
  Lines                  29277    29278    +1     
==================================================
+ Hits                   24410    24411    +1     
  Misses                  4867     4867
Flag Coverage Δ
5.10-m5n.metal 83.61% <83.33%> (+<0.01%) ⬆️
5.10-m6a.metal 82.93% <83.33%> (-0.01%) ⬇️
5.10-m6g.metal 80.35% <83.33%> (+<0.01%) ⬆️
5.10-m6i.metal 83.60% <83.33%> (-0.01%) ⬇️
5.10-m7a.metal-48xl 82.93% <83.33%> (+<0.01%) ⬆️
5.10-m7g.metal 80.35% <83.33%> (+<0.01%) ⬆️
5.10-m7i.metal-24xl 83.57% <83.33%> (-0.02%) ⬇️
5.10-m7i.metal-48xl 83.58% <83.33%> (+<0.01%) ⬆️
5.10-m8g.metal-24xl 80.35% <83.33%> (+<0.01%) ⬆️
5.10-m8g.metal-48xl 80.35% <83.33%> (+<0.01%) ⬆️
6.1-m5n.metal 83.62% <83.33%> (-0.01%) ⬇️
6.1-m6a.metal 82.97% <83.33%> (-0.01%) ⬇️
6.1-m6g.metal 80.35% <83.33%> (+<0.01%) ⬆️
6.1-m6i.metal 83.62% <83.33%> (-0.01%) ⬇️
6.1-m7a.metal-48xl 82.95% <83.33%> (+<0.01%) ⬆️
6.1-m7g.metal 80.35% <83.33%> (+<0.01%) ⬆️
6.1-m7i.metal-24xl 83.64% <83.33%> (-0.01%) ⬇️
6.1-m7i.metal-48xl 83.64% <83.33%> (-0.02%) ⬇️
6.1-m8g.metal-24xl 80.34% <83.33%> (+<0.01%) ⬆️
6.1-m8g.metal-48xl 80.35% <83.33%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

zulinx86
zulinx86 requested changes Feb 23, 2026
View reviewed changes

@zulinx86 zulinx86 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the first commit's message, the commit hash points to a commit that does not exist in upstream. That should point to 7ee43cc.

Some typo fixes mixed in the second commit (5460dfe), but those should be in the first commit.

Comment thread src/vmm/src/utils/mod.rs Outdated
@ShadowCurse
fix: open files with read modifier
0488464 
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>
@ShadowCurse
cleanup: remove redundant std::result::Result imports
606d479 
No functional change. Just cleanup.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@ShadowCurse
changelog: add note about FIFO fix
4f86587 
Add note about fix in the firecracker-microvm#5698 PR.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@Manciukic

Manciukic commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

kani timed out, retrying

@ShadowCurse
ShadowCurse merged commit 3a12ebf into firecracker-microvm:firecracker-v1.14 Feb 24, 2026
6 of 7 checks passed
@ShadowCurse
ShadowCurse deleted the save_fifos_1_14 branch February 24, 2026 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Manciukic Manciukic Manciukic approved these changes
@zulinx86 zulinx86 zulinx86 approved these changes
@xmarcalx xmarcalx Awaiting requested review from xmarcalx
@kalyazin kalyazin Awaiting requested review from kalyazin
@pb8o pb8o Awaiting requested review from pb8o pb8o is a code owner

Assignees

@ShadowCurse ShadowCurse

Labels

Status: Awaiting review Indicates that a pull request is ready to be reviewed Type: Fix Indicates a fix to existing code

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ShadowCurse @Manciukic @zulinx86