Re: [Qemu-devel] [PATCH v11 2/9] Add migration capabilites

[Juan Quintela]

Orit Wasserman <address@hidden> 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 <address@hidden>
> +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.