From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MVTEP-0000EV-Fg for qemu-devel@nongnu.org; Mon, 27 Jul 2009 12:39:21 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MVTEK-0000C5-Uo for qemu-devel@nongnu.org; Mon, 27 Jul 2009 12:39:21 -0400 Received: from [199.232.76.173] (port=33681 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MVTEK-0000Bw-El for qemu-devel@nongnu.org; Mon, 27 Jul 2009 12:39:16 -0400 Received: from mx2.redhat.com ([66.187.237.31]:34845) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MVTEJ-0001yG-Mv for qemu-devel@nongnu.org; Mon, 27 Jul 2009 12:39:16 -0400 Date: Mon, 27 Jul 2009 13:46:09 -0300 From: Glauber Costa Subject: Re: [Qemu-devel] Re: kvm: savevm is broken for me Message-ID: <20090727164609.GB4776@poweredge.glommer> References: <20090723185844.474f7c62@doriath> <20090724105402.1ea81ad5@doriath> <20090724112053.62b67d03@doriath> <60E5AA22-8B6F-488A-BD95-FA3B28935D23@suse.de> <20090724194235.GA19431@poweredge.glommer> <20090724200927.GE4019@poweredge.glommer> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Jan Kiszka , Anthony Liguori , qemu-devel , Luiz Capitulino On Sat, Jul 25, 2009 at 12:01:59AM +0200, Alexander Graf wrote: > > On 24.07.2009, at 22:09, Glauber Costa wrote: > >> On Fri, Jul 24, 2009 at 09:45:28PM +0200, Alexander Graf wrote: >>> >>> On 24.07.2009, at 21:42, Glauber Costa wrote: >>> >>>> 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 wrote: >>>>>> >>>>>>> >>>>>>> On 24.07.2009, at 15:54, Luiz Capitulino wrote: >>>>>>> >>>>>>>> On Fri, 24 Jul 2009 00:06:13 +0200 >>>>>>>> Alexander Graf 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 >>>>>>>>>> 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: >>> >>> But who calls kvm_physical_sync_dirty_bitmap with !(mem->flags & >>> KVM_MEM_LOG_DIRTY_PAGES)? Sounds rather wrong to me. >> I'd have to dig, but from code inspection, >> it seems that ram_save_live does it, in its first interaction. >> >> This can probably be improved, but I don't think it is wrong semantics >> anyway. We can sync dirty bitmaps without having mem->flags & >> KVM_MEM_LOG_DIRTY_PAGES, >> and just got nothing written to qemu dirty bitmap. >> >> Anyway, even if we decide to fix ram_save_live to not do it, >> getting into an endless loop in that case is buggy. So it is really >> orthogonal. > > Looking at the code again it's probably best to remove the if(). Doing a > break isn't exactly right either because it could hide dirty logs in > later slots. It's up to you. As an alternative, we could do start_addr = mem->phys_addr + mem->len, or something like it, therefore skipping the whole slot.