Re: [RFC PATCH 8/8] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function

[Zheng Chuan]

On 2020/8/5 0:28, Eric Blake wrote:
> On 7/24/20 10:11 PM, Chuan Zheng wrote:
>> From: Zheng Chuan <zhengchuan@huawei.com>
>>
>> Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be 
>> called
>> by libvirt api.
>>
>> Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
>> Signed-off-by: YanYing Zhang <ann.zhuangyanying@huawei.com>
>> ---
> 
>> +++ b/qapi/migration.json
>> @@ -1621,3 +1621,27 @@
>>   ##
>>   { 'event': 'UNPLUG_PRIMARY',
>>     'data': { 'device-id': 'str' } }
>> +
>> +##
>> +# @cal_dirty_rate:
> 
> New QMP commands should be named favoring '-' over '_'; also, it doesn't hurt 
> to spell it out:
> 
> calculate-dirty-rate
> 
Hi, Eric.
Thank you for your review.
I'll fix that in V2.

>> +#
>> +# start calculating dirty rate for vm
>> +#
>> +# @value: time for sample dirty pages
> 
> In what unit?
> 
The unit is second, i'll make comments in V2:)

>> +#
>> +# Since: 5.1
> 
> We've missed 5.1; this will need to be updated to 5.2.
> 
>> +#
>> +# Example:
>> +#   {"command": "cal_dirty_rate", "data": {"value": 1} }
>> +#
>> +##
>> +{ 'command': 'cal_dirty_rate', 'data': {'value': 'int64'} }
>> +
>> +##
>> +# @get_dirty_rate:
> 
> get-dirty-rate, except that we tend to use 'query-' as the prefix for 
> commands that read values.
> 
>> +#
>> +# get dirty rate for vm
>> +#
>> +# Since: 5.1
> 
> 5.2
> 
> What units is the rate expressed in?
> 
The unit is MB/s, i'll make comments in V2:)
> 
>> +##
>> +{ 'command': 'get_dirty_rate', 'returns': 'int64' }
>> diff --git a/qapi/pragma.json b/qapi/pragma.json
>> index cffae27..ecd294b 100644
>> --- a/qapi/pragma.json
>> +++ b/qapi/pragma.json
>> @@ -10,7 +10,8 @@
>>           'query-migrate-cache-size',
>>           'query-tpm-models',
>>           'query-tpm-types',
>> -        'ringbuf-read' ],
>> +        'ringbuf-read',
>> +        'get_dirty_rate' ],
>>       'name-case-whitelist': [
>>           'ACPISlotType',             # DIMM, visible through 
>> query-acpi-ospm-status
>>           'CpuInfoMIPS',              # PC, visible through query-cpu
>>
>