On 08/13/2012 01:39 PM, Luiz Capitulino wrote: > On Fri, 29 Jun 2012 18:43:57 +0200 > Juan Quintela <quint...@redhat.com> 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 <quint...@redhat.com> >> --- >> 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 ebl...@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
