On 01/13/2016 02:18 AM, Changlong Xie wrote: > From: Wen Congyang <we...@cn.fujitsu.com> > > Signed-off-by: Wen Congyang <we...@cn.fujitsu.com> > Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com> > Signed-off-by: Gonglei <arei.gong...@huawei.com> > Signed-off-by: Changlong Xie <xiecl.f...@cn.fujitsu.com> > --- > block/Makefile.objs | 1 + > block/replication-comm.c | 66 +++++ > block/replication.c | 590 > +++++++++++++++++++++++++++++++++++++++ > include/block/replication-comm.h | 50 ++++ > qapi/block-core.json | 13 + > 5 files changed, 720 insertions(+) > create mode 100644 block/replication-comm.c > create mode 100644 block/replication.c > create mode 100644 include/block/replication-comm.h >
Just a high-level overview, mainly on the user-visible interface and things that caught my eye. > +++ b/block/replication-comm.c > @@ -0,0 +1,66 @@ > +/* > + * Replication Block filter > + * > + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO., LTD. > + * Copyright (c) 2015 Intel Corporation > + * Copyright (c) 2015 FUJITSU LIMITED Do you want to claim 2016 in any of this? > + > +enum { > + BLOCK_REPLICATION_NONE, /* block replication is not started > */ > + BLOCK_REPLICATION_RUNNING, /* block replication is running */ > + BLOCK_REPLICATION_FAILOVER, /* failover is running in background > */ > + BLOCK_REPLICATION_FAILOVER_FAILED, /* failover failed*/ Space before */ > + BLOCK_REPLICATION_DONE, /* block replication is done(after > failover) */ > +}; Should this be an enum type in a .json file? > + > +static int replication_open(BlockDriverState *bs, QDict *options, > + int flags, Error **errp) > +{ > + > +fail: > + qemu_opts_del(opts); > + /* propagate error */ > + if (local_err) { > + error_propagate(errp, local_err); > + } It's safe to call error_propagate() unconditionally (you could drop the 'if (local_err)'). > +static coroutine_fn int replication_co_discard(BlockDriverState *bs, > + int64_t sector_num, > + int nb_sectors) > +{ > + BDRVReplicationState *s = bs->opaque; > + int ret; > + > + ret = replication_get_io_status(s); > + if (ret < 0) { > + return ret; > + } > + > + if (ret == 1) { > + /* It is secondary qemu and failover are running*/ Space before */ > +static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp) > +{ > + Error *local_err = NULL; > + int ret; > + > + if (!s->secondary_disk->job) { > + error_setg(errp, "Backup job is cancelled unexpectedly"); Maybe s/is/was/ > +static void replication_start(BlockReplicationState *brs, ReplicationMode > mode, > + Error **errp) > +{ > + BlockDriverState *bs = brs->bs; > + BDRVReplicationState *s = brs->bs->opaque; > + int64_t active_length, hidden_length, disk_length; > + AioContext *aio_context; > + Error *local_err = NULL; > + > + if (s->replication_state != BLOCK_REPLICATION_NONE) { > + error_setg(errp, "Block replication is running or done"); > + return; > + } > + > + if (s->mode != mode) { > + error_setg(errp, "The parameter mode's value is invalid, needs %d," > + " but receives %d", s->mode, mode); s/receives/got/ > +static void replication_do_checkpoint(BlockReplicationState *brs, Error > **errp) > +{ > + BDRVReplicationState *s = brs->bs->opaque; > + > + if (s->replication_state != BLOCK_REPLICATION_RUNNING) { > + error_setg(errp, "Block replication is not running"); > + return; > + } > + > + if (s->error) { > + error_setg(errp, "I/O error occurs"); s/occurs/occurred/ > +++ b/qapi/block-core.json > @@ -1975,6 +1975,19 @@ > '*read-pattern': 'QuorumReadPattern' } } > > ## > +# @ReplicationMode > +# > +# An enumeration of replication modes. > +# > +# @primary: Primary mode, the vm's state will be sent to secondary QEMU. > +# > +# @secondary: Secondary mode, receive the vm's state from primary QEMU. > +# > +# Since: 2.6 > +## > +{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] } > + Interface looks okay. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature