Re: [Qemu-devel] [PATCH v12 13/13] Add XBZRLE statistics

[Eric Blake]

On 06/19/2012 09:43 AM, Orit Wasserman wrote:
> Signed-off-by: Benoit Hudzia <address@hidden>
> Signed-off-by: Petter Svard <address@hidden>
> Signed-off-by: Aidan Shribman <address@hidden>
> Signed-off-by: Orit Wasserman <address@hidden>
> ---

> +++ b/qapi-schema.json
> @@ -263,7 +263,28 @@
>  # Since: 0.14.0.
>  ##
>  { 'type': 'MigrationStats',
> -  'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' } }
> +  'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int', 
> 'duplicate': 'int', 'norm': 'int' } }

What does 'norm' mean?  Is there something wrong with using an actual
word instead of an abbreviation?

Should 'duplicate' and 'norm' be marked optional, to reflect the fact
that they were not always present in the output?  Should the listing be:

# Since: 0.14.0, 'duplicate' and 'norm' since 1.2.

> +
> +##
> +# @CacheStats
> +#
> +# Detailed XBZRLE migration cache statistics
> +#
> +# @cache_size: XBZRLE cache size
> +#
> +# @xbzrle_bytes: amount of bytes already transferred to the target VM
> +#
> +# @xbzrle_pages: amount of pages transferred to the target VM
> +#
> +# @xbzrle_cache_miss: numer of cache miss

s/numer/number

> +#
> +# @xbzrle_overflow: number of overflows
> +#
> +# Since: 1.1

1.2

> +##
> +{ 'type': 'CacheStats',
> +  'data': {'cache_size': 'int', 'xbzrle_bytes': 'int', 'xbzrle_pages': 'int',
> +           'xbzrle_cache_miss': 'int', 'xbzrle_overflow': 'int' } }

s/_/-/g - prefer '-' over '_' in QMP


> +++ b/qmp-commands.hx
> @@ -2098,6 +2098,13 @@ 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.
> +  it is a json-object with the following XBZRLE information:

s/it/It/

> +         - "cache size": XBZRLE cache size
> +              - "xbzrle_bytes": total XBZRLE bytes transferred
> +      - "xbzrle_pages": number of XBZRLE compressed pages

Alignment.  Again, '-' instead of '_'.

> +5. Migration is being performed and XBZRLE is active:
> +
> +-> { "execute": "query-migrate" }
> +<- {
> +      "return":{
> +         "status":"active",
> +         "ram":{
> +            "total":1057024,
> +            "remaining":1053304,
> +            "transferred":3720
> +         },

Where's the capabilities member?

> +         "cache":{
> +         "size": 1024

No TABs.

> +            "xbzrle_transferred":20971520,

Especially not when you mix TAB and space indentation in the same example.

> +         "xbzrle_pages":2444343,
> +         "xbzrle_cache_miss:2244,

s/miss:/miss"/

> +         "xbzrle_overflow":34434
> +         }
> +      }
> +   }
> +
>  EQMP
>  
>      {
> 

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature