[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v15 7/9] Introduce new APIs to do replication op
From: |
Wen Congyang |
Subject: |
Re: [Qemu-devel] [PATCH v15 7/9] Introduce new APIs to do replication operation |
Date: |
Fri, 19 Feb 2016 16:55:18 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 |
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 <address@hidden>
>>>> Signed-off-by: zhanghailiang <address@hidden>
>>>> Signed-off-by: Gonglei <address@hidden>
>>>> Signed-off-by: Changlong Xie <address@hidden>
>>>> ---
>>>> 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 <address@hidden>
>>>> + *
>>>> + * 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 <address@hidden>
>>>> + *
>>>> + * 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 */
>>>>
>>>
>>>
>>>
>>>
>>> .
>>>
>>
>>
>>
>>
>> .
>>
>
>
>
>
> .
>
- [Qemu-devel] [PATCH v15 2/9] Store parent BDS in BdrvChild, (continued)
- [Qemu-devel] [PATCH v15 2/9] Store parent BDS in BdrvChild, Changlong Xie, 2016/02/04
- [Qemu-devel] [PATCH v15 4/9] Link backup into block core, Changlong Xie, 2016/02/04
- [Qemu-devel] [PATCH v15 6/9] auto complete active commit, Changlong Xie, 2016/02/04
- [Qemu-devel] [PATCH v15 5/9] docs: block replication's description, Changlong Xie, 2016/02/04
- [Qemu-devel] [PATCH v15 1/9] unblock backup operations in backing file, Changlong Xie, 2016/02/04
- [Qemu-devel] [PATCH v15 3/9] Backup: clear all bitmap when doing block checkpoint, Changlong Xie, 2016/02/04
- [Qemu-devel] [PATCH v15 7/9] Introduce new APIs to do replication operation, Changlong Xie, 2016/02/04
- Re: [Qemu-devel] [PATCH v15 7/9] Introduce new APIs to do replication operation, Changlong Xie, 2016/02/14
[Qemu-devel] [PATCH v15 9/9] support replication driver in blockdev-add, Changlong Xie, 2016/02/04
[Qemu-devel] [PATCH v15 8/9] Implement new driver for block replication, Changlong Xie, 2016/02/04
Re: [Qemu-devel] [PATCH v15 0/9] Block replication for continuous checkpoints, Changlong Xie, 2016/02/15
Re: [Qemu-devel] [PATCH v15 0/9] Block replication for continuous checkpoints, Hailiang Zhang, 2016/02/17
Re: [Qemu-devel] [PATCH v15 0/9] Block replication for continuous checkpoints, Changlong Xie, 2016/02/23