Re: [Qemu-block] [PATCH v17 6/8] Introduce new APIs to do replication operation

[Changlong Xie]

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/