From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:56465)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <amit.shah@redhat.com>) id 1ZHSAw-0007Rb-17
	for qemu-devel@nongnu.org; Tue, 21 Jul 2015 03:41:18 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <amit.shah@redhat.com>) id 1ZHSAq-0000i6-UV
	for qemu-devel@nongnu.org; Tue, 21 Jul 2015 03:41:17 -0400
Received: from mx1.redhat.com ([209.132.183.28]:44754)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <amit.shah@redhat.com>) id 1ZHSAq-0000hN-PA
	for qemu-devel@nongnu.org; Tue, 21 Jul 2015 03:41:12 -0400
Date: Tue, 21 Jul 2015 13:10:47 +0530
From: Amit Shah <amit.shah@redhat.com>
Message-ID: <20150721074047.GF10280@grmbl.mre>
References: <1434450415-11339-1-git-send-email-dgilbert@redhat.com>
	<1434450415-11339-23-git-send-email-dgilbert@redhat.com>
	<87380sl2rx.fsf@neno.neno> <20150713171340.GK2492@work-vm>
	<87mvz0hqx3.fsf@neno.neno>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87mvz0hqx3.fsf@neno.neno>
Subject: Re: [Qemu-devel] [PATCH v7 22/42] migrate_start_postcopy: Command
 to trigger transition to postcopy
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Juan Quintela <quintela@redhat.com>
Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, liang.z.li@intel.com, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, qemu-devel@nongnu.org, luis@cs.umu.se, pbonzini@redhat.com, david@gibson.dropbear.id.au

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.


		Amit