qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH RFC 3/4] Curling: the sender


From: junqing . wang
Subject: Re: [Qemu-devel] [PATCH RFC 3/4] Curling: the sender
Date: Wed, 11 Sep 2013 15:31:42 +0800 (CST)

hi,

>> +    bool create = false;
> >This variable is never set.
It is set in the following 'if' block.
+        create = true;    <<=======

>> -    migration_bitmap = bitmap_new(ram_pages); >> -    bitmap_set(migration_bitmap, 0, ram_pages); >> -    migration_dirty_pages = ram_pages; >> +    if (!ft_enabled() || !migration_bitmap)  { >> +        migration_bitmap = bitmap_new(ram_pages); >> + bitmap_set(migration_bitmap, 0, ram_pages); >> + migration_dirty_pages = ram_pages; >> + create = true;       <========== >> + }

>Nothing in this patch sets the migration_bitmap to anything.
Let me explain all the odd 'if'  block:
1 >> +    if (!ft_enabled() || !migration_bitmap)  {
2 >> +    if (!ft_enabled() || create) {
3 >> +    if (!ft_enabled()) {

As I mentioned in the commit log:
>> We need to handle the variables related to live migration very >> carefully. So the new migration does not restart from the very >> begin of the migration, instead, it continues the previous >> migration.

Some variables should not be reset after one migration, because
the next one need these variables to continue the migration.
This explains all the "if ft_enabled()"

Besides, some variables need to be initialized at the first migration of curling.
That explains the "if create" and "if  !migration_bitmap" >> +                if (ft_enabled()) { >> +                    if (old_vm_running) { >> +                        qemu_mutex_lock_iothread(); >> +                        vm_start(); >> +                        qemu_mutex_unlock_iothread(); >> + >> +                        current_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); >> +                        time_spent = current_time - migration_start_time; >> +                        DPRINTF("this migration lasts for %" PRId64 "ms\n", >> +                                time_spent); >> +                        if (time_spent < time_window) { >> +                            g_usleep((time_window - time_spent)*1000); > >Why are we waiting here?  If we are migration faster than allowed,  why >we are waiting?
Looping fast is not good, that means we enter iothread lock and do vm stop more frequently. The performance will drop and vm user will experience input stall if we do not sleep.

How to deal with this is a difficult issue, any suggestion is welcomed.

THIS IS ONE OF THE TWO MAIN PROBLEMS.  The other one is related to the magic number 0xfeedcafe.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]