Commit 1908a559 authored by Kevin Wolf's avatar Kevin Wolf
Browse files

job: Move defer_to_main_loop to Job 



Move the defer_to_main_loop functionality from BlockJob to Job.

The code can be simplified because we can use job->aio_context in
job_defer_to_main_loop_bh() now, instead of having to access the
BlockDriverState.

Probably taking the data->aio_context lock in addition was already
unnecessary in the old code because we didn't actually make use of
anything protected by the old AioContext except getting the new
AioContext, in case it changed between scheduling the BH and running it.
But it's certainly unnecessary now that the BDS isn't accessed at all
any more.

Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
Reviewed-by: default avatarMax Reitz <mreitz@redhat.com>
Reviewed-by: default avatarJohn Snow <jsnow@redhat.com>
parent 08be6fe2

block/backup.c

+4 −3
Original line number Diff line number Diff line
@@ -317,11 +317,12 @@ typedef struct { 
    int ret;

} BackupCompleteData;



static void backup_complete(BlockJob *job, void *opaque)

static void backup_complete(Job *job, void *opaque)

{

    BlockJob *bjob = container_of(job, BlockJob, job);

    BackupCompleteData *data = opaque;



    block_job_completed(job, data->ret);

    block_job_completed(bjob, data->ret);

    g_free(data);

}
@@ -519,7 +520,7 @@ static void coroutine_fn backup_run(void *opaque) 


    data = g_malloc(sizeof(*data));

    data->ret = ret;

    block_job_defer_to_main_loop(&job->common, backup_complete, data);

    job_defer_to_main_loop(&job->common.job, backup_complete, data);

}



static const BlockJobDriver backup_job_driver = {

block/commit.c

+6 −5
Original line number Diff line number Diff line
@@ -72,9 +72,10 @@ typedef struct { 
    int ret;

} CommitCompleteData;



static void commit_complete(BlockJob *job, void *opaque)

static void commit_complete(Job *job, void *opaque)

{

    CommitBlockJob *s = container_of(job, CommitBlockJob, common);

    CommitBlockJob *s = container_of(job, CommitBlockJob, common.job);

    BlockJob *bjob = &s->common;

    CommitCompleteData *data = opaque;

    BlockDriverState *top = blk_bs(s->top);

    BlockDriverState *base = blk_bs(s->base);
@@ -90,7 +91,7 @@ static void commit_complete(BlockJob *job, void *opaque) 
     * the normal backing chain can be restored. */

    blk_unref(s->base);



    if (!job_is_cancelled(&s->common.job) && ret == 0) {

    if (!job_is_cancelled(job) && ret == 0) {

        /* success */

        ret = bdrv_drop_intermediate(s->commit_top_bs, base,

                                     s->backing_file_str);
@@ -114,7 +115,7 @@ static void commit_complete(BlockJob *job, void *opaque) 
     * block_job_finish_sync()), block_job_completed() won't free it and

     * therefore the blockers on the intermediate nodes remain. This would

     * cause bdrv_set_backing_hd() to fail. */

    block_job_remove_all_bdrv(job);

    block_job_remove_all_bdrv(bjob);



    block_job_completed(&s->common, ret);

    g_free(data);
@@ -211,7 +212,7 @@ out: 


    data = g_malloc(sizeof(*data));

    data->ret = ret;

    block_job_defer_to_main_loop(&s->common, commit_complete, data);

    job_defer_to_main_loop(&s->common.job, commit_complete, data);

}



static const BlockJobDriver commit_job_driver = {

block/mirror.c

+8 −7
Original line number Diff line number Diff line
@@ -484,9 +484,10 @@ typedef struct { 
    int ret;

} MirrorExitData;



static void mirror_exit(BlockJob *job, void *opaque)

static void mirror_exit(Job *job, void *opaque)

{

    MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);

    MirrorBlockJob *s = container_of(job, MirrorBlockJob, common.job);

    BlockJob *bjob = &s->common;

    MirrorExitData *data = opaque;

    AioContext *replace_aio_context = NULL;

    BlockDriverState *src = s->source;
@@ -568,7 +569,7 @@ static void mirror_exit(BlockJob *job, void *opaque) 
     * the blockers on the intermediate nodes so that the resulting state is

     * valid. Also give up permissions on mirror_top_bs->backing, which might

     * block the removal. */

    block_job_remove_all_bdrv(job);

    block_job_remove_all_bdrv(bjob);

    bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL,

                            &error_abort);

    bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abort);
@@ -576,9 +577,9 @@ static void mirror_exit(BlockJob *job, void *opaque) 
    /* We just changed the BDS the job BB refers to (with either or both of the

     * bdrv_replace_node() calls), so switch the BB back so the cleanup does

     * the right thing. We don't need any permissions any more now. */

    blk_remove_bs(job->blk);

    blk_set_perm(job->blk, 0, BLK_PERM_ALL, &error_abort);

    blk_insert_bs(job->blk, mirror_top_bs, &error_abort);

    blk_remove_bs(bjob->blk);

    blk_set_perm(bjob->blk, 0, BLK_PERM_ALL, &error_abort);

    blk_insert_bs(bjob->blk, mirror_top_bs, &error_abort);



    block_job_completed(&s->common, data->ret);
@@ -901,7 +902,7 @@ immediate_exit: 
    if (need_drain) {

        bdrv_drained_begin(bs);

    }

    block_job_defer_to_main_loop(&s->common, mirror_exit, data);

    job_defer_to_main_loop(&s->common.job, mirror_exit, data);

}



