On Thu, Oct 12, 2023 at 09:20:28PM +0300, Vladimir Sementsov-Ogievskiy wrote: > Hi all! > > We now have a problem in downstream: > > We have a vhost-user server, which doesn't support > VHOST_USER_SET_VRING_ENABLE. As I understand, vhost-user specification allows > it. So the server behaves as follows: > > 1. in GET_FEATURES, it sets VHOST_USER_F_PROTOCOL_FEATURES, to report support > for protocol features. > > 2. if this flag VHOST_USER_F_PROTOCOL_FEATURES is set in SET_FEATURES, it > reports an error > > In my opinion, this doesn't violate the specification, maybe I'm wrong.
well it doesn't but it's a quality of implementation issue. if you set a feature bit in GET_FEATURES you should fully expect clients to set this bit. > Newer QEMU, after commits > > 02b61f38d357490 "hw/virtio: incorporate backend features in features" > and > 4daa5054c599c8a > "vhost: enable vrings in vhost_dev_start() for vhost-user devices" > > actually doesn't support such behavior, as QEMU assumes that if we have the > flag in GET_FEATURES, it is supported by SET_FEATURES. > > > I don't see any possibility to clearly support in QEMU both servers which > supports vring enable/disable and older servers that doesn't. > > ===== > > I also want to clarify, > > 1. is "feature negotiated" means set both in GET_FEATURES and SET_FEATURES, > as this term is not directly defined. generally both but yes, it is unfortunately vague in virtio spec too. > 2. What VHOST_USER_SET_VRING_ENABLE should do for vhost-user-blk? What meant > by "side effects" in this case? All IO requests should just fail, not > touching the actual storage? we discussed this recently upstream. I think most devices should just not process the ring. network is one of exceptions where you can harmlessly discard packets and guest does not care much. > 3. Except for VHOST_USER_SET_VRING_ENABLE, there is another command require > not just VHOST_USER_F_PROTOCOL_FEATURES being present in GET_FEATURES, but > "negotiated" i.e., as I understand being then set in SET_OPTIONS by client. > That's VHOST_USER_SET_SLAVE_REQ_FD. Why? Is it a mistake? generally frontend must set a bit before using it. this way backend knows what is going to be used. > 4. Also, in VHOST_USER_SET_FEATURES definition: > > Feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` signals > back-end support for ``VHOST_USER_GET_PROTOCOL_FEATURES`` and > ``VHOST_USER_SET_PROTOCOL_FEATURES``. > > > That seems wrong, as the flag in SET command is about vring enable/disable > related behavior. I think we meant frontend use not backend support. > -- > Best regards, > Vladimir
