From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=34482 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OqV8U-0007yM-8E for qemu-devel@nongnu.org; Tue, 31 Aug 2010 14:00:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OqV8O-0000Rl-Jy for qemu-devel@nongnu.org; Tue, 31 Aug 2010 14:00:42 -0400 Received: from mail-qw0-f45.google.com ([209.85.216.45]:59830) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OqV8O-0000Rh-EM for qemu-devel@nongnu.org; Tue, 31 Aug 2010 14:00:36 -0400 Received: by qwh5 with SMTP id 5so5726791qwh.4 for ; Tue, 31 Aug 2010 11:00:35 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1283174836-6330-1-git-send-email-stefanha@linux.vnet.ibm.com> <1283174836-6330-2-git-send-email-stefanha@linux.vnet.ibm.com> From: Blue Swirl Date: Tue, 31 Aug 2010 18:00:15 +0000 Message-ID: Subject: Re: [Qemu-devel] Re: [PATCH 01/14] trace: Add trace-events file for declaring trace events Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: qemu-devel@nongnu.org, Anthony Liguori , Stefan Hajnoczi , Prerna Saxena On Tue, Aug 31, 2010 at 8:46 AM, Stefan Hajnoczi wrote= : > On Mon, Aug 30, 2010 at 10:01 PM, Blue Swirl wrote= : >> On Mon, Aug 30, 2010 at 8:42 PM, Blue Swirl wrote= : >>> On Mon, Aug 30, 2010 at 8:10 PM, malc wrote: >>>> On Mon, 30 Aug 2010, Blue Swirl wrote: >>>> >>>>> On Mon, Aug 30, 2010 at 1:27 PM, Stefan Hajnoczi >>>>> wrote: >>>>> > This patch introduces the trace-events file where trace events can = be >>>>> > declared like so: >>>>> > >>>>> > qemu_malloc(size_t size) "size %zu" >>>>> > qemu_free(void *ptr) "ptr %p" >>>>> > >>>>> > These trace event declarations are processed by a new tool called >>>>> > tracetool to generate code for the trace events. =C2=A0Trace event >>>>> > declarations are independent of the backend tracing system (LTTng U= ser >>>>> > Space Tracing, ftrace markers, DTrace). >>>>> >>>>> I think the tool does not work if 'sh' is not 'bash'. For example, on >>>>> OpenBSD I got: >>>> >>>> Well, it does work with ash. >>>> >>>>> >>>>> config-host.mak is out-of-date, running configure >>>>> >>>>> Error: invalid trace backend >>>>> Please choose a supported trace backend. >>>>> >>>>> =C2=A0 GEN =C2=A0 trace.h >>>>> /src/qemu/tracetool[176]: no closing quote >>>>> >>>>> This shows the problem: >>>>> sh -x ../tracetool --nop --check-backend >>>>> + set -f >>>>> ../tracetool[176]: no closing quote >>>> >>>> `set -f' is a valid construct according to: >>>> http://www.opengroup.org/onlinepubs/009695399/utilities/set.html >>>> >>>> The problem is likely elsewhere. >>> >>> Right, the offending lines are: >>> =C2=A0 =C2=A0echo ${1%%(*} >>> and >>> =C2=A0 =C2=A0args=3D${1#*(} >>> >>> If I remove both of those, the errors are gone. >>> >> >> This patch fixes the problem. Double quotes do not help. >> >> diff --git a/tracetool b/tracetool >> index d640100..01de580 100755 >> --- a/tracetool >> +++ b/tracetool >> @@ -29,14 +29,14 @@ EOF >> =C2=A0# Get the name of a trace event >> =C2=A0get_name() >> =C2=A0{ >> - =C2=A0 =C2=A0echo ${1%%(*} >> + =C2=A0 =C2=A0echo ${1%%\(*} >> =C2=A0} >> >> =C2=A0# Get the argument list of a trace event, including types and name= s >> =C2=A0get_args() >> =C2=A0{ >> =C2=A0 =C2=A0 local args >> - =C2=A0 =C2=A0args=3D${1#*(} >> + =C2=A0 =C2=A0args=3D${1#*\(} >> =C2=A0 =C2=A0 args=3D${args%)*} >> =C2=A0 =C2=A0 echo "$args" >> =C2=A0} > > Thanks for finding and fixing this! =C2=A0I have been testing with dash i= n > addition to bash. > > I'd like to write tracetool in Python (or Perl, if needed) to > eliminate issues like this. =C2=A0I chose shell though to avoid the > dependency on Python. =C2=A0QEMU currently uses Perl for texi2pod.pl and > Python for QMP scripts but both are optional. =C2=A0Tracetool isn't > optional because it runs even when tracing is disabled ("nop" backend > generates empty stub functions). =C2=A0How do you feel about tracetool in > Python? I'd like to avoid additional dependencies. One way to solve this could be to ship QEMU with default-trace.[ch] which are used for the nop case, then tracetool implementation would not matter much. But these files would need to be updated, in synch, whenever new trace points are added which is fragile. Maybe a bit of CPP magic added to trace-events could simplify (or even eliminate) the script, something like we do with .hx files.