Skip to content

Conversation

@Manciukic
Copy link
Contributor

@Manciukic Manciukic commented Mar 12, 2025

Changes

This patchset upgrades the rust toolchain to 1.85.0 and the edition to 2024. Furthermore, the dev container is bumped to v79.
It looks like a big change but there's no functional changes. The biggest pieces are:

  • rustfmt has a different opinion in 2024, leading to many reformatted lines
  • gen is now a reserved keyword, so I've updated all generated files to use the generated/ prefix instead. I've also updated and rerun bindgen.sh.
  • unsafe now needs to be used explicitly inside unsafe functions
  • unnecessary ref in match have been removed
  • operator ordering needs to be explicit with parenthesis

Should I add a changelog entry? There's no user-visible change.

Reason

As part of the new release cycle, we're updating the rust toolchain and edition.

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 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.
  • NA? I have mentioned all user-facing changes in CHANGELOG.md.
  • NA If a specific issue led to this PR, this PR closes the issue.
  • NA 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.

@Manciukic Manciukic changed the title Upgrade Rust Toolchain to 1,85,0 and Edition to 2024 Upgrade Rust Toolchain to 1.85.0 and Edition to 2024 Mar 12, 2025
@codecov
Copy link

codecov bot commented Mar 12, 2025

Codecov Report

Attention: Patch coverage is 88.70968% with 14 lines in your changes missing coverage. Please review.

Project coverage is 83.15%. Comparing base (91cea5e) to head (a02e2bd).
Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
src/firecracker/src/main.rs 0.00% 3 Missing ⚠️
src/vmm/src/devices/virtio/iovec.rs 75.00% 2 Missing ⚠️
src/vmm/src/gdb/arch/aarch64.rs 0.00% 2 Missing ⚠️
src/vmm/src/vstate/vcpu/mod.rs 33.33% 2 Missing ⚠️
src/utils/src/arg_parser.rs 0.00% 1 Missing ⚠️
src/vmm/src/builder.rs 75.00% 1 Missing ⚠️
src/vmm/src/devices/virtio/vhost_user.rs 0.00% 1 Missing ⚠️
src/vmm/src/utils/signal.rs 50.00% 1 Missing ⚠️
src/vmm/src/vstate/memory.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5079   +/-   ##
=======================================
  Coverage   83.15%   83.15%           
=======================================
  Files         248      248           
  Lines       26896    26910   +14     
