From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35332) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XJh1K-0002OE-BA for qemu-devel@nongnu.org; Tue, 19 Aug 2014 06:52:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XJh1C-0007GW-PH for qemu-devel@nongnu.org; Tue, 19 Aug 2014 06:52:06 -0400 Message-ID: <53F32C49.9000106@suse.de> Date: Tue, 19 Aug 2014 12:51:53 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1408429031-1716-1-git-send-email-sam.mj@au1.ibm.com> In-Reply-To: <1408429031-1716-1-git-send-email-sam.mj@au1.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH V3] spapr: Fix stale HTAB during live migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Samuel Mendoza-Jonas , qemu-devel@nongnu.org, qemu-ppc@nongnu.org Cc: dgilbert@redhat.com On 19.08.14 08: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 > --- > Changes in v3: Pointed out by David, htab_save_iterate could > potentially try to read before htab_fd is open again. > Leave opening the fd to the functions trying to read. > Changes in v2: Forgot check on kvmppc_get_htab_fd return value > hw/ppc/spapr.c | 25 +++++++++++++++++++++++++ > include/hw/ppc/spapr.h | 1 + > 2 files changed, 26 insertions(+) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 3a6d26d..5b41318 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -997,6 +997,10 @@ 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; > + > + /* Check if we are overlapping a migration */ > + if (spapr->htab_fd > 0) > + spapr->need_reset = true; A "need_reset" field in the sPAPR machine with semantics of "we triggered a reset, so if there's a migration in flight please consider the full HTAB as invalid and thus reopen the fd to fetch all entries again" is not exactly obvious to me. How about something like "htab_save_should_reopen_fd" or an even better name you might come up with :). Alternatively you could save the last time stamp of when the HTAB was allocated and compare to that. Alex