Skip to content

Commit

Permalink
job: Move single job finalisation to Job
Browse files Browse the repository at this point in the history
This moves the finalisation of a single job from BlockJob to Job.

Some part of this code depends on job transactions, and job transactions
call this code, we introduce some temporary calls from Job functions to
BlockJob ones. This will be fixed once transactions move to Job, too.

Signed-off-by: Kevin Wolf <[email protected]>
Reviewed-by: Max Reitz <[email protected]>
  • Loading branch information
kevmw committed May 23, 2018
1 parent 139a9f0 commit 4ad3518
Show file tree
Hide file tree
Showing 10 changed files with 194 additions and 184 deletions.
22 changes: 11 additions & 11 deletions block/backup.c
Original file line number Diff line number Diff line change
Expand Up @@ -207,25 +207,25 @@ static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret)
}
}

static void backup_commit(BlockJob *job)
static void backup_commit(Job *job)
{
BackupBlockJob *s = container_of(job, BackupBlockJob, common);
BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
if (s->sync_bitmap) {
backup_cleanup_sync_bitmap(s, 0);
}
}

static void backup_abort(BlockJob *job)
static void backup_abort(Job *job)
{
BackupBlockJob *s = container_of(job, BackupBlockJob, common);
BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
if (s->sync_bitmap) {
backup_cleanup_sync_bitmap(s, -1);
}
}

static void backup_clean(BlockJob *job)
static void backup_clean(Job *job)
{
BackupBlockJob *s = container_of(job, BackupBlockJob, common);
BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
assert(s->target);
blk_unref(s->target);
s->target = NULL;
Expand Down Expand Up @@ -530,10 +530,10 @@ static const BlockJobDriver backup_job_driver = {
.free = block_job_free,
.user_resume = block_job_user_resume,
.start = backup_run,
.commit = backup_commit,
.abort = backup_abort,
.clean = backup_clean,
},
.commit = backup_commit,
.abort = backup_abort,
.clean = backup_clean,
.attached_aio_context = backup_attached_aio_context,
.drain = backup_drain,
};
Expand Down Expand Up @@ -678,8 +678,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
bdrv_reclaim_dirty_bitmap(bs, sync_bitmap, NULL);
}
if (job) {
backup_clean(&job->common);
block_job_early_fail(&job->common);
backup_clean(&job->common.job);
job_early_fail(&job->common.job);
}

return NULL;
Expand Down
2 changes: 1 addition & 1 deletion block/commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
if (commit_top_bs) {
bdrv_replace_node(commit_top_bs, top, &error_abort);
}
block_job_early_fail(&s->common);
job_early_fail(&s->common.job);
}


Expand Down
2 changes: 1 addition & 1 deletion block/mirror.c
Original file line number Diff line number Diff line change
Expand Up @@ -1257,7 +1257,7 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs,

g_free(s->replaces);
blk_unref(s->target);
block_job_early_fail(&s->common);
job_early_fail(&s->common.job);
}

bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL,
Expand Down
142 changes: 25 additions & 117 deletions blockjob.c
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job)
block_job_txn_ref(txn);
}

static void block_job_txn_del_job(BlockJob *job)
void block_job_txn_del_job(BlockJob *job)
{
if (job->txn) {
QLIST_REMOVE(job, txn_list);
Expand Down Expand Up @@ -262,101 +262,12 @@ const BlockJobDriver *block_job_driver(BlockJob *job)
return job->driver;
}

static void block_job_decommission(BlockJob *job)
{
assert(job);
job->job.busy = false;
job->job.paused = false;
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);
}

static void block_job_do_dismiss(BlockJob *job)
{
block_job_decommission(job);
}

static void block_job_conclude(BlockJob *job)
{
job_state_transition(&job->job, JOB_STATUS_CONCLUDED);
if (job->job.auto_dismiss || !job_started(&job->job)) {
block_job_do_dismiss(job);
}
}

static void block_job_update_rc(BlockJob *job)
{
if (!job->ret && job_is_cancelled(&job->job)) {
job->ret = -ECANCELED;
}
if (job->ret) {
job_state_transition(&job->job, JOB_STATUS_ABORTING);
}
}

