[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 06/16] block/mirror: conservative mirror_exit
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH v5 06/16] block/mirror: conservative mirror_exit refactor |
Date: |
Thu, 6 Sep 2018 12:57:20 -0400 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Thu, Sep 06, 2018 at 09:02:15AM -0400, John Snow wrote:
> For purposes of minimum code movement, refactor the mirror_exit
> callback to use the post-finalization callbacks in a trivial way.
>
> Signed-off-by: John Snow <address@hidden>
> ---
> block/mirror.c | 39 ++++++++++++++++++++++++++++-----------
> 1 file changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index bd3e908710..a92b4702c5 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -79,6 +79,7 @@ typedef struct MirrorBlockJob {
> int max_iov;
> bool initial_zeroing_ongoing;
> int in_active_write_counter;
> + bool prepared;
> } MirrorBlockJob;
>
> typedef struct MirrorBDSOpaque {
> @@ -607,7 +608,7 @@ static void mirror_wait_for_all_io(MirrorBlockJob *s)
> }
> }
>
> -static void mirror_exit(Job *job)
> +static int mirror_exit_common(Job *job)
> {
> MirrorBlockJob *s = container_of(job, MirrorBlockJob, common.job);
> BlockJob *bjob = &s->common;
> @@ -617,7 +618,13 @@ static void mirror_exit(Job *job)
> BlockDriverState *target_bs = blk_bs(s->target);
> BlockDriverState *mirror_top_bs = s->mirror_top_bs;
> Error *local_err = NULL;
> - int ret = job->ret;
> + bool abort = job->ret < 0;
> + int ret = 0;
> +
> + if (s->prepared) {
> + return 0;
> + }
> + s->prepared = true;
>
> bdrv_release_dirty_bitmap(src, s->dirty_bitmap);
>
> @@ -642,7 +649,7 @@ static void mirror_exit(Job *job)
> * required before it could become a backing file of target_bs. */
> bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL,
> &error_abort);
> - if (ret == 0 && s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
> + if (!abort && s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
> BlockDriverState *backing = s->is_none_mode ? src : s->base;
> if (backing_bs(target_bs) != backing) {
> bdrv_set_backing_hd(target_bs, backing, &local_err);
> @@ -658,11 +665,8 @@ static void mirror_exit(Job *job)
> aio_context_acquire(replace_aio_context);
> }
>
> - if (s->should_complete && ret == 0) {
> - BlockDriverState *to_replace = src;
> - if (s->to_replace) {
> - to_replace = s->to_replace;
> - }
> + if (s->should_complete && !abort) {
> + BlockDriverState *to_replace = s->to_replace ?: src;
>
> if (bdrv_get_flags(target_bs) != bdrv_get_flags(to_replace)) {
> bdrv_reopen(target_bs, bdrv_get_flags(to_replace), NULL);
> @@ -711,7 +715,18 @@ static void mirror_exit(Job *job)
> bdrv_unref(mirror_top_bs);
> bdrv_unref(src);
>
> - job->ret = ret;
> + return ret;
> +}
> +
> +static int mirror_prepare(Job *job)
> +{
> + return mirror_exit_common(job);
> +}
> +
> +static void mirror_abort(Job *job)
> +{
> + int ret = mirror_exit_common(job);
> + assert(ret == 0);
Not something to hold the series up, but in case a v6 is called for due to
other changes: I think it may be worth a comment in mirror_exit_common()
that if abort is true, and we don't return success, QEMU will hit an assert.
Mainly to prevent someone from including a call with a potential error
return in the abort path in the future.
Reviewed-by: Jeff Cody <address@hidden>
> }
>
> static void mirror_throttle(MirrorBlockJob *s)
> @@ -1132,7 +1147,8 @@ static const BlockJobDriver mirror_job_driver = {
> .user_resume = block_job_user_resume,
> .drain = block_job_drain,
> .run = mirror_run,
> - .exit = mirror_exit,
> + .prepare = mirror_prepare,
> + .abort = mirror_abort,
> .pause = mirror_pause,
> .complete = mirror_complete,
> },
> @@ -1149,7 +1165,8 @@ static const BlockJobDriver commit_active_job_driver = {
> .user_resume = block_job_user_resume,
> .drain = block_job_drain,
> .run = mirror_run,
> - .exit = mirror_exit,
> + .prepare = mirror_prepare,
> + .abort = mirror_abort,
> .pause = mirror_pause,
> .complete = mirror_complete,
> },
> --
> 2.14.4
>
- [Qemu-devel] [PATCH v5 00/16] jobs: Job Exit Refactoring Pt 2, John Snow, 2018/09/06
- [Qemu-devel] [PATCH v5 09/16] tests/test-blockjob: remove exit callback, John Snow, 2018/09/06
- [Qemu-devel] [PATCH v5 03/16] block/stream: add block job creation flags, John Snow, 2018/09/06
- [Qemu-devel] [PATCH v5 10/16] tests/test-blockjob-txn: move .exit to .clean, John Snow, 2018/09/06
- [Qemu-devel] [PATCH v5 01/16] block/commit: add block job creation flags, John Snow, 2018/09/06
- [Qemu-devel] [PATCH v5 05/16] block/mirror: don't install backing chain on abort, John Snow, 2018/09/06