I don't think this is right.
On Tue, Dec 22, 2020 at 03:41:26PM -0800, Dylan Reid wrote:
>
> The `CONFIGURE_MEM_SLOTS` feature is specified by vhost, not by virtio.
> Check the vhost flags for it being set.
>
Agreed - VHOST_USER_PROTCOL_F_CONFIGURE_MEM_SLOTS is negotiated at the
vhost-user, not virtio layer. The dev->protocol_features flags are taken
from the VHOST_USER_GET_PROTOCOL_FEATURES message, which retrieves the
supported vhost-user features from the backend. See libvhost-user for a
simple reference implementation.
The VHOST_USER_GET_FEATURES message retrieves the virtio protocol
features, so checking features instead of dev->protocol_features would
incorrectly check the supported virtio features instead of the vhost-user
ones.
Am I missing something here?
> I noticed this while testing a new vhost implementation that doesn't yet
> support configuring memory slots and retested with dpdk's block example
> as well.
Could you clarify what you tested with DPDK? I haven't had problems
testing other backends which do not support configuring memory slots.
>
> Signed-off-by: Dylan Reid <dgr...@dylanreid.com>
> ---
> hw/virtio/vhost-user.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 2fdd5daf74..faa0e133f2 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1860,7 +1860,7 @@ static int vhost_user_backend_init(struct vhost_dev
> *dev, void *opaque)
> }
>
> /* get max memory regions if backend supports configurable RAM slots
> */
> - if (!virtio_has_feature(dev->protocol_features,
> + if (!virtio_has_feature(features,
> VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS)) {
> u->user->memory_slots = VHOST_MEMORY_BASELINE_NREGIONS;
> } else {
> --
> 2.25.1
>
>
~
