Re: [Qemu-devel] [PATCH 2/3] migration: Cancel migration at exit

[Peter Xu <peterx@redhat.com>]

On Fri, Sep 15, 2017 at 04:20:26PM +0800, Fam Zheng wrote:

[...]

> > >  void qmp_migrate_set_cache_size(int64_t value, Error **errp)
> > >  {
> > >      MigrationState *s = migrate_get_current();
> > > diff --git a/vl.c b/vl.c
> > > index fb1f05b937..abbe61f40b 100644
> > > --- a/vl.c
> > > +++ b/vl.c
> > > @@ -87,6 +87,7 @@ int main(int argc, char **argv)
> > >  #include "sysemu/blockdev.h"
> > >  #include "hw/block/block.h"
> > >  #include "migration/misc.h"
> > > +#include "migration/savevm.h"
> > >  #include "migration/snapshot.h"
> > >  #include "migration/global_state.h"
> > >  #include "sysemu/tpm.h"
> > > @@ -4799,6 +4800,8 @@ int main(int argc, char **argv, char **envp)
> > >      iothread_stop_all();
> > >  
> > >      pause_all_vcpus();
> > > +    migrate_cancel();
> > 
> > IIUC this is an async cancel, so when reach here the migration thread
> > can still be alive.  Then...
> > 
> > > +    qemu_savevm_state_cleanup();
> > 
> > ... Here calling qemu_savevm_state_cleanup() may be problematic if
> > migration thread has not yet quitted.
> > 
> > I'm thinking whether we should make migrate_fd_cancel() wait until the
> > migration thread finishes (state change to CANCELLED).  Then the
> > migration thread will do the cleanup, and here we can avoid calling
> > qemu_savevm_state_cleanup() as well.
> 
> But if the migration thread is stuck and CANCELLED is never reached, we'll hang
> here?

Maybe we can add timeout. Anyway, if it gets stuck, I do see it a
migration bug, and in that case I'm not sure force return after
timeout would be anything wiser.

Dave/Juan may have better idea though.

-- 
Peter Xu