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

[Changlong Xie]

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

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
>