From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59325) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKIig-0002sH-Ut for qemu-devel@nongnu.org; Tue, 05 Jul 2016 01:16:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bKIic-0002U5-Nr for qemu-devel@nongnu.org; Tue, 05 Jul 2016 01:16:25 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:37293) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKIic-0002Tt-EB for qemu-devel@nongnu.org; Tue, 05 Jul 2016 01:16:22 -0400 Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.11/8.16.0.11) with SMTP id u6558kM8015607 for ; Tue, 5 Jul 2016 01:16:21 -0400 Received: from e23smtp03.au.ibm.com (e23smtp03.au.ibm.com [202.81.31.145]) by mx0a-001b2d01.pphosted.com with ESMTP id 23x8wvuc71-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 05 Jul 2016 01:16:21 -0400 Received: from localhost by e23smtp03.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 5 Jul 2016 15:16:18 +1000 Date: Tue, 5 Jul 2016 10:46:07 +0530 From: Bharata B Rao Reply-To: bharata@linux.vnet.ibm.com References: <1467693772-7391-1-git-send-email-bharata@linux.vnet.ibm.com> <1467693772-7391-2-git-send-email-bharata@linux.vnet.ibm.com> <20160705045613.GE2251@voom.fritz.box> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160705045613.GE2251@voom.fritz.box> Message-Id: <20160705051607.GA12977@in.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v0 1/5] cpu: Factor out cpu vmstate_[un]register into separate routines List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, imammedo@redhat.com, groug@kaod.org, nikunj@linux.vnet.ibm.com, pbonzini@redhat.com On Tue, Jul 05, 2016 at 02:56:13PM +1000, David Gibson wrote: > On Tue, Jul 05, 2016 at 10:12:48AM +0530, Bharata B Rao wrote: > > Consolidates cpu vmstate_[un]register calls into separate routines. > > No functionality change except that vmstate_unregister calls are > > now done under !CONFIG_USER_ONLY to match with vmstate_register calls. > > > > Signed-off-by: Bharata B Rao > > Reviewed-by: David Gibson > > > --- > > exec.c | 47 ++++++++++++++++++++++++++++------------------- > > 1 file changed, 28 insertions(+), 19 deletions(-) > > > > diff --git a/exec.c b/exec.c > > index 0122ef7..8ce8e90 100644 > > --- a/exec.c > > +++ b/exec.c > > @@ -594,9 +594,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx) > > /* Return the AddressSpace corresponding to the specified index */ > > return cpu->cpu_ases[asidx].as; > > } > > -#endif > > > > -#ifndef CONFIG_USER_ONLY > > static DECLARE_BITMAP(cpu_index_map, MAX_CPUMASK_BITS); > > > > static int cpu_get_free_index(Error **errp) > > @@ -617,6 +615,31 @@ static void cpu_release_index(CPUState *cpu) > > { > > bitmap_clear(cpu_index_map, cpu->cpu_index, 1); > > } > > + > > +static void cpu_vmstate_register(CPUState *cpu) > > +{ > > + CPUClass *cc = CPU_GET_CLASS(cpu); > > + > > + if (qdev_get_vmsd(DEVICE(cpu)) == NULL) { > > + vmstate_register(NULL, cpu->cpu_index, &vmstate_cpu_common, cpu); > > + } > > + if (cc->vmsd != NULL) { > > + vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu); > > + } > > +} > > + > > +static void cpu_vmstate_unregister(CPUState *cpu) > > +{ > > + CPUClass *cc = CPU_GET_CLASS(cpu); > > + > > + if (cc->vmsd != NULL) { > > + vmstate_unregister(NULL, cc->vmsd, cpu); > > + } > > + if (qdev_get_vmsd(DEVICE(cpu)) == NULL) { > > + vmstate_unregister(NULL, &vmstate_cpu_common, cpu); > > + } > > +} > > + > > Given you're factoring this out, would it make sense to defined no-op > versions for CONFIG_USER_ONLY, to reduce the amount of ifdefs at the > call site? I did that in a subsequent patch that moved the calls to these routines into cpu_common_[un]realize(), but ended up in some unrelated issue and hence didn't include that patch yet. But as you note, we could do that with the existing code itself. Commit 741da0d3 limited the vmstate_register calls to !CONFIG_USER_ONLY. I am still not sure why cpu vmstate registration isn't needed for CONFIG_USER_ONLY Regards, Bharata.