From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51950) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCT0q-0003BP-Mn for qemu-devel@nongnu.org; Mon, 13 Jun 2016 10:38:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bCT0m-0006iT-Lo for qemu-devel@nongnu.org; Mon, 13 Jun 2016 10:38:48 -0400 Received: from roura.ac.upc.es ([147.83.33.10]:51380) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCT0m-0006hX-A4 for qemu-devel@nongnu.org; Mon, 13 Jun 2016 10:38:44 -0400 From: =?utf-8?Q?Llu=C3=ADs_Vilanova?= References: <145641255678.30097.2919142707547689749.stgit@localhost> <145641258612.30097.7127731954660712163.stgit@localhost> <30b46b30-fbc0-31a4-6210-657b205c121f@redhat.com> <87lh29cmip.fsf@fimbulvetr.bsc.es> Date: Mon, 13 Jun 2016 16:38:34 +0200 In-Reply-To: <87lh29cmip.fsf@fimbulvetr.bsc.es> (=?utf-8?Q?=22Llu=C3=ADs?= Vilanova"'s message of "Mon, 13 Jun 2016 14:15:58 +0200") Message-ID: <87r3c140id.fsf@fimbulvetr.bsc.es> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 4/6] trace: Add per-vCPU tracing states for events with the 'vcpu' property List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, Blue Swirl , Riku Voipio , Eduardo Habkost , Stefan Hajnoczi , Andreas =?utf-8?Q?F=C3=A4rber?= Llu=C3=ADs Vilanova writes: > Paolo Bonzini writes: >> First of all, a generic problem I see with your patches is that the >> newly-introduced APIs are not providing a good abstraction. >> If something is only used internally, as is the case for >> trace_event_get_cpu_id, you don't need accessors. On the other hand, >> when you have a repeated expression such as >> trace_event_get_cpu_id(ev) !=3D trace_event_cpu_count() >> then you need an API such as trace_event_is_vcpu(ev). >> Another small ugliness is that you are using "vcpu" in trace-events and >> in the generated files, but "cpu" in the C file. My suggestion is to >> prefix functions with vcpu_trace_event if they refer to per-VCPU trace >> events, and only use the VCPU ids in those functions. > I'll fix these two. BTW, I'd rather keep the getters for this series, if only for the sake of tracing API consistency (e.g., we already have 'trace_event_get_id()'). I will send a separate series removing the existing superfluous asserts (I = won't be adding more on this series), and can extend it to remove the trivial get= ters on the tracing API if that's necessary. Thanks, Lluis