Skip to content

Conversation

@ShadowCurse
Copy link
Contributor

Changes

  • fix clippy warnings
  • replace u32 virtio device type with actual VirtioDeviceType enum
  • add a common id method to VirtioDevice trait
  • implement a common kick method in VirtioDevice trait that re-triggers queue eventfds instead of
    doing manual queue reprocessing

Reason

Maintenance

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 force-pushed the maintenance3 branch 3 times, most recently from ae25a1d to ed4d917 Compare December 4, 2025 15:11
@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

❌ Patch coverage is 71.17117% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.22%. Comparing base (5baa588) to head (9d651da).

Files with missing lines Patch % Lines
src/vmm/src/devices/virtio/device.rs 11.11% 16 Missing ⚠️
src/vmm/src/devices/virtio/balloon/device.rs 44.44% 5 Missing ⚠️
src/vmm/src/devices/virtio/vsock/device.rs 42.85% 4 Missing ⚠️
.../vmm/src/devices/virtio/block/vhost_user/device.rs 70.00% 3 Missing ⚠️
src/vmm/src/devices/virtio/pmem/device.rs 0.00% 3 Missing ⚠️
src/vmm/src/devices/virtio/block/device.rs 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5557      +/-   ##
==========================================
- Coverage   83.23%   83.22%   -0.02%     
==========================================
  Files         277      277              
  Lines       29265    29282      +17     
==========================================
+ Hits        24359    24370      +11     
- Misses       4906     4912       +6     
Flag Coverage Δ
5.10-m5n.metal 83.56% <71.17%> (-0.01%) ⬇️
5.10-m6a.metal 82.89% <71.17%> (-0.01%) ⬇️
5.10-m6g.metal 80.17% <71.17%> (-0.02%) ⬇️
5.10-m6i.metal 83.56% <71.17%> (-0.01%) ⬇️
5.10-m7a.metal-48xl 82.89% <71.17%> (-0.01%) ⬇️
5.10-m7g.metal 80.17% <71.17%> (-0.02%) ⬇️
5.10-m7i.metal-24xl 83.54% <71.17%> (-0.01%) ⬇️
5.10-m7i.metal-48xl 83.53% <71.17%> (-0.02%) ⬇️
5.10-m8g.metal-24xl 80.16% <71.17%> (-0.02%) ⬇️
5.10-m8g.metal-48xl 80.17% <71.17%> (-0.02%) ⬇️
6.1-m5n.metal 83.59% <71.17%> (-0.01%) ⬇️
6.1-m6a.metal 82.93% <71.17%> (-0.01%) ⬇️
6.1-m6g.metal 80.16% <71.17%> (-0.03%) ⬇️
6.1-m6i.metal 83.59% <71.17%> (-0.01%) ⬇️
6.1-m7a.metal-48xl 82.91% <71.17%> (-0.01%) ⬇️
6.1-m7g.metal 80.16% <71.17%> (-0.03%) ⬇️
6.1-m7i.metal-24xl 83.60% <71.17%> (-0.01%) ⬇️
6.1-m7i.metal-48xl 83.60% <71.17%> (-0.02%) ⬇️
6.1-m8g.metal-24xl 80.16% <71.17%> (-0.03%) ⬇️
6.1-m8g.metal-48xl 80.16% <71.17%> (-0.02%) ⬇️

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.

@ShadowCurse ShadowCurse force-pushed the maintenance3 branch 3 times, most recently from ba3cd70 to 694289e Compare December 4, 2025 16:02
@ShadowCurse ShadowCurse self-assigned this Dec 4, 2025
@ShadowCurse ShadowCurse added Status: Awaiting review Indicates that a pull request is ready to be reviewed Type: Enhancement Indicates new feature requests labels Dec 4, 2025
@ShadowCurse ShadowCurse marked this pull request as ready for review December 4, 2025 17:02
@ShadowCurse ShadowCurse force-pushed the maintenance3 branch 2 times, most recently from a0966ff to 6587ebb Compare December 8, 2025 14:13
@ShadowCurse ShadowCurse force-pushed the maintenance3 branch 3 times, most recently from c15a161 to e2e15fe Compare December 9, 2025 12:30
/// Represent it as u8 to give it a known size.
/// All used types fit in u8.
#[allow(clippy::cast_possible_truncation)]
#[repr(u8)]
Copy link
Contributor

Choose a reason for hiding this comment

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

QEMU uses u16 and there's actually already almost 40 of them!
https://elixir.bootlin.com/qemu/v7.2.10/source/include/hw/virtio/virtio.h#L113

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just selected the smallest type because it can hold 256 values which is over the top for what we need. This is also correlated to the fact that sometimes we need to convert this type to u16 or u32, so u8 (o u16) will work.

Copy link
Contributor

Choose a reason for hiding this comment

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

can we do u16 for consistency then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer to keep it u8 as this is the most flexible type in case we will do some other change that would benefit from having virtio type as u8.

Replace fmt.toml with new rustfmt.toml

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Fix new clippy warnings

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Instead of using `u32` values directly from generated `virtio_ids`
create an enum backed by u8 with all types we actually use

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Add a trait method to VirtioDevice to get a unique device id.
This can be used in default imps of some other methods to
print current device id in case of an error/warning etc.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Replace different `kick` implementations with a default implementation
in the `VirtioDevice` trait.
Change the `kick` logic to just re-trigger queue events instead of
manually calling different internal queue processing functions.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Update names for remaining BK pipelines. This
is needed because BK labels are used as a part of
the s3 path where the BK artifacts are uploaded.
Removing emojis and normalizing names simplifies
these paths.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
These variables are no longer needed for A/B since
A/B scripts use `metrics.json` files which are always
emitted, instead of `test-report.json`.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Fix incorrect instructions about running `tools/ab_test.py`
with manually collected test results.
Move A/B visualization section after manual test collection
section as `tools/ab_plot.py` script is using same inputs as
`tools/ab_test.py`

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
There is no need for build directory or ci artifacts to be present
to run style checks.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Awaiting review Indicates that a pull request is ready to be reviewed Type: Enhancement Indicates new feature requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants