chore: move rust-vmm deps to workspace dependencies by hanbings · Pull Request #942 · rust-vmm/vhost-device

hanbings · 2026-03-03T15:16:34Z

Summary of the PR

This PR moves rust-vmm related dependencies to workspace dependencies, and alphabetically reorders some of the crate declarations.

Closes #906

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

All commits in this PR have Signed-Off-By trailers (with
git commit -s), and the commit message has max 60 characters for the
summary and max 75 characters for each description line.
All added/changed functionality has a corresponding unit/integration
test.
All added/changed public-facing functionality has entries in the "Upcoming
Release" section of CHANGELOG.md (if no such section exists, please create one).
Any newly added unsafe code is properly documented.

stefano-garzarella · 2026-03-03T16:07:18Z

We like to have all the information in the git history and usually PR description will not be part of that, so please avoid commit with empty description, and especially add the reason why we are doing that.

For example in this case one of the reason is to have a single place for the entire workspace where we can handle versions of rust-vmm dependencies.

stefano-garzarella

Should we cover also crates in staging/ ?

hanbings · 2026-03-03T17:58:26Z

We like to have all the information in the git history and usually PR description will not be part of that, so please avoid commit with empty description, and especially add the reason why we are doing that.

For example in this case one of the reason is to have a single place for the entire workspace where we can handle versions of rust-vmm dependencies.

That makes sense. Should I use rebase to update the commit message?

stefano-garzarella · 2026-03-04T09:27:24Z

We like to have all the information in the git history and usually PR description will not be part of that, so please avoid commit with empty description, and especially add the reason why we are doing that.
For example in this case one of the reason is to have a single place for the entire workspace where we can handle versions of rust-vmm dependencies.

That makes sense. Should I use rebase to update the commit message?

Yes, rebase and force push. Our policy is to avoid any fixup in a PR (e.g. no new commits to fix code added by commits in the same PR), so it's perfectly fine to rebase the PR, update commits (code or description/title) and force push.

hanbings · 2026-03-07T04:51:28Z

We like to have all the information in the git history and usually PR description will not be part of that, so please avoid commit with empty description, and especially add the reason why we are doing that.
For example in this case one of the reason is to have a single place for the entire workspace where we can handle versions of rust-vmm dependencies.

That makes sense. Should I use rebase to update the commit message?

Yes, rebase and force push. Our policy is to avoid any fixup in a PR (e.g. no new commits to fix code added by commits in the same PR), so it's perfectly fine to rebase the PR, update commits (code or description/title) and force push.

Thank you for the review. I have made the changes in the latest commit.

RuoqingHe

Code-wise looking good, it would be better if you could put the "sorting and formatting" changes into a separate commit, and use taplo to format it

hanbings · 2026-03-11T17:46:48Z

Code-wise looking good, it would be better if you could put the "sorting and formatting" changes into a separate commit, and use taplo to format it

Thank you for the review! I'll address the sorting/formatting changes with taplo in a separate follow-up PR.

hanbings · 2026-03-11T17:53:07Z

Should we cover also crates in staging/ ?

I've added workspace dependencies in staging/Cargo.toml as well since it has its own separate workspace. It feels a bit redundant to me, but consolidating staging/ into the root workspace would require CI changes too — not sure what the preferred approach is here.

stefano-garzarella · 2026-03-30T14:04:56Z

Code-wise looking good, it would be better if you could put the "sorting and formatting" changes into a separate commit, and use taplo to format it

Thank you for the review! I'll address the sorting/formatting changes with taplo in a separate follow-up PR.

I still see "sorting and formatting" changes in a single commit, can you split them in another commit as suggested by @RuoqingHe ?

Please also rebase this since there are conflicts.

stefano-garzarella · 2026-03-30T14:07:48Z

Should we cover also crates in staging/ ?

I've added workspace dependencies in staging/Cargo.toml as well since it has its own separate workspace. It feels a bit redundant to me, but consolidating staging/ into the root workspace would require CI changes too — not sure what the preferred approach is here.

Do you think it's redundant because we have a single crate there?
I see, but IMO is still good, since it will allow us to promote crates from staging to the main workspace without touching their Cargo.toml

hanbings · 2026-04-08T13:13:09Z

I still see "sorting and formatting" changes in a single commit, can you split them in another commit as suggested by @RuoqingHe ?

Please also rebase this since there are conflicts.

I've resolved the conflicts via rebase and will move the sorting changes into a separate PR.

Do you think it's redundant because we have a single crate there? I see, but IMO is still good, since it will allow us to promote crates from staging to the main workspace without touching their Cargo.toml