static void mirror_complete(BlockJob *job, Error **errp)

block/stream.c

+7 −7
Original line number Diff line number Diff line
@@ -58,16 +58,16 @@ typedef struct { 
    int ret;

} StreamCompleteData;



static void stream_complete(BlockJob *job, void *opaque)

static void stream_complete(Job *job, void *opaque)

{

    StreamBlockJob *s = container_of(job, StreamBlockJob, common);

    StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);

    BlockJob *bjob = &s->common;

    StreamCompleteData *data = opaque;

    BlockDriverState *bs = blk_bs(job->blk);

    BlockDriverState *bs = blk_bs(bjob->blk);

    BlockDriverState *base = s->base;

    Error *local_err = NULL;



    if (!job_is_cancelled(&s->common.job) && bs->backing &&

        data->ret == 0) {

    if (!job_is_cancelled(job) && bs->backing && data->ret == 0) {

        const char *base_id = NULL, *base_fmt = NULL;

        if (base) {

            base_id = s->backing_file_str;
@@ -88,7 +88,7 @@ out: 
    /* Reopen the image back in read-only mode if necessary */

    if (s->bs_flags != bdrv_get_flags(bs)) {

        /* Give up write permissions before making it read-only */

        blk_set_perm(job->blk, 0, BLK_PERM_ALL, &error_abort);

        blk_set_perm(bjob->blk, 0, BLK_PERM_ALL, &error_abort);

        bdrv_reopen(bs, s->bs_flags, NULL);

    }
@@ -205,7 +205,7 @@ out: 
    /* Modify backing chain and close BDSes in main loop */

    data = g_malloc(sizeof(*data));

    data->ret = ret;

    block_job_defer_to_main_loop(&s->common, stream_complete, data);

    job_defer_to_main_loop(&s->common.job, stream_complete, data);

}



static const BlockJobDriver stream_job_driver = {

blockjob.c

+5 −52
Original line number Diff line number Diff line
@@ -360,7 +360,7 @@ static void block_job_decommission(BlockJob *job) 
    job->completed = true;

    job->busy = false;

    job->paused = false;

    job->deferred_to_main_loop = true;

    job->job.deferred_to_main_loop = true;

    block_job_txn_del_job(job);

    job_state_transition(&job->job, JOB_STATUS_NULL);

    job_unref(&job->job);
@@ -515,7 +515,7 @@ static int block_job_finish_sync(BlockJob *job, 
    /* block_job_drain calls block_job_enter, and it should be enough to

     * induce progress until the job completes or moves to the main thread.

    */

    while (!job->deferred_to_main_loop && !job->completed) {

    while (!job->job.deferred_to_main_loop && !job->completed) {

        block_job_drain(job);

    }

    while (!job->completed) {
@@ -729,7 +729,7 @@ void block_job_cancel(BlockJob *job, bool force) 
    block_job_cancel_async(job, force);

    if (!block_job_started(job)) {

        block_job_completed(job, -ECANCELED);

    } else if (job->deferred_to_main_loop) {

    } else if (job->job.deferred_to_main_loop) {

        block_job_completed_txn_abort(job);

    } else {

        block_job_enter(job);
@@ -1045,7 +1045,7 @@ static void block_job_enter_cond(BlockJob *job, bool(*fn)(BlockJob *job)) 
    if (!block_job_started(job)) {

        return;

    }

    if (job->deferred_to_main_loop) {

    if (job->job.deferred_to_main_loop) {

        return;

    }
@@ -1060,7 +1060,7 @@ static void block_job_enter_cond(BlockJob *job, bool(*fn)(BlockJob *job)) 
        return;

    }



    assert(!job->deferred_to_main_loop);

    assert(!job->job.deferred_to_main_loop);

    timer_del(&job->sleep_timer);

    job->busy = true;

    block_job_unlock();
@@ -1166,50 +1166,3 @@ BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err, 
    }

    return action;

}
 


typedef struct {

    BlockJob *job;

    AioContext *aio_context;

    BlockJobDeferToMainLoopFn *fn;

    void *opaque;

} BlockJobDeferToMainLoopData;



static void block_job_defer_to_main_loop_bh(void *opaque)

{

    BlockJobDeferToMainLoopData *data = opaque;

    AioContext *aio_context;



    /* Prevent race with block_job_defer_to_main_loop() */

    aio_context_acquire(data->aio_context);



    /* Fetch BDS AioContext again, in case it has changed */

    aio_context = blk_get_aio_context(data->job->blk);

    if (aio_context != data->aio_context) {

        aio_context_acquire(aio_context);

    }



    data->fn(data->job, data->opaque);



    if (aio_context != data->aio_context) {

        aio_context_release(aio_context);

    }



    aio_context_release(data->aio_context);



    g_free(data);

}



void block_job_defer_to_main_loop(BlockJob *job,

                                  BlockJobDeferToMainLoopFn *fn,

                                  void *opaque)

{

    BlockJobDeferToMainLoopData *data = g_malloc(sizeof(*data));

    data->job = job;

    data->aio_context = blk_get_aio_context(job->blk);

    data->fn = fn;

    data->opaque = opaque;

    job->deferred_to_main_loop = true;



    aio_bh_schedule_oneshot(qemu_get_aio_context(),

                            block_job_defer_to_main_loop_bh, data);

}