[VHOST USER SPEC PATCH] vhost-user.rst: add clarifying language about protocol negotiation

Alex Bennée posted 1 patch 3 years, 8 months ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210226111619.21178-1-alex.bennee@linaro.org
There is a newer version of this series
docs/interop/vhost-user.rst | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
[VHOST USER SPEC PATCH] vhost-user.rst: add clarifying language about protocol negotiation
Posted by Alex Bennée 3 years, 8 months ago
In practice the protocol negotiation between vhost master and slave
occurs before the final feature negotiation between backend and
frontend. This has lead to an inconsistency between the rust-vmm vhost
implementation and the libvhost-user library in their approaches to
checking if all the requirements for REPLY_ACK processing were met.
As this is purely a function of the protocol negotiation and not of
interest to the frontend lets make the language clearer about the
requirements for a successfully negotiated protocol feature.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Jiang Liu <gerry@linux.alibaba.com>
---
 docs/interop/vhost-user.rst | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index d6085f7045..3ac221a8c7 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -301,12 +301,22 @@ If *slave* detects some error such as incompatible features, it may also
 close the connection. This should only happen in exceptional circumstances.
 
 Any protocol extensions are gated by protocol feature bits, which
-allows full backwards compatibility on both master and slave.  As
-older slaves don't support negotiating protocol features, a feature
+allows full backwards compatibility on both master and slave. As older
+slaves don't support negotiating protocol features, a device feature
 bit was dedicated for this purpose::
 
   #define VHOST_USER_F_PROTOCOL_FEATURES 30
 
+However as the protocol negotiation something that only occurs between
+parts of the backend implementation it is permissible to for the master
+to mask the feature bit from the guest. As noted for the
+``VHOST_USER_GET_PROTOCOL_FEATURES`` and
+``VHOST_USER_SET_PROTOCOL_FEATURES`` messages this occurs before a
+final ``VHOST_USER_SET_FEATURES`` comes from the guest. So the
+enabling of protocol features need only require the advertising of the
+feature by the slave and the successful get/set protocol features
+sequence.
+  
 Starting and stopping rings
 ---------------------------
 
-- 
2.20.1


Re: [VHOST USER SPEC PATCH] vhost-user.rst: add clarifying language about protocol negotiation
Posted by no-reply@patchew.org 3 years, 8 months ago
Patchew URL: https://patchew.org/QEMU/20210226111619.21178-1-alex.bennee@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210226111619.21178-1-alex.bennee@linaro.org
Subject: [VHOST USER SPEC PATCH] vhost-user.rst: add clarifying language about protocol negotiation

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210226111619.21178-1-alex.bennee@linaro.org -> patchew/20210226111619.21178-1-alex.bennee@linaro.org
Switched to a new branch 'test'
93ba1ce vhost-user.rst: add clarifying language about protocol negotiation

=== OUTPUT BEGIN ===
ERROR: trailing whitespace
#48: FILE: docs/interop/vhost-user.rst:319:
+  $

total: 1 errors, 0 warnings, 24 lines checked

Commit 93ba1ce0fb2a (vhost-user.rst: add clarifying language about protocol negotiation) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210226111619.21178-1-alex.bennee@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [virtio-dev] [VHOST USER SPEC PATCH] vhost-user.rst: add clarifying language about protocol negotiation
Posted by Stefan Hajnoczi 3 years, 8 months ago
On Fri, Feb 26, 2021 at 11:16:19AM +0000, Alex Bennée wrote:
> In practice the protocol negotiation between vhost master and slave
> occurs before the final feature negotiation between backend and
> frontend. This has lead to an inconsistency between the rust-vmm vhost
> implementation and the libvhost-user library in their approaches to
> checking if all the requirements for REPLY_ACK processing were met.
> As this is purely a function of the protocol negotiation and not of
> interest to the frontend lets make the language clearer about the
> requirements for a successfully negotiated protocol feature.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Jiang Liu <gerry@linux.alibaba.com>
> ---
>  docs/interop/vhost-user.rst | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)

