Re: [Qemu-block] [Qemu-devel] [PATCH v22 23/30] qmp: add persistent flag to block-dirty-bitmap-add

[Vladimir Sementsov-Ogievskiy]

07.07.2017 10:54, Markus Armbruster wrote:
> QAPI schema review only...  I apologize for its lateness.
>
> Vladimir Sementsov-Ogievskiy <address@hidden> writes:
>
> > Add optional 'persistent' flag to qmp command block-dirty-bitmap-add.
> > Default is false.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> > Signed-off-by: Denis V. Lunev <address@hidden>
> > Reviewed-by: Max Reitz <address@hidden>
> > Reviewed-by: John Snow <address@hidden>
> > ---
> [...]
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index f85c2235c7..13f98ec146 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -1561,10 +1561,16 @@
> >   # @granularity: the bitmap granularity, default is 64k for
> >   #               block-dirty-bitmap-add
> >   #
> > +# @persistent: the bitmap is persistent, i.e. it will be saved to the
> > +#              corresponding block device image file on its close. For now only
> > +#              Qcow2 disks support persistent bitmaps. Default is false for
> > +#              block-dirty-bitmap-add. (Since: 2.10)
> "for block-dirty-bitmap-add" suggests there could be other users, with
> different (but unspecified) defaults.  What about replacing the sentence
> by "(default: false)"?
>
> Please wrap your comment lines around column 70.

Why 70, is it written somewhere? There are a lot of lines over 70 
characters in this file, so, as series are already in Max's block branch 
I think it would be better to fix the whole file, if it is really needed.

> > +#
> >   # Since: 2.4
> >   ##
> >   { 'struct': 'BlockDirtyBitmapAdd',
> > -  'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32' } }
> > +  'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32',
> > +            '*persistent': 'bool' } }
> >   
> >   ##
> >   # @block-dirty-bitmap-add:


--
Best regards,
Vladimir