Re: [Qemu-block] [Qemu-devel] [RFC PATCH 06/12] qapi: add bitmap info

[Vladimir Sementsov-Ogievskiy]

17.05.2018 00:15, John Snow wrote:
> On 05/14/2018 10:30 AM, Vladimir Sementsov-Ogievskiy wrote:
> > 12.05.2018 04:25, John Snow wrote:
> > > Add some of the necessary scaffolding for reporting bitmap information.
> > >
> > > Signed-off-by: John Snow <address@hidden>
> > > ---
> > >    qapi/block-core.json | 60
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++-
> > >    1 file changed, 59 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > index c50517bff3..8f33f41ce7 100644
> > > --- a/qapi/block-core.json
> > > +++ b/qapi/block-core.json
> > > @@ -33,6 +33,61 @@
> > >                'date-sec': 'int', 'date-nsec': 'int',
> > >                'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
> > >    +##
> > > +# @BitmapTypeEnum:
> > > +#
> > > +# An enumeration of possible serialized bitmap types.
> > > +#
> > > +# @dirty-tracking: This bitmap records information on dirty
> > > +#                  segments within the file.
> > > +#
> > > +# @unknown: This bitmap is of an unknown or reserved type.
> > > +#
> > > +# Since: 2.13
> > > +##
> > > +{ 'enum': 'BitmapTypeEnum', 'data': [ 'dirty-tracking', 'unknown' ] }
> > > +
> > > +##
> > > +# @BitmapFlagEnum:
> > > +#
> > > +# An enumeration of possible flags for serialized bitmaps.
> > > +#
> > > +# @in-use: This bitmap is considered to be in-use, and may now be
> > > inconsistent.
> > > +#
> > > +# @auto: This bitmap must reflect any and all changes to the file it
> > > describes.
> > > +#        The type of this bitmap must be @DirtyTrackingBitmap.
> > logical, but I don't see this restriction in the spec. May be we need to
> > update the spec
> 1: auto
>
> "The bitmap must reflect all changes of the virtual disk by any
> application that would write to this qcow2 file (including writes,
> snapshot switching, etc.). The type of this bitmap must be 'dirty
> tracking bitmap'."
>
> Actually, this looks correct now that I'm looking at the spec again.
> I've used a terser phrasing but I think it's correct.

another thought: why must? We know nothing about other types.. May be 
for other type this flag will have similar or other meaning.. For me, 
this flag looks like a property of dirty-tracking bitmap, not the thing 
which dictates only that type.

> > > +#
> > > +# @extra-data-compatible: This bitmap has extra information
> > > associated with it.
> > no, this flag means, that extra data is compatible. So, if you don't
> > know what is this extra data, you can read and modify the bitmap,
> > leaving this data as is. If this flag is unset, and there are some extra
> > data, bitmap must not be used.
> >
> > Finally, this spec should be consistent (or, may be better, duplicate)
> > spec from docs/interop/qcow2.txt..
> I might suggest a rewrite of this portion of the spec as it's a little
> unclear to me.
>
> I've given this portion a rewrite.
>
> > > +#
> > > +# @unknown: This bitmap has unknown or reserved properties.
> > Better is only "reserved flags" (not unknown and not properties), they
> > are reserved by spec.
> >
> > > +#
> > > +# Since: 2.13
> > > +##
> > > +{ 'enum': 'BitmapFlagEnum', 'data': [ 'in-use', 'auto',
> > > +                                      'extra-data-compatible',
> > > 'unknown' ] }
> > > +
> > > +##
> > > +# @BitmapInfo:
> > > +#
> > > +# @name: The name of the bitmap.
> > > +#
> > > +# @type: The type of bitmap.
> > > +#
> > > +# @granularity: Bitmap granularity, in bytes.
> > > +#
> > > +# @count: Overall bitmap dirtiness, in bytes.
> > > +#
> > > +# @flags: Bitmap flags, if any.
> > > +#
> > > +# Since: 2.13
> > > +#
> > > +##
> > > +{ 'struct': 'BitmapInfo',
> > > +  'data': { 'name': 'str', 'type': 'BitmapTypeEnum', 'granularity':
> > > 'int',
> > > +            'count': 'int', '*flags': ['BitmapFlagEnum']
> > may be worth add 'has-extra-data'
> >
> > > +  }
> > > +}
> > > +
> > >    ##
> > >    # @ImageInfoSpecificQCow2EncryptionBase:
> > >    #
> > > @@ -69,6 +124,8 @@
> > >    # @encrypt: details about encryption parameters; only set if image
> > >    #           is encrypted (since 2.10)
> > >    #
> > > +# @bitmaps: list of image bitmaps (since 2.13)
> > > +#
> > >    # Since: 1.7
> > >    ##
> > >    { 'struct': 'ImageInfoSpecificQCow2',
> > > @@ -77,7 +134,8 @@
> > >          '*lazy-refcounts': 'bool',
> > >          '*corrupt': 'bool',
> > >          'refcount-bits': 'int',
> > > -      '*encrypt': 'ImageInfoSpecificQCow2Encryption'
> > > +      '*encrypt': 'ImageInfoSpecificQCow2Encryption',
> > > +      '*bitmaps': ['BitmapInfo']
> > >      } }
> > >      ##


--
Best regards,
Vladimir