From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55838) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XooNE-0001Y6-Te for qemu-devel@nongnu.org; Thu, 13 Nov 2014 01:59:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XooN7-0002gV-7J for qemu-devel@nongnu.org; Thu, 13 Nov 2014 01:59:20 -0500 Received: from mail-pd0-f177.google.com ([209.85.192.177]:46924) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XooN6-0002fg-VE for qemu-devel@nongnu.org; Thu, 13 Nov 2014 01:59:13 -0500 Received: by mail-pd0-f177.google.com with SMTP id v10so13666342pde.8 for ; Wed, 12 Nov 2014 22:59:11 -0800 (PST) Message-ID: <546456B7.4060901@ozlabs.ru> Date: Thu, 13 Nov 2014 17:59:03 +1100 From: Alexey Kardashevskiy 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> In-Reply-To: <1415168221-2324-2-git-send-email-sam.mj@au1.ibm.com> Content-Type: text/plain; charset=koi8-r Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] spapr: Fix stale HTAB during live migration (KVM) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Samuel Mendoza-Jonas , qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 11/05/2014 05:17 PM, 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); > + if (spapr->htab_fd > 0) { s/>/>=/ htab_fd == 0 is correct. > + spapr->htab_fd_stale = true; > + } > + pthread_mutex_unlock(&spapr->htab_mutex); > } else { > if (!spapr->htab) { > /* Allocate an htab if we don't yet have one */ > @@ -850,6 +857,31 @@ static void spapr_reset_htab(sPAPREnvironment *spapr) > } > } > > +/* A guest reset will cause spapr->htab_fd to become stale if being used. Multiline comment starts with /* and \n. > + * Reopen the file descriptor to make sure the whole HTAB is properly read. > + */ > +static int spapr_check_htab_fd(sPAPREnvironment *spapr) > +{ > + int rc = 0; > + > + pthread_mutex_lock(&spapr->htab_mutex); > + > + if (spapr->htab_fd_stale) { > + close(spapr->htab_fd); > + spapr->htab_fd = kvmppc_get_htab_fd(false); > + if (spapr->htab_fd < 0) { > + error_report("Unable to open fd for reading hash table from KVM: " > + "%s", strerror(errno)); > + rc = -1; > + } > + spapr->htab_fd_stale = false; > + } > + > + pthread_mutex_unlock(&spapr->htab_mutex); > + return rc; > +} > + > + 2 empty lines, should be one. > static void ppc_spapr_reset(void) > { > PowerPCCPU *first_ppc_cpu; > @@ -984,7 +1016,10 @@ static int htab_save_setup(QEMUFile *f, void *opaque) > } else { > assert(kvm_enabled()); > > + pthread_mutex_lock(&spapr->htab_mutex); > spapr->htab_fd = kvmppc_get_htab_fd(false); > + spapr->htab_fd_stale = false; > + pthread_mutex_unlock(&spapr->htab_mutex); > if (spapr->htab_fd < 0) { > fprintf(stderr, "Unable to open fd for reading hash table from KVM: %s\n", > strerror(errno)); > @@ -1137,6 +1172,11 @@ static int htab_save_iterate(QEMUFile *f, void *opaque) > if (!spapr->htab) { > assert(kvm_enabled()); > > + rc = spapr_check_htab_fd(spapr); > + if (rc < 0) { > + return rc; > + } > + > rc = kvmppc_save_htab(f, spapr->htab_fd, > MAX_KVM_BUF_SIZE, MAX_ITERATION_NS); > if (rc < 0) { > @@ -1168,6 +1208,11 @@ static int htab_save_complete(QEMUFile *f, void *opaque) > > assert(kvm_enabled()); > > + rc = spapr_check_htab_fd(spapr); > + if (rc < 0) { > + return rc; > + } > + > rc = kvmppc_save_htab(f, spapr->htab_fd, MAX_KVM_BUF_SIZE, -1); > if (rc < 0) { > return rc; > @@ -1355,6 +1400,8 @@ static void ppc_spapr_init(MachineState *machine) > spapr->htab_shift++; > } > > + pthread_mutex_init(&spapr->htab_mutex, NULL); > + > /* Set up Interrupt Controller before we create the VCPUs */ > spapr->icp = xics_system_init(smp_cpus * kvmppc_smt_threads() / smp_threads, > XICS_IRQS); > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 749daf4..5e29bec 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -37,6 +37,8 @@ typedef struct sPAPREnvironment { > int htab_save_index; > bool htab_first_pass; > int htab_fd; > + bool htab_fd_stale; > + pthread_mutex_t htab_mutex; > } sPAPREnvironment; > > #define H_SUCCESS 0 > -- Alexey