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
