Re: [Qemu-devel] [Qemu-block] [PATCH v15 7/9] Introduce new APIs to do replication operation

[Changlong Xie <xiecl.fnst@cn.fujitsu.com>]

On 03/05/2016 12:13 AM, Stefan Hajnoczi wrote:
> On Fri, Feb 05, 2016 at 12:18:06PM +0800, Changlong Xie wrote:
>> 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 <wency@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);
>

Hi Stefan

Thanks for your comments.

> These typedefs are likely to collide with system headers.  Please prefix
> the name.
>
> That said, I don't think Start, Stop, Checkpoint, GetError are
> necessary.  Just declare the prototypes in struct ReplicationOps.  No
> user passes single function pointers, they always pass ReplicationOps.
> Therefore it's not necessary to declare types for these functions at
> all.

Will try that.

>
>> +
>> +struct ReplicationState {
>> +    void *opaque;
>> +    ReplicationOps *ops;
>> +    QLIST_ENTRY(ReplicationState) node;
>> +};
>> +
>> +struct ReplicationOps{
>> +    Start start;
>> +    Checkpoint checkpoint;
>> +    GetError get_error;
>> +    Stop stop;
>> +};
>
> Please add doc comments that explain the semantics of these functions.
> For examples, see include/qom/object.h.  This documentation should allow
> someone implementing a new replication listener to understand the
> constraints and the purpose of these functions.

Surely


Thanks
	-Xie
>