Re: [Qemu-devel] [PATCH v4 4/7] mirror: Add commit_job_type to perform commit with mirror code

[Fam Zheng]

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 <address@hidden>
> > ---
> >  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