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

[Juan Quintela]

Orit Wasserman <address@hidden> wrote:
> On 06/01/2012 01:57 PM, Juan Quintela wrote:
>> 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.
>
> In this implementation we can't disable a capability , do you see a
> need to add it ?

As we continue adding capabilities, I guess that at least for
testing. it is going to be needed.  Specially if we decide the path that
Anthony suggested:

set_capababilities(interesction(caps_source, caps_target))

if we do something like that, and we _know_ that we don't want a
capabilitie because we know it dont' work for our load, it sounds like a
good idea to have a good way, and the other reason is the next comment.
If we could have a capability that is _not_ a bool, we need to be able
to assign a value anyways.  Notice that I still don't know if we are
going to need it.  But can see one reason one way or another.

>
>> - 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.
> That is a good idea I will change it in next patch set.

Thanks, Juan.