Save/restore UART state to/from snapshots#5764
Merged
ilstam merged 4 commits intofirecracker-microvm:mainfrom Mar 31, 2026
Merged
Save/restore UART state to/from snapshots#5764ilstam merged 4 commits intofirecracker-microvm:mainfrom
ilstam merged 4 commits intofirecracker-microvm:mainfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5764 +/- ##
==========================================
+ Coverage 83.02% 83.04% +0.02%
==========================================
Files 276 276
Lines 29430 29466 +36
==========================================
+ Hits 24435 24471 +36
Misses 4995 4995
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Manciukic
reviewed
Mar 16, 2026
Contributor
Manciukic
left a comment
There was a problem hiding this comment.
let's add a changelog item as well for this as well
9082575 to
2e9fcf0
Compare
b14b444 to
089666d
Compare
089666d to
584bdc3
Compare
584bdc3 to
6658ebf
Compare
ShadowCurse
reviewed
Mar 31, 2026
Manciukic
reviewed
Mar 31, 2026
Contributor
Manciukic
left a comment
There was a problem hiding this comment.
just a couple of minor issues
The VMM does not save the UART state when taking a snapshot. Upon restoring it uses a weird hack in emulate_serial_init() where it enables RX (but no TX) interrupts after a snapshot restore. This hack apart form weird is also wrong as the guest driver might have never enabled RX interrupts in the first place. A comment in the same function acknowledges that the correct thing to do is save and restore the entire device state. Because we don't re-enable TX interrupts after a snapshot restore, the guest driver might hang if it relies on those TX interrupts as reported in [1]. Fix this by saving the UART state in snapshots and restoring it from them. Bump the SNAPSHOT_VERSION appropriately. [1] - firecracker-microvm#5730 Reported-by: EJ Ciramella <ejc3@meta.com> Signed-off-by: Ilias Stamatis <ilstam@amazon.com>
Add a simple unit test that injects a few bytes into the UART's FIFO, stores it's state, create a new device from the saved state and is able to read back the correct bytes. Signed-off-by: Ilias Stamatis <ilstam@amazon.com>
6658ebf to
d4678ac
Compare
Commit 7e619f4 ("devices: serial: Save/restore UART state to/from snapshots") fixed a bug where the guest driver would hang after a snapshot restore because Firecracker didn't properly restore the state of the UART device. Add a test_serial_active_tx_snapshot test case (which mostly duplicates the existing test_serial_after_snapshot). Start an unbounded transmission before taking the snapshot. This will saturate the TX buffer of the emulated UART and will make the guest driver enable TX interrupts. Without the fix, the test case fails because the driver is stuck waiting for a TX interrupt that will never be delivered after the snapshot is restored. Note that the serial.tx("") that sends a newline to flush the buffer in the original test case isn't needed anymore since sending Ctrl+C will achieve the same result. Signed-off-by: Ilias Stamatis <ilstam@amazon.com>
firecracker-microvm#5705 Signed-off-by: Ilias Stamatis <ilstam@amazon.com>
d4678ac to
5d75957
Compare
ShadowCurse
approved these changes
Mar 31, 2026
Manciukic
approved these changes
Mar 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The VMM does not save the UART state when taking a snapshot. Upon
restoring it uses a weird hack in emulate_serial_init() where it enables
RX (but no TX) interrupts after a snapshot restore. This hack apart form
weird is also wrong as the guest driver might have never enabled RX
interrupts in the first place. A comment in the same function
acknowledges that the correct thing to do is save and restore the entire
device state.
Because we don't re-enable TX interrupts after a snapshot restore, the
guest driver might hang if it relies on those TX interrupts as reported
in 1.
Fix this by saving the UART state in snapshots and restoring it from
them.
1 - #5730
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
tools/devtool checkbuild --allto verify that the PR passesbuild checks on all supported architectures.
tools/devtool checkstyleto verify that the PR passes theautomated style checks.
how they are solving the problem in a clear and encompassing way.
in the PR.
CHANGELOG.md.Runbook for Firecracker API changes.
integration tests.
TODO.rust-vmm.