On Tue, Jun 09, 2020 at 06:02:14PM +0100, Stefan Hajnoczi wrote: > QEMU enabled several feature bits for non-net devices without allowing > the device backend to control them. This only works when the device > backend implements support for those features. It won't work for new > features like the packed virtqueue layout, where proper feature > negotiation will be needed. > > Document the legacy behavior and specify that device backends must > report features so that we can avoid problems in the future. > > Cc: Ben Walker <benjamin.wal...@intel.com> > Cc: Sebastien Boeuf <sebastien.bo...@intel.com> > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com> > --- > CCing SPDK and cloud-hypervisor folks in case they are affected. DPDK > isn't affected since vhost-user-net performs full feature negotiation. > --- > docs/interop/vhost-user.rst | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst > index 3b1b6602c7..dfadee411d 100644 > --- a/docs/interop/vhost-user.rst > +++ b/docs/interop/vhost-user.rst > @@ -290,6 +290,27 @@ bit was dedicated for this purpose:: > > #define VHOST_USER_F_PROTOCOL_FEATURES 30 > > +Feature negotiation > +------------------- > +The master fetches features from the backend using the > +``VHOST_USER_GET_FEATURES`` message. The feature bits correspond to those > from > +the virtio specification, VHOST_F_LOG_ALL (26), and > +``VHOST_USER_F_PROTOCOL_FEATURES`` (30).
Seems to partially duplicate the description of the message VHOST_USER_GET_FEATURES, except that is missing description of VHOST_F_LOG_ALL. How about tweaking that instead of adding more text? BTW description of VHOST_USER_GET_FEATURES has some typos worth fixing ... > +Backends must report all supported feature bits. If a feature bit is set then > +the master may set it in the ``VHOST_USER_SET_FEATURES`` message. If a > feature > +bit is cleared then the master must not set it in the > +``VHOST_USER_SET_FEATURES`` message. Again let's extend description of VHOST_USER_SET_FEATURES if that's unclear. BTW description of VHOST_USER_SET_FEATURES has some typos worth fixing ... > + > +For devices other than the networking device, masters may assume the > following > +feature bits are always set in ``VHOST_USER_GET_FEATURES`` for compatibility > +with legacy backend implementations that do not report them correctly: > +* ``VIRTIO_F_RING_INDIRECT_DESC`` > +* ``VIRTIO_F_RING_EVENT_IDX`` > +* ``VIRTIO_F_VERSION_1`` > +* ``VIRTIO_F_NOTIFY_ON_EMPTY`` > +* ``VIRTIO_F_ANY_LAYOUT`` > + > Starting and stopping rings > --------------------------- How common are these backends? Anything shipped for a while? IIUC we are not talking about years of history here, so I really think we should just enforce what spec always said, rather than work around some broken clients. > -- > 2.26.2 >
