On Wed, Nov 02, 2016 at 01:50:53PM -0400, John Snow wrote: > Add an explicit start field to specify the entrypoint. We already have > ownership of the coroutine itself AND managing the lifetime of the > coroutine, let's take control of creation of the coroutine, too. > > This will allow us to delay creation of the actual coroutine until we > know we'll actually start a BlockJob in block_job_start. This avoids > the sticky question of how to "un-create" a Coroutine that hasn't been > started yet. > > Signed-off-by: John Snow <js...@redhat.com> > --- > block/backup.c | 25 +++++++++++++------------ > block/commit.c | 3 ++- > block/mirror.c | 4 +++- > block/stream.c | 3 ++- > include/block/blockjob_int.h | 3 +++ > 5 files changed, 23 insertions(+), 15 deletions(-) > > diff --git a/block/backup.c b/block/backup.c > index 734a24c..4ed4494 100644 > --- a/block/backup.c > +++ b/block/backup.c > @@ -323,17 +323,6 @@ static void backup_drain(BlockJob *job) > } > } > > -static const BlockJobDriver backup_job_driver = { > - .instance_size = sizeof(BackupBlockJob), > - .job_type = BLOCK_JOB_TYPE_BACKUP, > - .set_speed = backup_set_speed, > - .commit = backup_commit, > - .abort = backup_abort, > - .clean = backup_clean, > - .attached_aio_context = backup_attached_aio_context, > - .drain = backup_drain, > -}; > - > static BlockErrorAction backup_error_action(BackupBlockJob *job, > bool read, int error) > { > @@ -542,6 +531,18 @@ static void coroutine_fn backup_run(void *opaque) > block_job_defer_to_main_loop(&job->common, backup_complete, data); > } > > +static const BlockJobDriver backup_job_driver = { > + .instance_size = sizeof(BackupBlockJob), > + .job_type = BLOCK_JOB_TYPE_BACKUP, > + .start = backup_run, > + .set_speed = backup_set_speed, > + .commit = backup_commit, > + .abort = backup_abort, > + .clean = backup_clean, > + .attached_aio_context = backup_attached_aio_context, > + .drain = backup_drain, > +}; > +
Some code movement here in addition to the .start addition, but to a better place (I am guessing that is intentional). > void backup_start(const char *job_id, BlockDriverState *bs, > BlockDriverState *target, int64_t speed, > MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap, > @@ -653,7 +654,7 @@ void backup_start(const char *job_id, BlockDriverState > *bs, > > block_job_add_bdrv(&job->common, target); > job->common.len = len; > - job->common.co = qemu_coroutine_create(backup_run, job); > + job->common.co = qemu_coroutine_create(job->common.driver->start, job); > block_job_txn_add_job(txn, &job->common); > qemu_coroutine_enter(job->common.co); > return; > diff --git a/block/commit.c b/block/commit.c > index e1eda89..20d27e2 100644 > --- a/block/commit.c > +++ b/block/commit.c > @@ -205,6 +205,7 @@ static const BlockJobDriver commit_job_driver = { > .instance_size = sizeof(CommitBlockJob), > .job_type = BLOCK_JOB_TYPE_COMMIT, > .set_speed = commit_set_speed, > + .start = commit_run, > }; > > void commit_start(const char *job_id, BlockDriverState *bs, > @@ -288,7 +289,7 @@ void commit_start(const char *job_id, BlockDriverState > *bs, > s->backing_file_str = g_strdup(backing_file_str); > > s->on_error = on_error; > - s->common.co = qemu_coroutine_create(commit_run, s); > + s->common.co = qemu_coroutine_create(s->common.driver->start, s); > > trace_commit_start(bs, base, top, s, s->common.co); > qemu_coroutine_enter(s->common.co); > diff --git a/block/mirror.c b/block/mirror.c > index b2c1fb8..659e09c 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -920,6 +920,7 @@ static const BlockJobDriver mirror_job_driver = { > .instance_size = sizeof(MirrorBlockJob), > .job_type = BLOCK_JOB_TYPE_MIRROR, > .set_speed = mirror_set_speed, > + .start = mirror_run, > .complete = mirror_complete, > .pause = mirror_pause, > .attached_aio_context = mirror_attached_aio_context, > @@ -930,6 +931,7 @@ static const BlockJobDriver commit_active_job_driver = { > .instance_size = sizeof(MirrorBlockJob), > .job_type = BLOCK_JOB_TYPE_COMMIT, > .set_speed = mirror_set_speed, > + .start = mirror_run, > .complete = mirror_complete, > .pause = mirror_pause, > .attached_aio_context = mirror_attached_aio_context, > @@ -1007,7 +1009,7 @@ static void mirror_start_job(const char *job_id, > BlockDriverState *bs, > } > } > > - s->common.co = qemu_coroutine_create(mirror_run, s); > + s->common.co = qemu_coroutine_create(s->common.driver->start, s); > trace_mirror_start(bs, s, s->common.co, opaque); > qemu_coroutine_enter(s->common.co); > } > diff --git a/block/stream.c b/block/stream.c > index b05856b..92309ff 100644 > --- a/block/stream.c > +++ b/block/stream.c > @@ -218,6 +218,7 @@ static const BlockJobDriver stream_job_driver = { > .instance_size = sizeof(StreamBlockJob), > .job_type = BLOCK_JOB_TYPE_STREAM, > .set_speed = stream_set_speed, > + .start = stream_run, > }; > > void stream_start(const char *job_id, BlockDriverState *bs, > @@ -254,7 +255,7 @@ void stream_start(const char *job_id, BlockDriverState > *bs, > s->bs_flags = orig_bs_flags; > > s->on_error = on_error; > - s->common.co = qemu_coroutine_create(stream_run, s); > + s->common.co = qemu_coroutine_create(s->common.driver->start, s); > trace_stream_start(bs, base, s, s->common.co); > qemu_coroutine_enter(s->common.co); > } > diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h > index 60d91a0..8223822 100644 > --- a/include/block/blockjob_int.h > +++ b/include/block/blockjob_int.h > @@ -47,6 +47,9 @@ struct BlockJobDriver { > /** 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; > + > /** > * Optional callback for job types whose completion must be triggered > * manually. > -- > 2.7.4 > Reviewed-by: Jeff Cody <jc...@redhat.com>