Re: [Qemu-block] [PATCH RFC v2 4/6] replication: fix code logic with the new shared_disk option

[Hailiang Zhang]

On 2016/12/20 20:42, Changlong Xie wrote:
> On 12/05/2016 04:35 PM, zhanghailiang wrote:
> > Some code logic only be needed in non-shared disk, here
> > we adjust these codes to prepare for shared disk scenario.
> >
> > Signed-off-by: zhanghailiang <address@hidden>
> > ---
> >    block/replication.c | 47 ++++++++++++++++++++++++++++-------------------
> >    1 file changed, 28 insertions(+), 19 deletions(-)
> >
> > diff --git a/block/replication.c b/block/replication.c
> > index 35e9ab3..6574cc2 100644
> > --- a/block/replication.c
> > +++ b/block/replication.c
> > @@ -531,21 +531,28 @@ static void replication_start(ReplicationState *rs, 
> > ReplicationMode mode,
> >                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);
> >
> > -        job = backup_job_create(NULL, s->secondary_disk->bs, 
> > s->hidden_disk->bs,
> > -                                0, MIRROR_SYNC_MODE_NONE, NULL, false,
> > +        /*
> > +         * Only in the case of non-shared disk,
> > +         * the backup job is in the secondary side
> > +         */
> > +        if (!s->is_shared_disk) {
> > +            bdrv_op_block_all(top_bs, s->blocker);
> > +            bdrv_op_unblock(top_bs, BLOCK_OP_TYPE_DATAPLANE, s->blocker);
> > +            job = backup_job_create(NULL, s->secondary_disk->bs,
> > +                                s->hidden_disk->bs, 0,
> > +                                MIRROR_SYNC_MODE_NONE, NULL, false,
> >                                    BLOCKDEV_ON_ERROR_REPORT,
> >                                    BLOCKDEV_ON_ERROR_REPORT, BLOCK_JOB_INTERNAL,
> >                                    backup_job_completed, bs, NULL, &local_err);
>
> Coding style here.

Will fix it in next version, thanks.

> > -        if (local_err) {
> > -            error_propagate(errp, local_err);
> > -            backup_job_cleanup(bs);
> > -            aio_context_release(aio_context);
> > -            return;
> > +            if (local_err) {
> > +                error_propagate(errp, local_err);
> > +                backup_job_cleanup(bs);
> > +                aio_context_release(aio_context);
> > +                return;
> > +            }
> > +            block_job_start(job);
> >            }
> > -        block_job_start(job);
> >
> >            secondary_do_checkpoint(s, errp);
> >            break;
> > @@ -575,14 +582,16 @@ static void replication_do_checkpoint(ReplicationState 
> > *rs, Error **errp)
> >        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;
> > +        if (!s->is_shared_disk) {
> > +            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;
> > @@ -663,7 +672,7 @@ static void replication_stop(ReplicationState *rs, bool 
> > failover, Error **errp)
> >             * before the BDS is closed, because we will access hidden
> >             * disk, secondary disk in backup_job_completed().
> >             */
> > -        if (s->secondary_disk->bs->job) {
> > +        if (!s->is_shared_disk && s->secondary_disk->bs->job) {
> >                block_job_cancel_sync(s->secondary_disk->bs->job);
> >            }
>
>
>
> .