[Qemu-block] [PATCH RFC 4/7] replication: Split out backup_do_checkpoint() from secondary_do_checkpoint()
The helper backup_do_checkpoint() will be used for primary related codes. Here we split it out from secondary_do_checkpoint(). Besides, it is unnecessary to call backup_do_checkpoint() in replication starting and normally stop replication path. We only need call it while do real checkpointing. Signed-off-by: zhanghailiang --- block/replication.c | 36 +++- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/block/replication.c b/block/replication.c index 2a2fdb2..d687ffc 100644 --- a/block/replication.c +++ b/block/replication.c @@ -320,20 +320,8 @@ static bool replication_recurse_is_first_non_filter(BlockDriverState *bs, static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp) { -Error *local_err = NULL; int ret; -if (!s->secondary_disk->bs->job) { -error_setg(errp, "Backup job was cancelled unexpectedly"); -return; -} - -backup_do_checkpoint(s->secondary_disk->bs->job, &local_err); -if (local_err) { -error_propagate(errp, local_err); -return; -} - ret = s->active_disk->bs->drv->bdrv_make_empty(s->active_disk->bs); if (ret < 0) { error_setg(errp, "Cannot make active disk empty"); @@ -539,6 +527,8 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode, aio_context_release(aio_context); return; } + +secondary_do_checkpoint(s, errp); break; default: aio_context_release(aio_context); @@ -547,10 +537,6 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode, s->replication_state = BLOCK_REPLICATION_RUNNING; -if (s->mode == REPLICATION_MODE_SECONDARY) { -secondary_do_checkpoint(s, errp); -} - s->error = 0; aio_context_release(aio_context); } @@ -560,13 +546,29 @@ static void replication_do_checkpoint(ReplicationState *rs, Error **errp) BlockDriverState *bs = rs->opaque; BDRVReplicationState *s; AioContext *aio_context; +Error *local_err = NULL; aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); s = bs->opaque; -if (s->mode == REPLICATION_MODE_SECONDARY) { +switch (s->mode) { +case REPLICATION_MODE_PRIMARY: +break; +case REPLICATION_MODE_SECONDARY: +if (!s->secondary_disk->bs->job) { +error_setg(errp, "Backup job was cancelled unexpectedly"); +break; +} +backup_do_checkpoint(s->secondary_disk->bs->job, &local_err); +if (local_err) { +error_propagate(errp, local_err); +break; +} secondary_do_checkpoint(s, errp); +break; +default: +abort(); } aio_context_release(aio_context); } -- 1.8.3.1
Re: [Qemu-block] [PATCH RFC 4/7] replication: Split out backup_do_checkpoint() from secondary_do_checkpoint()
On 10/20/2016 09:57 PM, zhanghailiang wrote: The helper backup_do_checkpoint() will be used for primary related codes. Here we split it out from secondary_do_checkpoint(). Besides, it is unnecessary to call backup_do_checkpoint() in replication starting and normally stop replication path. This patch is unnecessary. We *really* need clean backup_job->done_bitmap in replication_start/stop path. We only need call it while do real checkpointing. Signed-off-by: zhanghailiang --- block/replication.c | 36 +++- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/block/replication.c b/block/replication.c index 2a2fdb2..d687ffc 100644 --- a/block/replication.c +++ b/block/replication.c @@ -320,20 +320,8 @@ static bool replication_recurse_is_first_non_filter(BlockDriverState *bs, static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp) { -Error *local_err = NULL; int ret; -if (!s->secondary_disk->bs->job) { -error_setg(errp, "Backup job was cancelled unexpectedly"); -return; -} - -backup_do_checkpoint(s->secondary_disk->bs->job, &local_err); -if (local_err) { -error_propagate(errp, local_err); -return; -} - ret = s->active_disk->bs->drv->bdrv_make_empty(s->active_disk->bs); if (ret < 0) { error_setg(errp, "Cannot make active disk empty"); @@ -539,6 +527,8 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode, aio_context_release(aio_context); return; } + +secondary_do_checkpoint(s, errp); break; default: aio_context_release(aio_context); @@ -547,10 +537,6 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode, s->replication_state = BLOCK_REPLICATION_RUNNING; -if (s->mode == REPLICATION_MODE_SECONDARY) { -secondary_do_checkpoint(s, errp); -} - s->error = 0; aio_context_release(aio_context); } @@ -560,13 +546,29 @@ static void replication_do_checkpoint(ReplicationState *rs, Error **errp) BlockDriverState *bs = rs->opaque; BDRVReplicationState *s; AioContext *aio_context; +Error *local_err = NULL; aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); s = bs->opaque; -if (s->mode == REPLICATION_MODE_SECONDARY) { +switch (s->mode) { +case REPLICATION_MODE_PRIMARY: +break; +case REPLICATION_MODE_SECONDARY: +if (!s->secondary_disk->bs->job) { +error_setg(errp, "Backup job was cancelled unexpectedly"); +break; +} +backup_do_checkpoint(s->secondary_disk->bs->job, &local_err); +if (local_err) { +error_propagate(errp, local_err); +break; +} secondary_do_checkpoint(s, errp); +break; +default: +abort(); } aio_context_release(aio_context); }
Re: [Qemu-block] [PATCH RFC 4/7] replication: Split out backup_do_checkpoint() from secondary_do_checkpoint()
On 2016/10/26 9:40, Changlong Xie wrote: On 10/20/2016 09:57 PM, zhanghailiang wrote: The helper backup_do_checkpoint() will be used for primary related codes. Here we split it out from secondary_do_checkpoint(). Besides, it is unnecessary to call backup_do_checkpoint() in replication starting and normally stop replication path. This patch is unnecessary. We *really* need clean backup_job->done_bitmap in replication_start/stop path. After we support internal job ('BLOCK_JOB_INTERNAL'), do we still need to call backup_do? IMHO, we don't have to clean 'done_bitmap', because its initial value is zero, and we don't have to call it in stop path either, the backup job will be cleaned. We only need call it while do real checkpointing. Signed-off-by: zhanghailiang --- block/replication.c | 36 +++- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/block/replication.c b/block/replication.c index 2a2fdb2..d687ffc 100644 --- a/block/replication.c +++ b/block/replication.c @@ -320,20 +320,8 @@ static bool replication_recurse_is_first_non_filter(BlockDriverState *bs, static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp) { -Error *local_err = NULL; int ret; -if (!s->secondary_disk->bs->job) { -error_setg(errp, "Backup job was cancelled unexpectedly"); -return; -} - -backup_do_checkpoint(s->secondary_disk->bs->job, &local_err); -if (local_err) { -error_propagate(errp, local_err); -return; -} - ret = s->active_disk->bs->drv->bdrv_make_empty(s->active_disk->bs); if (ret < 0) { error_setg(errp, "Cannot make active disk empty"); @@ -539,6 +527,8 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode, aio_context_release(aio_context); return; } + +secondary_do_checkpoint(s, errp); break; default: aio_context_release(aio_context); @@ -547,10 +537,6 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode, s->replication_state = BLOCK_REPLICATION_RUNNING; -if (s->mode == REPLICATION_MODE_SECONDARY) { -secondary_do_checkpoint(s, errp); -} - s->error = 0; aio_context_release(aio_context); } @@ -560,13 +546,29 @@ static void replication_do_checkpoint(ReplicationState *rs, Error **errp) BlockDriverState *bs = rs->opaque; BDRVReplicationState *s; AioContext *aio_context; +Error *local_err = NULL; aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); s = bs->opaque; -if (s->mode == REPLICATION_MODE_SECONDARY) { +switch (s->mode) { +case REPLICATION_MODE_PRIMARY: +break; +case REPLICATION_MODE_SECONDARY: +if (!s->secondary_disk->bs->job) { +error_setg(errp, "Backup job was cancelled unexpectedly"); +break; +} +backup_do_checkpoint(s->secondary_disk->bs->job, &local_err); +if (local_err) { +error_propagate(errp, local_err); +break; +} secondary_do_checkpoint(s, errp); +break; +default: +abort(); } aio_context_release(aio_context); } .