Re: [Qemu-devel] [PATCH RFC v3 02/27] migration: Introduce capability 'colo' to migration

[zhanghailiang <zhang.zhanghailiang@huawei.com>]

On 2015/2/17 5:57, Eric Blake wrote:
> On 02/11/2015 08:16 PM, zhanghailiang wrote:
>> This capability allows Primary VM (PVM) to be continuously checkpointed
>> to secondary VM.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
>> ---
>>   include/migration/migration.h |  1 +
>>   migration/migration.c         | 15 +++++++++++++++
>>   qapi-schema.json              |  5 ++++-
>>   3 files changed, 20 insertions(+), 1 deletion(-)
>>
>
>> +++ b/migration/migration.c
>> @@ -276,6 +276,15 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
>>       }
>>
>>       for (cap = params; cap; cap = cap->next) {
>> +#ifndef CONFIG_COLO
>> +        if (cap->value->capability == MIGRATION_CAPABILITY_COLO &&
>> +            cap->value->state) {
>> +            error_setg(errp, "COLO is not currently supported, please"
>> +                             " configure with --enable-colo option in order to"
>> +                             " support COLO feature");
>> +            continue;
>> +        }
>> +#endif
>>           s->enabled_capabilities[cap->value->capability] = cap->value->state;
>>       }
>
> Yuck.  This means that probing whether colo is supported requires a
> usage test (try setting the capability with migrate-set-capabilities and
> see if it fails) instead of a query test (list the current capabilities;
> if colo is in the set then it is supported).  Can you figure out a way
> to avoid exposing the colo capability if !CONFIG_COLO, so that
> query-migate-capabilities is sufficient to learn if colo is supported?
>

What about using colo_supported() function to instead of compile macro ?
Like what we have done in v2 version:
in colo.c

bool colo_supported(void)
{
     return true;
}

in stubs/migration-colo.c
bool colo_supported(void)
{
     return false;
}

And then we call this function in qmp_migrate_set_capabilities
@@ -292,15 +295,13 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
      }

      for (cap = params; cap; cap = cap->next) {
-#ifndef CONFIG_COLO
          if (cap->value->capability == MIGRATION_CAPABILITY_COLO &&
-            cap->value->state) {
+            cap->value->state && !colo_supported()) {
              error_setg(errp, "COLO is not currently supported, please"
                               " configure with --enable-colo option in order to"
                               " support COLO feature");
              continue;
          }
-#endif
          s->enabled_capabilities[cap->value->capability] = cap->value->state;
      }
  }

For qmp_query_migrate_capabilities we call it like:

@@ -158,6 +158,9 @@ MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp)

      caps = NULL; /* silence compiler warning */
      for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
+        if (i == MIGRATION_CAPABILITY_COLO && !colo_supported()) {
+            continue;
+        }

Thanks,
zhanghailiang