This functionality is gated by `postcopy` feature.
Now backend supports processing of POSTCOPY messages.
This is only available when `xen` feature is not in use.
Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
New feature `postcopy` is introduced. It acts as a gate for a
new functionality.
This feature is not compatible with `xen` feature because `xen`
can map memory regions on demand and this does not work with uffd.
Now frondned supports sending POSTCOPY messages to the backend.
The POSTCOPY messages are only usable when
VHOST_USER_PROTOCOL_F_PAGEFAULT feature is negotiated.
The messages and their descriptions are:
- VHOST_USER_POSTCOPY_ADVISE:
When the front-end sends this message to the backend,
the back-end must open a userfaultfd for later use
and send it's fd to the front-end.
- VHOST_USER_POSTCOPY_LISTEN
When the back-end receives this message it must ensure
that shared memory is registered with userfaultfd to
cause faulting of non-present pages.
This is always sent sometime after a VHOST_USER_POSTCOPY_ADVISE.
- VHOST_USER_POSTCOPY_END
When the back-end receives this message it must disable the
userfaultfd. The reply is an acknowledgement only.
Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
New code was added to the `BackendReqHandler` that is not feasible to
test right now. The rest of the code are just definitions.
Signed-off-by: German Maglione <gmaglione@redhat.com>
Let's add the `VHOST_USER_PROTOCOL_F_DEVICE_STATE` feature flag, and
handling the messages to support tha backend internal state migration.
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: German Maglione <gmaglione@redhat.com>
If `VHOST_USER_PROTOCOL_F_DEVICE_STATE` has been negotiated, these
methods will be called on receiving the `SET_DEVICE_STATE_FD` and
`CHECK_DEVICE_STATE` messages.
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: German Maglione <gmaglione@redhat.com>
A new set of messages have been introduced in the vhost-user protocol
to support the migration of devices with back-end's internal state[0].
These are the additions to the protocol:
- New vhost-user protocol feature VHOST_USER_PROTOCOL_F_DEVICE_STATE
- SET_DEVICE_STATE_FD function: Front-end and back-end negotiate a file
descriptor over which to transfer the state. It also includes
establishing the direction of transfer and migration phase.
- CHECK_DEVICE_STATE: After the state has been transferred through the
file descriptor, the front-end invokes this function to verify
success. There is no in-band way (through the file descriptor) to
indicate failure, so we need to check explicitly.
[0] 019233096c
We leave the VHOST_USER_PROTOCOL_F_DEVICE_STATE feature flag for a
future commit.
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: German Maglione <gmaglione@redhat.com>
This is based on `rust-vmm-ci/.buildkite/test_description.json`.
We can't run rust-vmm-ci tests because they enable all the features with
`--all-features` and our crates have features that may not be compatible with
others (e.g. `xen`). Waiting to solve this problem in rust-vmm-ci (see
https://github.com/rust-vmm/rust-vmm-ci/issues/152), we use a custom
pipeline based on that but that does not use `--all-features`.
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
This was introduced in 7f326dd314, but
doesn't work with qemu versions < 7.2. SET_FEATURES is received before
SET_VRING_KICK, we will enable the vrings in set_features() and disabled
them later in set_vring_kick().
Currently, the libvhostuser in qemu disables the vring upon receiving
RESET_OWNER, but that message is currently deprecated. There is not a
sane place to disable the vring, since according to the spec we can only
do that upon receiving RESET_OWNER (deprecated), RESET_DEVICE (currently
not supported in this crate), and SET_VRING_ENABLE.
The current state of the vhost-user spec is a mess, we need a more
formal spec that just a wall of english.
Signed-off-by: German Maglione <gmaglione@redhat.com>
According to the vhost-user spec (as the code shows) the vring status
must not be modified if `VHOST_USER_F_PROTOCOL_FEATURES` has been
negotiated.
Signed-off-by: German Maglione <gmaglione@redhat.com>
Before this release, vhost-user devices using this crate were
limited to 32 memslots. This does not support well the integration
with virtio-mem.
For more details: https://issues.redhat.com/browse/RHEL-15317
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Let's support up to 509 mem slots, just like vhost in the kernel usually
does. This is required to properly support memory hotplug, either using
multiple DIMMs (ACPI supports up to 256) or using virtio-mem.
The 509 used to be the KVM limit, it supported 512, but 3 were
used for internal purposes. Currently, KVM supports more than 512, but
it usually doesn't make use of more than ~260 (i.e., 256 DIMMs + boot
memory), except when other memory devices like PCI devices with BARs are
used. So, 509 seems to work well for vhost in the kernel.
Details can be found in the QEMU change that made virtio-mem consume
up to 256 mem slots across all virtio-mem devices. [1]
509 mem slots implies 509 VMAs/mappings in the worst case (even though,
in practice with virtio-mem we won't be seeing more than ~260 in most
setups).
With max_map_count under Linux defaulting to 64k, 509 mem slots
still correspond to less than 1% of the maximum number of mappings.
There are plenty left for the application to consume.
[1] https://lore.kernel.org/all/20230926185738.277351-1-david@redhat.com/
Signed-off-by: David Hildenbrand <david@redhat.com>
Signed-off-by: German Maglione <gmaglione@redhat.com>
This release only includes a dependency update, which is visible in the
public API through `VhostBackend::set_vring_kick` and
`VhostBackend::set_vring_call`.
Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
The update brings a fix for a security vulnerability behind
feature-gated code not used by vhost (the `with-serde` feature),
see GHSA-875g-mfp6-g7f9
Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
In many places we access the `VhostUserHandler::vrings` array many
times and at the beginning of each function we check the index to
avoid OOB.
It happened that we forgot about this check (see #211), so let's
avoid this error-prone pattern by taking the reference to the vring
using `Vec::get()`, and avoiding multiple indexing.
Suggested-by: Erik Schilling <erik.schilling@linaro.org>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
VHostUserHandler::set_vring_base() lacks validation of the input
parameter `index`, which is used for indexing an internal vector and
could potentially lead to a Out-of-Bounds write resulting in a panic.
Closes#211
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Having all the workspace crates under the crates/ directory is
unnecessary. Rust documentation itself recommends all crates to be in
the root directory:
https://doc.rust-lang.org/book/ch14-03-cargo-workspaces.html#creating-the-second-package-in-the-workspace
I paste the text content here, in case the online page ever changes or
becomes unavailable:
## Creating the Second Package in the Workspace
Next, let’s create another member package in the workspace and call it add_one. Change the top-level Cargo.toml to specify the add_one path in the members list:
Filename: Cargo.toml
[workspace]
members = [
"adder",
"add_one",
]
Then generate a new library crate named add_one:
$ cargo new add_one --lib
Created library `add_one` package
Your add directory should now have these directories and files:
├── Cargo.lock
├── Cargo.toml
├── add_one
│ ├── Cargo.toml
│ └── src
│ └── lib.rs
├── adder
│ ├── Cargo.toml
│ └── src
│ └── main.rs
└── target
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
The vhost-user spec has been updated to clarify enabling and
disabling vrings. The current spec states that upon receiving a
VHOST_USER_SET_FEATURES message from the front-end without
VHOST_USER_F_PROTOCOL_FEATURES set, the back-end must enable
all rings immediately. Also, while processing the rings (whether
they are enabled or not), the back-end must support changing some
configuration aspects on the fly.
However, the vhost-user-backend implementation will disable all
vring whenever this feature is negotiated, which means that every
VHOST_USER_SET_FEATURES message could unintentionally halt the
device.
This is an incorrect interpretation of the spec, and is problematic
because the VHOST_F_LOG_ALL feature is also set or cleared by
VHOST_USER_SET_FEATURES, which happens during migration.
And doing so should not disable any vring nor halt the device.
Signed-off-by: German Maglione <gmaglione@redhat.com>
Currently, some vhost-user-messages structs are defined with just
'repr(packed)', and also implement the 'ByteValue' trait implying that
they can be initialized safely from an array of bytes.
This is not safe, because the default memory representation does not
guarantee that the fields will not be reordered.
Signed-off-by: German Maglione <gmaglione@redhat.com>
Let's remove commented vhost-user message definitions, some of the
message definition are not supported and the other is duplicated
(i.e., VhostUserLog is already defined).
Signed-off-by: German Maglione <gmaglione@redhat.com>
@stefano-garzarella asked me whether I would be willing to help with
reviews here.
While my availability will probably be a lot worse starting with next
year, I should still be able to do some reviews or ocassional releases.
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
NOOP and MAX_CMD are not present in the vhost-user protocol, these are
use only to verify that the value is correct before transmuting it to
an enum variant.
This has two problems, firstly it exposes of non-standard values in a
public interface, this internal implementaion detail could be
confusing, and secondly it assumes that the values are consecutive with
no "holes" between them.
This results in having to define requests that are not actually
supported, and error prone, which can generate UB when transmuting.
Instead, it is better to implement TryFrom<u32> for enums that need to
convert between variants and u32. This will allow, in the next commit,
to eliminate the need to use an unsafe transmute.
Signed-off-by: German Maglione <gmaglione@redhat.com>
This was tested on vhost-device successfully [1]. Lets enable it here!
[1] 072fadaaba
Suggested-by: Patrick Roy <roypat@amazon.co.uk>
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Release a new version with the following breaking changes:
- replaced master/slave with frontend/backend in public API
- changed VhostUserBackend::handle_request() return type
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Release a new version with the following breaking changes:
- replaced master/slave with frontend/backend in public API
- replaced master/slave with frontend/backend in crate features
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
After updating rust edition to 2021, cargo complains about the
resolver:
warning: some crates are on edition 2021 which defaults to
`resolver = "2"`, but virtual workspaces default to
`resolver = "1"`
note: to keep the current resolver, specify
`workspace.resolver = "1"` in the workspace root's manifest
note: to use the edition 2021 resolver, specify
`workspace.resolver = "2"` in the workspace root's manifest
Let's specify to use the edition 2021 resolver.
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
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>
