Re: [PATCH v3 5/5] vhost-user: add skip_drain param to GET_VRING_BASE

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3396 bytes --]

On Mon, May 25, 2026 at 05:34:37PM -0400, Raphael Norwitz wrote:
> Apologies for the late reply.
> 
> On 5/12/26 1:55 AM, Alexandr Moshkov wrote:
> > Gentle ping :)
> > 
> 
> [...]
> 
> > > > > > On 4/16/26 5:26 AM, Alexandr Moshkov wrote:
> > > > > > > Greetings! Thanks for reply!
> > > > > > > 
> > > > > > > 
> > > > > > > On 4/15/26 20:21, Raphael Norwitz wrote:
> > > > > > > > I’m not sure I like using the num field in
> > > > > > > > vhost_vring_state to set magic values which
> > > > > > > > affect protocol behavior for GET_VRING_BASE. It
> > > > > > > > feels like a hack to me. I would think the
> > > > > > > > proper solution if we want to support migration
> > > > > > > > from new to old would be to have new use a
> > > > > > > > different new message entirely. Can we do that?
> > > > > > > 
> > > > > > > I think we can, but I thought at first that this
> > > > > > > will be almost a complete copy of GET_VRING_BASE
> > > > > > > message, with the exception of waiting for drain of
> > > > > > > requests, so I choose to expand existing message.
> > > > > > > 
> > > > > > 
> > > > > > I think a new message would be cleaner. Anyone else have thoughts?
> > > > > > 
> > > > > > > If this is not an appropriate approach, is it better
> > > > > > > to make a new message like GET_VRING_BASE or a
> > > > > > > separate message used together with default
> > > > > > > GET_VRING_BASE message (for example, message for
> > > > > > > setting some kind of status on the server)? What
> > > > > > > should I name this new message?
> > > > > > > 
> > > > > > 
> > > > > > Maybe GET_VRING_BASE_SKIP_DRAIN? How would a separate
> > > > > > message used together with default GET_VRING_BASE
> > > > > > message work?
> > > > > 
> > > > > I was thinking about a message something like
> > > > > SET_VRING_ENABLE - for example SET_SKIP_DRAIN_ENABLE, that
> > > > > would enable/disable some state (skip_drain) in the backend.
> > > > 
> > > > I'd need to see the flow in more detail but sounds promising.
> > > 
> > > On migration vhost-user-blk firstly send SET_SKIP_DRAIN_ENABLE with
> > > num = 1 message if `inflight-migration` device parameter
> > > and VHOST_USER_PROTOCOL_F_GET_VRING_BASE_INFLIGHT enabled. Then send
> > > GET_VRING_BASE and continue work as usual.
> > > 
> > > On SET_SKIP_DRAIN_ENABLE backend sets inner state (skip_drain) so
> > > when GET_VRING_BASE is called and skip_drain is true the drain will
> > > skipped.
> > > 
> > > What do you think?
> > > 
> 
> I'm happy with that in theory but would like more clarity on the details. In
> particular, I'm not sure what you mean by "vhost-user-blk firstly send
> SET_SKIP_DRAIN_ENABLE". To add a new message I would think we would have to
> do a vhost-user protocol feature negotiation to gate it.
> 
> Also what are the precise semantics for SET_SKIP_DRAIN_ENABLE? Would it be
> sent on backend connect or when we're about to migrate?
> 
> I was hoping others would comment but at this point I'd suggest drafting the
> code and then we can re-review.

Adding a new GET_VRING_BASE_SKIP_DRAIN message seems simpler to me than
a stateful SET_SKIP_DRAIN_ENABLE where the back-end needs to stash the
enable_skip_drain state and the front-end would have to toggle the state
if it switches between skip drain and classic behavior.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]