That makes sense. I've retained the change in the new commit.

RuoqingHe · 2026-04-16T03:38:48Z

+
 members = [
-  "vhost-device-video",
+    "vhost-device-video",


Suggested change

"vhost-device-video",

"vhost-device-video",

Thanks for your patiance on this work ❤️, one step away from there

It's my pleasure, and thank you for your review. : )

Move rust-vmm deps to workspace level for unified management - Migrate rust-vmm related dependencies (e.g. vhost, vm-memory) to workspace-level Cargo.toml for centralized version control - Sub-crates under the staging/ are still managed by the Cargo.toml within staging/, while also declaring the dependencies at the workspace level temp Signed-off-by: hanbings <hanbings@hanbings.io>

RuoqingHe

Looking good, thanks @hanbings

stefano-garzarella · 2026-04-20T07:24:25Z

+vhost-user-backend = { workspace = true }
+virtio-bindings = { workspace = true }
+virtio-queue = { workspace = true }
+virtio-vsock = { workspace = true }


Do we really need to move virtio-vsock in the workspace?

I'm not sure if it will be used by any device other than vhost-device-vsock.

stefano-garzarella · 2026-04-20T07:27:17Z

-virtio-queue = "0.17"
-vm-memory = "0.17.1"
-vmm-sys-util = "0.15"
+vhost = { workspace = true, features = ["vhost-user-backend"] }


the vhost-user-backend feature is already enabled in the workspace

stefano-garzarella · 2026-04-20T07:29:31Z

Please check the commit 7e81c68 description.

E.g. there a random temp, plus it seems there are a lot of repetitions.
Also, if possible, avoid the bullet list.

stefano-garzarella · 2026-04-20T07:30:45Z

+virtio-bindings = "0.2.5"
+virtio-queue = "0.17"
+vm-memory = "0.17.1"
+vmm-sys-util = "0.15"


It seems there is no a new line here, maybe it's not an issue, but if you can fix it, it will avoid new diff in the future when it will be edited by another editor.

hanbings requested review from dorindabassey, epilys, mz-pdm, stefano-garzarella, stsquad and vireshk as code owners March 3, 2026 15:16

stefano-garzarella reviewed Mar 3, 2026

View reviewed changes

Comment thread vhost-device-can/Cargo.toml Outdated

hanbings force-pushed the chore/deps branch from 13bcbde to ea0ae70 Compare March 7, 2026 04:48

RuoqingHe reviewed Mar 7, 2026

View reviewed changes

hanbings force-pushed the chore/deps branch 2 times, most recently from 1d904e0 to 3aa0c5a Compare March 11, 2026 17:24

hanbings requested a review from stefano-garzarella March 15, 2026 11:51

stefano-garzarella reviewed Mar 30, 2026

View reviewed changes

Comment thread staging/Cargo.toml Outdated

hanbings force-pushed the chore/deps branch 3 times, most recently from 0ba247e to fc5c8f2 Compare April 8, 2026 13:06

hanbings requested review from RuoqingHe and stefano-garzarella April 15, 2026 17:16

RuoqingHe reviewed Apr 16, 2026

View reviewed changes

hanbings force-pushed the chore/deps branch 2 times, most recently from 1258e6a to 3e42375 Compare April 19, 2026 18:19

hanbings force-pushed the chore/deps branch from 3e42375 to 7e81c68 Compare April 19, 2026 18:21

hanbings requested a review from RuoqingHe April 19, 2026 18:36

RuoqingHe approved these changes Apr 20, 2026

View reviewed changes

stefano-garzarella requested changes Apr 20, 2026

View reviewed changes

Conversation

hanbings commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of the PR

Requirements

Uh oh!

stefano-garzarella commented Mar 3, 2026

Uh oh!

stefano-garzarella left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hanbings commented Mar 3, 2026

Uh oh!

stefano-garzarella commented Mar 4, 2026

Uh oh!

hanbings commented Mar 7, 2026

Uh oh!

RuoqingHe left a comment

Choose a reason for hiding this comment

Uh oh!

hanbings commented Mar 11, 2026

Uh oh!

hanbings commented Mar 11, 2026

Uh oh!

stefano-garzarella commented Mar 30, 2026

Uh oh!

Uh oh!

stefano-garzarella commented Mar 30, 2026

Uh oh!

hanbings commented Apr 8, 2026

Uh oh!

RuoqingHe Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

hanbings Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

RuoqingHe left a comment

Choose a reason for hiding this comment

Uh oh!

stefano-garzarella Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

stefano-garzarella Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

stefano-garzarella Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

stefano-garzarella Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hanbings commented Mar 3, 2026 •

edited

Loading