On 02/19/2016 04:41 PM, Hailiang Zhang wrote: > Hi, > > On 2016/2/15 9:13, Wen Congyang wrote: >> On 02/15/2016 08:57 AM, Hailiang Zhang wrote: >>> On 2016/2/5 12:18, Changlong Xie wrote: >>>> Signed-off-by: Wen Congyang <we...@cn.fujitsu.com> >>>> Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com> >>>> Signed-off-by: Gonglei <arei.gong...@huawei.com> >>>> Signed-off-by: Changlong Xie <xiecl.f...@cn.fujitsu.com> >>>> --- >>>> Makefile.objs | 1 + >>>> qapi/block-core.json | 13 ++++++++ >>>> replication.c | 94 >>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> replication.h | 53 +++++++++++++++++++++++++++++ >>>> 4 files changed, 161 insertions(+) >>>> create mode 100644 replication.c >>>> create mode 100644 replication.h >>>> >>>> diff --git a/Makefile.objs b/Makefile.objs >>>> index 06b95c7..a8c74b7 100644 >>>> --- a/Makefile.objs >>>> +++ b/Makefile.objs >>>> @@ -15,6 +15,7 @@ block-obj-$(CONFIG_POSIX) += aio-posix.o >>>> block-obj-$(CONFIG_WIN32) += aio-win32.o >>>> block-obj-y += block/ >>>> block-obj-y += qemu-io-cmds.o >>>> +block-obj-y += replication.o >>>> >>>> block-obj-m = block/ >>>> >>>> diff --git a/qapi/block-core.json b/qapi/block-core.json >>>> index 7e9e8fe..12362b8 100644 >>>> --- a/qapi/block-core.json >>>> +++ b/qapi/block-core.json >>>> @@ -2002,6 +2002,19 @@ >>>> '*read-pattern': 'QuorumReadPattern' } } >>>> >>>> ## >>>> +# @ReplicationMode >>>> +# >>>> +# An enumeration of replication modes. >>>> +# >>>> +# @primary: Primary mode, the vm's state will be sent to secondary QEMU. >>>> +# >>>> +# @secondary: Secondary mode, receive the vm's state from primary QEMU. >>>> +# >>>> +# Since: 2.6 >>>> +## >>>> +{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] } >>>> + >>>> +## >>>> # @BlockdevOptions >>>> # >>>> # Options for creating a block device. >>>> diff --git a/replication.c b/replication.c >>>> new file mode 100644 >>>> index 0000000..e8ac2f0 >>>> --- /dev/null >>>> +++ b/replication.c >>>> @@ -0,0 +1,94 @@ >>>> +/* >>>> + * Replication filter >>>> + * >>>> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. >>>> + * Copyright (c) 2016 Intel Corporation >>>> + * Copyright (c) 2016 FUJITSU LIMITED >>>> + * >>>> + * Author: >>>> + * Wen Congyang <we...@cn.fujitsu.com> >>>> + * >>>> + * 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 "replication.h" >>>> + >>>> +static QLIST_HEAD(, ReplicationState) replication_states; >>>> + >>>> +ReplicationState *replication_new(void *opaque, ReplicationOps *ops) >>>> +{ >>>> + ReplicationState *rs; >>>> + >>>> + rs = g_new0(ReplicationState, 1); >>>> + rs->opaque = opaque; >>>> + rs->ops = ops; >>>> + QLIST_INSERT_HEAD(&replication_states, rs, node); >>>> + >>>> + return rs; >>>> +} >>>> + >>>> +void replication_remove(ReplicationState *rs) >>>> +{ >>>> + QLIST_REMOVE(rs, node); >>>> + g_free(rs); >>>> +} >>>> + >>>> +/* >>>> + * The caller of the function *MUST* make sure vm stopped >>>> + */ >>>> +void replication_start_all(ReplicationMode mode, Error **errp) >>>> +{ >>> >>> Is this public API is only used for block ? >>> If yes, I'd like it with a 'block_' prefix. >> >> No, we hope it can be used for nic too. >> > > OK, i got why you designed these APIs, I like this idea that > use the callback/notifier to notify the status of COLO for block/nic. > > But let's do something more graceful. > For COLO, we can consider it has four states: > Prepare/start checkpoint(with VM stopped)/finish checkpoint (with VM > resumed)/failover. > So here we need four operation functions according to these four states.
Where is do_checkpoint? > I'd like these public APIs with a 'colo_' prefix, I hope this API can also be used by other solution such as microcheckpoint. > > What about colo_replication_prepare()/colo_replication_start_checkpoint()/ > > colo_replication_finish_checkpoint()/colo_replication_failover() ? > > Besides, It's better to rename the replicaton.c, and move it to the > 'migration' folder. > Because it seems to be only used by COLO, maybe colo_ops.c or maybe another > choice > move all these codes to colo-comm.c. (If we do this, we need to merge > colo-comm.c > first as prerequisite). This api is used by block driver, so qemu-nbd, qemu-img also needs it. Thanks Wen Congyang > > Thanks, > Hailiang > >> >>> >>>> + ReplicationState *rs, *next; >>>> + >>>> + QLIST_FOREACH_SAFE(rs, &replication_states, node, next) { >>>> + if (rs->ops && rs->ops->start) { >>>> + rs->ops->start(rs, mode, errp); >>>> + } >>>> + if (*errp != NULL) { >>> >>> This is incorrect, you miss checking if errp is NULL, >>> if errp is NULL, there will be an error that accessing memory at address >>> 0x0. >>> Same with other places in this patch. >>> >>>> + return; >>>> + } >>>> + } >>>> +} >>>> + >>>> +void replication_do_checkpoint_all(Error **errp) >>>> +{ >>>> + ReplicationState *rs, *next; >>>> + >>>> + QLIST_FOREACH_SAFE(rs, &replication_states, node, next) { >>>> + if (rs->ops && rs->ops->checkpoint) { >>>> + rs->ops->checkpoint(rs, errp); >>>> + } >>>> + if (*errp != NULL) { >>>> + return; >>> >>>> + } >>>> + } >>>> +} >>>> + >>>> +void replication_get_error_all(Error **errp) >>>> +{ >>>> + ReplicationState *rs, *next; >>>> + >>>> + QLIST_FOREACH_SAFE(rs, &replication_states, node, next) { >>>> + if (rs->ops && rs->ops->get_error) { >>>> + rs->ops->get_error(rs, errp); >>>> + } >>>> + if (*errp != NULL) { >>> >>>> + return; >>>> + } >>>> + } >>>> +} >>>> + >>>> +void replication_stop_all(bool failover, Error **errp) >>>> +{ >>>> + ReplicationState *rs, *next; >>>> + >>>> + QLIST_FOREACH_SAFE(rs, &replication_states, node, next) { >>>> + if (rs->ops && rs->ops->stop) { >>>> + rs->ops->stop(rs, failover, errp); >>>> + } >>>> + if (*errp != NULL) { >>>> + return; >>>> + } >>>> + } >>>> +} >>>> diff --git a/replication.h b/replication.h >>>> new file mode 100644 >>>> index 0000000..faea649 >>>> --- /dev/null >>>> +++ b/replication.h >>>> @@ -0,0 +1,53 @@ >>>> +/* >>>> + * Replication filter >>>> + * >>>> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. >>>> + * Copyright (c) 2016 Intel Corporation >>>> + * Copyright (c) 2016 FUJITSU LIMITED >>>> + * >>>> + * Author: >>>> + * Wen Congyang <we...@cn.fujitsu.com> >>>> + * >>>> + * 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 REPLICATION_H >>>> +#define REPLICATION_H >>>> + >>>> +#include "sysemu/sysemu.h" >>>> + >>>> +typedef struct ReplicationOps ReplicationOps; >>>> +typedef struct ReplicationState ReplicationState; >>>> +typedef void (*Start)(ReplicationState *rs, ReplicationMode mode, Error >>>> **errp); >>>> +typedef void (*Stop)(ReplicationState *rs, bool failover, Error **errp); >>>> +typedef void (*Checkpoint)(ReplicationState *rs, Error **errp); >>>> +typedef void (*GetError)(ReplicationState *rs, Error **errp); >>>> + >>>> +struct ReplicationState { >>>> + void *opaque; >>>> + ReplicationOps *ops; >>>> + QLIST_ENTRY(ReplicationState) node; >>>> +}; >>>> + >>>> +struct ReplicationOps{ >>>> + Start start; >>>> + Checkpoint checkpoint; >>>> + GetError get_error; >>>> + Stop stop; >>>> +}; >>>> + >>>> + >>>> +ReplicationState *replication_new(void *opaque, ReplicationOps *ops); >>>> + >>>> +void replication_remove(ReplicationState *rs); >>>> + >>>> +void replication_start_all(ReplicationMode mode, Error **errp); >>>> + >>>> +void replication_do_checkpoint_all(Error **errp); >>>> + >>>> +void replication_get_error_all(Error **errp); >>>> + >>>> +void replication_stop_all(bool failover, Error **errp); >>>> + >>>> +#endif /* REPLICATION_H */ >>>> >>> >>> >>> >>> >>> . >>> >> >> >> >> >> . >> > > > > > . >