From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:47914)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <xiecl.fnst@cn.fujitsu.com>) id 1aqWKi-0002QY-2e
	for qemu-devel@nongnu.org; Wed, 13 Apr 2016 21:44:36 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <xiecl.fnst@cn.fujitsu.com>) id 1aqWKh-0002l6-1S
	for qemu-devel@nongnu.org; Wed, 13 Apr 2016 21:44:36 -0400
Message-ID: <570EF6A7.4010806@cn.fujitsu.com>
Date: Thu, 14 Apr 2016 09:47:19 +0800
From: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
MIME-Version: 1.0
References: <1460362979-15146-1-git-send-email-xiecl.fnst@cn.fujitsu.com>
	<1460362979-15146-7-git-send-email-xiecl.fnst@cn.fujitsu.com>
	<20160413124757.GC9272@stefanha-x1.localdomain>
In-Reply-To: <20160413124757.GC9272@stefanha-x1.localdomain>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v17 6/8] Introduce new APIs to do
 replication operation
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: qemu devel <qemu-devel@nongnu.org>, Fam Zheng <famz@redhat.com>, Max Reitz <mreitz@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Kevin Wolf <kwolf@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Dong Eddie <eddie.dong@intel.com>, Jiang Yunhong <yunhong.jiang@intel.com>, Wen Congyang <wency@cn.fujitsu.com>, qemu block <qemu-block@nongnu.org>, "Michael R. Hines" <mrhines@linux.vnet.ibm.com>, Eric Blake <eblake@redhat.com>, zhanghailiang <zhang.zhanghailiang@huawei.com>, Gonglei <arei.gonglei@huawei.com>

On 04/13/2016 08:47 PM, Stefan Hajnoczi wrote:
> On Mon, Apr 11, 2016 at 04:22:57PM +0800, Changlong Xie wrote:
>> +/*
>> + * The caller of the function MUST make sure vm stopped
>> + */
>> +void replication_start_all(ReplicationMode mode, Error **errp)
>> +{
>> +    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 && *errp) {
>> +            return;
>> +        }
>
> This function returns immediately on error if the caller provided errp.
> It continues if the caller did not provide errp.
>
> I'm not sure if you wanted this difference in behavior.
>
> The following always returns immediately on error, even when the caller
> did not provide errp:
>

I just notice that, if errp == NULL and error happens in 
rs->ops->{start,...}, we could never detect it. I'll check all 
replication callbacks carefully

Thanks
	-Xie
> void replication_start_all(ReplicationMode mode, Error **errp)
> {
>      ReplicationState *rs, *next;
>      Error *local_err = NULL;
>
>      QLIST_FOREACH_SAFE(rs, &replication_states, node, next) {
>          if (rs->ops && rs->ops->start) {
>              rs->ops->start(rs, mode, &local_err);
>          }
>          if (local_err) {
> 	    error_propagate(errp, local_err);
>              return;
>          }
>> +/**
>> + * SECTION:replication.h
>> + * @title:Base Replication System
>> + * @short_description: interfaces for handle replication
>> + *
>> + * The Replication Model provides a framework for handle Replication
>
> s/handle/handling/
>