static int block_job_prepare(BlockJob *job)
{
if (job->ret == 0 && job->driver->prepare) {
job->ret = job->driver->prepare(job);
}
return job->ret;
}

static void block_job_commit(BlockJob *job)
{
assert(!job->ret);
if (job->driver->commit) {
job->driver->commit(job);
}
}

static void block_job_abort(BlockJob *job)
{
assert(job->ret);
if (job->driver->abort) {
job->driver->abort(job);
}
}

static void block_job_clean(BlockJob *job)
{
if (job->driver->clean) {
job->driver->clean(job);
if (job->job.ret == 0 && job->driver->prepare) {
job->job.ret = job->driver->prepare(job);
}
}

static int block_job_finalize_single(BlockJob *job)
{
assert(job_is_completed(&job->job));

/* Ensure abort is called for late-transactional failures */
block_job_update_rc(job);

if (!job->ret) {
block_job_commit(job);
} else {
block_job_abort(job);
}
block_job_clean(job);

if (job->cb) {
job->cb(job->opaque, job->ret);
}

/* Emit events only if we actually started */
if (job_started(&job->job)) {
if (job_is_cancelled(&job->job)) {
job_event_cancelled(&job->job);
} else {
job_event_completed(&job->job);
}
}

block_job_txn_del_job(job);
block_job_conclude(job);
return 0;
return job->job.ret;
}

static void block_job_cancel_async(BlockJob *job, bool force)
Expand Down Expand Up @@ -424,8 +335,8 @@ static int block_job_finish_sync(BlockJob *job,
while (!job_is_completed(&job->job)) {
aio_poll(qemu_get_aio_context(), true);
}
ret = (job_is_cancelled(&job->job) && job->ret == 0)
? -ECANCELED : job->ret;
ret = (job_is_cancelled(&job->job) && job->job.ret == 0)
? -ECANCELED : job->job.ret;
job_unref(&job->job);
return ret;
}
Expand Down Expand Up @@ -466,7 +377,7 @@ static void block_job_completed_txn_abort(BlockJob *job)
assert(job_is_cancelled(&other_job->job));
block_job_finish_sync(other_job, NULL, NULL);
}
block_job_finalize_single(other_job);
job_finalize_single(&other_job->job);
aio_context_release(ctx);
}

Expand All @@ -478,6 +389,11 @@ static int block_job_needs_finalize(BlockJob *job)
return !job->job.auto_finalize;
}

static int block_job_finalize_single(BlockJob *job)
{
return job_finalize_single(&job->job);
}

static void block_job_do_finalize(BlockJob *job)
{
int rc;
Expand Down Expand Up @@ -516,7 +432,7 @@ static void block_job_completed_txn_success(BlockJob *job)
if (!job_is_completed(&other_job->job)) {
return;
}
assert(other_job->ret == 0);
assert(other_job->job.ret == 0);
}

block_job_txn_apply(txn, block_job_transition_to_pending, false);
Expand Down Expand Up @@ -601,14 +517,14 @@ void block_job_dismiss(BlockJob **jobptr, Error **errp)
return;
}

block_job_do_dismiss(job);
job_do_dismiss(&job->job);
*jobptr = NULL;
}

void block_job_cancel(BlockJob *job, bool force)
{
if (job->job.status == JOB_STATUS_CONCLUDED) {
block_job_do_dismiss(job);
job_do_dismiss(&job->job);
return;
}
block_job_cancel_async(job, force);
Expand Down Expand Up @@ -691,8 +607,8 @@ BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
info->status = job->job.status;
info->auto_finalize = job->job.auto_finalize;
info->auto_dismiss = job->job.auto_dismiss;
info->has_error = job->ret != 0;
info->error = job->ret ? g_strdup(strerror(-job->ret)) : NULL;
info->has_error = job->job.ret != 0;
info->error = job->job.ret ? g_strdup(strerror(-job->job.ret)) : NULL;
return info;
}

Expand Down Expand Up @@ -729,8 +645,8 @@ static void block_job_event_completed(Notifier *n, void *opaque)
return;
}

if (job->ret < 0) {
msg = strerror(-job->ret);
if (job->job.ret < 0) {
msg = strerror(-job->job.ret);
}

qapi_event_send_block_job_completed(job_type(&job->job),
Expand Down Expand Up @@ -787,7 +703,7 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
}

job = job_create(job_id, &driver->job_driver, blk_get_aio_context(blk),
flags, errp);
flags, cb, opaque, errp);
if (job == NULL) {
blk_unref(blk);
return NULL;
Expand All @@ -799,8 +715,6 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,

job->driver = driver;
job->blk = blk;
job->cb = cb;
job->opaque = opaque;

job->finalize_cancelled_notifier.notify = block_job_event_cancelled;
job->finalize_completed_notifier.notify = block_job_event_completed;
Expand Down Expand Up @@ -828,7 +742,7 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,

block_job_set_speed(job, speed, &local_err);
if (local_err) {
block_job_early_fail(job);
job_early_fail(&job->job);
error_propagate(errp, local_err);
return NULL;
}
Expand All @@ -847,20 +761,14 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
return job;
}

void block_job_early_fail(BlockJob *job)
{
assert(job->job.status == JOB_STATUS_CREATED);
block_job_decommission(job);
}

void block_job_completed(BlockJob *job, int ret)
{
assert(job && job->txn && !job_is_completed(&job->job));
assert(blk_bs(job->blk)->job == job);
job->ret = ret;
block_job_update_rc(job);
trace_block_job_completed(job, ret, job->ret);
if (job->ret) {
job->job.ret = ret;
job_update_rc(&job->job);
trace_block_job_completed(job, ret, job->job.ret);
if (job->job.ret) {
block_job_completed_txn_abort(job);
} else {
block_job_completed_txn_success(job);
Expand Down
9 changes: 0 additions & 9 deletions include/block/blockjob.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,6 @@ typedef struct BlockJob {
/** Rate limiting data structure for implementing @speed. */
RateLimit limit;

/** The completion function that will be called when the job completes. */
BlockCompletionFunc *cb;

/** Block other operations when block job is running */
Error *blocker;

Expand All @@ -94,12 +91,6 @@ typedef struct BlockJob {
/** BlockDriverStates that are involved in this block job */
GSList *nodes;

/** The opaque value that is passed to the completion function. */
void *opaque;

/** ret code passed to block_job_completed. */
int ret;

BlockJobTxn *txn;
QLIST_ENTRY(BlockJob) txn_list;
} BlockJob;
Expand Down
36 changes: 0 additions & 36 deletions include/block/blockjob_int.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,34 +54,6 @@ struct BlockJobDriver {
*/
int (*prepare)(BlockJob *job);

/**
* If the callback is not NULL, it will be invoked when all the jobs
* belonging to the same transaction complete; or upon this job's
* completion if it is not in a transaction. Skipped if NULL.
*
* All jobs will complete with a call to either .commit() or .abort() but
* never both.
*/
void (*commit)(BlockJob *job);

/**
* If the callback is not NULL, it will be invoked when any job in the
* same transaction fails; or upon this job's failure (due to error or
* cancellation) if it is not in a transaction. Skipped if NULL.
*
* All jobs will complete with a call to either .commit() or .abort() but
* never both.
*/
void (*abort)(BlockJob *job);

/**
* If the callback is not NULL, it will be invoked after a call to either
* .commit() or .abort(). Regardless of which callback is invoked after
* completion, .clean() will always be called, even if the job does not
* belong to a transaction group.
*/
void (*clean)(BlockJob *job);

/*
* If the callback is not NULL, it will be invoked before the job is
* resumed in a new AioContext. This is the place to move any resources
Expand Down Expand Up @@ -155,14 +127,6 @@ void block_job_yield(BlockJob *job);
*/
int64_t block_job_ratelimit_get_delay(BlockJob *job, uint64_t n);

/**
* block_job_early_fail:
* @bs: The block device.
*
* The block job could not be started, free it.
*/
void block_job_early_fail(BlockJob *job);

/**
* block_job_completed:
* @job: The job being completed.
Expand Down
Loading

0 comments on commit 4ad3518

Please sign in to comment.