I had difficulty understanding this change and its purpose. I think it's
emphasizing what the spec already says: VHOST_USER_SET_PROTOCOL_FEATURES
can be sent after VHOST_USER_F_PROTOCOL_FEATURES was reported by
VHOST_USER_GET_FEATURES.

BTW Paolo has just sent a patch here to use the terms "frontend" and
"backend" with different meanings from how you are using them:
https://lists.nongnu.org/archive/html/qemu-devel/2021-02/msg08347.html

> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index d6085f7045..3ac221a8c7 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -301,12 +301,22 @@ If *slave* detects some error such as incompatible features, it may also
>  close the connection. This should only happen in exceptional circumstances.
>  
>  Any protocol extensions are gated by protocol feature bits, which
> -allows full backwards compatibility on both master and slave.  As
> -older slaves don't support negotiating protocol features, a feature
> +allows full backwards compatibility on both master and slave. As older
> +slaves don't support negotiating protocol features, a device feature
>  bit was dedicated for this purpose::
>  
>    #define VHOST_USER_F_PROTOCOL_FEATURES 30
>  
> +However as the protocol negotiation something that only occurs between

Missing "is". Shortening the sentence fixes that without losing clarity:
s/something that/negotiation/

> +parts of the backend implementation it is permissible to for the master

"vhost-user device backend" is often used to refer to the slave (to
avoid saying the word "slave") but "backend" is being used in a
different sense here. That is confusing.

> +to mask the feature bit from the guest.

I think this sentence effectively says "the master MAY mask the
VHOST_USER_F_PROTOCOL_FEATURES bit from the VIRTIO feature bits". That
is not really accurate since VIRTIO devices do not advertise this
feature bit and so it can never be negotiated through the VIRTIO feature
negotiation process.

How about referring to the details from the VIRTIO 1.1 specification
instead. Something like this:

  Note that VHOST_USER_F_PROTOCOL_FEATURES is the UNUSED (30) feature
  bit defined in `VIRTIO 1.1 6.3 Legacy Interface: Reserved Feature Bits
  <https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-4130003>`_.
  VIRTIO devices do not advertise this feature bit and therefore VIRTIO
  drivers cannot negotiate it.

  This reserved feature bit was reused by the vhost-user protocol to add
  vhost-user protocol feature negotiation in a backwards compatible
  fashion. Old vhost-user master and slave implementations continue to
  work even though they are not aware of vhost-user protocol feature
  negotiation.

> As noted for the
> +``VHOST_USER_GET_PROTOCOL_FEATURES`` and
> +``VHOST_USER_SET_PROTOCOL_FEATURES`` messages this occurs before a
> +final ``VHOST_USER_SET_FEATURES`` comes from the guest.

I couldn't find any place where vhost-user.rst states that
VHOST_USER_SET_PROTOCOL_FEATURES has to come before
VHOST_USER_SET_FEATURES?

The only order I found was:

1. VHOST_USER_GET_FEATURES to determine whether protocol features are
   supported.
2. VHOST_USER_GET_PROTOCOL_FEATURES to fetch available protocol feature bits.
3. VHOST_USER_SET_PROTOCOL_FEATURES to set protocol feature bits.
4. Using functionality that depends on enabled protocol feature bits.

Is the purpose of this sentence to add a new requirement to the spec
that "VHOST_USER_SET_PROTOCOL_FEATURES MUST be sent before
VHOST_USER_SET_FEATURES"?

> So the
> +enabling of protocol features need only require the advertising of the
> +feature by the slave and the successful get/set protocol features
> +sequence.

"the feature" == VHOST_USER_F_PROTOCOL_FEATURES?

Stefan
Re: [virtio-dev] [VHOST USER SPEC PATCH] vhost-user.rst: add clarifying language about protocol negotiation
Posted by Alex Bennée 3 years, 8 months ago
Stefan Hajnoczi <stefanha@redhat.com> writes:

