| Message ID | 1380542578-2387-5-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 09/30/2013 06:02 AM, Fam Zheng wrote: > Commit active layer will be implemented in block/mirror.c, prepare a new > job type to let it have a right type name for the user. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > block/mirror.c | 12 +++++++++++- > blockdev.c | 2 +- > include/block/block_int.h | 2 ++ > 3 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/block/mirror.c b/block/mirror.c > index af6851f..20dcfb6 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -532,10 +532,19 @@ static const BlockJobType mirror_job_type = { > .complete = mirror_complete, > }; > > +static const BlockJobType commit_job_type = { > + .instance_size = sizeof(MirrorBlockJob), > + .job_type = "commit", I still wonder if we should complete the conversion over to a QAPI enum type for all valid job types prior to hard-coding open strings through yet more of the code base. As long as we don't have introspection done yet, we can still make the switch, and in the long run, having an enum of valid job types seems like it will be better for maintenance, all with no change to what is sent over the wire in QMP. > @@ -390,6 +391,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, > int64_t speed, int64_t granularity, int64_t buf_size, > MirrorSyncMode mode, BlockdevOnError on_source_error, > BlockdevOnError on_target_error, > + bool commit_job, If we DO create a QAPI enum for job type, then this parameter would be the enum type, rather than a bool.
On Tue, 10/01 11:13, Eric Blake wrote: > On 09/30/2013 06:02 AM, Fam Zheng wrote: > > Commit active layer will be implemented in block/mirror.c, prepare a new > > job type to let it have a right type name for the user. > > > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > block/mirror.c | 12 +++++++++++- > > blockdev.c | 2 +- > > include/block/block_int.h | 2 ++ > > 3 files changed, 14 insertions(+), 2 deletions(-) > > > > diff --git a/block/mirror.c b/block/mirror.c > > index af6851f..20dcfb6 100644 > > --- a/block/mirror.c > > +++ b/block/mirror.c > > @@ -532,10 +532,19 @@ static const BlockJobType mirror_job_type = { > > .complete = mirror_complete, > > }; > > > > +static const BlockJobType commit_job_type = { > > + .instance_size = sizeof(MirrorBlockJob), > > + .job_type = "commit", > > I still wonder if we should complete the conversion over to a QAPI enum > type for all valid job types prior to hard-coding open strings through > yet more of the code base. As long as we don't have introspection done > yet, we can still make the switch, and in the long run, having an enum > of valid job types seems like it will be better for maintenance, all > with no change to what is sent over the wire in QMP. > > > @@ -390,6 +391,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, > > int64_t speed, int64_t granularity, int64_t buf_size, > > MirrorSyncMode mode, BlockdevOnError on_source_error, > > BlockdevOnError on_target_error, > > + bool commit_job, > > If we DO create a QAPI enum for job type, then this parameter would be > the enum type, rather than a bool. > Good point, I'll work on a QAPI enum and base this on it. Fam
diff --git a/block/mirror.c b/block/mirror.c index af6851f..20dcfb6 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -532,10 +532,19 @@ static const BlockJobType mirror_job_type = { .complete = mirror_complete, }; +static const BlockJobType commit_job_type = { + .instance_size = sizeof(MirrorBlockJob), + .job_type = "commit", + .set_speed = mirror_set_speed, + .iostatus_reset= mirror_iostatus_reset, + .complete = mirror_complete, +}; + void mirror_start(BlockDriverState *bs, BlockDriverState *target, int64_t speed, int64_t granularity, int64_t buf_size, MirrorSyncMode mode, BlockdevOnError on_source_error, BlockdevOnError on_target_error, + bool commit_job, BlockDriverCompletionFunc *cb, void *opaque, Error **errp) { @@ -573,7 +582,8 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, base = bs->backing_hd; } - s = block_job_create(&mirror_job_type, bs, speed, cb, opaque, errp); + s = block_job_create(commit_job ? &commit_job_type : &mirror_job_type, + bs, speed, cb, opaque, errp); if (!s) { return; } diff --git a/blockdev.c b/blockdev.c index 3acd330..032913e 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1963,7 +1963,7 @@ void qmp_drive_mirror(const char *device, const char *target, } mirror_start(bs, target_bs, speed, granularity, buf_size, sync, - on_source_error, on_target_error, + on_source_error, on_target_error, false, block_job_cb, bs, &local_err); if (local_err != NULL) { bdrv_unref(target_bs); diff --git a/include/block/block_int.h b/include/block/block_int.h index 3eeb6fe..77a6295 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -377,6 +377,7 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base, * @mode: Whether to collapse all images in the chain to the target. * @on_source_error: The action to take upon error reading from the source. * @on_target_error: The action to take upon error writing to the target. + * @commit_job: Whether the job type string should be "commit". * @cb: Completion function for the job. * @opaque: Opaque pointer value passed to @cb. * @errp: Error object. @@ -390,6 +391,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, int64_t speed, int64_t granularity, int64_t buf_size, MirrorSyncMode mode, BlockdevOnError on_source_error, BlockdevOnError on_target_error, + bool commit_job, BlockDriverCompletionFunc *cb, void *opaque, Error **errp);
Commit active layer will be implemented in block/mirror.c, prepare a new job type to let it have a right type name for the user. Signed-off-by: Fam Zheng <famz@redhat.com> --- block/mirror.c | 12 +++++++++++- blockdev.c | 2 +- include/block/block_int.h | 2 ++ 3 files changed, 14 insertions(+), 2 deletions(-)