| Message ID | 1438159544-6224-3-git-send-email-zhang.zhanghailiang@huawei.com |
|---|---|
| State | New |
| Headers | show |
On 07/29/2015 02:45 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> > --- Just focusing on the interface. > @@ -338,6 +339,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; > + } Interesting - you completely omit the capability if it can't be set to true; so the presence of the capability is therefore the witness of whether it works. Which means it is a true runtime probe, rather than a static introspection, and therefore more accurate :) > if (head == NULL) { > head = g_malloc0(sizeof(*caps)); > caps = head; > @@ -478,6 +482,13 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, > } > > for (cap = params; cap; cap = cap->next) { > + if (cap->value->capability == MIGRATION_CAPABILITY_COLO && > + 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; > + } This says that you error out if colo:true is requested but not supported, but silently ignore colo:false even when it is unsupported. Isn't it better to error out that colo is unsupported, regardless of enabled true/false being requested, since you explicitly avoided advertising the feature? > +++ b/qapi-schema.json > @@ -529,11 +529,14 @@ > # @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 VM in primary side Long line. Please wrap to fit within 80 columns. > +# will be migrated continuously to VM in secondary side. (since 2.4) You've missed 2.4; change this to 2.5. Grammar suggestion: s/of VM in primary/of the VM on the primary/ s/to VM in secondary/to the VM on the secondary/ > +++ b/qmp-commands.hx > @@ -3434,6 +3434,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" : COLO FT state (json-bool) Acronym soup. Might be nice to state more human-readable text about what 'colo' actually controls (stating COarse-grain LOcking fault tolerance would help).
On 2015/8/29 5:54, Eric Blake wrote: > On 07/29/2015 02:45 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> >> --- > > Just focusing on the interface. > Thank you very much for the review. >> @@ -338,6 +339,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; >> + } > > Interesting - you completely omit the capability if it can't be set to > true; so the presence of the capability is therefore the witness of > whether it works. Which means it is a true runtime probe, rather than a > static introspection, and therefore more accurate :) > Yes, it will help users to know if they can set this capability or not. I can't figure out a better way to do this thing ;) >> if (head == NULL) { >> head = g_malloc0(sizeof(*caps)); >> caps = head; >> @@ -478,6 +482,13 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, >> } >> >> for (cap = params; cap; cap = cap->next) { >> + if (cap->value->capability == MIGRATION_CAPABILITY_COLO && >> + 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; >> + } > > This says that you error out if colo:true is requested but not > supported, but silently ignore colo:false even when it is unsupported. > Isn't it better to error out that colo is unsupported, regardless of > enabled true/false being requested, since you explicitly avoided > advertising the feature? > You are right, we should tell people colo is unsupported whether the set or unset this capability, will fix in next version. >> +++ b/qapi-schema.json >> @@ -529,11 +529,14 @@ >> # @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 VM in primary side > > Long line. Please wrap to fit within 80 columns. > OK, will fix it. >> +# will be migrated continuously to VM in secondary side. (since 2.4) > > You've missed 2.4; change this to 2.5. > > Grammar suggestion: > s/of VM in primary/of the VM on the primary/ > s/to VM in secondary/to the VM on the secondary/ > >> +++ b/qmp-commands.hx >> @@ -3434,6 +3434,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" : COLO FT state (json-bool) > > Acronym soup. Might be nice to state more human-readable text about what > 'colo' actually controls (stating COarse-grain LOcking fault tolerance > would help). > OK, i will address all the problems 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 a2f8ed0..5fd61a3 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 fd4f99b..70ca246 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 */ @@ -338,6 +339,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; @@ -478,6 +482,13 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, } for (cap = params; cap; cap = cap->next) { + if (cap->value->capability == MIGRATION_CAPABILITY_COLO && + 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; + } s->enabled_capabilities[cap->value->capability] = cap->value->state; } } @@ -906,6 +917,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 4342a08..136dc4f 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -529,11 +529,14 @@ # @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 VM in primary side +# will be migrated continuously to VM in secondary side. (since 2.4) +# # 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 ba630b1..bb49a1a 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3434,6 +3434,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" : COLO FT state (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; +}