> On Fri, Feb 26, 2021 at 11:16:19AM +0000, Alex Bennée wrote:
>> In practice the protocol negotiation between vhost master and slave
>> occurs before the final feature negotiation between backend and
>> frontend. This has lead to an inconsistency between the rust-vmm vhost
>> implementation and the libvhost-user library in their approaches to
>> checking if all the requirements for REPLY_ACK processing were met.
>> As this is purely a function of the protocol negotiation and not of
>> interest to the frontend lets make the language clearer about the
>> requirements for a successfully negotiated protocol feature.
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Cc: Jiang Liu <gerry@linux.alibaba.com>
>> ---
>>  docs/interop/vhost-user.rst | 14 ++++++++++++--
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> I had difficulty understanding this change and its purpose. I think it's
> emphasizing what the spec already says: VHOST_USER_SET_PROTOCOL_FEATURES
> can be sent after VHOST_USER_F_PROTOCOL_FEATURES was reported by
> VHOST_USER_GET_FEATURES.

Well and also the protocol feature is considered negotiated after that
sequence and doesn't require the feature bit to also be negotiated. I
think I read the spec properly when I submitted:

  https://github.com/rust-vmm/vhost/pull/24

however it was implied rather than explicit. I was hoping to make that
clearer but obviously I've failed with this iteration.

> BTW Paolo has just sent a patch here to use the terms "frontend" and
> "backend" with different meanings from how you are using them:
> https://lists.nongnu.org/archive/html/qemu-devel/2021-02/msg08347.html

Yeah we have mixed up terminology - the relationship between QEMU and a
vhost-user daemon is separate from the relationship between a VirtIO
device driver (in the guest) and the device implementation (as done by
the combination of QEMU and the vhost-user daemon).

I wish we had clearer terminology sections throughout both specs.

>
>> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
>> index d6085f7045..3ac221a8c7 100644
>> --- a/docs/interop/vhost-user.rst
>> +++ b/docs/interop/vhost-user.rst
>> @@ -301,12 +301,22 @@ If *slave* detects some error such as incompatible features, it may also
>>  close the connection. This should only happen in exceptional circumstances.
>>  
>>  Any protocol extensions are gated by protocol feature bits, which
>> -allows full backwards compatibility on both master and slave.  As
>> -older slaves don't support negotiating protocol features, a feature
>> +allows full backwards compatibility on both master and slave. As older
>> +slaves don't support negotiating protocol features, a device feature
>>  bit was dedicated for this purpose::
>>  
>>    #define VHOST_USER_F_PROTOCOL_FEATURES 30
>>  
>> +However as the protocol negotiation something that only occurs between
>
> Missing "is". Shortening the sentence fixes that without losing clarity:
> s/something that/negotiation/
>
>> +parts of the backend implementation it is permissible to for the master
>
> "vhost-user device backend" is often used to refer to the slave (to
> avoid saying the word "slave") but "backend" is being used in a
> different sense here. That is confusing.
>
>> +to mask the feature bit from the guest.
>
> I think this sentence effectively says "the master MAY mask the
> VHOST_USER_F_PROTOCOL_FEATURES bit from the VIRTIO feature bits". That
> is not really accurate since VIRTIO devices do not advertise this
> feature bit and so it can never be negotiated through the VIRTIO feature
> negotiation process.
>
> How about referring to the details from the VIRTIO 1.1 specification
> instead. Something like this:
>
>   Note that VHOST_USER_F_PROTOCOL_FEATURES is the UNUSED (30) feature
>   bit defined in `VIRTIO 1.1 6.3 Legacy Interface: Reserved Feature Bits
>   <https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-4130003>`_.
>   VIRTIO devices do not advertise this feature bit and therefore VIRTIO
>   drivers cannot negotiate it.
>
>   This reserved feature bit was reused by the vhost-user protocol to add
>   vhost-user protocol feature negotiation in a backwards compatible
>   fashion. Old vhost-user master and slave implementations continue to
>   work even though they are not aware of vhost-user protocol feature
>   negotiation.

OK - so does that mean that feature bit will remain UNUSED for ever
more?

What about other feature bits? Is it permissible for the
master/requester/vhost-user front-end/QEMU to filter any other feature
bits the slave/vhost-user backend/daemon may offer from being read by
the guest driver when it reads the feature bits?

