Re: [Qemu-devel] [PATCH v12 15/15] rdma: account for the time spent in MIG_STATE_SETUP through QMP

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

On 06/27/2013 06:58 PM, Eric Blake wrote:
> On 06/25/2013 07:35 PM, mrhines@linux.vnet.ibm.com wrote:
>> From: "Michael R. Hines" <mrhines@us.ibm.com>
>>
>> Using the previous patches, we're now able to timestamp the SETUP
>> state. Once we have this time, let the user know about it in the
>> schema.
>>
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>> Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>
> Usually, Reviewed-by lines are listed _after_ S-o-b lines - signature
> lines are typically chronological, but the patch has to be signed before
> a review can have any weight at getting the patch into a pull request :)
>
>> ---
>>   hmp.c                         |    4 ++++
>>   include/migration/migration.h |    1 +
>>   migration.c                   |    9 +++++++++
>>   qapi-schema.json              |    9 ++++++++-
>>   4 files changed, 22 insertions(+), 1 deletion(-)
>>
>> +++ b/qapi-schema.json
>> @@ -578,6 +578,12 @@
>>   #        expected downtime in milliseconds for the guest in last walk
>>   #        of the dirty bitmap. (since 1.3)
>>   #
>> +# @setup-time: #optional amount of setup time spent _before_ the iterations
>> +#        begin but _after_ the QMP command is issued. This is designed to
> In what units?  One can easily assume milliseconds, based on the docs of
> other elapsed time parameters, but being explicit never hurts.
>
>> +#        provide an accounting of any activities (such as RDMA pinning) which
>> +#        may be expensive, but do not actually occur during the iterative
>> +#        migration rounds themselves. (since 1.6)
>> +#
>>   # Since: 0.14.0
>>   ##
>>   { 'type': 'MigrationInfo',
>> @@ -586,7 +592,8 @@
>>              '*xbzrle-cache': 'XBZRLECacheStats',
>>              '*total-time': 'int',
>>              '*expected-downtime': 'int',
>> -           '*downtime': 'int'} }
>> +           '*downtime': 'int',
>> +           '*setup-time' : 'int'} }
> We typically don't have space before ':' (as seen in the other lines
> just above).  I can live with the patch as-is, but if you respin the
> series for other things, then fix those two things before adding
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
Good catches.....thank you.