[1.12 Backport] Fix greedy block device #5277
Merged
Conversation
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
The `max_size` field is public, so no need for a getter. Signed-off-by: Egor Lazarchuk <[email protected]>
The size of queue set by the driver must be always less or equal to the queue size in FC. This is checked before device activation. This removes the need for `actual_size` function. Signed-off-by: Egor Lazarchuk <[email protected]>
Currently block device has a guest notification logic inside it's request processing loop. This can create a situation when guest can continuously add more requests to the queue, making the whole request processing loop arbitrary long. This is an issue, since it block any other IO from being processed. The solution is to simply notify guest one time, after all current requests are processed. Signed-off-by: Egor Lazarchuk <[email protected]> Signed-off-by: Riccardo Mancini <[email protected]>
VIRTIO spec states: ``` After the device writes a descriptor index into the used ring: If the idx field in the used ring (which determined where that descriptor index was placed) was equal to used_event, the device MUST send a notification. ``` The current implementation does not follow this very precisely. It bumps used ring index when new descriptors are added to the used ring. But the check if the notification is needed is postponed to later processing stage. To be more VIRTIO spec compliant simply move the logic for updating the used ring index into the later processing stage as well, just before the check if the notification should be send. Signed-off-by: Egor Lazarchuk <[email protected]>
ShadowCurse
requested review from
xmarcalx,
kalyazin,
pb8o and
Manciukic
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## firecracker-v1.12 #5277 +/- ##
=====================================================
- Coverage 83.09% 83.07% -0.02%
=====================================================
Files 250 250
Lines 26892 26893 +1
=====================================================
- Hits 22346 22342 -4
- Misses 4546 4551 +5
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:
|
roypat
previously approved these changes
Jun 23, 2025
bchalios
previously approved these changes
Jun 23, 2025
ShadowCurse
added
Status: Awaiting review
kalyazin
reviewed
Jun 23, 2025
Add a new changelog entry for the block device fairness fix. Signed-off-by: Riccardo Mancini <[email protected]>
ShadowCurse
force-pushed
the
fair_devices_1.12_backport
branch
from
5b1d5ac to
73dc513
Compare
kalyazin
approved these changes
Jun 23, 2025
roypat
approved these changes
Jun 23, 2025
ShadowCurse
merged commit b12cb30
into
firecracker-microvm:firecracker-v1.12
5 of 7 checks passed
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.
Changes
Backport of #5260 to 1.12 branch
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 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.