>
>> As noted for the
>> +``VHOST_USER_GET_PROTOCOL_FEATURES`` and
>> +``VHOST_USER_SET_PROTOCOL_FEATURES`` messages this occurs before a
>> +final ``VHOST_USER_SET_FEATURES`` comes from the guest.
>
> I couldn't find any place where vhost-user.rst states that
> VHOST_USER_SET_PROTOCOL_FEATURES has to come before
> VHOST_USER_SET_FEATURES?
>
> The only order I found was:
>
> 1. VHOST_USER_GET_FEATURES to determine whether protocol features are
>    supported.
> 2. VHOST_USER_GET_PROTOCOL_FEATURES to fetch available protocol feature bits.
> 3. VHOST_USER_SET_PROTOCOL_FEATURES to set protocol feature bits.
> 4. Using functionality that depends on enabled protocol feature bits.
>
> Is the purpose of this sentence to add a new requirement to the spec
> that "VHOST_USER_SET_PROTOCOL_FEATURES MUST be sent before
> VHOST_USER_SET_FEATURES"?

No I don't want to add a new sequence requirement. But if SET_FEATURES
doesn't acknowledge the VHOST_USER_F_PROTOCOL_FEATURES bit should that
stop the processing of
VHOST_USER_GET_PROTOCOL_FEATURES/VHOST_USER_SET_PROTOCOL_FEATURES
messages? AFAICT SET_FEATURES should be irrelevant to the negotiation of
the PROTOCOL_FEATURES right?

>> So the
>> +enabling of protocol features need only require the advertising of the
>> +feature by the slave and the successful get/set protocol features
>> +sequence.
>
> "the feature" == VHOST_USER_F_PROTOCOL_FEATURES?

yes.

>
> Stefan


-- 
Alex Bennée

Re: [virtio-dev] [VHOST USER SPEC PATCH] vhost-user.rst: add clarifying language about protocol negotiation
Posted by Stefan Hajnoczi 3 years, 8 months ago
On Mon, Mar 01, 2021 at 11:38:47AM +0000, Alex Bennée wrote:
> Stefan Hajnoczi <stefanha@redhat.com> writes:
> > On Fri, Feb 26, 2021 at 11:16:19AM +0000, Alex Bennée wrote:
> >> +However as the protocol negotiation something that only occurs between
> >
> > Missing "is". Shortening the sentence fixes that without losing clarity:
> > s/something that/negotiation/
> >
> >> +parts of the backend implementation it is permissible to for the master
> >
> > "vhost-user device backend" is often used to refer to the slave (to
> > avoid saying the word "slave") but "backend" is being used in a
> > different sense here. That is confusing.
> >
> >> +to mask the feature bit from the guest.
> >
> > I think this sentence effectively says "the master MAY mask the
> > VHOST_USER_F_PROTOCOL_FEATURES bit from the VIRTIO feature bits". That
> > is not really accurate since VIRTIO devices do not advertise this
> > feature bit and so it can never be negotiated through the VIRTIO feature
> > negotiation process.
> >
> > How about referring to the details from the VIRTIO 1.1 specification
> > instead. Something like this:
> >
> >   Note that VHOST_USER_F_PROTOCOL_FEATURES is the UNUSED (30) feature
> >   bit defined in `VIRTIO 1.1 6.3 Legacy Interface: Reserved Feature Bits
> >   <https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-4130003>`_.
> >   VIRTIO devices do not advertise this feature bit and therefore VIRTIO
> >   drivers cannot negotiate it.
> >
> >   This reserved feature bit was reused by the vhost-user protocol to add
> >   vhost-user protocol feature negotiation in a backwards compatible
> >   fashion. Old vhost-user master and slave implementations continue to
> >   work even though they are not aware of vhost-user protocol feature
> >   negotiation.
> 
> OK - so does that mean that feature bit will remain UNUSED for ever
> more?

It's unlikely to be repurposed in VIRTIO. It can never be used by VIRTIO
in a situation that overlaps with vhost-user. That leaves cases that
don't overlap with vhost-user but that is unlikely too since the bit had
a previous meaning (before vhost-user) and repurposing it would cause
confusion for very old drivers or devices.

> What about other feature bits? Is it permissible for the
> master/requester/vhost-user front-end/QEMU to filter any other feature
> bits the slave/vhost-user backend/daemon may offer from being read by
> the guest driver when it reads the feature bits?

