|
From: | Changlong Xie |
Subject: | Re: [Qemu-devel] [PATCH v14 7/8] Implement new driver for block replication |
Date: | Wed, 20 Jan 2016 15:45:23 +0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 |
On 01/20/2016 08:04 AM, Eric Blake wrote:
On 01/13/2016 02:18 AM, Changlong Xie wrote:From: Wen Congyang <address@hidden> Signed-off-by: Wen Congyang <address@hidden> Signed-off-by: zhanghailiang <address@hidden> Signed-off-by: Gonglei <address@hidden> Signed-off-by: Changlong Xie <address@hidden> --- 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.hJust 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 LIMITEDDo 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
[Prev in Thread] | Current Thread | [Next in Thread] |