On 07/27/2012 01:48 AM, Eric Blake wrote:
> On 07/25/2012 08:50 AM, Orit Wasserman wrote:
>> Signed-off-by: Benoit Hudzia <benoit.hud...@sap.com>
>> Signed-off-by: Petter Svard <pett...@cs.umu.se>
>> Signed-off-by: Aidan Shribman <aidan.shrib...@sap.com>
>> Signed-off-by: Orit Wasserman <owass...@redhat.com>
>> Signed-off-by: Juan Quintela <quint...@redhat.com>
>> ---
>> +++ b/qapi-schema.json
>> @@ -273,6 +273,26 @@
>> { 'type': 'MigrationStats',
>> 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
>> 'total_time': 'int', '*duplicate': 'int', '*normal': 'int' } }
>> +##
>> +# @XBZRLECacheStats
>> +#
>> +# Detailed XBZRLE migration cache statistics
>> +#
>> +# @cache_size: XBZRLE cache size
>
> s/cache_size/cache-size/, and so on throughout this struct (it is new,
> so it should use '-' instead of '_'); especially since you already made
> that change in the JSON itself.
>
ok
>> +#
>> +# @xbzrle_bytes: amount of bytes already transferred to the target VM
>> +#
>> +# @xbzrle_pages: amount of pages transferred to the target VM
>> +#
>> +# @xbzrle_cache_miss: number of cache miss
>> +#
>> +# @xbzrle_overflow: number of overflows
>> +#
>> +# Since: 1.2
>> +##
>> +{ 'type': 'XBZRLECacheStats',
>> + 'data': {'cache-size': 'int', '*xbzrle-bytes': 'int', '*xbzrle-pages':
>> 'int',
>> + '*xbzrle-cache-miss': 'int', '*xbzrle-overflow': 'int' } }
>
> Why are you marking four of the five fields optional here, but not in
> the text above? I don't think any of them should be optional.
If the migration is not active only cache size will be available (requested by
Luiz).
I will add it to the comment.
>
>>
>> ##
>> # @MigrationInfo
>> @@ -292,11 +312,15 @@
>> # status, only returned if status is 'active' and it is a block
>> # migration
>> #
>> +# @xbzrle-cache: #optional @XBZRLECacheStats containing detailed XBZRLE
>> +# migration statistics (since 1.2)
>
> Now _this_ field is indeed optional - it should appear in the output
> only when xbzrle is enabled. You may want to mention that in the
> description (just like the previous field mentions that it was only
> available for a block migration).
ok
>
>> +++ b/qmp-commands.hx
>> @@ -2106,10 +2106,16 @@ The main json-object contains the following:
>> - "transferred": amount transferred (json-int)
>> - "remaining": amount remaining (json-int)
>> - "total": total (json-int)
>> +- "cache": only present if "status" and XBZRLE is active.
>
> Naming mismatch; you named it 'xbzrle-cache' above.
ok
>
>> + It is a json-object with the following XBZRLE information:
>> + - "cache-size": XBZRLE cache size
>> + - "xbzrle-bytes": total XBZRLE bytes transferred
>
> Just so I'm clear, is xbzrle-bytes is the number of compressed bytes
> sent over the wire, or the number of uncompressed bytes? Likewise, at
> the top level, is 'total' the number of bytes sent over the wire, or the
> number of bytes after decompression?
xbzrle-bytes are the compressed bytes sent on the wire.
total is the total bytes sent on the wire (including xbzrle_bytes and
duplicates ...).
>
> That is, if I compare this field against 'total', which field will
> always be bigger? Knowing this will let me compute my percentage of
> savings due to compressed pages.
total will be always bigger.
>
>> + - "xbzrle-pages": number of XBZRLE compressed pages
>> + - "cache-miss": number of cache misses
>> + - "overflow": number of XBZRLE overflows
>> Examples:
>>
>> 1. Before the first migration
>> -
>> -> { "execute": "query-migrate" }
>
> Spurious whitespace change.
>
Thanks,
Orit