Yes, the vhost-user frontend can decide how it wants to expose
VHOST_USER_GET_FEATURES feature bits on the VIRTIO device:

1. Pass-through. Allow the vhost-user device backend to control the
   feature bit.
2. Disabling. Clear a feature bit because it cannot be supported for
   some reason (e.g. VIRTIO 1.1 packed vrings are not implemented and
   therefore enabling them would prevent live migration).
3. Enabling. Enable a feature bit that does not rely on vhost-user
   device backend support. For example, message-signalled interrupts
   for virtio-mmio.

> 
> >
> >> As noted for the
> >> +``VHOST_USER_GET_PROTOCOL_FEATURES`` and
> >> +``VHOST_USER_SET_PROTOCOL_FEATURES`` messages this occurs before a
> >> +final ``VHOST_USER_SET_FEATURES`` comes from the guest.
> >
> > I couldn't find any place where vhost-user.rst states that
> > VHOST_USER_SET_PROTOCOL_FEATURES has to come before
> > VHOST_USER_SET_FEATURES?
> >
> > The only order I found was:
> >
> > 1. VHOST_USER_GET_FEATURES to determine whether protocol features are
> >    supported.
> > 2. VHOST_USER_GET_PROTOCOL_FEATURES to fetch available protocol feature bits.
> > 3. VHOST_USER_SET_PROTOCOL_FEATURES to set protocol feature bits.
> > 4. Using functionality that depends on enabled protocol feature bits.
> >
> > Is the purpose of this sentence to add a new requirement to the spec
> > that "VHOST_USER_SET_PROTOCOL_FEATURES MUST be sent before
> > VHOST_USER_SET_FEATURES"?
> 
> No I don't want to add a new sequence requirement. But if SET_FEATURES
> doesn't acknowledge the VHOST_USER_F_PROTOCOL_FEATURES bit should that
> stop the processing of
> VHOST_USER_GET_PROTOCOL_FEATURES/VHOST_USER_SET_PROTOCOL_FEATURES
> messages? AFAICT SET_FEATURES should be irrelevant to the negotiation of
> the PROTOCOL_FEATURES right?

