From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:46992)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1YLEAx-0003CQ-HQ
	for qemu-devel@nongnu.org; Tue, 10 Feb 2015 12:00:55 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1YLEAc-0000ZY-Kr
	for qemu-devel@nongnu.org; Tue, 10 Feb 2015 12:00:39 -0500
Received: from mx1.redhat.com ([209.132.183.28]:40371)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1YLEAc-0000Yr-D5
	for qemu-devel@nongnu.org; Tue, 10 Feb 2015 12:00:18 -0500
From: Juan Quintela <quintela@redhat.com>
In-Reply-To: <1423584999-13946-3-git-send-email-dgilbert@redhat.com> (David
	Alan Gilbert's message of "Tue, 10 Feb 2015 16:16:38 +0000")
References: <1423584999-13946-1-git-send-email-dgilbert@redhat.com>
	<1423584999-13946-3-git-send-email-dgilbert@redhat.com>
Date: Tue, 10 Feb 2015 18:00:16 +0100
Message-ID: <87386ditwf.fsf@neno.neno>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH 2/3] Add migrate -u option for -incoming
	pause
Reply-To: quintela@redhat.com
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: "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com>
Cc: liang.z.li@intel.com, qemu-devel@nongnu.org, amit.shah@redhat.com, pbonzini@redhat.com

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Once a qemu has been started with -incoming pause   the
> migration can be started by issuing:
>
> migrate -u uri
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> -		      "(base image shared between src and destination)",
> +		      "(base image shared between src and destination)"
> +                      "\n\t\t\t -u unpauses an incoming migration started with "
> +                      "-incoming pause using the given uri.",

Spaces vs tabs.

> +        -u to unpause an incoming migration started with -incoming pause

more spaces

> -    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err);
> +    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, !!unpause, unpause,
> +                &err);

I don't claim to understand QMP, but this whole bussines of !!foo, foo
is getting confusing, no?

No, this is not relaced to this patch.

>  {
>      Error *local_err = NULL;
>      MigrationState *s = migrate_get_current();
> @@ -450,6 +450,25 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>          return;
>      }
>  

I would preffer something like:

    if (runstate_check(RUN_STATE_INMIGRATE)) {
        if (unpause) {
            ... unpause code
        }
    } else {
        error_setg(errp, "Guest is waiting for an incoming migration");
        return;
    }

    if (unpause) {
        error_setg(errp, "Guest is waiting for an incoming migration");
        return;
    }

    if (s->state == MIG_STATE_ACTIVE || s->state == MIG_STATE_SETUP ||
        s->state == MIG_STATE_CANCELLING) {
        error_set(errp, QERR_MIGRATION_ACTIVE);
        return;
    }

    if (qemu_savevm_state_blocked(errp)) {
        return;
    }

.... and now continue with the rest ...




Thinking more about this problem, I am not sure this is the "cleanest
approach".   What do you think of:

- create RUN_STATE_INMIGRATE_PAUSED
  bonus: no need of paused_incoming variable

- create a new migrate_incoming command

And then we have cleaner separation of what we are doing?

Later, Juan.