Re: [Qemu-devel] Re: kvm: savevm is broken for me

[Glauber Costa <glommer@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]

On Fri, Jul 24, 2009 at 04:27:38PM +0200, Alexander Graf wrote:
>
> On 24.07.2009, at 16:20, Luiz Capitulino wrote:
>
>> On Fri, 24 Jul 2009 16:18:02 +0200
>> Alexander Graf <agraf@suse.de> wrote:
>>
>>>
>>> On 24.07.2009, at 15:54, Luiz Capitulino wrote:
>>>
>>>> On Fri, 24 Jul 2009 00:06:13 +0200
>>>> Alexander Graf <agraf@suse.de> wrote:
>>>>
>>>>> On 23.07.2009, at 23:58, Luiz Capitulino wrote:
>>>>>
>>>>>>
>>>>>> Hi there,
>>>>>>
>>>>>> If I try to 'savevm' with latest Anthony's tree (HEAD
>>>>>> 6f725c139ae975646c44ace77bf796318a5783da) QEMU will hang and I  
>>>>>> know
>>>>>> it's not saving anything because vmstat shows no disk activity
>>>>>> (which
>>>>>> is the opposite behavior of when 'savevm' works).
>>>>>>
>>>>>> According to 'git bisect' the week's winner is:
>>>>>>
>>>>>> """
>>>>>> commit bd8367761236cd5c435598aeb2f1b8240c09b059
>>>>>> Author: Alexander Graf <agraf@suse.de>
>>>>>> Date:   Fri Jul 17 13:51:48 2009 +0200
>>>>>>
>>>>>>  Fake dirty loggin when it's not there
>>>>>> """
>>>>>>
>>>>>> Indeed, reverting this makes 'savevm' work for me.
>>>>>
>>>>>
>>>>> Yikes. I have no idea why, but just revert the patch :-). I don't
>>>>> really need it anymore since we now have dirty logging in PPC.
>>>>>
>>>>> Maybe it's the if (is dirty logging really enabled?) in the
>>>>> beginning?
>>>>
>>>> Yes, it's the if. I have removed it and savevm works.
>>>>
>>>> Can you submit the fix please?
>>>
>>> Eh - what fix? You found that something doesn't set or evaluate the
>>> flags right. That still doesn't tell us who behaves incorrectly.
>>
>> Ah, okay. I thought you meant the 'if' _was_ the bug.
>>
>> Better to revert then?
>
> Even better yet to find out who's doing something wrong :-).

It's actually quite simple.
you continue'd instead of break'ing during the dirty log loop,
which would put us in an infinite loop

Patch follows:



[-- Attachment #2: 0001-remove-infinite-loop-for-migration.patch --]
[-- Type: text/plain, Size: 1006 bytes --]

>From d249467b75dea5fad2c305d88edd447e8745494f Mon Sep 17 00:00:00 2001
From: Glauber Costa <glommer@redhat.com>
Date: Fri, 24 Jul 2009 15:32:46 -0400
Subject: [PATCH] remove infinite loop for migration

Linux is so good, that it can finish an endless loop in 5 seconds.
Unfortunately, not everybody uses Linux, and while kvm is still linux-only,
this is likely to change. So remove an endless loop in migration code
recently introduced.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 kvm-all.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 824bb4c..b8b9539 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -333,7 +333,7 @@ int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
 
         /* We didn't activate dirty logging? Don't care then. */
         if(!(mem->flags & KVM_MEM_LOG_DIRTY_PAGES)) {
-            continue;
+            break;
         }
 
         size = ((mem->memory_size >> TARGET_PAGE_BITS) + 7) / 8;
-- 
1.6.2.2