Alex Bennée <alex.ben...@linaro.org> writes:
> Alyssa Ross <h...@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....@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.ben...@linaro.org> > >> >> Cc: Michael S. Tsirkin <m...@redhat.com> >> Signed-off-by: Alyssa Ross <h...@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.ben...@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
