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 <address@hidden>
> > Signed-off-by: Yang Hongyang <address@hidden>
> > Signed-off-by: Gonglei <address@hidden>
> > Signed-off-by: Lai Jiangshan <address@hidden>
> > ---
> >   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