On 07/27/2016 12:17 AM, Max Reitz wrote:
On 26.07.2016 10:15, Changlong Xie wrote:From: Wen Congyang <we...@cn.fujitsu.com>Signed-off-by: Wen Congyang <we...@cn.fujitsu.com> Signed-off-by: Changlong Xie <xiecl.f...@cn.fujitsu.com> Signed-off-by: Wang WeiWei <wangww.f...@cn.fujitsu.com> Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com> Signed-off-by: Gonglei <arei.gong...@huawei.com> --- block/Makefile.objs | 1 + block/replication.c | 658 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 659 insertions(+) create mode 100644 block/replication.c[...]diff --git a/block/replication.c b/block/replication.c new file mode 100644 index 0000000..ec35348 --- /dev/null +++ b/block/replication.c @@ -0,0 +1,658 @@[...]+static void replication_start(ReplicationState *rs, ReplicationMode mode, + Error **errp) +{[...]+ /* start backup job now */ + error_setg(&s->blocker, + "Block device is in use by internal backup job"); + + top_bs = bdrv_lookup_bs(s->top_id, s->top_id, errp);I think you should pass NULL instead of errp...+ if (!top_bs || !check_top_bs(top_bs, bs)) { + error_setg(errp, "No top_bs or it is invalid");...or if you don't, then you should not call this function if top_bs is NULL. Otherwise you'll probably get a failed assertion in error_setv() because *errp is not NULL.
Thanks for pointing it out. if top_is is NULL, *errp will be set in bdrv_lookup_bs(). Then we'll get failed assertion in error_setv(). Will
fix it.
+ reopen_backing_file(s, false, NULL); + aio_context_release(aio_context); + return; + } + bdrv_op_block_all(top_bs, s->blocker); + bdrv_op_unblock(top_bs, BLOCK_OP_TYPE_DATAPLANE, s->blocker);Shouldn't you make sure that top_bs is a root node? The first patch in
Indeed, it should be a root node
Kevin's "block: Accept node-name in all node level QMP commands" series introduces the bdrv_is_root_node() function for that purpose. Maybe that check should be put into check_top_bs().
I think we just need check top_bs is a root node or not one time before stepping in check_top_bs().
if (!top_bs || !bdrv_is_root_node(top_bs) ||
!check_top_bs(top_bs, bs)) {
Max+ + backup_start("replication-backup", s->secondary_disk->bs, + s->hidden_disk->bs, 0, MIRROR_SYNC_MODE_NONE, NULL, + BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT, + backup_job_completed, s, NULL, &local_err); + if (local_err) { + error_propagate(errp, local_err); + backup_job_cleanup(s); + aio_context_release(aio_context); + return; + } + break;
