Re: [Qemu-devel] [PATCH 06/13] Add spent time for migration

[Eric Blake]

On 08/13/2012 01:39 PM, Luiz Capitulino wrote:
> On Fri, 29 Jun 2012 18:43:57 +0200
> Juan Quintela <address@hidden> wrote:
> 
>> We add time spent for migration to the output of "info migrate"
>> command.  'total_time' means time since the start fo migration if
>> migration is 'active', and total time of migration if migration is
>> completed.  As we are also interested in transferred ram when
>> migration completes, adding all ram statistics
> 
> I see this has already been merged and am sorry for being late with my
> review, but it turns out that there are a few issues to be addressed in
> this patch, comments inlined below.
> 
> Another point is that this patch extends the query-migrate command. We've
> decided not to extend QMP commands, however I think that we should relax
> that restriction for query commands, because the client doesn't need to know
> the new fields in advance.
> 
>>
>> Signed-off-by: Juan Quintela <address@hidden>
>> ---
>>  hmp.c            |    2 ++
>>  migration.c      |   11 +++++++++++
>>  migration.h      |    1 +
>>  qapi-schema.json |   12 +++++++++---
>>  4 files changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/hmp.c b/hmp.c
>> index b9cec1d..4c6d4ae 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -145,6 +145,8 @@ void hmp_info_migrate(Monitor *mon)
>>                         info->ram->remaining >> 10);
>>          monitor_printf(mon, "total ram: %" PRIu64 " kbytes\n",
>>                         info->ram->total >> 10);
>> +        monitor_printf(mon, "total time: %" PRIu64 " milliseconds\n",
>> +                       info->ram->total_time);
> 
> This adds a new line to the HMP output between the end of the ram stats and
> the disk stats. Iirc libvirt parses this output when in non-json mode, 
> although
> I don't think it ever does it for disk migration.
> 
> Eric, does libvirt do that?

Non-issue.  Libvirt insists on using QMP (JSON mode) if qemu >= 0.15,
precisely so that changes to HMP do not affect libvirt parsing.

>> +++ b/migration.c
>> @@ -131,6 +131,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>>          info->ram->transferred = ram_bytes_transferred();
>>          info->ram->remaining = ram_bytes_remaining();
>>          info->ram->total = ram_bytes_total();
>> +        info->ram->total_time = qemu_get_clock_ms(rt_clock)
>> +            - s->total_time;
>>
> 
> I really don't think that 'total_time' pertains to the ram stats info, I think
> it should be in the MigrationInfo dict.

Yes, Juan took care of that in
https://lists.gnu.org/archive/html/qemu-devel/2012-08/msg02142.html

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

signature.asc
Description: OpenPGP digital signature