Re: [PATCH v2 0/3] vhost-user: Lift Max Ram Slots Limitation

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Feb 19, 2020 at 12:33:24AM -0500, Raphael Norwitz wrote:
> On Mon, Feb 10, 2020 at 11:04:28AM -0500, Michael S. Tsirkin wrote:
> > 
> > On Sun, Feb 09, 2020 at 12:14:42PM -0500, Raphael Norwitz wrote:
> > > On Thu, Feb 06, 2020 at 03:33:13AM -0500, Michael S. Tsirkin wrote:
> > > > 
> > > > On Wed, Jan 15, 2020 at 09:57:03PM -0500, Raphael Norwitz wrote:
> > > > > 
> > > > > Changes since V1:
> > > > >     * Kept the assert in vhost_user_set_mem_table_postcopy, but moved it
> > > > >       to prevent corruption
> > > > >     * Made QEMU send a single VHOST_USER_GET_MAX_MEMSLOTS message at
> > > > >       startup and cache the returned value so that QEMU does not need to
> > > > >       query the backend every time vhost_backend_memslots_limit is called.
> > > > 
> > > > I'm a bit confused about what happens on reconnect.
> > > > Can you clarify pls?
> > > > 
> > > >From what I can see, backends which support reconnect call vhost_dev_init,
> > > which then calls vhost_user_backend_init(), as vhost-user-blk does here:
> > > https://github.com/qemu/qemu/blob/master/hw/block/vhost-user-blk.c#L315. The
> > > ram slots limit is fetched in vhost_user_backend_init() so every time the
> > > device reconnects the limit should be refetched. 
> > 
> > Right. Point is, we might have validated using an old limit.
> > Reconnect needs to verify limit did not change or at least
> > did not decrease.
> > 
> > -- 
> > MST
> Good point - I did not consider this case. Could we keep the slots limit in
> the VhostUserState instead?
> 
> Say vhost_user_init() initializes the limit inside the VhostUserState to 0. Then,
> vhost_user_backend_init() checks if this limit is 0. If so, this is the initial
> connection and qemu fetches the limit from the backend, ensures the returned
> value is nonzero, and sets the limit the VhostUserState. If not, qemu knows this
> is a reconnect and queries the backend slots limit. If the returned value does
> not equal the limit in the VhostUserState, vhost_user_backend_init() returns an
> error.
> 
> Thoughts?

Right.
Or if we really want to, check backend value is >= the saved one.
Basically same thing we do with features.

-- 
MST