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

[Amit Shah]

On (Mon) 13 Jul 2015 [20:07:52], Juan Quintela wrote:
> "Dr. David Alan Gilbert" <address@hidden> wrote:
> > * Juan Quintela (address@hidden) 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.


                Amit