Re: [PATCH v3 4/7] hw/virtio: ensure a valid host_feature set for virtio-user-gpio

[Stefan Hajnoczi]

On Mon, 28 Nov 2022 at 11:42, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> There was a disconnect here because vdev->host_features was set to
> random rubbish. This caused a weird negotiation between the driver and
> device that took no account of the features provided by the backend.
> To fix this we must set vdev->host_features once we have initialised
> the vhost backend.
>
> [AJB: however this is confusing because AFAICT none of the other
> vhost-user devices do this.]
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  hw/virtio/vhost-user-gpio.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/hw/virtio/vhost-user-gpio.c b/hw/virtio/vhost-user-gpio.c
> index 5851cb3bc9..b2496c824c 100644
> --- a/hw/virtio/vhost-user-gpio.c
> +++ b/hw/virtio/vhost-user-gpio.c
> @@ -228,6 +228,12 @@ static int vu_gpio_connect(DeviceState *dev, Error 
> **errp)
>          return ret;
>      }
>
> +    /*
> +     * Once we have initialised the vhost backend we can finally set
> +     * the what host features are available for this device.
> +     */
> +    vdev->host_features = vhost_dev->features;

vdev->host_feature is already set by virtio_bus_device_plugged ->
vu_gpio_get_features.

Something is still wrong.

My understanding is: ->realize() performs a blocking connect so when
it returns and virtio_bus_device_plugged() runs, we'll be able to
fetch the backend features from ->get_features(). The assumption is
that the backend features don't change across reconnection (I think).

vhost-user-gpio seems to follow the same flow as the other vhost-user
devices (vhost-user-net is special, so let's ignore it), so I don't
understand why it's necessary to manually assign ->host_features here?

Stefan