Re: [RFC PATCH] docs/interop: define STANDALONE protocol feature for vhost-user

[Stefano Garzarella <sgarzare@redhat.com>]

On Thu, Jul 06, 2023 at 05:31:15PM +0100, Alex Bennée wrote:
> Alex Bennée <alex.bennee@linaro.org> writes:
>
> > Currently QEMU has to know some details about the back-end to be able
> > to setup the guest. While various parts of the setup can be delegated
> > to the backend (for example config handling) this is a very piecemeal
> > approach.
> >
> > This patch suggests a new feature flag (VHOST_USER_PROTOCOL_F_STANDALONE)
> > which the back-end can advertise which allows a probe message to be
> > sent to get all the details QEMU needs to know in one message.
> >
> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >
> > ---
> > Initial RFC for discussion. I intend to prototype this work with QEMU
> > and one of the rust-vmm vhost-user daemons.
> > ---
> >  docs/interop/vhost-user.rst | 37 +++++++++++++++++++++++++++++++++++++
> >  hw/virtio/vhost-user.c      |  8 ++++++++
> >  2 files changed, 45 insertions(+)
> >
> > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> > index 5a070adbc1..85b1b1583a 100644
> > --- a/docs/interop/vhost-user.rst
> > +++ b/docs/interop/vhost-user.rst
> > @@ -275,6 +275,21 @@ Inflight description
> >
> >  :queue size: a 16-bit size of virtqueues
> >
> > +Backend specifications
> > +^^^^^^^^^^^^^^^^^^^^^^
> > +
> > ++-----------+-------------+------------+------------+
> > +| device id | config size |   min_vqs  |   max_vqs  |
> > ++-----------+-------------+------------+------------+
> > +
> > +:device id: a 32-bit value holding the VirtIO device ID
> > +
> > +:config size: a 32-bit value holding the config size (see ``VHOST_USER_GET_CONFIG``)
> > +
> > +:min_vqs: a 32-bit value holding the minimum number of vqs supported
> > +
> > +:max_vqs: a 32-bit value holding the maximum number of vqs supported, must be >= min_vqs
> > +
> >  C structure
> >  -----------
> >
> > @@ -296,6 +311,7 @@ In QEMU the vhost-user message is implemented with the following struct:
> >            VhostUserConfig config;
> >            VhostUserVringArea area;
> >            VhostUserInflight inflight;
> > +          VhostUserBackendSpecs specs;
> >        };
> >    } QEMU_PACKED VhostUserMsg;
> >
> > @@ -316,6 +332,7 @@ replies. Here is a list of the ones that do:
> >  * ``VHOST_USER_GET_VRING_BASE``
> >  * ``VHOST_USER_SET_LOG_BASE`` (if ``VHOST_USER_PROTOCOL_F_LOG_SHMFD``)
> >  * ``VHOST_USER_GET_INFLIGHT_FD`` (if ``VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD``)
> > +* ``VHOST_USER_GET_BACKEND_SPECS`` (if ``VHOST_USER_PROTOCOL_F_STANDALONE``)
> >
> >  .. seealso::
> >
> > @@ -885,6 +902,13 @@ Protocol features
> >    #define VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS  15
> >    #define VHOST_USER_PROTOCOL_F_STATUS               16
> >    #define VHOST_USER_PROTOCOL_F_XEN_MMAP             17
> > +  #define VHOST_USER_PROTOCOL_F_STANDALONE           18
> > +
> > +Some features are only valid in the presence of other supporting
> > +features. In the case of ``VHOST_USER_PROTOCOL_F_STANDALONE`` the
> > +backend must also support ``VHOST_USER_PROTOCOL_F_CONFIG`` and
> > +``VHOST_USER_PROTOCOL_F_STATUS``.
> > +
>
> This is too tight a restriction as not all VirtIO backends manage a
> config space. So I suggest the following:
>
>  Some features are only valid in the presence of other supporting
>  features. In the case of ``VHOST_USER_PROTOCOL_F_STANDALONE`` the
>  backend must also support ``VHOST_USER_PROTOCOL_F_STATUS`` and
>  optionally ``VHOST_USER_PROTOCOL_F_CONFIG`` (if there is a config space).

Right, but could we describe it more as a kind of dependence between
features?

Something like this:

Some features depend on others to be supported:

* ``VHOST_USER_PROTOCOL_F_STANDALONE`` depends on:

  * ``VHOST_USER_PROTOCOL_F_STATUS``
  * ``VHOST_USER_PROTOCOL_F_CONFIG`` (if there is a config space)

Thanks,
Stefano