From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37975) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ad7yX-0006M1-NL for qemu-devel@nongnu.org; Mon, 07 Mar 2016 22:06:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ad7yW-0003N5-Od for qemu-devel@nongnu.org; Mon, 07 Mar 2016 22:06:21 -0500 Message-ID: <56DE4215.9000104@cn.fujitsu.com> Date: Tue, 8 Mar 2016 11:08:05 +0800 From: Changlong Xie MIME-Version: 1.0 References: <1454645888-28826-1-git-send-email-xiecl.fnst@cn.fujitsu.com> <1454645888-28826-8-git-send-email-xiecl.fnst@cn.fujitsu.com> <20160304161327.GE9130@stefanha-x1.localdomain> In-Reply-To: <20160304161327.GE9130@stefanha-x1.localdomain> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v15 7/9] Introduce new APIs to do replication operation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Fam Zheng , qemu block , Jiang Yunhong , Dong Eddie , qemu devel , "Michael R. Hines" , Max Reitz , Gonglei , Stefan Hajnoczi , Paolo Bonzini , "Dr. David Alan Gilbert" , zhanghailiang 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 >> + * >> + * 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 >