Commit 22dffcbe authored by John Snow's avatar John Snow Committed by Max Reitz
Browse files

block/commit: refactor commit to use job callbacks 



Use the component callbacks; prepare, abort, and clean.

NB: prepare is only called when the job has not yet failed;
and abort can be called after prepare.

complete -> prepare -> abort -> clean
complete -> abort -> clean

During refactor, a potential problem with bdrv_drop_intermediate
was identified, the patched behavior is no worse than the pre-patch
behavior, so leave a FIXME for now to be fixed in a future patch.

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
Reviewed-by: default avatarMax Reitz <mreitz@redhat.com>
Message-id: 20180906130225.5118-5-jsnow@redhat.com
Reviewed-by: default avatarJeff Cody <jcody@redhat.com>
Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
parent cf6320df

block/commit.c

+51 −41
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ typedef struct CommitBlockJob { 
    BlockDriverState *commit_top_bs;

    BlockBackend *top;

    BlockBackend *base;

    BlockDriverState *base_bs;

    BlockdevOnError on_error;

    int base_flags;

    char *backing_file_str;
@@ -68,61 +69,67 @@ static int coroutine_fn commit_populate(BlockBackend *bs, BlockBackend *base, 
    return 0;

}



static void commit_exit(Job *job)

static int commit_prepare(Job *job)

{

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

    BlockJob *bjob = &s->common;

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

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

    BlockDriverState *commit_top_bs = s->commit_top_bs;

    bool remove_commit_top_bs = false;



    /* Make sure commit_top_bs and top stay around until bdrv_replace_node() */

    bdrv_ref(top);

    bdrv_ref(commit_top_bs);



    /* Remove base node parent that still uses BLK_PERM_WRITE/RESIZE before

     * the normal backing chain can be restored. */

    blk_unref(s->base);

    s->base = NULL;



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

        /* success */

        job->ret = bdrv_drop_intermediate(s->commit_top_bs, base,

    /* FIXME: bdrv_drop_intermediate treats total failures and partial failures

     * identically. Further work is needed to disambiguate these cases. */

    return bdrv_drop_intermediate(s->commit_top_bs, s->base_bs,

                                  s->backing_file_str);

    } else {

        /* XXX Can (or should) we somehow keep 'consistent read' blocked even

}



static void commit_abort(Job *job)

{

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

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



    /* Make sure commit_top_bs and top stay around until bdrv_replace_node() */

    bdrv_ref(top_bs);

    bdrv_ref(s->commit_top_bs);



    if (s->base) {

        blk_unref(s->base);

    }



    /* free the blockers on the intermediate nodes so that bdrv_replace_nodes

     * can succeed */

    block_job_remove_all_bdrv(&s->common);



    /* If bdrv_drop_intermediate() failed (or was not invoked), remove the

     * commit filter driver from the backing chain now. Do this as the final

     * step so that the 'consistent read' permission can be granted.

     *

     * XXX Can (or should) we somehow keep 'consistent read' blocked even

     * after the failed/cancelled commit job is gone? If we already wrote

     * something to base, the intermediate images aren't valid any more. */

        remove_commit_top_bs = true;

    bdrv_child_try_set_perm(s->commit_top_bs->backing, 0, BLK_PERM_ALL,

                            &error_abort);

    bdrv_replace_node(s->commit_top_bs, backing_bs(s->commit_top_bs),

                      &error_abort);



    bdrv_unref(s->commit_top_bs);

    bdrv_unref(top_bs);

}



static void commit_clean(Job *job)

{

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



    /* restore base open flags here if appropriate (e.g., change the base back

     * to r/o). These reopens do not need to be atomic, since we won't abort

     * even on failure here */

    if (s->base_flags != bdrv_get_flags(base)) {

        bdrv_reopen(base, s->base_flags, NULL);

    if (s->base_flags != bdrv_get_flags(s->base_bs)) {

        bdrv_reopen(s->base_bs, s->base_flags, NULL);

    }



    g_free(s->backing_file_str);

    blk_unref(s->top);



    /* If there is more than one reference to the job (e.g. if called from

     * job_finish_sync()), 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(bjob);



    /* If bdrv_drop_intermediate() didn't already do that, remove the commit

     * filter driver from the backing chain. Do this as the final step so that

     * the 'consistent read' permission can be granted.  */

    if (remove_commit_top_bs) {

        bdrv_child_try_set_perm(commit_top_bs->backing, 0, BLK_PERM_ALL,

                                &error_abort);

        bdrv_replace_node(commit_top_bs, backing_bs(commit_top_bs),

                          &error_abort);

    }



    bdrv_unref(commit_top_bs);

    bdrv_unref(top);

}



static int coroutine_fn commit_run(Job *job, Error **errp)
@@ -211,7 +218,9 @@ static const BlockJobDriver commit_job_driver = { 
        .user_resume   = block_job_user_resume,

        .drain         = block_job_drain,

        .run           = commit_run,

        .exit          = commit_exit,

        .prepare       = commit_prepare,

        .abort         = commit_abort,

        .clean         = commit_clean

    },

};
@@ -345,6 +354,7 @@ void commit_start(const char *job_id, BlockDriverState *bs, 
    if (ret < 0) {

        goto fail;

    }

    s->base_bs = base;



    /* Required permissions are already taken with block_job_add_bdrv() */

    s->top = blk_new(0, BLK_PERM_ALL);