From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32943) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aP2tE-0001Yn-VQ for qemu-devel@nongnu.org; Fri, 29 Jan 2016 01:50:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aP2tA-0002P4-QY for qemu-devel@nongnu.org; Fri, 29 Jan 2016 01:50:40 -0500 Received: from e23smtp08.au.ibm.com ([202.81.31.141]:53134) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aP2tA-0002Om-8u for qemu-devel@nongnu.org; Fri, 29 Jan 2016 01:50:36 -0500 Received: from localhost by e23smtp08.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 29 Jan 2016 16:50:30 +1000 Date: Fri, 29 Jan 2016 12:19:33 +0530 From: Bharata B Rao Message-ID: <20160129064933.GF13713@in.ibm.com> References: <1453960195-15181-1-git-send-email-bharata@linux.vnet.ibm.com> <1453960195-15181-14-git-send-email-bharata@linux.vnet.ibm.com> <56AA8E99.8020006@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56AA8E99.8020006@redhat.com> Subject: Re: [Qemu-devel] [PATCH v7 13/13] hmp: Add "info ppc-cpu-cores" command Reply-To: bharata@linux.vnet.ibm.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: mjrosato@linux.vnet.ibm.com, qemu-devel@nongnu.org, ehabkost@redhat.com, imammedo@redhat.com, aik@ozlabs.ru, agraf@suse.de, mdroth@linux.vnet.ibm.com, qemu-ppc@nongnu.org, tyreld@linux.vnet.ibm.com, nfont@linux.vnet.ibm.com, pbonzini@redhat.com, afaerber@suse.de, david@gibson.dropbear.id.au On Thu, Jan 28, 2016 at 02:56:41PM -0700, Eric Blake wrote: > On 01/27/2016 10:49 PM, Bharata B Rao wrote: > > This is the hmp equivalent of "query ppc-cpu-cores" > > The QMP command is spelled "query-ppc-cpu-cores". > > Most HMP commands prefer '_' over '-'; so this should be 'info > ppc_cpu_cores'. I see that a few commands have '-' but as you note, majority of them use '_'. Though I personally prefer '-', if HMP convention is to go with '_', will change in the next iteration. > > > > > Signed-off-by: Bharata B Rao > > --- > > hmp-commands-info.hx | 16 ++++++++++++++++ > > hmp.c | 31 +++++++++++++++++++++++++++++++ > > hmp.h | 1 + > > 3 files changed, 48 insertions(+) > > > > > +++ b/hmp.c > > @@ -2375,3 +2375,34 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict) > > > > qapi_free_RockerOfDpaGroupList(list); > > } > > + > > +void hmp_info_ppc_cpu_cores(Monitor *mon, const QDict *qdict) > > +{ > > + Error *err = NULL; > > + PPCCPUCoreList *ppc_cpu_core_list = qmp_query_ppc_cpu_cores(&err); > > + PPCCPUCoreList *s = ppc_cpu_core_list; > > + CpuInfoList *thread; > > + > > + while (s) { > > + monitor_printf(mon, "PowerPC CPU device: \"%s\"\n", > > + s->value->id ? s->value->id : ""); > > This should probably be checking s->value->has_id rather than assuming > that s->value->id will be NULL when not present (well, I'd like to clean > up qapi to avoid the need for has_FOO when FOO is a pointer, but we're > not there yet). Ok, will switch to s->value->has_id ? s->value->id : "") > > > + monitor_printf(mon, " hotplugged: %s\n", > > + s->value->hotplugged ? "true" : "false"); > > + monitor_printf(mon, " hotpluggable: %s\n", > > + s->value->hotpluggable ? "true" : "false"); > > + monitor_printf(mon, " Threads:\n"); > > + for (thread = s->value->threads; thread; thread = thread->next) { > > + monitor_printf(mon, " CPU #%" PRId64 ":", thread->value->CPU); > > + monitor_printf(mon, " nip=0x%016" PRIx64, > > + thread->value->u.ppc->nip); > > This uses value->u.ppc without first checking that the discriminator > value->arch is set to CPU_INFO_ARCH_PPC; could that be a problem down > the road? Can't think of any potential problems as this command is PowerPC specific. BTW can you please let me know what else is needed from QEMU end to drive this PowerPC CPU core device hotplug from libvirt ? Regards, Bharata.