In order to allow zero-cost exchanging of the concrete bitmap and vring
types, a lot of the generic code required using a tuple of
`<VringType, BitmapType>` for parameterizing the impl's. Once code is
also oblivious of the concrete backend (and a lot of code is), this
tuple turns into a triplet. Juggling these three single letter generic
parameters while making sure that all the type constraints (that are
also changing depending on the abstraction layer) does not feel very
ergonomic.
While one can argue that within this crate, this would be fine since
people probably know the internals, the design choice is leaking out
into every consumer of vhost-user-backend. Instead of just being able
to reference "some backend" one needs to copy a lot of boilerplate code
for also passing the other type parameters (again, while making sure
that all the constraints are met).
Instead, this commit changes things to utilize associated types [1].
This makes the Bitmap and Vring types part of the backend trait and
requires the implementations to spell them out. Code that just wants to
use the backend without needing to know the details can now just use
the trait without needing to specify the Bitmap and Vring types again.
Where needed, restricting Bitmap and Vring further is still possible
(though one no longer needs to copy all the existing restrictions and
can keep the code more maintainable by only listing new ones).
Overall, my main target was to improve the ergonomics of the consumers
of the crate. But I think the change also improves the readability and
maintainability within this crate. Combined, this hopefully justifies
the small code breakage in consumers.
No functional changes intended.
No change in type flexibility intended.
BREAKING CHANGE, consumers of the lib will need to adjust their code
(but it should improve the general readability).
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
[1] https://doc.rust-lang.org/book/ch19-03-advanced-traits.html#specifying-placeholder-types-in-trait-definitions-with-associated-types
The return value of VhostUserBackend::handle_event() is
undocumented and difficult to interpret.
The current implementation used it to interrupt the event
loop as it does when we receive an exit event.
All current implementations checked (rust-vmm/vhost-device,
virtiofsd) return an error or always false, effectively not
using this feature.
Since we already have a mechanism for breaking the event loop,
we can avoid this ambiguous and redundant feature.
Closes#144
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Updates the requirements on [nix](https://github.com/nix-rust/nix) to permit the latest version.
- [Changelog](https://github.com/nix-rust/nix/blob/master/CHANGELOG.md)
- [Commits](https://github.com/nix-rust/nix/compare/v0.26.0...v0.27.1)
---
updated-dependencies:
- dependency-name: nix
dependency-type: direct:production
...
Signed-off-by: dependabot[bot] <support@github.com>
nix 0.27 required these changes:
- memfd_create() is now only available with crate feature `fs`.
- the return value of memfd_create() is now an OwnedFd and not RawFd
anymore.
We are using nix only for testing, so no functional changes at all.
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
We figured that all vhost-device daemons just ended up copying code from
each other. During an earlier attempt to solve that with a helper crate
for vhost-device [1], it was suggested to extend the vhost-user-backend
API directly. This way more people can benefit from this.
The function just groups together some functions that (at least in
vhost-device) are always called together. Some usually "expected" error
results are filtered out. The remaining additions are extending the mock
backend so that it allows sensible testing of exit events and the test
for the functionality itself.
[1] https://github.com/rust-vmm/vhost-device/pull/362
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Scoping the threads allows us to just borrow the values from the thread and
removes the need for the clones with the slightly awkward names.
Also, we no longer need to remember to join threads (happens
automatically upon end of scope).
No functional changes intended.
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
The function used to return a tuple, but no longer does that.
Fixes: e8beb23 ("epoll: refine the way to manage event id")
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
After updating the CI the coverage value changed causing failures:
Current code coverage (76.33%) deviates by 7.67% from the
previous code coverage 84.00%.
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Clippy reported the following suggestions:
error: useless use of `vec!`
--> crates/vhost/src/vhost_user/connection.rs:676:20
|
676 | let buf1 = vec![0x1, 0x2, 0x3, 0x4];
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: you can use an array directly: `[0x1, 0x2, 0x3, 0x4]`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_vec
= note: `-D clippy::useless-vec` implied by `-D warnings`
error: useless use of `vec!`
--> crates/vhost/src/vhost_user/connection.rs:706:20
|
706 | let buf1 = vec![0x1, 0x2, 0x3, 0x4];
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: you can use an array directly: `[0x1, 0x2, 0x3, 0x4]`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_vec
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Clippy reported the following suggestions:
error: incorrect implementation of `clone` on a `Copy` type
--> crates/vhost/src/vhost_kern/vhost_binding.rs:134:29
|
134 | fn clone(&self) -> Self {
| _____________________________^
135 | | Self::new()
136 | | }
| |_____^ help: change this to: `{ *self }`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#incorrect_clone_impl_on_copy_type
= note: `#[deny(clippy::incorrect_clone_impl_on_copy_type)]` on by default
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Following vhost-user specification changes,
replace all uses of slave/master with backend/frontend
for the vhost-user-backend crate.
Signed-off-by: Albert Esteve <aesteve@redhat.com>
Following vhost-user specification, replace
all uses of master/slave with backend/frontend
in the vhost crate.
Signed-off-by: Albert Esteve <aesteve@redhat.com>
`extract_request_body()` assumes these types are POD as a safety
invariant, which is only possible if `VhostUserMsgValidator` is itself
an `unsafe trait` or depends on an `unsafe trait` with the correct
invariants.
`VhostUserMemoryRegion` is the only type that implemented
`VhostUserMsgValidator` and did not yet implement `ByteValued`
Signed-off-by: Manish Goregaokar <manishsmail@gmail.com>
Commit 8a4ba9d ("vhost-user-backend: fetch 'used' index from guest")
fixes an issue introduced in v0.10.0 that affects all vhost-user
backends like virtiofsd [1] or rust-vmm's vhost-device.
I easily reproduced the problem with vhost-device-vsock as well:
just restart the guest kernel and the device no longer works.
[1] https://gitlab.com/virtio-fs/virtiofsd/-/issues/120
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
commit 958cdec2b8 stopped GET_VRING_BASE
from resetting the vring. This was required because when the VM is
stopped/resumed the index will be 0 instead of the correct value.
However, after the guest driver changes, for instance, after reboot,
the 'used' index should be reset to 0. The bug fixed in that commit had
the unintended side effect of setting the 'used' index to 0 after a
driver change.
QEMU's vhost-user library sets the 'used' index when receiving
SET_VRING_ADDR, _probably_ to make sure that the VQ is configured.
Perhaps the appropriate place is when receiving the first kick.
A better solution would be to send the 'used' index in SET_VRING_BASE,
as is done when using packed VQs.
To keep compatibility with QEMU and just in case, any implementation
expects the 'used' index to be set when receiving SET_VRING_ADDR let's
fetch the 'used' index from the guest when receiving that message.
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: German Maglione <gmaglione@redhat.com>
In the previous commit I forgot to update the version in
vhost/Cargo.toml, let's set it to 0.8.1
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
We experienced a small dependency issue with the `vm-memory` crate
features, let's release this version to fix it.
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Since 4029089f ('vhost: Add support for Xen memory mappings') the
feature backend-mmap of the vm-memory crate is no longer just a
dev-dependency, as we're unconditionally importing GuestMemoryMmap from
'src/backend.rs'. Make it a general build dependency.
Signed-off-by: Sergio Lopez <slp@redhat.com>
Release a new version with Xen support.
We broke the build with the default features, but not yet made the
release, so this should be the real release.
Fixes: b128901 ("vhost: Release v0.8.0")
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
The `vhost-user` feature isn't enabled by default and using it without
protection in backend.rs makes the vhost only build fails.
Fix it by protecting it with the `#[cfg]` block.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Migrate to a newer version of the vhost and other dependencies and add
support for xen memory mappings. Add a corresponding xen feature for
vhost-user-backend crate.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Automatically enable the VhostUserProtocolFeatures::XEN_MMAP feature for
backends for Xen specific builds. With these the backends don't need to
enable this feature and can directly support Xen.
Suggested-by: Erik Schilling <erik.schilling@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
The vm-memory crate now supports Xen specific memory mappings via a
special feature: "xen".
Add a corresponding feature for vhost crate and add support for Xen
memory regions. Update various dependencies to align to the same version
of vm-memory crate.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
No need to repeat the struct definition here. The only purpose is to
register an event_fd at epoll if one exists.
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
A lot of tests are creating objects of this structure directly, it would
be better if they all use a function instead to do so.
This will also be beneficial for future commits where more fields will
be added to this structure.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
VhostUserSingleMemoryRegion contains an extra padding before
VhostUserMemoryRegion, and everything else remains the same. Lets reuse
the same structure instead of duplicating implementation here.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
The spec specifies that on receiving `GET_VRING_BASE` the backend should
stop the vring, but not that it must be reset. This is intended for
`VHOST_USER_RESET_DEVICE`, also in this case the spec makes a
difference between stopping and disabling the ring.
The spec also doesn't forbid to send `VHOST_USER_SET_VRING_ENABLE` to
enable the vring after receiving `GET_VRING_BASE` or sending more
`GET_VRING_BASE` messages, which would always respond 0. Moreover, qemu
doesn't reset the vring either.
Signed-off-by: German Maglione <gmaglione@redhat.com>
Clippy warns saying:
error: this `impl` can be derived
Derive them instead to fix those.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
In rust-vmm-ci we are enabling `clippy::undocumented_unsafe_blocks` as
errors, so let's comment all unsafe blocks to avoid failures in the
CI.
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Unlike other vhost backends (e.g. vhost-net and vhost-vsock), vDPA
backends can not work with host virtual address (HVA), instead they
expect I/O virtual address (IOVA). The IOVA can be mapped 1:1 with
guest physical address (GPA) when no IOMMU is involved. This is why
the default implementation of `set_vring_addr()` from Trait
`VhostBackend` is no longer working with vDPA backends. To solve this
issue, this patch provides a custom `set_vring_addr()` implementation
for Trait `VhostKernVdpa`.
Fixes: #164
Signed-off-by: Bo Chen <chen.bo@intel.com>
Set up the stage to release vhost v0.7.0 and vhost-user-backend v0.9.0
to accommodate to the new vm-memory, virtio-bindings and virtio-queue
releases.
Signed-off-by: Sergio Lopez <slp@redhat.com>
When the frontend sends the `GET_VRING_BASE` message, we should return
the vring's last available index and stop the vring. To return the
correct value we should not reset the queue before getting its value,
otherwise we will always return 0.
Signed-off-by: German Maglione <gmaglione@redhat.com>
`VhostBackend::set_vring_addr()` receives a vring config data which
contains the addresses of desc table, used ring and avail ring.
`VhostBackend::is_valid()` checks the addresses in the guest address space.
`VHOST_SET_VRING_ADDR` uses the addresses in the host address space.
However, the method doesn't convert those addresses.
To address this issue, the addresses passed by the config are checked in
the guest address space. Then, they are converted by
`VringConfigData::to_vhost_vring_addr()` into the host address space to
setup the vring on the kernel.
Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
There are a lot of slave requests supported by vhost-user protocol, it
isn't just about FS_MAP and FS_UNMAP. Rename the files and declarations
to slave request specific names.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Since VhostUserMsgHeader implements ByteValued, it is supposed to be
safe to construct from any correctly-sized arbitrary byte array.
But that means we can do this:
let bytes = b"\xFF\xFF\xFF\xFF\x00\x00\x00\x00\x00\x00\x00\x00";
let header = VhostUserMsgHeader::<MasterReq>::from_slice(bytes).unwrap();
header.get_code()
constructing an invalid MasterReq, using only functions that are
marked as safe. Constructing an invalid enum value is undefined
behavior in Rust, so this API is unsound. This wasn't considered by
the safety comment in VhostUserMsgHeader::get_code, which only
considered the safety of requests that were valid enum variants.
If the vhost-user frontend process sends a message that the backend
doesn't recognise, that's exactly what will happen, so the UB can be
triggered from an external process (but a trusted one).
To fix this, we need to check whether the value is valid _before_
converting it. Req::is_valid is changed to be a non-instance method,
so it can be called before constructing the Req.
VhostUserMsgHeader::get_code is changed to return a Result, to
accomodate the case where the request number is not a valid value for
R.
Signed-off-by: Alyssa Ross <alyssa.ross@unikie.com>
