Re: [Qemu-devel] [PATCH] Introduce "info migrate-times" monitor command

[Michal Novotny <minovotn@redhat.com>]

On 07/14/2011 11:44 AM, Paolo Bonzini wrote:
> On 07/14/2011 10:45 AM, Michal Novotny wrote:
>>>  Please inline all these instead of adding new functions.
>> Do you mean to implement as macros? I'm trying since yesterday and it's
>> not that simple because the variable has to be accessible from 3 files -
>> arch_init.c, savevm.c and migration.c. So I need to figure out what file
>> to put the variables to to make it working fine.
> I would like to avoid using them in multiple files.
>
> The simplest change is to remove migration.c from the list.  The 
> "waiting for input" handling can be moved to qemu_savevm_state_begin and 
> qemu_savevm_state_iterate.  The monitor code can be moved to savevm.c as 
> well.
>
> And also, it makes no sense that arch_init.c includes savevm-related 
> code.  It was done simply to avoid compiling savevm more than once (to 
> put it in Makefile.objs instead of Makefile.target).  If you move that 
> code back to savevm.c, the list of files goes down to one.
>
> Paolo
I already sent the version 2 of my patch including both time measuring
support for each of the 4 parts of loadvm and also for savevm. The
monitor command implementation has been moved to the savevm.c and I have
to react to some points from your e-mail although I read it now (after
it's been already sent to the qemu-devel list).

What do you mean by removing migration.c from the list? Do you mean
doing no modifications to this file? The wait for input stage makes time
differences if it's moved into the qemu_savevm_state_{begin|iterate} -
although it's about milliseconds. If we consider this is acceptable we
need to take in mind that this function may be called many times which
would result into some bigger differences when we sum the time values
together.

The arch_init.c to include savevm-related code doesn't make any sense in
general which is the thing I have to agree but if we want to measure the
ram_save_live() per each stage we need to alter the ram_save_live()
function which is present in the arch_init.c file (this is directly in
vl.c file for RHEL-6 version of qemu however for upstream version we
need to modify arch_init.c to provide the most precise RAM transfer
values per each stage). We can move the ram_save_live() function into
some other file, like savevm.c or vl.c... Is this what you mean by "If
you move that code back to savevm.c" ? The code is in vl.c file for
RHEL-6, not savevm.c and I don't know whether it ever was in savevm.c .

Thanks,
Michal

-- 
Michal Novotny <minovotn@redhat.com>, RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org