On Wed, Jul 12, 2023 at 11:17:04AM +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 be > 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.
Huh. I don't know why we don't set VHOST_USER_F_PROTOCOL_FEATURES on all calls though. I think it's a bug. Let's fix that first of all? Then we can still document behaviour of existing buggy QEMU. > This other 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 should clarify this in the documentation. > > Signed-off-by: Hanna Czenczek <hre...@redhat.com> > --- > docs/interop/vhost-user.rst | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst > index 5a070adbc1..ca0e899765 100644 > --- a/docs/interop/vhost-user.rst > +++ b/docs/interop/vhost-user.rst > @@ -383,12 +383,23 @@ and stop ring upon receiving > ``VHOST_USER_GET_VRING_BASE``. > > Rings can be enabled or disabled by ``VHOST_USER_SET_VRING_ENABLE``. > > -If ``VHOST_USER_F_PROTOCOL_FEATURES`` has not been negotiated, the > -ring starts directly in the enabled state. > - > -If ``VHOST_USER_F_PROTOCOL_FEATURES`` has been negotiated, the ring is > -initialized in a disabled state and is enabled by > -``VHOST_USER_SET_VRING_ENABLE`` with parameter 1. > +Between initialization and the first ``VHOST_USER_SET_FEATURES`` call, it > +is implementation-defined whether each ring is enabled or disabled. > + > +If ``VHOST_USER_SET_FEATURES`` does not negotiate > +``VHOST_USER_F_PROTOCOL_FEATURES``, each ring, when started, will be > +enabled immediately. > + > +If ``VHOST_USER_SET_FEATURES`` does negotiate > +``VHOST_USER_F_PROTOCOL_FEATURES``, each ring will remain in the disabled > +state until ``VHOST_USER_SET_VRING_ENABLE`` enables it with parameter 1. > + > +Back-end implementations that support ``VHOST_USER_F_PROTOCOL_FEATURES`` > +should implement this by initializing each ring in a disabled state, and > +enabling them when ``VHOST_USER_SET_FEATURES`` is used without > +negotiating ``VHOST_USER_F_PROTOCOL_FEATURES``. Other than that, rings > +should only be enabled and disabled through > +``VHOST_USER_SET_VRING_ENABLE``. > > While processing the rings (whether they are enabled or not), the back-end > must support changing some configuration aspects on the fly. > -- > 2.41.0
