Re: [Qemu-devel] [PATCH v4 31/32] migration, qmp: new command "migrate-pause"

[Peter Xu]

On Mon, Dec 04, 2017 at 05:10:29PM +0000, Dr. David Alan Gilbert wrote:
> * Peter Xu (address@hidden) wrote:
> > On Fri, Dec 01, 2017 at 04:53:28PM +0000, Dr. David Alan Gilbert wrote:
> > > * Peter Xu (address@hidden) wrote:
> > > > It is used to manually trigger the postcopy pause state.  It works just
> > > > like when we found the migration stream failed during postcopy, but
> > > > provide an explicit way for user in case of misterious socket hangs.
> > > > 
> > > > Signed-off-by: Peter Xu <address@hidden>
> > > 
> > > Can we change the name to something like 'migrate-disconnect' - pause
> > > is a bit easy to confuse with other things and this is really more
> > > an explicit network disconnect (Is it worth just making it a flag to
> > > migrate-cancel?)
> > 
> > Then I would prefer to reuse the migrate_cancel command.  
> > 
> > Actually this reminded me about what would happen now if someone on
> > src VM sends a "migrate_cancel" during postcopy active.  It should
> > crash the VM, right?
> > 
> > Considering above, I'm thinking whether we should just make it a
> > default behavior that when do migrate_cancel during postcopy-active we
> > just do a pause instead of real cancel. After all it cannot re-start
> > the VM any more on source, so IMHO a real cancel does not mean much
> > here.  More importantly, what if someone wants to manually trigger
> > this pause but accidentally forgot to type that new flag (say,
> > -D[isconnect])?  It'll crash the VM directly.
> > 
> > What do you think?
> 
> Yes, that's OK, just be careful about race conditions between the
> states,  for example what happens if you do a cancel and you enter
> migrate_fd_cancel in postcopy-active, but before you can actually
> cancel you end up completing,

If I am going to modify that, migrate_fd_cancel won't be called if we
are in postcopy-active state, instead we'll just do the disconnect
only.

For finally solving all the races between QMP commands and migration
thread, I do think (again) that we need locks or some other sync
method.  I really hope we can have this fixed in QEMU 2.12.  Basically
we will need to go over every migration command to see whether it'll
need to take the migration lock (to be added) or not.  With that,
it'll save a lot of our future time IMHO thinking about races.

> or the opposite where you do a
> migrate-start-postcopy almost immediately before migrade-cancel;
> do you get to cancel in teh active or postcopy-active state?

This is a good example that at least migrate-start-postcopy is
synchronized somehow nicely with the migration thread using a single
variable (actually it can be non-atomic operation I think, anyway, no
race is here as long as we are delivering the message via a single
variable, and qmp command is the only writter).  For this command I
think it's pretty safe.  After all, user should not run that command
too fast if he/she wants a paused postcopy, at least he/she should do
query-migration before that to make sure the state is postcopy-active.
So IMHO this is totally fine.

Thanks,

-- 
Peter Xu