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

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1972 bytes --]

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);

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.

> +
> +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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]