From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:34545)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1SaPYL-0006aj-8n
	for qemu-devel@nongnu.org; Fri, 01 Jun 2012 06:58:02 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1SaPYG-0007Y5-8w
	for qemu-devel@nongnu.org; Fri, 01 Jun 2012 06:57:56 -0400
Received: from mx1.redhat.com ([209.132.183.28]:21524)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1SaPYG-0007Xp-0Z
	for qemu-devel@nongnu.org; Fri, 01 Jun 2012 06:57:52 -0400
From: Juan Quintela <quintela@redhat.com>
In-Reply-To: <1337691425-6022-3-git-send-email-owasserm@redhat.com> (Orit
	Wasserman's message of "Tue, 22 May 2012 15:56:58 +0300")
References: <1337691425-6022-1-git-send-email-owasserm@redhat.com>
	<1337691425-6022-3-git-send-email-owasserm@redhat.com>
Date: Fri, 01 Jun 2012 12:57:31 +0200
Message-ID: <871ulz9u5g.fsf@elfo.elfo>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v11 2/9] Add migration capabilites
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: Orit Wasserman <owasserm@redhat.com>
Cc: peter.maydell@linaro.org, aliguori@us.ibm.com, stefanha@gmail.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, blauwirbel@gmail.com, chegu_vinod@hp.com, avi@redhat.com, pbonzini@redhat.com, eblake@redhat.com

Orit Wasserman <owasserm@redhat.com> wrote:
> Add migration capabiltes that can be queried by the management.
> The managment can query the source QEMU and the destination QEMU in order to
> verify both support some  migration capability (currently only XBZRLE).
> The managment can enable a capabilty for the next migration by using
> migrate_set_parameter command.
>
> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
> +void qmp_migrate_set_parameter(const char *parameter, Error **errp)
> +{
> +    MigrationState *s = migrate_get_current();
> +    int i;
> +
> +    if (s->state == MIG_STATE_ACTIVE) {
> +        error_set(errp, QERR_MIGRATION_ACTIVE);
> +        return;
> +    }
> +
> +    for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
> +        if (strcmp(parameter, MigrationCapability_lookup[i]) == 0) {
> +            s->enabled_capabilities[i] = true;
> +            return;
> +        }
> +    }
> +
> +    error_set(errp, QERR_INVALID_PARAMETER, parameter);
> +}

Two things here:
- Is there a way to disable capabilities?  it seems no.
- Would we want in the future capabilities that are not "bool"?  Just
  asking loud, I haven't thought a lot about this.  Fixing it as a
  paramenter, it would make trivial to fix previous comment: cap:true vs
  cap:false, or whatever syntax we want.

>      memset(s, 0, sizeof(*s));
>      s->bandwidth_limit = bandwidth_limit;
>      s->params = *params;
> +    memcpy(s->enabled_capabilities, enabled_capabilities,
> +           sizeof(enabled_capabilities));
>  
> -    s->bandwidth_limit = bandwidth_limit;
>      s->state = MIG_STATE_SETUP;

Nice catch/cleanup.


> diff --git a/savevm.c b/savevm.c
> index dd66f2c..42937a0 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1711,7 +1711,7 @@ static int qemu_savevm_state(QEMUFile *f)
>      int ret;
>      MigrationParams params = {
>          .blk = 0,
> -        .shared = 0
> +        .shared = 0,
>      };
>  
>      if (qemu_savevm_state_blocked(NULL)) {

This belongs to previous patch?

Later, Juan.