Re: [Qemu-devel] [PATCH v7 22/42] migrate_start_postcopy: Command to trigger transition to postcopy

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Amit Shah (amit.shah@redhat.com) wrote:
> On (Mon) 13 Jul 2015 [20:07:52], Juan Quintela wrote:
> > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> > > * Juan Quintela (quintela@redhat.com) wrote:
> 
> > >> > +void qmp_migrate_start_postcopy(Error **errp)
> > >> > +{
> > >> > +    MigrationState *s = migrate_get_current();
> > >> > +
> > >> > +    if (!migrate_postcopy_ram()) {
> > >> > +        error_setg(errp, "Enable postcopy with migration_set_capability before"
> > >> > +                         " the start of migration");
> > >> > +        return;
> > >> > +    }
> > >> > +
> > >> > +    if (s->state == MIGRATION_STATUS_NONE) {
> > >> 
> > >> I would claim that this check should be:
> > >> 
> > >>     if (s->state != MIGRATION_STATUS_ACTIVE) {
> > >> ??
> > >> 
> > >> FAILED, COMPLETED, CANCELL* don't make sense, right?
> > >
> > > What I'm trying to catch here is people doing:
> > >      migrate_start_postcopy
> > >      migrate tcp:pppp:whereever
> > >
> > >   which wont work, because migrate_init reinitialises
> > > the flag that start previously set.
> > >
> > > However, I also don't want to create a race, since what you do is
> > > typically:
> > >      migrate  tcp:pppp:whereever
> > >    <wait some time, get bored>
> > >      migrate_start_postcopy
> > >
> > > if you're unlucky, and the migration finishes just
> > > at the same time you do the migrate_start_postcopy, do you
> > > want migrate_start_postcopy to fail?  My guess was it
> > > was best for it not to fail, in this case.
> > 
> > Change the order, if it is ACTIVE: do the postcopy thing, otherwise, do
> > the clause that is protected now?  Moving to postcopy only make sense if
> > we are in active.
> 
> Yeah, I tend to agree, because in the cases where migration has failed
> or has been cancelled, we'll end up setting the postcopy bit.  Then,
> upon the next migration, this bit could get reused - resulting in the
> previous condition of setting postcopy bit before starting migration.

No, that doesn't happen;  the bit is cleared at the start of migration
so that race condition doesn't exist.

Dave

> 
> 
> 		Amit
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK