Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump rust-vmm-ci manually to use rustvmm/dev:v28 #493

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

stefano-garzarella
Copy link
Member

Summary of the PR

This new version contains alsa and pipewire libraries to build vhost-device-sound audio backends.

Requirements

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

  • All commits in this PR are signed (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
Copy link
Member Author

@dorindabassey @epilys @MatiasVara CI now partially covers vhost-device-sound, but we still have some issues:

Can you take a look?

@epilys
Copy link
Member

epilys commented Oct 23, 2023

I see various different errors:

  • pkg-config has not been configured to support cross-compilation should that change in the CI image?

  • audio_backends::pipewire::tests::test_pipewire_backend_success panics

  • clippy: error: unsafe block missing a safety comment: vhost-device-sound/src/audio_backends/pipewire.rs:78:27

  • clippy: error: unsafe block missing a safety comment: vhost-device-sound/src/audio_backends/pipewire.rs:340:33

  • test errors: (does it expect a pipewire server running?)

    test audio_backends::pipewire::tests::test_pipewire_backend_panics - should panic ... FAILED
    test audio_backends::pipewire::tests::test_pipewire_backend_success ... FAILED
    failures:
    ---- audio_backends::pipewire::tests::test_pipewire_backend_panics stdout ----
    thread 'audio_backends::pipewire::tests::test_pipewire_backend_panics' panicked at 'Failed to connect to core: CreationFailed', vhost-device-sound/src/audio_backends/pipewire.rs:84:42
    note: panic did not contain expected string
          panic message: `"Failed to connect to core: CreationFailed"`,
     expected substring: `"Stream does not exist"`
    ---- audio_backends::pipewire::tests::test_pipewire_backend_success stdout ----
    thread 'audio_backends::pipewire::tests::test_pipewire_backend_success' panicked at 'Failed to connect to core: CreationFailed', vhost-device-sound/src/audio_backends/pipewire.rs:84:42
    

@dorindabassey
Copy link
Contributor

@dorindabassey @epilys @MatiasVara CI now partially covers vhost-device-sound, but we still have some issues:

* `alsa-sys v0.3.1` doesn't build on `musl`
  https://buildkite.com/rust-vmm/vhost-device-ci/builds/1827#018b5b65-79ba-4499-91b7-fcd22ece063e

* two pipewire's unit tests fails (maybe because we need to start pipewire in the container)
  https://buildkite.com/rust-vmm/vhost-device-ci/builds/1827#018b5b65-79c6-4555-9c45-dc582a60274f

* clippy suggests adding safety comments in the pipewire backend
  https://buildkite.com/rust-vmm/vhost-device-ci/builds/1827#018b5b65-79d5-4489-b4e6-40b1b6f676ef

Can you take a look?

I'm looking into this

@stefano-garzarella
Copy link
Member Author

@dorindabassey @epilys @MatiasVara CI now partially covers vhost-device-sound, but we still have some issues:

* `alsa-sys v0.3.1` doesn't build on `musl`
  https://buildkite.com/rust-vmm/vhost-device-ci/builds/1827#018b5b65-79ba-4499-91b7-fcd22ece063e

* two pipewire's unit tests fails (maybe because we need to start pipewire in the container)
  https://buildkite.com/rust-vmm/vhost-device-ci/builds/1827#018b5b65-79c6-4555-9c45-dc582a60274f

These are still open

* clippy suggests adding safety comments in the pipewire backend
  https://buildkite.com/rust-vmm/vhost-device-ci/builds/1827#018b5b65-79d5-4489-b4e6-40b1b6f676ef

This was fixed by #495 (thanks @dorindabassey)

@stefano-garzarella
Copy link
Member Author

@vireshk @stsquad I think we can merge this PR while we fix the other issues, since I guess the remaining ones need changes in rust-vmm-container

@stefano-garzarella
Copy link
Member Author

It looks like we introduced a little issue in the tests with #487 : https://buildkite.com/rust-vmm/vhost-device-ci/builds/1842#018b6131-868d-4f04-a3bb-c7f78243fb78

@epilys can you take a look?

@epilys
Copy link
Member

epilys commented Oct 24, 2023

@stefano-garzarella pushed a fix in a PR

@epilys epilys mentioned this pull request Oct 24, 2023
This new version contains alsa and pipewire libraries to build
vhost-device-sound audio backends.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
@stefano-garzarella
Copy link
Member Author

Rebased, since #496 is now merged.

@vireshk vireshk enabled auto-merge (rebase) October 25, 2023 07:38
@vireshk vireshk disabled auto-merge October 26, 2023 09:37
@vireshk
Copy link
Collaborator

vireshk commented Oct 26, 2023

Since 3 people have reviewed it already (two non-owners), I am going to force push it anyway.

@vireshk vireshk merged commit b2e3f0b into rust-vmm:main Oct 26, 2023
2 checks passed
@stefano-garzarella
Copy link
Member Author

* `pkg-config has not been configured to support cross-compilation` should that change in the CI image?

@epilys do you know how to fix this?

After merging rust-vmm/rust-vmm-container#92 that should be the only thing left to have the CI all green in staging.

@epilys
Copy link
Member

epilys commented Nov 1, 2023

@stefano-garzarella Looking into it now, hopefully it will only need some debian packages...

@epilys
Copy link
Member

epilys commented Nov 1, 2023

Setting export PKG_CONFIG_ALLOW_CROSS=1 was enough to get through that, but:

  1. I can't build pipewire with musl, it complains stdbool.h is missing. I tried installing musl-dev and setting the library path to musl's dir in .cargo/config.toml:

    [target.x86_64-unknown-linux-musl]
    linker = "ld"
    rustflags = ["-Ctarget-feature=-crt-static", "-Clink-self-contained=on", "-L/usr/lib/x86_64-linux-musl", "-Clink-args=--dynamic-linker /lib/ld-musl-x86_64.so.1"]

    but no dice. Any ideas?

  2. Disabling default features with --no-default-features gives me errors from rust-vmm crates:

Compilation errors..

   Compiling vhost-user-backend v0.10.1
error[E0277]: the trait bound `<M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory` is not satisfied
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:145:23
    |
145 |             .add_used(self.mem.memory().deref(), desc_index, len)
    |              -------- ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `vm_memory::guest_memory::GuestMemory` is not implemented for `<M as GuestAddressSpace>::M`
    |              |
    |              required by a bound introduced by this call
    |
note: required by a bound in `add_used`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/lib.rs:193:20
    |
193 |     fn add_used<M: GuestMemory>(&mut self, mem: &M, head_index: u16, len: u32)
    |                    ^^^^^^^^^^^ required by this bound in `QueueT::add_used`
help: consider further restricting the associated type
    |
143 |     pub fn add_used(&mut self, desc_index: u16, len: u32) -> Result<(), VirtQueError> where <M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory {
    |                                                                                       +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

error[E0277]: the trait bound `<M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory` is not satisfied
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:159:40
    |
159 |         self.queue.enable_notification(self.mem.memory().deref())
    |                    ------------------- ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `vm_memory::guest_memory::GuestMemory` is not implemented for `<M as GuestAddressSpace>::M`
    |                    |
    |                    required by a bound introduced by this call
    |
note: required by a bound in `enable_notification`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/lib.rs:201:31
    |
201 |     fn enable_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<bool, Error>;
    |                               ^^^^^^^^^^^ required by this bound in `QueueT::enable_notification`
help: consider further restricting the associated type
    |
158 |     pub fn enable_notification(&mut self) -> Result<bool, VirtQueError> where <M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory {
    |                                                                         +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

error[E0277]: the trait bound `<M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory` is not satisfied
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:164:41
    |
164 |         self.queue.disable_notification(self.mem.memory().deref())
    |                    -------------------- ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `vm_memory::guest_memory::GuestMemory` is not implemented for `<M as GuestAddressSpace>::M`
    |                    |
    |                    required by a bound introduced by this call
    |
note: required by a bound in `disable_notification`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/lib.rs:204:32
    |
204 |     fn disable_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<(), Error>;
    |                                ^^^^^^^^^^^ required by this bound in `QueueT::disable_notification`
help: consider further restricting the associated type
    |
163 |     pub fn disable_notification(&mut self) -> Result<(), VirtQueError> where <M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory {
    |                                                                        +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

error[E0277]: the trait bound `<M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory` is not satisfied
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:169:39
    |
169 |         self.queue.needs_notification(self.mem.memory().deref())
    |                    ------------------ ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `vm_memory::guest_memory::GuestMemory` is not implemented for `<M as GuestAddressSpace>::M`
    |                    |
    |                    required by a bound introduced by this call
    |
note: required by a bound in `needs_notification`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/lib.rs:212:30
    |
212 |     fn needs_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<bool, Error>;
    |                              ^^^^^^^^^^^ required by this bound in `QueueT::needs_notification`
help: consider further restricting the associated type
    |
168 |     pub fn needs_notification(&mut self) -> Result<bool, VirtQueError> where <M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory {
    |                                                                        +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

error[E0308]: mismatched types
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:185:41
    |
185 |             .try_set_desc_table_address(GuestAddress(desc_table))?;
    |              -------------------------- ^^^^^^^^^^^^^^^^^^^^^^^^ expected `GuestAddress`, found a different `GuestAddress`
    |              |
    |              arguments to this method are incorrect
    |
    = note: `GuestAddress` and `GuestAddress` have similar names, but are actually distinct types
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.12.2/src/guest_memory.rs:109:1
    |
109 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.13.1/src/guest_memory.rs:111:1
    |
111 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `vm_memory` are being used?
note: method defined here
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/queue.rs:132:12
    |
132 |     pub fn try_set_desc_table_address(&mut self, desc_table: GuestAddress) -> Result<(), Error> {
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0308]: mismatched types
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:187:41
    |
187 |             .try_set_avail_ring_address(GuestAddress(avail_ring))?;
    |              -------------------------- ^^^^^^^^^^^^^^^^^^^^^^^^ expected `GuestAddress`, found a different `GuestAddress`
    |              |
    |              arguments to this method are incorrect
    |
    = note: `GuestAddress` and `GuestAddress` have similar names, but are actually distinct types
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.12.2/src/guest_memory.rs:109:1
    |
109 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.13.1/src/guest_memory.rs:111:1
    |
111 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `vm_memory` are being used?
note: method defined here
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/queue.rs:147:12
    |
147 |     pub fn try_set_avail_ring_address(&mut self, avail_ring: GuestAddress) -> Result<(), Error> {
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0308]: mismatched types
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:189:40
    |
189 |             .try_set_used_ring_address(GuestAddress(used_ring))
    |              ------------------------- ^^^^^^^^^^^^^^^^^^^^^^^ expected `GuestAddress`, found a different `GuestAddress`
    |              |
    |              arguments to this method are incorrect
    |
    = note: `GuestAddress` and `GuestAddress` have similar names, but are actually distinct types
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.12.2/src/guest_memory.rs:109:1
    |
109 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.13.1/src/guest_memory.rs:111:1
    |
111 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `vm_memory` are being used?
note: method defined here
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/queue.rs:161:12
    |
161 |     pub fn try_set_used_ring_address(&mut self, used_ring: GuestAddress) -> Result<(), Error> {
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0277]: the trait bound `<M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory` is not satisfied
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:210:23
    |
210 |             .used_idx(self.mem.memory().deref(), Ordering::Relaxed)
    |              -------- ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `vm_memory::guest_memory::GuestMemory` is not implemented for `<M as GuestAddressSpace>::M`
    |              |
    |              required by a bound introduced by this call
    |
note: required by a bound in `used_idx`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/lib.rs:190:20
    |
190 |     fn used_idx<M: GuestMemory>(&self, mem: &M, order: Ordering) -> Result<Wrapping<u16>, Error>;
    |                    ^^^^^^^^^^^ required by this bound in `QueueT::used_idx`
help: consider further restricting the associated type
    |
208 |     fn queue_used_idx(&self) -> Result<u16, VirtQueError> where <M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory {
    |                                                           +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Some errors have detailed explanations: E0277, E0308.
For more information about an error, try `rustc --explain E0277`.
error: could not compile `vhost-user-backend` (lib) due to 8 previous errors

@stefano-garzarella
Copy link
Member Author

Setting export PKG_CONFIG_ALLOW_CROSS=1 was enough to get through that, but:

1. I can't build pipewire with musl, it complains `stdbool.h` is missing. I tried installing `musl-dev` and setting the library path to musl's dir in `.cargo/config.toml`:
   ```toml
   [target.x86_64-unknown-linux-musl]
   linker = "ld"
   rustflags = ["-Ctarget-feature=-crt-static", "-Clink-self-contained=on", "-L/usr/lib/x86_64-linux-musl", "-Clink-args=--dynamic-linker /lib/ld-musl-x86_64.so.1"]
   ```

   but no dice. Any ideas?

Nope :-(

2. Disabling default features with `--no-default-features` gives me errors from `rust-vmm` crates:

Compilation errors..

Strange.

What about moving this discussion to a new issue?
I'll try to help if I can find some time ;-)

@epilys
Copy link
Member

epilys commented Nov 14, 2023

Tracked here rust-vmm/rust-vmm-ci#146

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants