| Message ID | 1441182199-8328-3-git-send-email-zhang.zhanghailiang@huawei.com |
|---|---|
| State | New |
| Headers | show |
On 09/02/2015 02:22 AM, zhanghailiang wrote: > We add helper function colo_supported() to indicate whether > colo is supported or not, with which we use to control whether or not > showing 'colo' string to users, they can use qmp command > 'query-migrate-capabilities' or hmp command 'info migrate_capabilities' > to learn if colo is supported. > > Cc: Juan Quintela <quintela@redhat.com> > Cc: Amit Shah <amit.shah@redhat.com> > Cc: Eric Blake <eblake@redhat.com> > Cc: Markus Armbruster <armbru@redhat.com> > 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> > --- > +++ b/migration/colo.c > @@ -0,0 +1,18 @@ > +/* > + * COarse-grain LOck-stepping Virtual Machines for Non-stop Service (COLO) > + * (a.k.a. Fault Tolerance or Continuous Replication) > + * > + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO., LTD. > + * Copyright (c) 2015 FUJITSU LIMITED > + * Copyright (c) 2015 Intel Corporation > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or > + * later. See the COPYING file in the top-level directory. > + */ > + > +#include "migration/colo.h" > + > +bool colo_supported(void) > +{ > + return true; > +} So this exists because you have a configure option that says whether to include or exclude this .o file (the backup stub version returns false if this file is not included)... > diff --git a/migration/migration.c b/migration/migration.c > index 662e77e..593cac0 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -29,6 +29,7 @@ > #include "trace.h" > #include "qapi/util.h" > #include "qapi-event.h" > +#include "migration/colo.h" > > #define MAX_THROTTLE (32 << 20) /* Migration speed throttling */ > > @@ -345,6 +346,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; > + } ...and here, you use the result to explicitly remove the colo migration property from the runtime result of query-migrate-capabilities if support was not compiled in. That's a good thing, because it means that a client can call query-migrate-capabilities, and immediately know if qemu supports colo by whether the migration property is present. Especially since the new query-qmp-schema WILL show colo as a member of the enum of possible values. Knowing that the enum supports a value doesn't tell you whether runtime supports use of the value, so your approach definitely helps in a place where static introspection doesn't solve everything. > if (head == NULL) { > head = g_malloc0(sizeof(*caps)); > caps = head; > @@ -485,6 +489,13 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, > } > > for (cap = params; cap; cap = cap->next) { > + if (cap->value->capability == MIGRATION_CAPABILITY_COLO && > + !colo_supported()) { > + error_setg(errp, "COLO is not currently supported, please" > + " configure with --enable-colo option in order to" > + " support COLO feature"); > + continue; Likewise, you explicitly reject attempts to change the property, where introspection says the enum supports the property. It might be acceptable to silently permit attempts to set the capability to false (the value it already has) and only reject attempts to set it to true, but I'm not sure if the difference is worth it. > + } > s->enabled_capabilities[cap->value->capability] = cap->value->state; > } > } > @@ -913,6 +924,12 @@ int64_t migrate_xbzrle_cache_size(void) > return s->xbzrle_cache_size; > } > > +bool migrate_enable_colo(void) > +{ > + MigrationState *s = migrate_get_current(); > + return s->enabled_capabilities[MIGRATION_CAPABILITY_COLO]; Function name is wrong - it sounds like an imperative command (call this to enable colo), when it is really a query command (call this to learn if colo is enabled). Maybe migrate_colo_enabled()? > +} > + > /* migration thread support */ > > static void *migration_thread(void *opaque) > diff --git a/qapi-schema.json b/qapi-schema.json > index 5f45571..b14d1f4 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -529,11 +529,15 @@ > # @auto-converge: If enabled, QEMU will automatically throttle down the guest > # to speed up convergence of RAM migration. (since 1.6) > # > +# @colo: If enabled, migration will never end, and the state of the VM on the > +# primary side will be migrated continuously to the VM on secondary > +# side. (since 2.5) You may want to name this property 'x-colo' to mark it experimental; especially since there are still a lot of other things waiting to go in and we are getting closer to 2.5 freeze. Making the property experimental will relieve the pressure of having to get everything right on the first try, although it also means that libvirt won't use the property until we graduate it from experimental 'x-colo' to fully-supported 'colo'.
On 2015/10/3 0:02, Eric Blake wrote: > On 09/02/2015 02:22 AM, zhanghailiang wrote: >> We add helper function colo_supported() to indicate whether >> colo is supported or not, with which we use to control whether or not >> showing 'colo' string to users, they can use qmp command >> 'query-migrate-capabilities' or hmp command 'info migrate_capabilities' >> to learn if colo is supported. >> >> Cc: Juan Quintela <quintela@redhat.com> >> Cc: Amit Shah <amit.shah@redhat.com> >> Cc: Eric Blake <eblake@redhat.com> >> Cc: Markus Armbruster <armbru@redhat.com> >> 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> >> --- > >> +++ b/migration/colo.c >> @@ -0,0 +1,18 @@ >> +/* >> + * COarse-grain LOck-stepping Virtual Machines for Non-stop Service (COLO) >> + * (a.k.a. Fault Tolerance or Continuous Replication) >> + * >> + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO., LTD. >> + * Copyright (c) 2015 FUJITSU LIMITED >> + * Copyright (c) 2015 Intel Corporation >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or >> + * later. See the COPYING file in the top-level directory. >> + */ >> + >> +#include "migration/colo.h" >> + >> +bool colo_supported(void) >> +{ >> + return true; >> +} > > So this exists because you have a configure option that says whether to > include or exclude this .o file (the backup stub version returns false > if this file is not included)... > >> diff --git a/migration/migration.c b/migration/migration.c >> index 662e77e..593cac0 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -29,6 +29,7 @@ >> #include "trace.h" >> #include "qapi/util.h" >> #include "qapi-event.h" >> +#include "migration/colo.h" >> >> #define MAX_THROTTLE (32 << 20) /* Migration speed throttling */ >> >> @@ -345,6 +346,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; >> + } > > ...and here, you use the result to explicitly remove the colo migration > property from the runtime result of query-migrate-capabilities if > support was not compiled in. That's a good thing, because it means that > a client can call query-migrate-capabilities, and immediately know if > qemu supports colo by whether the migration property is present. > > Especially since the new query-qmp-schema WILL show colo as a member of > the enum of possible values. Knowing that the enum supports a value > doesn't tell you whether runtime supports use of the value, so your > approach definitely helps in a place where static introspection doesn't > solve everything. > Yes, that is the reason we add it. >> if (head == NULL) { >> head = g_malloc0(sizeof(*caps)); >> caps = head; >> @@ -485,6 +489,13 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, >> } >> >> for (cap = params; cap; cap = cap->next) { >> + if (cap->value->capability == MIGRATION_CAPABILITY_COLO && >> + !colo_supported()) { >> + error_setg(errp, "COLO is not currently supported, please" >> + " configure with --enable-colo option in order to" >> + " support COLO feature"); >> + continue; > > Likewise, you explicitly reject attempts to change the property, where > introspection says the enum supports the property. It might be > acceptable to silently permit attempts to set the capability to false > (the value it already has) and only reject attempts to set it to true, > but I'm not sure if the difference is worth it. > Ha, maybe you have forgotten, in last version, you pointed out that maybe it was better to give this error message regardless of true/false being requested, and i think it is reasonable, so i will keep it like this ;) >> + } >> s->enabled_capabilities[cap->value->capability] = cap->value->state; >> } >> } >> @@ -913,6 +924,12 @@ int64_t migrate_xbzrle_cache_size(void) >> return s->xbzrle_cache_size; >> } >> >> +bool migrate_enable_colo(void) >> +{ >> + MigrationState *s = migrate_get_current(); >> + return s->enabled_capabilities[MIGRATION_CAPABILITY_COLO]; > > Function name is wrong - it sounds like an imperative command (call this > to enable colo), when it is really a query command (call this to learn > if colo is enabled). Maybe migrate_colo_enabled()? > Sounds reasonable, will fix in next version. >> +} >> + >> /* migration thread support */ >> >> static void *migration_thread(void *opaque) >> diff --git a/qapi-schema.json b/qapi-schema.json >> index 5f45571..b14d1f4 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -529,11 +529,15 @@ >> # @auto-converge: If enabled, QEMU will automatically throttle down the guest >> # to speed up convergence of RAM migration. (since 1.6) >> # >> +# @colo: If enabled, migration will never end, and the state of the VM on the >> +# primary side will be migrated continuously to the VM on secondary >> +# side. (since 2.5) > > You may want to name this property 'x-colo' to mark it experimental; > especially since there are still a lot of other things waiting to go in > and we are getting closer to 2.5 freeze. Making the property > experimental will relieve the pressure of having to get everything right > on the first try, although it also means that libvirt won't use the > property until we graduate it from experimental 'x-colo' to > fully-supported 'colo'. > Got it, will fix it in next version, thanks.
diff --git a/include/migration/colo.h b/include/migration/colo.h new file mode 100644 index 0000000..c60a590 --- /dev/null +++ b/include/migration/colo.h @@ -0,0 +1,20 @@ +/* + * COarse-grain LOck-stepping Virtual Machines for Non-stop Service (COLO) + * (a.k.a. Fault Tolerance or Continuous Replication) + * + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO., LTD. + * Copyright (c) 2015 FUJITSU LIMITED + * Copyright (c) 2015 Intel Corporation + * + * This work is licensed under the terms of the GNU GPL, version 2 or + * later. See the COPYING file in the top-level directory. + */ + +#ifndef QEMU_COLO_H +#define QEMU_COLO_H + +#include "qemu-common.h" + +bool colo_supported(void); + +#endif diff --git a/include/migration/migration.h b/include/migration/migration.h index 8334621..05de3a1 100644 --- a/include/migration/migration.h +++ b/include/migration/migration.h @@ -170,6 +170,7 @@ int xbzrle_decode_buffer(uint8_t *src, int slen, uint8_t *dst, int dlen); int migrate_use_xbzrle(void); int64_t migrate_xbzrle_cache_size(void); +bool migrate_enable_colo(void); int64_t xbzrle_cache_resize(int64_t new_size); diff --git a/migration/Makefile.objs b/migration/Makefile.objs index d929e96..5a25d39 100644 --- a/migration/Makefile.objs +++ b/migration/Makefile.objs @@ -1,4 +1,5 @@ common-obj-y += migration.o tcp.o +common-obj-$(CONFIG_COLO) += colo.o common-obj-y += vmstate.o common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o qemu-file-stdio.o common-obj-y += xbzrle.o diff --git a/migration/colo.c b/migration/colo.c new file mode 100644 index 0000000..2c40d2e --- /dev/null +++ b/migration/colo.c @@ -0,0 +1,18 @@ +/* + * COarse-grain LOck-stepping Virtual Machines for Non-stop Service (COLO) + * (a.k.a. Fault Tolerance or Continuous Replication) + * + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO., LTD. + * Copyright (c) 2015 FUJITSU LIMITED + * Copyright (c) 2015 Intel Corporation + * + * This work is licensed under the terms of the GNU GPL, version 2 or + * later. See the COPYING file in the top-level directory. + */ + +#include "migration/colo.h" + +bool colo_supported(void) +{ + return true; +} diff --git a/migration/migration.c b/migration/migration.c index 662e77e..593cac0 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -29,6 +29,7 @@ #include "trace.h" #include "qapi/util.h" #include "qapi-event.h" +#include "migration/colo.h" #define MAX_THROTTLE (32 << 20) /* Migration speed throttling */ @@ -345,6 +346,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; + } if (head == NULL) { head = g_malloc0(sizeof(*caps)); caps = head; @@ -485,6 +489,13 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, } for (cap = params; cap; cap = cap->next) { + if (cap->value->capability == MIGRATION_CAPABILITY_COLO && + !colo_supported()) { + error_setg(errp, "COLO is not currently supported, please" + " configure with --enable-colo option in order to" + " support COLO feature"); + continue; + } s->enabled_capabilities[cap->value->capability] = cap->value->state; } } @@ -913,6 +924,12 @@ int64_t migrate_xbzrle_cache_size(void) return s->xbzrle_cache_size; } +bool migrate_enable_colo(void) +{ + MigrationState *s = migrate_get_current(); + return s->enabled_capabilities[MIGRATION_CAPABILITY_COLO]; +} + /* migration thread support */ static void *migration_thread(void *opaque) diff --git a/qapi-schema.json b/qapi-schema.json index 5f45571..b14d1f4 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -529,11 +529,15 @@ # @auto-converge: If enabled, QEMU will automatically throttle down the guest # to speed up convergence of RAM migration. (since 1.6) # +# @colo: If enabled, migration will never end, and the state of the VM on the +# primary side will be migrated continuously to the VM on secondary +# side. (since 2.5) +# # Since: 1.2 ## { 'enum': 'MigrationCapability', 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks', - 'compress', 'events'] } + 'compress', 'events', 'colo'] } ## # @MigrationCapabilityStatus diff --git a/qmp-commands.hx b/qmp-commands.hx index e6ed580..b06c09c 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3489,6 +3489,7 @@ Query current migration capabilities - "rdma-pin-all" : RDMA Pin Page state (json-bool) - "auto-converge" : Auto Converge state (json-bool) - "zero-blocks" : Zero Blocks state (json-bool) + - "colo" : COarse-Grain LOck Stepping for Non-stop Service (json-bool) Arguments: diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index 9937a12..5b124a9 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -38,3 +38,4 @@ stub-obj-$(CONFIG_WIN32) += fd-register.o stub-obj-y += cpus.o stub-obj-y += kvm.o stub-obj-y += qmp_pc_dimm_device_list.o +stub-obj-y += migration-colo.o diff --git a/stubs/migration-colo.c b/stubs/migration-colo.c new file mode 100644 index 0000000..3d817df --- /dev/null +++ b/stubs/migration-colo.c @@ -0,0 +1,18 @@ +/* + * COarse-grain LOck-stepping Virtual Machines for Non-stop Service (COLO) + * (a.k.a. Fault Tolerance or Continuous Replication) + * + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO., LTD. + * Copyright (c) 2015 FUJITSU LIMITED + * Copyright (c) 2015 Intel Corporation + * + * This work is licensed under the terms of the GNU GPL, version 2 or + * later. See the COPYING file in the top-level directory. + */ + +#include "migration/colo.h" + +bool colo_supported(void) +{ + return false; +}