search

rust-vmm / vmm-reference

A VMM implementation based of rust-vmm components
Apache License 2.0
146 stars 61 forks source link

Fix discordance in dependencies #199

Closed cecton closed 2 years ago

cecton commented 2 years ago

I found some discordance in the dependencies:

This makes the update impossible because vm-vcpu keeps 0.8.0 while kvm-bindings updates to 0.9.

Either kvm-bindings needs to be set on 0.8.0 (which is equivalent to ~0.8.0 or >=0.8.0 && <0.9.0) or either vm-vcpu needs to be set on >=0.8.0. [edit: spelling]

[0] [18:00:34] ~/r/v/s/vm-vcpu main > cargo update
    Updating git repository `https://github.com/rust-vmm/linux-loader.git`
    Updating crates.io index
    Updating git repository `https://github.com/rust-vmm/vm-device`
    Updating git repository `https://github.com/rust-vmm/vm-virtio.git`
    Updating git submodule `https://github.com/rust-vmm/rust-vmm-ci.git`
    Updating ansi_term v0.11.0 -> v0.12.1
    Updating bitflags v1.2.1 -> v1.3.2
    Updating cfg-if v0.1.10 -> v1.0.0
    Updating clap v2.33.3 -> v2.34.0
    Updating hermit-abi v0.1.17 -> v0.1.19
    Updating libc v0.2.91 -> v0.2.111
    Updating log v0.4.6 -> v0.4.14
    Updating unicode-width v0.1.8 -> v0.1.9
    Updating virtio-blk v0.1.0 (https://github.com/rust-vmm/vm-virtio.git#333eb784) -> #e4a42064
    Updating virtio-device v0.1.0 (https://github.com/rust-vmm/vm-virtio.git#333eb784) -> #e4a42064
    Updating virtio-queue v0.1.0 (https://github.com/rust-vmm/vm-virtio.git#333eb784) -> #e4a42064
      Adding vm-memory v0.7.0
      Adding vmm-sys-util v0.9.0
[0] [18:04:28] ~/r/v/s/vm-vcpu main > cargo check
   Compiling libc v0.2.111
    Checking bitflags v1.3.2
    Checking vmm-sys-util v0.9.0
    Checking vm-memory v0.6.0
    Checking vmm-sys-util v0.8.0
    Checking kvm-bindings v0.5.0
    Checking arch v0.1.0 (/home/cecile/repos/vmm-reference/src/arch)
    Checking kvm-ioctls v0.11.0
    Checking vm-vcpu-ref v0.1.0 (/home/cecile/repos/vmm-reference/src/vm-vcpu-ref)
    Checking vm-vcpu v0.1.0 (/home/cecile/repos/vmm-reference/src/vm-vcpu)
error[E0308]: mismatched types
   --> src/vm-vcpu/src/vm.rs:401:29
    |
401 |             .register_irqfd(&event, irq_number)
    |                             ^^^^^^ expected struct `vmm_sys_util::linux::eventfd::EventFd`, found `&EventFd`
    |
    = note: expected reference `&vmm_sys_util::linux::eventfd::EventFd`
               found reference `&&EventFd`

For more information about this error, try `rustc --explain E0308`.
error: could not compile `vm-vcpu` due to previous error
lauralt commented 2 years ago

Hi, @cecton! Your PR seems to update all the dependencies of vmm-reference to the latest version, and the build fails. I think there would be a few changes required to make the build pass, so if you just want to update vmm-sys-util, you should change to vmm-sys-util = ">=0.8.0" in all the Cargo.tomls that have this dependency, and after that just cargo update -p vmm-sys-util instead of cargo update. Otherwise we should fix all the build errors.

cecton commented 2 years ago

Ah there are more issues :disappointed: I wanted to use this repo's crates to avoid copying too much code. But if there are other things broken it might not help me much.

I guess I will fix all the dependencies first to be sure to be on the right versions of things.

Do you want to keep vmm-sys-util = ">=0.8.0" on kvm-bindings btw? It's often preferable to keep things working in the "semver way" (so 0.8.0 or ~0.8.0).

andreeaflorescu commented 2 years ago

Do you want to keep vmm-sys-util = ">=0.8.0" on kvm-bindings btw? It's often preferable to keep things working in the "semver way" (so 0.8.0 or ~0.8.0).

We can update that as well, but it's not going to help much with this PR because we would need to first publish a new version of kvm-bindings.

cecton commented 2 years ago

This PR is only for the option of fixing this on this side indeed. But if we fix it on kvm-bindings in patch then we can close this one.

andreeaflorescu commented 2 years ago

We can fix it here temporarily so we can move forward. And we can also have a PR in kvm-bindings to fix it there, but that typically takes longer because you need to follow the release process which is not 0-cost.

cecton commented 2 years ago

Ok I'll do that hmm... "Soon" :tm:

cecton commented 2 years ago

Here is already this one. I just reverted the change on Cargo.lock. It will still fix things on my side, though tbh I don't really need this to be merged. I can use a path and do the small changes locally for myself for now.

cecton commented 2 years ago

The commit 'e2d6be7' should contain at least 3 lines: title, blank line and a sign-off one. Please check: https://www.midori-global.com/blog/2018/04/02/git-50-72-rule.

Alright I guess I need to rebase and squash and sign my commit xD

lauralt commented 2 years ago

Should we though update the vmm-sys-util dependency in all crates?