From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40951) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xm8Yn-0007Dt-DW for qemu-devel@nongnu.org; Wed, 05 Nov 2014 16:56:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xm8YX-0007pk-9J for qemu-devel@nongnu.org; Wed, 05 Nov 2014 16:56:13 -0500 Received: from e23smtp09.au.ibm.com ([202.81.31.142]:33328) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xm8YW-0007os-Li for qemu-devel@nongnu.org; Wed, 05 Nov 2014 16:55:57 -0500 Received: from /spool/local by e23smtp09.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 6 Nov 2014 07:55:51 +1000 Message-ID: <545A9CF3.6090504@au1.ibm.com> Date: Thu, 06 Nov 2014 08:56:03 +1100 From: Samuel Mendoza-Jonas MIME-Version: 1.0 References: <1415168221-2324-1-git-send-email-sam.mj@au1.ibm.com> <1415168221-2324-2-git-send-email-sam.mj@au1.ibm.com> <5459D87D.9010103@suse.de> In-Reply-To: <5459D87D.9010103@suse.de> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] spapr: Fix stale HTAB during live migration (KVM) Reply-To: sam.mj@au1.ibm.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf , qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 05/11/14 18:57, Alexander Graf wrote: > > > On 05.11.14 07:17, Samuel Mendoza-Jonas wrote: >> If a guest reboots during a running migration, changes to the >> hash page table are not necessarily updated on the destination. >> Opening a new file descriptor to the HTAB forces the migration >> handler to resend the entire table. >> >> Signed-off-by: Samuel Mendoza-Jonas >> --- >> hw/ppc/spapr.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ >> include/hw/ppc/spapr.h | 2 ++ >> 2 files changed, 49 insertions(+) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 0a2bfe6..1610c28 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -833,6 +833,13 @@ static void spapr_reset_htab(sPAPREnvironment *spapr) >> /* Kernel handles htab, we don't need to allocate one */ >> spapr->htab_shift = shift; >> kvmppc_kern_htab = true; >> + >> + /* Tell readers to update their file descriptor */ >> + pthread_mutex_lock(&spapr->htab_mutex); > > I don't think you can directly use pthread functions in hw/. These files > could be compiled on Windows which doesn't have pthread. Instead, please > use the QEMU wrappers from util/qemu-thread-posix.c. Thanks for catching this! > > Or maybe try and find out whether you actually do need the lock. Reboots > can only happen when triggered via a HCALL which takes the BQL. I don't > quite know how much the migration code became threaded, but I'd assume > that at least device migration would happen under the BQL or after > stopping the VM, but in a consistent place. > > So as long as we're guaranteed that the htab_fd_stale variable is set at > the final "send all device contents" phase, we should automatically > catch any reset that happened in between - even without a lock, no? Good point, we only really *need* the flag to be set at least before the call to save_live_complete(). I'll double check but yes, that should work. > > > Alex > -- ----------- LTC Ozlabs IBM