Re: [Qemu-devel] [RFC PATCH v2 07/12] mc: introduce additional QMP statistics for micro-checkpointing

["Michael R. Hines" <mrhines@linux.vnet.ibm.com>]

On 03/12/2014 05:45 AM, Eric Blake wrote:
>> +++ b/qapi-schema.json
>> @@ -603,6 +603,36 @@
>>              'cache-miss': 'int', 'overflow': 'int' } }
>>   
>>   ##
>> +# @MCStats
>> +#
>> +# Detailed Micro Checkpointing (MC) statistics
>> +#
>> +# @mbps: throughput of transmitting last MC
>> +#
>> +# @xmit-time: milliseconds to transmit last MC
> Trailing whitespace.
>
> Rather than abbreviate, how about naming this 'transmit-time'.

Acknowledged.

>> +#
>> +# @checkpoints: cummulative total number of MCs generated
> More trailing whitespace.  Please run your series through
> scripts/checkpatch.pl.
>
> s/cummulative total/cumulative/

Acknowledged.


>> +#
>> +# Since: 2.x
>> +##
>> +{ 'type': 'MCStats',
>> +  'data': {'mbps': 'number',
>> +           'xmit-time': 'uint64',
>> +           'log-dirty-time': 'uint64',
>> +           'migration-bitmap-time': 'uint64',
>> +           'ram-copy-time': 'uint64',
>> +           'checkpoints' : 'uint64',
>> +           'copy-mbps': 'number' }}
> Again, it helps to document the fields in the same order as they are
> declared (no, it's not a hard requirement, but being nice to readers is
> always worth the effort).

Acknowledged.

>> +
>> +##
>>   # @MigrationInfo
>>   #
>>   # Information about current migration process.
>> @@ -624,6 +654,8 @@
>>   #                migration statistics, only returned if XBZRLE feature is on and
>>   #                status is 'active' or 'completed' (since 1.2)
>>   #
>> +# @mc: #options @MCStats containing details Micro-Checkpointing statistics
> s/options/optional/ - I'm assuming it is optional because it only
> appears when MC is in use.
>
> 'mc' is a rather short name, maybe 'micro-checkpoint' is better?

Funny. I thought 'micro-checkpoint' was too long, particularly
in the ./configure output and the QEMU Monitor 'help' command output =)

> Missing a '(since 2.1)' designation (or 2.x, as you used above as a
> placeholder, although obviously we'd fix the .x before actually bringing
> into mainline)
>

Acknowledged.

- Michael