diff options
| -rw-r--r-- | block/backup.c | 2 | ||||
| -rw-r--r-- | block/commit.c | 2 | ||||
| -rw-r--r-- | block/gluster.c | 18 | ||||
| -rw-r--r-- | block/io.c | 19 | ||||
| -rw-r--r-- | block/mirror.c | 2 | ||||
| -rw-r--r-- | blockdev.c | 1 | ||||
| -rw-r--r-- | blockjob.c | 750 | ||||
| -rw-r--r-- | include/block/blockjob.h | 16 | ||||
| -rw-r--r-- | include/block/blockjob_int.h | 27 | ||||
| -rw-r--r-- | tests/test-blockjob-txn.c | 7 | ||||
| -rw-r--r-- | tests/test-blockjob.c | 10 |
11 files changed, 463 insertions, 391 deletions
diff --git a/block/backup.c b/block/backup.c index a4fb2884f9..5387fbd84e 100644 --- a/block/backup.c +++ b/block/backup.c @@ -692,7 +692,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, } if (job) { backup_clean(&job->common); - block_job_unref(&job->common); + block_job_early_fail(&job->common); } return NULL; diff --git a/block/commit.c b/block/commit.c index 76a0d98c6f..a3028b20f3 100644 --- a/block/commit.c +++ b/block/commit.c @@ -426,7 +426,7 @@ fail: if (commit_top_bs) { bdrv_set_backing_hd(overlay_bs, top, &error_abort); } - block_job_unref(&s->common); + block_job_early_fail(&s->common); } diff --git a/block/gluster.c b/block/gluster.c index 7c76cd0988..8ba3bcca0b 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -1275,7 +1275,14 @@ static int find_allocation(BlockDriverState *bs, off_t start, if (offs < 0) { return -errno; /* D3 or D4 */ } - assert(offs >= start); + + if (offs < start) { + /* This is not a valid return by lseek(). We are safe to just return + * -EIO in this case, and we'll treat it like D4. Unfortunately some + * versions of gluster server will return offs < start, so an assert + * here will unnecessarily abort QEMU. */ + return -EIO; + } if (offs > start) { /* D2: in hole, next data at offs */ @@ -1307,7 +1314,14 @@ static int find_allocation(BlockDriverState *bs, off_t start, if (offs < 0) { return -errno; /* D1 and (H3 or H4) */ } - assert(offs >= start); + + if (offs < start) { + /* This is not a valid return by lseek(). We are safe to just return + * -EIO in this case, and we'll treat it like H4. Unfortunately some + * versions of gluster server will return offs < start, so an assert + * here will unnecessarily abort QEMU. */ + return -EIO; + } if (offs > start) { /* diff --git a/block/io.c b/block/io.c index fdd7485c22..ed31810c0a 100644 --- a/block/io.c +++ b/block/io.c @@ -26,6 +26,7 @@ #include "trace.h" #include "sysemu/block-backend.h" #include "block/blockjob.h" +#include "block/blockjob_int.h" #include "block/block_int.h" #include "qemu/cutils.h" #include "qapi/error.h" @@ -301,16 +302,9 @@ void bdrv_drain_all_begin(void) bool waited = true; BlockDriverState *bs; BdrvNextIterator it; - BlockJob *job = NULL; GSList *aio_ctxs = NULL, *ctx; - while ((job = block_job_next(job))) { - AioContext *aio_context = blk_get_aio_context(job->blk); - - aio_context_acquire(aio_context); - block_job_pause(job); - aio_context_release(aio_context); - } + block_job_pause_all(); for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { AioContext *aio_context = bdrv_get_aio_context(bs); @@ -354,7 +348,6 @@ void bdrv_drain_all_end(void) { BlockDriverState *bs; BdrvNextIterator it; - BlockJob *job = NULL; for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { AioContext *aio_context = bdrv_get_aio_context(bs); @@ -365,13 +358,7 @@ void bdrv_drain_all_end(void) aio_context_release(aio_context); } - while ((job = block_job_next(job))) { - AioContext *aio_context = blk_get_aio_context(job->blk); - - aio_context_acquire(aio_context); - block_job_resume(job); - aio_context_release(aio_context); - } + block_job_resume_all(); } void bdrv_drain_all(void) diff --git a/block/mirror.c b/block/mirror.c index e86f8f8ad7..b9eb2a2ddb 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1252,7 +1252,7 @@ fail: g_free(s->replaces); blk_unref(s->target); - block_job_unref(&s->common); + block_job_early_fail(&s->common); } bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL, diff --git a/blockdev.c b/blockdev.c index c63f4e82c7..892d768574 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3715,7 +3715,6 @@ void qmp_block_job_resume(const char *device, Error **errp) } trace_qmp_block_job_resume(job); - block_job_iostatus_reset(job); block_job_user_resume(job); aio_context_release(aio_context); } diff --git a/blockjob.c b/blockjob.c index 6e489327ff..a0d7e29b83 100644 --- a/blockjob.c +++ b/blockjob.c @@ -55,35 +55,20 @@ struct BlockJobTxn { static QLIST_HEAD(, BlockJob) block_jobs = QLIST_HEAD_INITIALIZER(block_jobs); -static char *child_job_get_parent_desc(BdrvChild *c) -{ - BlockJob *job = c->opaque; - return g_strdup_printf("%s job '%s'", - BlockJobType_lookup[job->driver->job_type], - job->id); -} - -static const BdrvChildRole child_job = { - .get_parent_desc = child_job_get_parent_desc, - .stay_at_node = true, -}; - -static void block_job_drained_begin(void *opaque) -{ - BlockJob *job = opaque; - block_job_pause(job); -} - -static void block_job_drained_end(void *opaque) -{ - BlockJob *job = opaque; - block_job_resume(job); -} - -static const BlockDevOps block_job_dev_ops = { - .drained_begin = block_job_drained_begin, - .drained_end = block_job_drained_end, -}; +/* + * The block job API is composed of two categories of functions. + * + * The first includes functions used by the monitor. The monitor is + * peculiar in that it accesses the block job list with block_job_get, and + * therefore needs consistency across block_job_get and the actual operation + * (e.g. block_job_set_speed). The consistency is achieved with + * aio_context_acquire/release. These functions are declared in blockjob.h. + * + * The second includes functions used by the block job drivers and sometimes + * by the core block layer. These do not care about locking, because the + * whole coroutine runs under the AioContext lock, and are declared in + * blockjob_int.h. + */ BlockJob *block_job_next(BlockJob *job) { @@ -106,6 +91,80 @@ BlockJob *block_job_get(const char *id) return NULL; } +BlockJobTxn *block_job_txn_new(void) +{ + BlockJobTxn *txn = g_new0(BlockJobTxn, 1); + QLIST_INIT(&txn->jobs); + txn->refcnt = 1; + return txn; +} + +static void block_job_txn_ref(BlockJobTxn *txn) +{ + txn->refcnt++; +} + +void block_job_txn_unref(BlockJobTxn *txn) +{ + if (txn && --txn->refcnt == 0) { + g_free(txn); + } +} + +void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job) +{ + if (!txn) { + return; + } + + assert(!job->txn); + job->txn = txn; + + QLIST_INSERT_HEAD(&txn->jobs, job, txn_list); + block_job_txn_ref(txn); +} + +static void block_job_pause(BlockJob *job) +{ + job->pause_count++; +} + +static void block_job_resume(BlockJob *job) +{ + assert(job->pause_count > 0); + job->pause_count--; + if (job->pause_count) { + return; + } + block_job_enter(job); +} + +static void block_job_ref(BlockJob *job) +{ + ++job->refcnt; +} + +static void block_job_attached_aio_context(AioContext *new_context, + void *opaque); +static void block_job_detach_aio_context(void *opaque); + +static void block_job_unref(BlockJob *job) +{ + if (--job->refcnt == 0) { + BlockDriverState *bs = blk_bs(job->blk); + bs->job = NULL; + block_job_remove_all_bdrv(job); + blk_remove_aio_context_notifier(job->blk, + block_job_attached_aio_context, + block_job_detach_aio_context, job); + blk_unref(job->blk); + error_free(job->blocker); + g_free(job->id); + QLIST_REMOVE(job, job_list); + g_free(job); + } +} + static void block_job_attached_aio_context(AioContext *new_context, void *opaque) { @@ -145,6 +204,36 @@ static void block_job_detach_aio_context(void *opaque) block_job_unref(job); } +static char *child_job_get_parent_desc(BdrvChild *c) +{ + BlockJob *job = c->opaque; + return g_strdup_printf("%s job '%s'", + BlockJobType_lookup[job->driver->job_type], + job->id); +} + +static const BdrvChildRole child_job = { + .get_parent_desc = child_job_get_parent_desc, + .stay_at_node = true, +}; + +static void block_job_drained_begin(void *opaque) +{ + BlockJob *job = opaque; + block_job_pause(job); +} + +static void block_job_drained_end(void *opaque) +{ + BlockJob *job = opaque; + block_job_resume(job); +} + +static const BlockDevOps block_job_dev_ops = { + .drained_begin = block_job_drained_begin, + .drained_end = block_job_drained_end, +}; + void block_job_remove_all_bdrv(BlockJob *job) { GSList *l; @@ -175,90 +264,6 @@ int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs, return 0; } -void *block_job_create(const char *job_id, const BlockJobDriver *driver, - BlockDriverState *bs, uint64_t perm, - uint64_t shared_perm, int64_t speed, int flags, - BlockCompletionFunc *cb, void *opaque, Error **errp) -{ - BlockBackend *blk; - BlockJob *job; - int ret; - - if (bs->job) { - error_setg(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs)); - return NULL; - } - - if (job_id == NULL && !(flags & BLOCK_JOB_INTERNAL)) { - job_id = bdrv_get_device_name(bs); - if (!*job_id) { - error_setg(errp, "An explicit job ID is required for this node"); - return NULL; - } - } - - if (job_id) { - if (flags & BLOCK_JOB_INTERNAL) { - error_setg(errp, "Cannot specify job ID for internal block job"); - return NULL; - } - - if (!id_wellformed(job_id)) { - error_setg(errp, "Invalid job ID '%s'", job_id); - return NULL; - } - - if (block_job_get(job_id)) { - error_setg(errp, "Job ID '%s' already in use", job_id); - return NULL; - } - } - - blk = blk_new(perm, shared_perm); - ret = blk_insert_bs(blk, bs, errp); - if (ret < 0) { - blk_unref(blk); - return NULL; - } - - job = g_malloc0(driver->instance_size); - job->driver = driver; - job->id = g_strdup(job_id); - job->blk = blk; - job->cb = cb; - job->opaque = opaque; - job->busy = false; - job->paused = true; - job->pause_count = 1; - job->refcnt = 1; - - error_setg(&job->blocker, "block device is in use by block job: %s", - BlockJobType_lookup[driver->job_type]); - block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, &error_abort); - bs->job = job; - - blk_set_dev_ops(blk, &block_job_dev_ops, job); - bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker); - - QLIST_INSERT_HEAD(&block_jobs, job, job_list); - - blk_add_aio_context_notifier(blk, block_job_attached_aio_context, - block_job_detach_aio_context, job); - - /* Only set speed when necessary to avoid NotSupported error */ - if (speed != 0) { - Error *local_err = NULL; - - block_job_set_speed(job, speed, &local_err); - if (local_err) { - block_job_unref(job); - error_propagate(errp, local_err); - return NULL; - } - } - return job; -} - bool block_job_is_internal(BlockJob *job) { return (job->id == NULL); @@ -293,30 +298,10 @@ void block_job_start(BlockJob *job) bdrv_coroutine_enter(blk_bs(job->blk), job->co); } -void block_job_ref(BlockJob *job) -{ - ++job->refcnt; -} - -void block_job_unref(BlockJob *job) -{ - if (--job->refcnt == 0) { - BlockDriverState *bs = blk_bs(job->blk); - bs->job = NULL; - block_job_remove_all_bdrv(job); - blk_remove_aio_context_notifier(job->blk, - block_job_attached_aio_context, - block_job_detach_aio_context, job); - blk_unref(job->blk); - error_free(job->blocker); - g_free(job->id); - QLIST_REMOVE(job, job_list); - g_free(job); - } -} - static void block_job_completed_single(BlockJob *job) { + assert(job->completed); + if (!job->ret) { if (job->driver->commit) { job->driver->commit(job); @@ -354,11 +339,57 @@ static void block_job_completed_single(BlockJob *job) block_job_unref(job); } +static void block_job_cancel_async(BlockJob *job) +{ + if (job->iostatus != BLOCK_DEVICE_IO_STATUS_OK) { + block_job_iostatus_reset(job); + } + if (job->user_paused) { + /* Do not call block_job_enter here, the caller will handle it. */ + job->user_paused = false; + job->pause_count--; + } + job->cancelled = true; +} + +static int block_job_finish_sync(BlockJob *job, + void (*finish)(BlockJob *, Error **errp), + Error **errp) +{ + Error *local_err = NULL; + int ret; + + assert(blk_bs(job->blk)->job == job); + + block_job_ref(job); + + if (finish) { + finish(job, &local_err); + } + if (local_err) { + error_propagate(errp, local_err); + block_job_unref(job); + return -EBUSY; + } + /* 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) { + block_job_drain(job); + } + while (!job->completed) { + aio_poll(qemu_get_aio_context(), true); + } + ret = (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret; + block_job_unref(job); + return ret; +} + static void block_job_completed_txn_abort(BlockJob *job) { AioContext *ctx; BlockJobTxn *txn = job->txn; - BlockJob *other_job, *next; + BlockJob *other_job; if (txn->aborting) { /* @@ -367,29 +398,34 @@ static void block_job_completed_txn_abort(BlockJob *job) return; } txn->aborting = true; + block_job_txn_ref(txn); + /* We are the first failed job. Cancel other jobs. */ QLIST_FOREACH(other_job, &txn->jobs, txn_list) { ctx = blk_get_aio_context(other_job->blk); aio_context_acquire(ctx); } + + /* Other jobs are effectively cancelled by us, set the status for + * them; this job, however, may or may not be cancelled, depending + * on the caller, so leave it. */ QLIST_FOREACH(other_job, &txn->jobs, txn_list) { - if (other_job == job || other_job->completed) { - /* Other jobs are "effectively" cancelled by us, set the status for - * them; this job, however, may or may not be cancelled, depending - * on the caller, so leave it. */ - if (other_job != job) { - other_job->cancelled = true; - } - continue; + if (other_job != job) { + block_job_cancel_async(other_job); } - block_job_cancel_sync(other_job); - assert(other_job->completed); } - QLIST_FOREACH_SAFE(other_job, &txn->jobs, txn_list, next) { + while (!QLIST_EMPTY(&txn->jobs)) { + other_job = QLIST_FIRST(&txn->jobs); ctx = blk_get_aio_context(other_job->blk); + if (!other_job->completed) { + assert(other_job->cancelled); + block_job_finish_sync(other_job, NULL, NULL); + } block_job_completed_single(other_job); aio_context_release(ctx); } + + block_job_txn_unref(txn); } static void block_job_completed_txn_success(BlockJob *job) @@ -416,21 +452,6 @@ static void block_job_completed_txn_success(BlockJob *job) } } -void block_job_completed(BlockJob *job, int ret) -{ - assert(blk_bs(job->blk)->job == job); - assert(!job->completed); - job->completed = true; - job->ret = ret; - if (!job->txn) { - block_job_completed_single(job); - } else if (ret < 0 || block_job_is_cancelled(job)) { - block_job_completed_txn_abort(job); - } else { - block_job_completed_txn_success(job); - } -} - void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp) { Error *local_err = NULL; @@ -462,135 +483,36 @@ void block_job_complete(BlockJob *job, Error **errp) job->driver->complete(job, errp); } -void block_job_pause(BlockJob *job) -{ - job->pause_count++; -} - void block_job_user_pause(BlockJob *job) { job->user_paused = true; block_job_pause(job); } -static bool block_job_should_pause(BlockJob *job) -{ - return job->pause_count > 0; -} - bool block_job_user_paused(BlockJob *job) { - return job ? job->user_paused : 0; -} - -void coroutine_fn block_job_pause_point(BlockJob *job) -{ - assert(job && block_job_started(job)); - - if (!block_job_should_pause(job)) { - return; - } - if (block_job_is_cancelled(job)) { - return; - } - - if (job->driver->pause) { - job->driver->pause(job); - } - - if (block_job_should_pause(job) && !block_job_is_cancelled(job)) { - job->paused = true; - job->busy = false; - qemu_coroutine_yield(); /* wait for block_job_resume() */ - job->busy = true; - job->paused = false; - } - - if (job->driver->resume) { - job->driver->resume(job); - } -} - -void block_job_resume(BlockJob *job) -{ - assert(job->pause_count > 0); - job->pause_count--; - if (job->pause_count) { - return; - } - block_job_enter(job); + return job->user_paused; } void block_job_user_resume(BlockJob *job) { if (job && job->user_paused && job->pause_count > 0) { + block_job_iostatus_reset(job); job->user_paused = false; block_job_resume(job); } } -void block_job_enter(BlockJob *job) -{ - if (job->co && !job->busy) { - bdrv_coroutine_enter(blk_bs(job->blk), job->co); - } -} - void block_job_cancel(BlockJob *job) { if (block_job_started(job)) { - job->cancelled = true; - block_job_iostatus_reset(job); + block_job_cancel_async(job); block_job_enter(job); } else { block_job_completed(job, -ECANCELED); } } -bool block_job_is_cancelled(BlockJob *job) -{ - return job->cancelled; -} - -void block_job_iostatus_reset(BlockJob *job) -{ - job->iostatus = BLOCK_DEVICE_IO_STATUS_OK; - if (job->driver->iostatus_reset) { - job->driver->iostatus_reset(job); - } -} - -static int block_job_finish_sync(BlockJob *job, - void (*finish)(BlockJob *, Error **errp), - Error **errp) -{ - Error *local_err = NULL; - int ret; - - assert(blk_bs(job->blk)->job == job); - - block_job_ref(job); - - finish(job, &local_err); - if (local_err) { - error_propagate(errp, local_err); - block_job_unref(job); - return -EBUSY; - } - /* 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) { - block_job_drain(job); - } - while (!job->completed) { - aio_poll(qemu_get_aio_context(), true); - } - ret = (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret; - block_job_unref(job); - return ret; -} - /* A wrapper around block_job_cancel() taking an Error ** parameter so it may be * used with block_job_finish_sync() without the need for (rather nasty) * function pointer casts there. */ @@ -622,42 +544,6 @@ int block_job_complete_sync(BlockJob *job, Error **errp) return block_job_finish_sync(job, &block_job_complete, errp); } -void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns) -{ - assert(job->busy); - - /* Check cancellation *before* setting busy = false, too! */ - if (block_job_is_cancelled(job)) { - return; - } - - job->busy = false; - if (!block_job_should_pause(job)) { - co_aio_sleep_ns(blk_get_aio_context(job->blk), type, ns); - } - job->busy = true; - - block_job_pause_point(job); -} - -void block_job_yield(BlockJob *job) -{ - assert(job->busy); - - /* Check cancellation *before* setting busy = false, too! */ - if (block_job_is_cancelled(job)) { - return; - } - - job->busy = false; - if (!block_job_should_pause(job)) { - qemu_coroutine_yield(); - } - job->busy = true; - - block_job_pause_point(job); -} - BlockJobInfo *block_job_query(BlockJob *job, Error **errp) { BlockJobInfo *info; @@ -717,6 +603,236 @@ static void block_job_event_completed(BlockJob *job, const char *msg) &error_abort); } +/* + * API for block job drivers and the block layer. These functions are + * declared in blockjob_int.h. + */ + +void *block_job_create(const char *job_id, const BlockJobDriver *driver, + BlockDriverState *bs, uint64_t perm, + uint64_t shared_perm, int64_t speed, int flags, + BlockCompletionFunc *cb, void *opaque, Error **errp) +{ + BlockBackend *blk; + BlockJob *job; + int ret; + + if (bs->job) { + error_setg(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs)); + return NULL; + } + + if (job_id == NULL && !(flags & BLOCK_JOB_INTERNAL)) { + job_id = bdrv_get_device_name(bs); + if (!*job_id) { + error_setg(errp, "An explicit job ID is required for this node"); + return NULL; + } + } + + if (job_id) { + if (flags & BLOCK_JOB_INTERNAL) { + error_setg(errp, "Cannot specify job ID for internal block job"); + return NULL; + } + + if (!id_wellformed(job_id)) { + error_setg(errp, "Invalid job ID '%s'", job_id); + return NULL; + } + + if (block_job_get(job_id)) { + error_setg(errp, "Job ID '%s' already in use", job_id); + return NULL; + } + } + + blk = blk_new(perm, shared_perm); + ret = blk_insert_bs(blk, bs, errp); + if (ret < 0) { + blk_unref(blk); + return NULL; + } + + job = g_malloc0(driver->instance_size); + job->driver = driver; + job->id = g_strdup(job_id); + job->blk = blk; + job->cb = cb; + job->opaque = opaque; + job->busy = false; + job->paused = true; + job->pause_count = 1; + job->refcnt = 1; + + error_setg(&job->blocker, "block device is in use by block job: %s", + BlockJobType_lookup[driver->job_type]); + block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, &error_abort); + bs->job = job; + + blk_set_dev_ops(blk, &block_job_dev_ops, job); + bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker); + + QLIST_INSERT_HEAD(&block_jobs, job, job_list); + + blk_add_aio_context_notifier(blk, block_job_attached_aio_context, + block_job_detach_aio_context, job); + + /* Only set speed when necessary to avoid NotSupported error */ + if (speed != 0) { + Error *local_err = NULL; + + block_job_set_speed(job, speed, &local_err); + if (local_err) { + block_job_unref(job); + error_propagate(errp, local_err); + return NULL; + } + } + return job; +} + +void block_job_pause_all(void) +{ + BlockJob *job = NULL; + while ((job = block_job_next(job))) { + AioContext *aio_context = blk_get_aio_context(job->blk); + + aio_context_acquire(aio_context); + block_job_pause(job); + aio_context_release(aio_context); + } +} + +void block_job_early_fail(BlockJob *job) +{ + block_job_unref(job); +} + +void block_job_completed(BlockJob *job, int ret) +{ + assert(blk_bs(job->blk)->job == job); + assert(!job->completed); + job->completed = true; + job->ret = ret; + if (!job->txn) { + block_job_completed_single(job); + } else if (ret < 0 || block_job_is_cancelled(job)) { + block_job_completed_txn_abort(job); + } else { + block_job_completed_txn_success(job); + } +} + +static bool block_job_should_pause(BlockJob *job) +{ + return job->pause_count > 0; +} + +void coroutine_fn block_job_pause_point(BlockJob *job) +{ + assert(job && block_job_started(job)); + + if (!block_job_should_pause(job)) { + return; + } + if (block_job_is_cancelled(job)) { + return; + } + + if (job->driver->pause) { + job->driver->pause(job); + } + + if (block_job_should_pause(job) && !block_job_is_cancelled(job)) { + job->paused = true; + job->busy = false; + qemu_coroutine_yield(); /* wait for block_job_resume() */ + job->busy = true; + job->paused = false; + } + + if (job->driver->resume) { + job->driver->resume(job); + } +} + +void block_job_resume_all(void) +{ + BlockJob *job = NULL; + while ((job = block_job_next(job))) { + AioContext *aio_context = blk_get_aio_context(job->blk); + + aio_context_acquire(aio_context); + block_job_resume(job); + aio_context_release(aio_context); + } +} + +void block_job_enter(BlockJob *job) +{ + if (!block_job_started(job)) { + return; + } + if (job->deferred_to_main_loop) { + return; + } + + if (!job->busy) { + bdrv_coroutine_enter(blk_bs(job->blk), job->co); + } +} + +bool block_job_is_cancelled(BlockJob *job) +{ + return job->cancelled; +} + +void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns) +{ + assert(job->busy); + + /* Check cancellation *before* setting busy = false, too! */ + if (block_job_is_cancelled(job)) { + return; + } + + job->busy = false; + if (!block_job_should_pause(job)) { + co_aio_sleep_ns(blk_get_aio_context(job->blk), type, ns); + } + job->busy = true; + + block_job_pause_point(job); +} + +void block_job_yield(BlockJob *job) +{ + assert(job->busy); + + /* Check cancellation *before* setting busy = false, too! */ + if (block_job_is_cancelled(job)) { + return; + } + + job->busy = false; + if (!block_job_should_pause(job)) { + qemu_coroutine_yield(); + } + job->busy = true; + + block_job_pause_point(job); +} + +void block_job_iostatus_reset(BlockJob *job) +{ + if (job->iostatus == BLOCK_DEVICE_IO_STATUS_OK) { + return; + } + assert(job->user_paused && job->pause_count > 0); + job->iostatus = BLOCK_DEVICE_IO_STATUS_OK; +} + void block_job_event_ready(BlockJob *job) { job->ready = true; @@ -790,7 +906,6 @@ static void block_job_defer_to_main_loop_bh(void *opaque) aio_context_acquire(aio_context); } - data->job->deferred_to_main_loop = false; data->fn(data->job, data->opaque); if (aio_context != data->aio_context) { @@ -816,36 +931,3 @@ void block_job_defer_to_main_loop(BlockJob *job, aio_bh_schedule_oneshot(qemu_get_aio_context(), block_job_defer_to_main_loop_bh, data); } - -BlockJobTxn *block_job_txn_new(void) -{ - BlockJobTxn *txn = g_new0(BlockJobTxn, 1); - QLIST_INIT(&txn->jobs); - txn->refcnt = 1; - return txn; -} - -static void block_job_txn_ref(BlockJobTxn *txn) -{ - txn->refcnt++; -} - -void block_job_txn_unref(BlockJobTxn *txn) -{ - if (txn && --txn->refcnt == 0) { - g_free(txn); - } -} - -void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job) -{ - if (!txn) { - return; - } - - assert(!job->txn); - job->txn = txn; - - QLIST_INSERT_HEAD(&txn->jobs, job, txn_list); - block_job_txn_ref(txn); -} diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 9e906f7d7e..09c7c694b5 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -235,14 +235,6 @@ void block_job_complete(BlockJob *job, Error **errp); BlockJobInfo *block_job_query(BlockJob *job, Error **errp); /** - * block_job_pause: - * @job: The job to be paused. - * - * Asynchronously pause the specified job. - */ -void block_job_pause(BlockJob *job); - -/** * block_job_user_pause: * @job: The job to be paused. * @@ -260,14 +252,6 @@ void block_job_user_pause(BlockJob *job); bool block_job_user_paused(BlockJob *job); /** - * block_job_resume: - * @job: The job to be resumed. - * - * Resume the specified job. Must be paired with a preceding block_job_pause. - */ -void block_job_resume(BlockJob *job); - -/** * block_job_user_resume: * @job: The job to be resumed. * diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index 3f86cc5acc..f13ad05c0d 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -44,9 +44,6 @@ struct BlockJobDriver { /** Optional callback for job types that support setting a speed limit */ void (*set_speed)(BlockJob *job, int64_t speed, Error **errp); - /** Optional callback for job types that need to forward I/O status reset */ - void (*iostatus_reset)(BlockJob *job); - /** Mandatory: Entrypoint for the Coroutine. */ CoroutineEntry *start; @@ -159,21 +156,26 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns); void block_job_yield(BlockJob *job); /** - * block_job_ref: - * @bs: The block device. + * block_job_pause_all: + * + * Asynchronously pause all jobs. + */ +void block_job_pause_all(void); + +/** + * block_job_resume_all: * - * Grab a reference to the block job. Should be paired with block_job_unref. + * Resume all block jobs. Must be paired with a preceding block_job_pause_all. */ -void block_job_ref(BlockJob *job); +void block_job_resume_all(void); /** - * block_job_unref: + * block_job_early_fail: * @bs: The block device. * - * Release reference to the block job and release resources if it is the last - * reference. + * The block job could not be started, free it. */ -void block_job_unref(BlockJob *job); +void block_job_early_fail(BlockJob *job); /** * block_job_completed: @@ -239,7 +241,8 @@ typedef void BlockJobDeferToMainLoopFn(BlockJob *job, void *opaque); * @fn: The function to run in the main loop * @opaque: The opaque value that is passed to @fn * - * Execute a given function in the main loop with the BlockDriverState + * This function must be called by the main job coroutine just before it + * returns. @fn is executed in the main loop with the BlockDriverState * AioContext acquired. Block jobs must call bdrv_unref(), bdrv_close(), and * anything that uses bdrv_drain_all() in the main loop. * diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c index 0f80194e85..c77343fc04 100644 --- a/tests/test-blockjob-txn.c +++ b/tests/test-blockjob-txn.c @@ -167,6 +167,11 @@ static void test_pair_jobs(int expected1, int expected2) block_job_start(job1); block_job_start(job2); + /* Release our reference now to trigger as many nice + * use-after-free bugs as possible. + */ + block_job_txn_unref(txn); + if (expected1 == -ECANCELED) { block_job_cancel(job1); } @@ -187,8 +192,6 @@ static void test_pair_jobs(int expected1, int expected2) g_assert_cmpint(result1, ==, expected1); g_assert_cmpint(result2, ==, expected2); - - block_job_txn_unref(txn); } static void test_pair_jobs_success(void) diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c index 740e740398..23bdf1a932 100644 --- a/tests/test-blockjob.c +++ b/tests/test-blockjob.c @@ -116,11 +116,11 @@ static void test_job_ids(void) job[1] = do_test_id(blk[1], "id0", false); /* But once job[0] finishes we can reuse its ID */ - block_job_unref(job[0]); + block_job_early_fail(job[0]); job[1] = do_test_id(blk[1], "id0", true); /* No job ID specified, defaults to the backend name ('drive1') */ - block_job_unref(job[1]); + block_job_early_fail(job[1]); job[1] = do_test_id(blk[1], NULL, true); /* Duplicate job ID */ @@ -133,9 +133,9 @@ static void test_job_ids(void) /* This one is valid */ job[2] = do_test_id(blk[2], "id_2", true); - block_job_unref(job[0]); - block_job_unref(job[1]); - block_job_unref(job[2]); + block_job_early_fail(job[0]); + block_job_early_fail(job[1]); + block_job_early_fail(job[2]); destroy_blk(blk[0]); destroy_blk(blk[1]); |