Re: [PATCH v2 4/7] vhost: involve device backends in feature negotiation

[Stefan Hajnoczi]

On Tue, Jun 09, 2020 at 02:07:44PM -0400, Michael S. Tsirkin wrote:
> On Tue, Jun 09, 2020 at 06:02:15PM +0100, Stefan Hajnoczi wrote:
> > Many vhost devices in QEMU currently do not involve the device backend
> > in feature negotiation. This seems fine at first glance when no
> > device-specific feature bits are defined (virtio-net has many but some
> > devices have none).
> > 
> > Unfortunately this causes problems when QEMU's virtqueue implementation
> > and the device backend's implementation support different features.
> > QEMU must not report features to the guest that aren't supported by the
> > device backend.
> > 
> > For example, QEMU supports VIRTIO 1.1 packed virtqueues while many
> > existing vhost device backends do not. The device backend breaks when
> > the user sets packed=on. This should have been handled gracefully by
> > feature negotiation instead of resulting in a cryptic failure when the
> > device backend cannot parse the vring.
> > 
> > Introduce the vhost_old_default_feature_bits[] array so existing
> > devices can involve the device backend in feature negotiation.
> > libvhost-user does not report VIRTIO_RING_F_INDIRECT_DESC and other core
> > feature bits even though it implements them. Therefore
> > vhost_old_default_feature_bits[] only includes feature bits that can be
> > explicitly negotiated without breaking existing libvhost-user device
> > backends.
> 
> libvhost-user is in contrib. Can't we just fix it as appropriate?
> Work arounds have their cost ..

libvhost-user's behavior is unfortunate but the bigger problem is that
QEMU does not include backends in feature negotiation. Existing backends
for devices touched in this patch may have a broken
VHOST_USER_GET_FEATURES implementations and changing QEMU's behavior
will break them.

If you feel confident that third-party vhost-user backends will work,
then we can simplify this patch series. I think we should be very
careful but it's up to you.

Stefan

signature.asc
Description: PGP signature