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

[zhanghailiang]

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.zhanghaili...@huawei.com>
Signed-off-by: Yang Hongyang <yan...@cn.fujitsu.com>
Signed-off-by: Gonglei <arei.gong...@huawei.com>
Signed-off-by: Lai Jiangshan <la...@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