vhost-user-generic: add support #7221
Conversation
DemiMarie
force-pushed
the
generic-vhost-user
branch
6 times, most recently
from
aed497d to
cd6b328
Compare
DemiMarie
force-pushed
the
generic-vhost-user
branch
4 times, most recently
from
b101b21 to
64b241c
Compare
DemiMarie
force-pushed
the
generic-vhost-user
branch
2 times, most recently
from
2b46ed3 to
5c8e3f7
Compare
|
Can you give an example of what a |
DemiMarie
force-pushed
the
generic-vhost-user
branch
from
5c8e3f7 to
e084e44
Compare
|
A |
|
The QEMU patch referenced in #7189 -- https://lore.kernel.org/qemu-devel/[email protected]/ -- shows that it is still in RFC status. What's the latest status in QEMU? There is a flag How will this work in practice? Do you have a working backend to support this already? |
I don’t think the patch was accepted. QEMU does have a generic vhost-user device, but (like this patch) it requires parameters to be passed by the user rather than fetching them from the backend. QEMU also requires a trivial source-code patch to enable the feature, which this patch doesn’t need.
You are correct.
This works with an unmodified virtiofsd, but the caller must explicitly provide all of the parameters needed. If I did not make any copy-and-paste mistakes, I used the following option to make a virtio-fs device: --vhost-user-generic \
"socket=build/virtiofsd.sock,virtio_id=26,avail_features=$((0x5970000000)),avail_protocol_features=$((0x920b)),queue_size=1024,num_queues=1,min_queues=1,id=virtiofs0"This interface is deliberately low-level to allow maximum flexibility. Callers are expected to know the parameters for their particular device. Higher-level tooling is expected to provide a user-friendly interface. |
DemiMarie
force-pushed
the
generic-vhost-user
branch
3 times, most recently
from
887cdc6 to
7cccee1
Compare
DemiMarie
force-pushed
the
generic-vhost-user
branch
from
7cccee1 to
e8b8f38
Compare
|
Needs some clippy fixes. |
DemiMarie
force-pushed
the
generic-vhost-user
branch
2 times, most recently
from
595b465 to
986efa7
Compare
vmm/src/config.rs
Outdated
| let num_queues = parser | ||
| .convert("num_queues") | ||
| .map_err(Error::ParseVhostUser)? | ||
| .ok_or(Error::ParseVhostUserNumQueuesMissing)?; |
There was a problem hiding this comment.
Maybe it would be better to take queue_sizes, and infer num_queues from that? That would allow specifying queues of different types — e.g. crosvm GPU uses one queue of size 256, and another of size 16.
There was a problem hiding this comment.
Can you provide some more details here? I’m not sure how the API for this should look.
There was a problem hiding this comment.
--user-vhost-device queue_sizes=[256,16], and then you know there are two queues.
DemiMarie
force-pushed
the
generic-vhost-user
branch
6 times, most recently
from
7d529a8 to
301b753
Compare
DemiMarie
force-pushed
the
generic-vhost-user
branch
4 times, most recently
from
f7667e5 to
5ee5d30
Compare
DemiMarie
force-pushed
the
generic-vhost-user
branch
from
5ee5d30 to
e3fbf78
Compare
| // Copyright 2019 Intel Corporation. All Rights Reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| use std::io::Write as _; |
There was a problem hiding this comment.
I don't think this as _ is idiomatic. We don't do it anywhere else that I can see.
| VIRTIO_F_VERSION_1, VirtioCommon, VirtioDevice, VirtioInterrupt, VirtioSharedMemoryList, | ||
| }; | ||
|
|
||
| const DEFAULT_QUEUE_NUMBER: usize = 2; |
There was a problem hiding this comment.
Should we even have a default?
| #[repr(C, packed)] | ||
| pub struct VirtioVhostUserConfig { | ||
| pub num_request_queues: u32, | ||
| } |
| pub num_request_queues: u32, | ||
| } | ||
|
|
||
| // SAFETY: only a series of integers |
There was a problem hiding this comment.
| // SAFETY: only a series of integers | |
| // SAFETY: only an integer |
| 10, | ||
| &IntegerList(vec![u16::MAX.into(), 20u16.into()]), | ||
| virtio_devices::vhost_user::DEFAULT_VIRTIO_FEATURES, | ||
| ); |
There was a problem hiding this comment.
I think I'd rather see construction and checking of these inlined. As it is it's quite difficult to see what's actually being tested for each case.
vmm/src/device_manager.rs
Outdated
|
|
||
| /// Cannot create virtio-vhost-user device | ||
| #[error("Cannot create virtio-vhost-user device")] | ||
| CreateVirtioVhostUser(#[source] virtio_devices::vhost_user::Error), |
There was a problem hiding this comment.
Was this supposed to be renamed?
vmm/src/vm_config.rs
Outdated
| pub struct PvmemcontrolConfig {} | ||
|
|
||
| #[derive(Clone, Copy, Debug, PartialEq, Eq)] | ||
| pub struct AvailProtocolFeatures(pub VhostUserProtocolFeatures); |
vmm/src/vm_config.rs
Outdated
|
|
||
| pub fn default_vhost_user_config_num_queues() -> usize { | ||
| 1 | ||
| } |
There was a problem hiding this comment.
I think this should have been deleted.
DemiMarie
force-pushed
the
generic-vhost-user
branch
4 times, most recently
from
0cdc95c to
c52ba0a
Compare
This was breaking CI for PR cloud-hypervisor#7221. Signed-off-by: Demi Marie Obenour <[email protected]>
This breaks CI for cloud-hypervisor#7221. Signed-off-by: Demi Marie Obenour <[email protected]>
This breaks CI for cloud-hypervisor#7221. Signed-off-by: Demi Marie Obenour <[email protected]>
This adds support for a generic vhost-user-generic device. All information about this device must be provided to Cloud Hypervisor via the command-line or API. Fixes cloud-hypervisor#7189 Signed-off-by: Demi Marie Obenour <[email protected]>
DemiMarie
force-pushed
the
generic-vhost-user
branch
from
c52ba0a to
298d2b4
Compare
This breaks CI for cloud-hypervisor#7221. Signed-off-by: Demi Marie Obenour <[email protected]>
This breaks CI for cloud-hypervisor#7221. Signed-off-by: Demi Marie Obenour <[email protected]>
This breaks CI for cloud-hypervisor#7221. Signed-off-by: Demi Marie Obenour <[email protected]>
This breaks CI for cloud-hypervisor#7221. Signed-off-by: Demi Marie Obenour <[email protected]>
This breaks CI for cloud-hypervisor#7221. Signed-off-by: Demi Marie Obenour <[email protected]>
This breaks CI for cloud-hypervisor#7221. Signed-off-by: Demi Marie Obenour <[email protected]>
4 participants
This adds support for a generic vhost-user-generic device. All information about this device must be provided to Cloud Hypervisor via the command-line or API.
Fixes #7189