=======================================
+ Hits        22365    22377   +12     
- Misses       4531     4533    +2     
Flag Coverage Δ
5.10-c5n.metal 83.53% <92.37%> (+0.01%) ⬆️
5.10-m5n.metal 83.52% <92.37%> (+0.01%) ⬆️
5.10-m6a.metal 82.72% <91.52%> (+<0.01%) ⬆️
5.10-m6g.metal 79.60% <87.71%> (+<0.01%) ⬆️
5.10-m6i.metal 83.50% <91.52%> (+0.01%) ⬆️
5.10-m7g.metal 79.60% <87.71%> (+<0.01%) ⬆️
6.1-c5n.metal 83.57% <92.37%> (+0.01%) ⬆️
6.1-m5n.metal 83.55% <92.37%> (+<0.01%) ⬆️
6.1-m6a.metal 82.77% <91.52%> (+0.01%) ⬆️
6.1-m6g.metal 79.60% <87.71%> (+<0.01%) ⬆️
6.1-m6i.metal 83.55% <91.52%> (+0.01%) ⬆️
6.1-m7g.metal 79.60% <87.71%> (+<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.

@Manciukic
Copy link
Contributor Author

Kani is timing out but it's doing so also on the v78 devctr built yesterday, so it's doesn't seem to be caused by these changes.

@Manciukic Manciukic marked this pull request as ready for review March 13, 2025 12:27
@Manciukic Manciukic added the Status: Awaiting review Indicates that a pull request is ready to be reviewed label Mar 13, 2025
roypat
roypat previously approved these changes Mar 13, 2025
pb8o
pb8o previously approved these changes Mar 13, 2025
The section has been renamed and the old name has been deprecated.
This has no functional changes.

> The "poetry.dev-dependencies" section is deprecated and will
> be removed in a future version.
> Use "poetry.group.dev.dependencies" instead.

Signed-off-by: Riccardo Mancini <[email protected]>
I've run `poetry update --lock` inside `/tools/devctr`.

Signed-off-by: Riccardo Mancini <[email protected]>
This commit updates the rust toolchain in the devctr to 1.85.0.
Changes will take effect on the next devctr release.

Signed-off-by: Riccardo Mancini <[email protected]>
The clippy update now flags cases where operator precedence may lead to
unexpected results [1]. The fix is to explicitly wrap in parenthesis.

[1]: https://rust-lang.github.io/rust-clippy/master/index.html#precedence

Signed-off-by: Riccardo Mancini <[email protected]>
The rust toolchain version requires specifying which ABI the packed
representation refers to [1].
In all our cases, we want C ABI.

[1]: https://rust-lang.github.io/rust-clippy/master/index.html#repr_packed_without_abi

Signed-off-by: Riccardo Mancini <[email protected]>
The compiler complains that we're not consuming the output of `exec`.
Let's be explicit about ignoring it.

Signed-off-by: Riccardo Mancini <[email protected]>
Clippy complains that `map_or` can be simplified.
Replace it with `is_some_and`.

Signed-off-by: Riccardo Mancini <[email protected]>
The call is redundant as len() on a string returns the number of bytes
[1].

[1]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_as_bytes

Signed-off-by: Riccardo Mancini <[email protected]>
When applying fixes automatically, clippy doesn't know what to do in
this case. It's also objectively obscure.

Let's replace it with an explicit cast, specifying the pointer types,
and modernize the syntax to use `&raw mut`.

Signed-off-by: Riccardo Mancini <[email protected]>
The new version has rust toolchain v1.85.0

Signed-off-by: Riccardo Mancini <[email protected]>
Binding modifiers may only be written when the default binding mode
is `move`. For more information, see [1].

[1]: https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html

Signed-off-by: Riccardo Mancini <[email protected]>
In Rust 2024 these are considered unsafe operations, but
it's safe to edit them from a single threaded program.

Signed-off-by: Riccardo Mancini <[email protected]>
In Rust 2024 edition, it is required to mark which parts of an unsafe
functions are unsafe.

Signed-off-by: Riccardo Mancini <[email protected]>
In Rust 2024 edition, extern blocks must be marked unsafe.

Signed-off-by: Riccardo Mancini <[email protected]>
In Rust 2024 edition, gen is a keyword reserved for future use.
This change updates bindgen to change the destination of the generated
files.

I've tested this by rerunning bindgen.sh

The next change will switch all files to reference the new path.

Signed-off-by: Riccardo Mancini <[email protected]>
This change switches the generated files location from gen/ to
generated/, updating all references in the code.

Signed-off-by: Riccardo Mancini <[email protected]>
All issues have been fixed in the previous commits, so finally switch to
use 2024 edition.

Signed-off-by: Riccardo Mancini <[email protected]>
The new rust edition comes with different opinions on how code should be
formatted, mostly around ordering of dependencies.
Let's fix it.

Signed-off-by: Riccardo Mancini <[email protected]>
The re-formatting made clippy aware of this unneeded late
initialization. Let's fix it.

Signed-off-by: Riccardo Mancini <[email protected]>
@Manciukic
Copy link
Contributor Author

Manciukic commented Mar 13, 2025

The only change from the latest approved revision is to use &raw mut as suggested.

@roypat roypat enabled auto-merge (rebase) March 13, 2025 17:10
@roypat roypat merged commit 6cee4a8 into firecracker-microvm:main Mar 13, 2025
6 of 7 checks passed
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants