Re: [PATCH RESEND] docs: clarify absence of set_features in vhost-user

["Alex Bennée" <alex.bennee@linaro.org>]

Alyssa Ross <hi@alyssa.is> writes:

> The previous wording was (at least to me) ambiguous about whether a
> backend should enable features immediately after they were set using
> VHOST_USER_SET_PROTOCOL_FEATURES, or wait for support for protocol
> features to be acknowledged if it hasn't been yet before enabling
> those features.
>
> This patch attempts to make it clearer that
> VHOST_USER_SET_PROTOCOL_FEATURES should immediately enable features,
> even if support for protocol features has not yet been acknowledged,
> while still also making clear that the frontend SHOULD acknowledge
> support for protocol features.
>
> Previous discussion begins here:
> <https://lore.kernel.org/qemu-devel/87sgd1ktx9.fsf@alyssa.is/>

I totally missed this when I posted a similar attempt at clarification:

  Subject: [PATCH v2] vhost-user.rst: add clarifying language about protocol negotiation
  Date: Wed,  3 Mar 2021 14:50:11 +0000
  Message-Id: <20210303145011.14547-1-alex.bennee@linaro.org>

>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Alyssa Ross <hi@alyssa.is>
> ---
>  docs/interop/vhost-user.rst | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index d6085f7045..c42150331d 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -871,9 +871,9 @@ Master message types
>    ``VHOST_USER_GET_FEATURES``.
>  
>  .. Note::
> -   Slave that reported ``VHOST_USER_F_PROTOCOL_FEATURES`` must
> -   support this message even before ``VHOST_USER_SET_FEATURES`` was
> -   called.
> +   While QEMU should acknowledge ``VHOST_USER_F_PROTOCOL_FEATURES``, a
> +   backend must allow ``VHOST_USER_GET_PROTOCOL_FEATURES`` even if
> +   ``VHOST_USER_F_PROTOCOL_FEATURES`` has not been acknowledged yet.
>  
>  ``VHOST_USER_SET_PROTOCOL_FEATURES``
>    :id: 16
> @@ -886,8 +886,12 @@ Master message types
>    ``VHOST_USER_GET_FEATURES``.
>  
>  .. Note::
> -   Slave that reported ``VHOST_USER_F_PROTOCOL_FEATURES`` must support
> -   this message even before ``VHOST_USER_SET_FEATURES`` was called.
> +   While QEMU should acknowledge ``VHOST_USER_F_PROTOCOL_FEATURES``, a
> +   backend must allow ``VHOST_USER_SET_PROTOCOL_FEATURES`` even if
> +   ``VHOST_USER_F_PROTOCOL_FEATURES`` has not been acknowledged yet.
> +   The backend must not wait for ``VHOST_USER_SET_FEATURES`` before
> +   enabling protocol features requested with
> +   ``VHOST_USER_SET_PROTOCOL_FEATURES``.

I think this is perfectly fine clarification although I think there
might be a patch in flight which changes some of the master slave
terminology so with that resolved:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

However there is still the edge case of what happens after the vhost
pair have negotiated protocol features like REPLY_ACK should
VHOST_USER_F_PROTOCOL_FEATURES still be acknowledged by
VHOST_USER_SET_FEATURES.

Stefan's proposed wording which I incorporated in my patch made it clear
that VHOST_USER_F_PROTOCOL_FEATURES is never exposed to the guest by the
VMM due to it's UNUSED status. I would just like it explicit if it needs
to be preserved between the two sides of the VHOST_USER_*_FEATURES for
the negotiated protocol features to remain valid.

Perhaps you could incorporate some wording for that?

>  
>  ``VHOST_USER_SET_OWNER``
>    :id: 3


-- 
Alex Bennée