I agree, the value of VHOST_USER_F_PROTOCOL_FEATURES in
VHOST_USER_SET_FEATURES does not matter according to the spec:

  Only legal if feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` is
  present in ``VHOST_USER_GET_FEATURES``.

Since it does not mention "set in VHOST_USER_SET_FEATURES" we have to
assume existing vhost-user device backends do not care whether the
vhost-user frontend includes the bit in VHOST_USER_SET_FEATURES or not.

Stefan
Re: [virtio-dev] [VHOST USER SPEC PATCH] vhost-user.rst: add clarifying language about protocol negotiation
Posted by Michael S. Tsirkin 3 years, 8 months ago
On Mon, Mar 01, 2021 at 04:35:51PM +0000, Stefan Hajnoczi wrote:
> On Mon, Mar 01, 2021 at 11:38:47AM +0000, Alex Bennée wrote:
> > Stefan Hajnoczi <stefanha@redhat.com> writes:
> > > On Fri, Feb 26, 2021 at 11:16:19AM +0000, Alex Bennée wrote:
> > >> +However as the protocol negotiation something that only occurs between
> > >
> > > Missing "is". Shortening the sentence fixes that without losing clarity:
> > > s/something that/negotiation/
> > >
> > >> +parts of the backend implementation it is permissible to for the master
> > >
> > > "vhost-user device backend" is often used to refer to the slave (to
> > > avoid saying the word "slave") but "backend" is being used in a
> > > different sense here. That is confusing.
> > >
> > >> +to mask the feature bit from the guest.
> > >
> > > I think this sentence effectively says "the master MAY mask the
> > > VHOST_USER_F_PROTOCOL_FEATURES bit from the VIRTIO feature bits". That
> > > is not really accurate since VIRTIO devices do not advertise this
> > > feature bit and so it can never be negotiated through the VIRTIO feature
> > > negotiation process.
> > >
> > > How about referring to the details from the VIRTIO 1.1 specification
> > > instead. Something like this:
> > >
> > >   Note that VHOST_USER_F_PROTOCOL_FEATURES is the UNUSED (30) feature
> > >   bit defined in `VIRTIO 1.1 6.3 Legacy Interface: Reserved Feature Bits
> > >   <https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-4130003>`_.
> > >   VIRTIO devices do not advertise this feature bit and therefore VIRTIO
> > >   drivers cannot negotiate it.
> > >
> > >   This reserved feature bit was reused by the vhost-user protocol to add
> > >   vhost-user protocol feature negotiation in a backwards compatible
> > >   fashion. Old vhost-user master and slave implementations continue to
> > >   work even though they are not aware of vhost-user protocol feature
> > >   negotiation.
> > 
> > OK - so does that mean that feature bit will remain UNUSED for ever
> > more?
> 
> It's unlikely to be repurposed in VIRTIO. It can never be used by VIRTIO
> in a situation that overlaps with vhost-user. That leaves cases that
> don't overlap with vhost-user but that is unlikely too since the bit had
> a previous meaning (before vhost-user) and repurposing it would cause
> confusion for very old drivers or devices.

Yes, it's easier to just use higher bits.
If it ever is reused we will just send that bit separately.

> > What about other feature bits? Is it permissible for the
> > master/requester/vhost-user front-end/QEMU to filter any other feature
> > bits the slave/vhost-user backend/daemon may offer from being read by
> > the guest driver when it reads the feature bits?
> 
> Yes, the vhost-user frontend can decide how it wants to expose
> VHOST_USER_GET_FEATURES feature bits on the VIRTIO device:
> 
> 1. Pass-through. Allow the vhost-user device backend to control the
>    feature bit.
> 2. Disabling. Clear a feature bit because it cannot be supported for
>    some reason (e.g. VIRTIO 1.1 packed vrings are not implemented and
>    therefore enabling them would prevent live migration).
> 3. Enabling. Enable a feature bit that does not rely on vhost-user
>    device backend support. For example, message-signalled interrupts
>    for virtio-mmio.
> 
> > 
> > >
> > >> As noted for the
> > >> +``VHOST_USER_GET_PROTOCOL_FEATURES`` and
> > >> +``VHOST_USER_SET_PROTOCOL_FEATURES`` messages this occurs before a
> > >> +final ``VHOST_USER_SET_FEATURES`` comes from the guest.
> > >
> > > I couldn't find any place where vhost-user.rst states that
> > > VHOST_USER_SET_PROTOCOL_FEATURES has to come before
> > > VHOST_USER_SET_FEATURES?
> > >
> > > The only order I found was:
> > >
> > > 1. VHOST_USER_GET_FEATURES to determine whether protocol features are
> > >    supported.
> > > 2. VHOST_USER_GET_PROTOCOL_FEATURES to fetch available protocol feature bits.
> > > 3. VHOST_USER_SET_PROTOCOL_FEATURES to set protocol feature bits.
> > > 4. Using functionality that depends on enabled protocol feature bits.
> > >
> > > Is the purpose of this sentence to add a new requirement to the spec
> > > that "VHOST_USER_SET_PROTOCOL_FEATURES MUST be sent before
> > > VHOST_USER_SET_FEATURES"?
> > 
> > No I don't want to add a new sequence requirement. But if SET_FEATURES
> > doesn't acknowledge the VHOST_USER_F_PROTOCOL_FEATURES bit should that
> > stop the processing of
> > VHOST_USER_GET_PROTOCOL_FEATURES/VHOST_USER_SET_PROTOCOL_FEATURES
> > messages? AFAICT SET_FEATURES should be irrelevant to the negotiation of
> > the PROTOCOL_FEATURES right?
> 
> I agree, the value of VHOST_USER_F_PROTOCOL_FEATURES in
> VHOST_USER_SET_FEATURES does not matter according to the spec:
> 
>   Only legal if feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` is
>   present in ``VHOST_USER_GET_FEATURES``.
> 
> Since it does not mention "set in VHOST_USER_SET_FEATURES" we have to
> assume existing vhost-user device backends do not care whether the
> vhost-user frontend includes the bit in VHOST_USER_SET_FEATURES or not.
> 
> Stefan