Re: [Qemu-devel] [PATCH 31/46] Postcopy: Rework migration thread for postcopy mode

[Paolo Bonzini]

Il 28/08/2014 13:04, Dr. David Alan Gilbert ha scritto:
>> You don't really need the "else" if you have a continue.  However, do you
>> need _any_ of the "else" and "continue"?  Would the next iteration of the
>> "while" loop do anything else but invoking qemu_savevm_state_iterate.
> 
> Yes, I've dropped that 'else'; however, I've kept the continue - we're about
> 3 if's deep here inside the loop and there's a bunch of stuff at the end of
> the if's but still inside the loop that I'm not 100% sure I want to run
> again at this point (although it's probably OK).

My point is that the next iteration would start exactly with the else
(calling qemu_savevm_state_iterate) and then do the stuff at the end of
the if's (bandwidth calculation and all that).  So why not do that
immediately, without the "continue"?

> A lot of this stuff is done, but it's done at the point we transition into
> postcopy, not at the end (see postcopy_start).

Perhaps you can move the common parts to a separate function instead of
cut-and-paste? ;)

> However, I've not
> got the wakup_request and old_vm_running check; so I probably need to
> think where they should go; what's the purpose of the 
> qemu_system_wakeup_request
> there ? it seems to be getting the guest into running state - which is
> where I'd assumed it was already.

No, it doesn't have to---you could be doing non-live migration.
old_vm_running makes sure that if migration fails the VM restarts.  You
need to grab the state just before force-stopping the VM.

Regarding qemu_system_wakeup_request, it only does something if the
virtual machine is suspended-to-RAM; the call should be handling
migration of such a VM.  The idea is that since we don't transmit the
runstate, we just wakeup the VM on the destination.  But you need to
prepare the VM for that (which is basically a reset plus setting a
couple of ACPI registers).

The handling of the request is done here:

    if (qemu_wakeup_requested()) {
        pause_all_vcpus();
        cpu_synchronize_all_states();
        qemu_system_reset(VMRESET_SILENT);
        notifier_list_notify(&wakeup_notifiers, &wakeup_reason);
        wakeup_reason = QEMU_WAKEUP_REASON_NONE;
        resume_all_vcpus();
        qapi_event_send_wakeup(&error_abort);
    }


However, it might be broken or might be working by chance only.

Paolo