[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 5/5] blockjob: add Job parameter to BlockComp
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH v2 5/5] blockjob: add Job parameter to BlockCompletionFunc |
Date: |
Wed, 27 Jan 2016 10:25:10 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, 01/26 18:54, John Snow wrote:
> It will no longer be sufficient to rely on the opaque parameter
> containing a BDS, and there's no way to reliably include a
> self-reference to the job we're creating, so always pass the Job
> object forward to any callbacks.
>
> Signed-off-by: John Snow <address@hidden>
> ---
> block/backup.c | 2 +-
> block/commit.c | 2 +-
> block/mirror.c | 6 +++---
> block/stream.c | 2 +-
> blockdev.c | 14 +++++---------
> blockjob.c | 13 +++++++++++--
> include/block/block.h | 2 ++
> include/block/block_int.h | 10 +++++-----
> include/block/blockjob.h | 6 ++++--
> qemu-img.c | 4 ++--
> tests/test-blockjob-txn.c | 4 ++--
> 11 files changed, 37 insertions(+), 28 deletions(-)
>
> diff --git a/block/backup.c b/block/backup.c
> index 735cbe6..dd7e532 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -492,7 +492,7 @@ BlockJob *backup_start(BlockDriverState *bs,
> BlockDriverState *target,
> BdrvDirtyBitmap *sync_bitmap,
> BlockdevOnError on_source_error,
> BlockdevOnError on_target_error,
> - BlockCompletionFunc *cb, void *opaque,
> + BlockJobCompletionFunc *cb, void *opaque,
> BlockJobTxn *txn, Error **errp)
> {
> int64_t len;
> diff --git a/block/commit.c b/block/commit.c
> index 446a3ae..aca4b84 100644
> --- a/block/commit.c
> +++ b/block/commit.c
> @@ -203,7 +203,7 @@ static const BlockJobDriver commit_job_driver = {
>
> void commit_start(BlockDriverState *bs, BlockDriverState *base,
> BlockDriverState *top, int64_t speed,
> - BlockdevOnError on_error, BlockCompletionFunc *cb,
> + BlockdevOnError on_error, BlockJobCompletionFunc *cb,
> void *opaque, const char *backing_file_str, Error **errp)
> {
> CommitBlockJob *s;
> diff --git a/block/mirror.c b/block/mirror.c
> index b193e36..8016834 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -715,7 +715,7 @@ static BlockJob *mirror_start_job(BlockDriverState *bs,
> BlockdevOnError on_source_error,
> BlockdevOnError on_target_error,
> bool unmap,
> - BlockCompletionFunc *cb,
> + BlockJobCompletionFunc *cb,
> void *opaque, Error **errp,
> const BlockJobDriver *driver,
> bool is_none_mode, BlockDriverState *base)
> @@ -802,7 +802,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState
> *target,
> MirrorSyncMode mode, BlockdevOnError on_source_error,
> BlockdevOnError on_target_error,
> bool unmap,
> - BlockCompletionFunc *cb,
> + BlockJobCompletionFunc *cb,
> void *opaque, Error **errp)
> {
> bool is_none_mode;
> @@ -827,7 +827,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState
> *target,
> BlockJob *commit_active_start(BlockDriverState *bs, BlockDriverState *base,
> int64_t speed,
> BlockdevOnError on_error,
> - BlockCompletionFunc *cb,
> + BlockJobCompletionFunc *cb,
> void *opaque, Error **errp)
> {
> int64_t length, base_length;
> diff --git a/block/stream.c b/block/stream.c
> index 26a2990..0c8ae20 100644
> --- a/block/stream.c
> +++ b/block/stream.c
> @@ -217,7 +217,7 @@ static const BlockJobDriver stream_job_driver = {
> BlockJob *stream_start(BlockDriverState *bs, BlockDriverState *base,
> const char *backing_file_str, int64_t speed,
> BlockdevOnError on_error,
> - BlockCompletionFunc *cb,
> + BlockJobCompletionFunc *cb,
> void *opaque, Error **errp)
> {
> StreamBlockJob *s;
> diff --git a/blockdev.c b/blockdev.c
> index ad46aa8..9851405 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2854,28 +2854,24 @@ out:
> aio_context_release(aio_context);
> }
>
> -static void block_job_cb(void *opaque, int ret)
> +static void block_job_cb(BlockJob *job, int ret)
> {
> /* Note that this function may be executed from another AioContext
> besides
> * the QEMU main loop. If you need to access anything that assumes the
> * QEMU global mutex, use a BH or introduce a mutex.
> */
> -
> - BlockDriverState *bs = opaque;
> const char *msg = NULL;
>
> - trace_block_job_cb(bs, bs->job, ret);
> -
> - assert(bs->job);
> + trace_block_job_cb(job->bs, job, ret);
>
> if (ret < 0) {
> msg = strerror(-ret);
> }
>
> - if (block_job_is_cancelled(bs->job)) {
> - block_job_event_cancelled(bs->job);
> + if (block_job_is_cancelled(job)) {
> + block_job_event_cancelled(job);
> } else {
> - block_job_event_completed(bs->job, msg);
> + block_job_event_completed(job, msg);
> }
> }
>
> diff --git a/blockjob.c b/blockjob.c
> index 80adb9d..fe6873c 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -51,7 +51,7 @@ struct BlockJobTxn {
> };
>
> void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
> - int64_t speed, BlockCompletionFunc *cb,
> + int64_t speed, BlockJobCompletionFunc *cb,
> void *opaque, Error **errp)
> {
> BlockJob *job;
> @@ -90,6 +90,15 @@ void *block_job_create(const BlockJobDriver *driver,
> BlockDriverState *bs,
> return job;
> }
>
> +/**
> + * For generic callbacks to retrieve the data they submitted
> + * during callback registration.
> + */
> +void *block_job_data(BlockJob *job)
> +{
> + return job->opaque;
> +}
> +
> void block_job_ref(BlockJob *job)
> {
> ++job->refcnt;
> @@ -118,7 +127,7 @@ static void block_job_completed_single(BlockJob *job)
> job->driver->abort(job);
> }
> }
> - job->cb(job->opaque, job->ret);
> + job->cb(job, job->ret);
> if (job->txn) {
> block_job_txn_unref(job->txn);
> }
> diff --git a/include/block/block.h b/include/block/block.h
> index 25f36dc..ed6f2ee 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -16,6 +16,8 @@ typedef struct BdrvChild BdrvChild;
> typedef struct BdrvChildRole BdrvChildRole;
> typedef struct BlockJobTxn BlockJobTxn;
>
> +typedef void BlockJobCompletionFunc(BlockJob *job, int ret);
> +
> typedef struct BlockDriverInfo {
> /* in bytes, 0 if irrelevant */
> int cluster_size;
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 9a5cc39..271e922 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -601,7 +601,7 @@ int is_windows_drive(const char *filename);
> BlockJob *stream_start(BlockDriverState *bs, BlockDriverState *base,
> const char *base_id, int64_t speed,
> BlockdevOnError on_error,
> - BlockCompletionFunc *cb,
> + BlockJobCompletionFunc *cb,
> void *opaque, Error **errp);
>
> /**
> @@ -619,7 +619,7 @@ BlockJob *stream_start(BlockDriverState *bs,
> BlockDriverState *base,
> */
> void commit_start(BlockDriverState *bs, BlockDriverState *base,
> BlockDriverState *top, int64_t speed,
> - BlockdevOnError on_error, BlockCompletionFunc *cb,
> + BlockdevOnError on_error, BlockJobCompletionFunc *cb,
> void *opaque, const char *backing_file_str, Error **errp);
> /**
> * commit_active_start:
> @@ -635,7 +635,7 @@ void commit_start(BlockDriverState *bs, BlockDriverState
> *base,
> BlockJob *commit_active_start(BlockDriverState *bs, BlockDriverState *base,
> int64_t speed,
> BlockdevOnError on_error,
> - BlockCompletionFunc *cb,
> + BlockJobCompletionFunc *cb,
> void *opaque, Error **errp);
> /*
> * mirror_start:
> @@ -665,7 +665,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState
> *target,
> MirrorSyncMode mode, BlockdevOnError on_source_error,
> BlockdevOnError on_target_error,
> bool unmap,
> - BlockCompletionFunc *cb,
> + BlockJobCompletionFunc *cb,
> void *opaque, Error **errp);
>
> /*
> @@ -689,7 +689,7 @@ BlockJob *backup_start(BlockDriverState *bs,
> BlockDriverState *target,
> BdrvDirtyBitmap *sync_bitmap,
> BlockdevOnError on_source_error,
> BlockdevOnError on_target_error,
> - BlockCompletionFunc *cb, void *opaque,
> + BlockJobCompletionFunc *cb, void *opaque,
> BlockJobTxn *txn, Error **errp);
>
> void blk_set_bs(BlockBackend *blk, BlockDriverState *bs);
> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
> index d84ccd8..b233d27 100644
> --- a/include/block/blockjob.h
> +++ b/include/block/blockjob.h
> @@ -143,7 +143,7 @@ struct BlockJob {
> int64_t speed;
>
> /** The completion function that will be called when the job completes.
> */
> - BlockCompletionFunc *cb;
> + BlockJobCompletionFunc *cb;
>
> /** Block other operations when block job is running */
> Error *blocker;
> @@ -186,9 +186,11 @@ struct BlockJob {
> * called from a wrapper that is specific to the job type.
> */
> void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
> - int64_t speed, BlockCompletionFunc *cb,
> + int64_t speed, BlockJobCompletionFunc *cb,
> void *opaque, Error **errp);
>
> +void *block_job_data(BlockJob *job);
> +
> /**
> * block_job_sleep_ns:
> * @job: The job that calls the function.
> diff --git a/qemu-img.c b/qemu-img.c
> index 8d11708..cc96cc8 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -635,9 +635,9 @@ typedef struct CommonBlockJobCBInfo {
> Error **errp;
> } CommonBlockJobCBInfo;
>
> -static void common_block_job_cb(void *opaque, int ret)
> +static void common_block_job_cb(BlockJob *job, int ret)
> {
> - CommonBlockJobCBInfo *cbi = opaque;
> + CommonBlockJobCBInfo *cbi = block_job_data(job);
>
> if (ret < 0) {
> error_setg_errno(cbi->errp, -ret, "Block job failed");
> diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
> index 34747e9..3e8d17d 100644
> --- a/tests/test-blockjob-txn.c
> +++ b/tests/test-blockjob-txn.c
> @@ -66,9 +66,9 @@ typedef struct {
> int *result;
> } TestBlockJobCBData;
>
> -static void test_block_job_cb(void *opaque, int ret)
> +static void test_block_job_cb(BlockJob *job, int ret)
> {
> - TestBlockJobCBData *data = opaque;
> + TestBlockJobCBData *data = block_job_data(job);
> if (!ret && block_job_is_cancelled(&data->job->common)) {
> ret = -ECANCELED;
> }
> --
> 2.4.3
>
>
Reviewed-by: Fam Zheng <address@hidden>
- [Qemu-devel] [PATCH v2 0/5] block: reduce reliance on bs->job pointer, John Snow, 2016/01/26
- [Qemu-devel] [PATCH v2 2/5] block: Allow stream_start to return job references, John Snow, 2016/01/26
- [Qemu-devel] [PATCH v2 1/5] block: Allow mirror_start to return job references, John Snow, 2016/01/26
- [Qemu-devel] [PATCH v2 3/5] block: allow backup_start to return job references, John Snow, 2016/01/26
- [Qemu-devel] [PATCH v2 4/5] block/backup: Pack Notifier within BackupBlockJob, John Snow, 2016/01/26
- [Qemu-devel] [PATCH v2 5/5] blockjob: add Job parameter to BlockCompletionFunc, John Snow, 2016/01/26
- Re: [Qemu-devel] [PATCH v2 5/5] blockjob: add Job parameter to BlockCompletionFunc,
Fam Zheng <=