On 01/20/2016 08:04 AM, Eric Blake wrote:
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.
Hi eric, thanks for your patience.
+++ 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?
Will correct all Copyright issues in next version.
+
+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 */
Will fix this space issue
+ BLOCK_REPLICATION_DONE, /* block replication is done(after
failover) */
+};
Should this be an enum type in a .json file?
Since this is just an internal enum that only used in
block/replication.c, i think we don't need put it in *.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)').
You're right, i'll fix all relevant issue in my patchset.
+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 */
Will fix
+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/
will fix
+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/
will fix
+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/
Will fix
+++ 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.
Thanks
-Xie
