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

[John Snow]

On 05/17/2018 06:01 AM, Vladimir Sementsov-Ogievskiy wrote:
> 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 
 ---
    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.
> 

Yeah, there may be other kinds that want this behavior. The restriction
can probably be eliminated from the spec, I think, but that's to be
handled outside of this series.

I'll remove the restriction from the QAPI flag, because what "should" or
"must" happen is not relevant to what _is_ happening. :)

--js

Re: [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 
---
   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

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

[John Snow]

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 
>> ---
>>   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.

>> +#
>> +# @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']
>>     } }
>>     ##
> 
> 

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

[John Snow]

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 
>> ---
>>   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
> 

Where did I even get this idea?

>> +#
>> +# @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.
> 

Oh, understood! Thank you for clearing that up.

> Finally, this spec should be consistent (or, may be better, duplicate)
> spec from docs/interop/qcow2.txt..

*cough* I guess if I get enough things wrong it's fair to say "RTFM!"

>> +#
>> +# @unknown: This bitmap has unknown or reserved properties.
> 
> Better is only "reserved flags" (not unknown and not properties), they
> are reserved by spec.
> 

Yeah, I'll take that stance.

>> +#
>> +# 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'
> 

Since you pointed out the difference, I agree.

>> +  }
>> +}
>> +
>>   ##
>>   # @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']
>>     } }
>>     ##
> 
> 

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

[Vladimir Sementsov-Ogievskiy]

12.05.2018 04:25, John Snow wrote:

Add some of the necessary scaffolding for reporting bitmap information.

Signed-off-by: John Snow 
---
  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



+#
+# @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..

+#
+# @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