[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH RESEND] docs: clarify absence of set_features in vhost-user
From: |
Alex Bennée |
Subject: |
Re: [PATCH RESEND] docs: clarify absence of set_features in vhost-user |
Date: |
Thu, 17 Jun 2021 16:19:26 +0100 |
User-agent: |
mu4e 1.5.13; emacs 28.0.50 |
Alex Bennée <alex.bennee@linaro.org> writes:
> 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
General ping to the vhost-user spec maintainers. This was also mentioned
while merging:
https://github.com/rust-vmm/vhost/pull/24
--
Alex Bennée
- Re: [PATCH RESEND] docs: clarify absence of set_features in vhost-user,
Alex Bennée <=