Re: [PATCH v4 3/8] vhost-user.rst: Clarify enabling/disabling vrings

[Hanna Czenczek <hreitz@redhat.com>]

On 18.10.23 14:14, Michael S. Tsirkin wrote:
> On Wed, Oct 04, 2023 at 02:58:59PM +0200, Hanna Czenczek wrote:
>> Currently, the vhost-user documentation says that rings are to be
>> initialized in a disabled state when VHOST_USER_F_PROTOCOL_FEATURES is
>> negotiated.  However, by the time of feature negotiation, all rings have
>> already been initialized, so it is not entirely clear what this means.
>>
>> At least the vhost-user-backend Rust crate's implementation interpreted
>> it to mean that whenever this feature is negotiated, all rings are to
>> put into a disabled state, which means that every SET_FEATURES call
>> would disable all rings, effectively halting the device.  This is
>> problematic because the VHOST_F_LOG_ALL feature is also set or cleared
>> this way, which happens during migration.  Doing so should not halt the
>> device.
>>
>> Other implementations have interpreted this to mean that the device is
>> to be initialized with all rings disabled, and a subsequent SET_FEATURES
>> call that does not set VHOST_USER_F_PROTOCOL_FEATURES will enable all of
>> them.  Here, SET_FEATURES will never disable any ring.
>>
>> This interpretation does not suffer the problem of unintentionally
>> halting the device whenever features are set or cleared, so it seems
>> better and more reasonable.
>>
>> We can clarify this in the documentation by making it explicit that the
>> enabled/disabled state is tracked even while the vring is stopped.
>> Every vring is initialized in a disabled state, and SET_FEATURES without
>> VHOST_USER_F_PROTOCOL_FEATURES simply becomes one way to enable all
>> vrings.
>>
>> Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
>
> OK so I am expecting v5. My advice is to move patch 1 to end of patchset
> so we can defer it if we want to.

Already sent – I’ve just dropped patch 1, since it doesn’t add anything 
to the objective of the patch series itself:

https://lists.nongnu.org/archive/html/qemu-devel/2023-10/msg04727.html

Hanna