On Thu, 03/27 10:41, Eric Blake wrote:
> On 03/27/2014 03:09 AM, Fam Zheng wrote:
> > The new command pair is added to manage user created dirty bitmap. The
> > dirty bitmap's name is mandatory and must be unique for the same device,
> > but different devices can have bitmaps with the same names.
> >
> > Signed-off-by: Fam Zheng <f...@redhat.com>
> > ---
>
> > +++ b/qapi-schema.json
> > @@ -2209,6 +2209,51 @@
> > '*on-target-error': 'BlockdevOnError' } }
> >
> > ##
> > +# @DirtyBitmap
> > +#
> > +# @device: name of device which the bitmap is tracking
> > +#
> > +# @name: name of the dirty bitmap
> > +#
> > +# @granularity: #optional the bitmap granularity, default is 64k for
> > +# dirty-bitmap-add
>
> Optional, but only affects dirty-bitmap-add. You later document...
>
> > +# @dirty-bitmap-remove
> > +#
> > +# Remove a dirty bitmap on the device
> > +#
> > +# Setting granularity has no effect here.
>
> ...that it is silently ignored where it can't be used here, and again in
> 7/9 for both dirty-bitmap-disable and dirty-bitmap-enable.
>
> I think it would be smarter to do:
>
> { 'type': 'DirtyBitmap',
> 'data': { 'device': 'str', 'name': 'str' } }
>
> {'command': 'dirty-bitmap-add',
> 'data': { 'map': 'DirtyBitmap', '*granularity': 'int' } }
>
> Or:
>
> { 'type': 'DirtyBitmap',
> 'data': { 'device': 'str', 'name': 'str' } }
> { 'type': 'DirtyBitmapGranularity',
> 'base': 'DirtyBitmap',
> 'data': { '*granularity': 'int' } }
> {'command': 'dirty-bitmap-add',
> 'data': 'DirtyBitmapGranularity' }
>
>
> which says that the 'DirtyBitmap' struct has no optional members, and
> instead of silently ignoring an optional member in 3 commands, we
> instead write the one command that takes the optional argument when we
> actually care about it.
>
Yes, taking the later one since a type is needed for transaction support.
Thanks,
Fam
