> -----Original Message-----
> From: Qemu-devel <qemu-devel-
> bounces+chen.zhang=intel....@nongnu.org> On Behalf Of Lukas Straub
> Sent: Sunday, July 18, 2021 10:48 PM
> To: qemu-devel <qemu-devel@nongnu.org>
> Cc: Kevin Wolf <kw...@redhat.com>; Vladimir Sementsov-Ogievskiy
> <vsement...@virtuozzo.com>; qemu-block <qemu-bl...@nongnu.org>;
> Wen Congyang <wencongya...@huawei.com>; Xie Changlong
> <xiechanglon...@gmail.com>; Max Reitz <mre...@redhat.com>
> Subject: [PATCH v6 1/4] replication: Remove s->active_disk
>
> s->active_disk is bs->file. Remove it and use local variables instead.
>
> Signed-off-by: Lukas Straub <lukasstra...@web.de>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
Looks good for me.
Reviewed-by: Zhang Chen <chen.zh...@intel.com>
> ---
> block/replication.c | 34 +++++++++++++++++-----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/block/replication.c b/block/replication.c index
> 774e15df16..9ad2dfdc69 100644
> --- a/block/replication.c
> +++ b/block/replication.c
> @@ -35,7 +35,6 @@ typedef enum {
> typedef struct BDRVReplicationState {
> ReplicationMode mode;
> ReplicationStage stage;
> - BdrvChild *active_disk;
> BlockJob *commit_job;
> BdrvChild *hidden_disk;
> BdrvChild *secondary_disk;
> @@ -307,8 +306,10 @@ out:
> return ret;
> }
>
> -static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp)
> +static void secondary_do_checkpoint(BlockDriverState *bs, Error **errp)
> {
> + BDRVReplicationState *s = bs->opaque;
> + BdrvChild *active_disk = bs->file;
> Error *local_err = NULL;
> int ret;
>
> @@ -323,13 +324,13 @@ static void
> secondary_do_checkpoint(BDRVReplicationState *s, Error **errp)
> return;
> }
>
> - if (!s->active_disk->bs->drv) {
> + if (!active_disk->bs->drv) {
> error_setg(errp, "Active disk %s is ejected",
> - s->active_disk->bs->node_name);
> + active_disk->bs->node_name);
> return;
> }
>
> - ret = bdrv_make_empty(s->active_disk, errp);
> + ret = bdrv_make_empty(active_disk, errp);
> if (ret < 0) {
> return;
> }
> @@ -458,6 +459,7 @@ static void replication_start(ReplicationState *rs,
> ReplicationMode mode,
> BlockDriverState *bs = rs->opaque;
> BDRVReplicationState *s;
> BlockDriverState *top_bs;
> + BdrvChild *active_disk;
> int64_t active_length, hidden_length, disk_length;
> AioContext *aio_context;
> Error *local_err = NULL;
> @@ -495,15 +497,14 @@ static void replication_start(ReplicationState *rs,
> ReplicationMode mode,
> case REPLICATION_MODE_PRIMARY:
> break;
> case REPLICATION_MODE_SECONDARY:
> - s->active_disk = bs->file;
> - if (!s->active_disk || !s->active_disk->bs ||
> - !s->active_disk->bs->backing) {
> + active_disk = bs->file;
> + if (!active_disk || !active_disk->bs ||
> + !active_disk->bs->backing) {
> error_setg(errp, "Active disk doesn't have backing file");
> aio_context_release(aio_context);
> return;
> }
>
> - s->hidden_disk = s->active_disk->bs->backing;
> + s->hidden_disk = active_disk->bs->backing;
> if (!s->hidden_disk->bs || !s->hidden_disk->bs->backing) {
> error_setg(errp, "Hidden disk doesn't have backing file");
> aio_context_release(aio_context); @@ -518,7 +519,7 @@ static void
> replication_start(ReplicationState *rs, ReplicationMode mode,
> }
>
> /* verify the length */
> - active_length = bdrv_getlength(s->active_disk->bs);
> + active_length = bdrv_getlength(active_disk->bs);
> hidden_length = bdrv_getlength(s->hidden_disk->bs);
> disk_length = bdrv_getlength(s->secondary_disk->bs);
> if (active_length < 0 || hidden_length < 0 || disk_length < 0 || @@ -
> 530,9 +531,9 @@ static void replication_start(ReplicationState *rs,
> ReplicationMode mode,
> }
>
> /* Must be true, or the bdrv_getlength() calls would have failed */
> - assert(s->active_disk->bs->drv && s->hidden_disk->bs->drv);
> + assert(active_disk->bs->drv && s->hidden_disk->bs->drv);
>
> - if (!s->active_disk->bs->drv->bdrv_make_empty ||
> + if (!active_disk->bs->drv->bdrv_make_empty ||
> !s->hidden_disk->bs->drv->bdrv_make_empty) {
> error_setg(errp,
> "Active disk or hidden disk doesn't support
> make_empty"); @@ -
> 586,7 +587,7 @@ static void replication_start(ReplicationState *rs,
> ReplicationMode mode,
> s->stage = BLOCK_REPLICATION_RUNNING;
>
> if (s->mode == REPLICATION_MODE_SECONDARY) {
> - secondary_do_checkpoint(s, errp);
> + secondary_do_checkpoint(bs, errp);
> }
>
> s->error = 0;
> @@ -615,7 +616,7 @@ static void
> replication_do_checkpoint(ReplicationState *rs, Error **errp)
> }
>
> if (s->mode == REPLICATION_MODE_SECONDARY) {
> - secondary_do_checkpoint(s, errp);
> + secondary_do_checkpoint(bs, errp);
> }
> aio_context_release(aio_context);
> }
> @@ -652,7 +653,6 @@ static void replication_done(void *opaque, int ret)
> if (ret == 0) {
> s->stage = BLOCK_REPLICATION_DONE;
>
> - s->active_disk = NULL;
> s->secondary_disk = NULL;
> s->hidden_disk = NULL;
> s->error = 0;
> @@ -705,7 +705,7 @@ static void replication_stop(ReplicationState *rs, bool
> failover, Error **errp)
> }
>
> if (!failover) {
> - secondary_do_checkpoint(s, errp);
> + secondary_do_checkpoint(bs, errp);
> s->stage = BLOCK_REPLICATION_DONE;
> aio_context_release(aio_context);
> return;
> @@ -713,7 +713,7 @@ static void replication_stop(ReplicationState *rs, bool
> failover, Error **errp)
>
> s->stage = BLOCK_REPLICATION_FAILOVER;
> s->commit_job = commit_active_start(
> - NULL, s->active_disk->bs, s->secondary_disk->bs,
> + NULL, bs->file->bs, s->secondary_disk->bs,
> JOB_INTERNAL, 0, BLOCKDEV_ON_ERROR_REPORT,
> NULL, replication_done, bs, true, errp);
> break;
> --
> 2.20.1
