Am 12.01.2016 um 01:36 hat John Snow geschrieben: > 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 <[email protected]> > --- > 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, 39 insertions(+), 26 deletions(-) > > diff --git a/block/backup.c b/block/backup.c > index 58c76be..cadb880 100644 > --- a/block/backup.c > +++ b/block/backup.c > @@ -506,7 +506,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 a5d02aa..ef4fd5a 100644 > --- a/block/commit.c > +++ b/block/commit.c > @@ -202,7 +202,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 92706ab..18134e4 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -714,7 +714,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) > @@ -801,7 +801,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; > @@ -826,7 +826,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 1dfeac0..1bd8220 100644 > --- a/block/stream.c > +++ b/block/stream.c > @@ -216,7 +216,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 9b37ace..6713ecb 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -2855,28 +2855,28 @@ 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; > + BlockDriverState *bs = job->bs; > const char *msg = NULL; > > - trace_block_job_cb(bs, bs->job, ret); > + trace_block_job_cb(bs, job, ret);
You could directly use job->bs here, then the last user of the local variable bs is gone. > - assert(bs->job); > + assert(job); That's a bit late. If job == NULL, we have already segfaulted. I'd suggest to just remove the assertion now that the job is directly passed. Kevin
