On Thu, Jun 17, 2021 at 04:19:26PM +0100, Alex Bennée wrote: > > 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
Alyssa or Alex: Please send another revision rebased on qemu.git/master. Michael Tsirkin is the vhost-user.rst maintainer but I can help with reviewing an updated patch. Stefan
signature.asc
Description: PGP signature
