From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:47479) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QGso0-0002SI-Nc for qemu-devel@nongnu.org; Mon, 02 May 2011 09:04:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QGsnz-0002lf-EZ for qemu-devel@nongnu.org; Mon, 02 May 2011 09:04:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:16028) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QGsnz-0002lb-5x for qemu-devel@nongnu.org; Mon, 02 May 2011 09:04:51 -0400 Date: Mon, 2 May 2011 16:04:32 +0300 From: Alon Levy Message-ID: <20110502130432.GC3633@playa.tlv.redhat.com> References: <4DBAAA92.60409@redhat.com> <20110429221944.GD13837@amber.local> <4DBE731E.1050905@redhat.com> <20110502101200.GG17629@playa.redhat.com> <4DBE869A.4010403@redhat.com> <20110502103315.GA3633@playa.tlv.redhat.com> <4DBE8978.1030207@siemens.com> <20110502115008.GB3633@playa.tlv.redhat.com> <4DBEA6F6.6020201@siemens.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4DBEA6F6.6020201@siemens.com> Subject: Re: [Qemu-devel] debug logging List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: qemu-devel , Markus Armbruster , Stefan Hajnoczi , Gerd Hoffmann On Mon, May 02, 2011 at 02:43:34PM +0200, Jan Kiszka wrote: > On 2011-05-02 13:50, Alon Levy wrote: > > On Mon, May 02, 2011 at 01:00:01PM +0200, Markus Armbruster wrote: > >> Jan Kiszka writes: > >> > >>> On 2011-05-02 12:33, Alon Levy wrote: > >>>> On Mon, May 02, 2011 at 12:25:30PM +0200, Gerd Hoffmann wrote: > >>>>> Hi, > >>>>> > >>>>>> dbg_print takes care of making it standard to have a loglevel and prefix, sounds > >>>>>> good, but I'd still like to know if it is acceptable to also redirect with -debug, > >>>>>> I guess reusing the DeviceState then, instead of my added struct, so just letting > >>>>>> DeviceState.debug_chardev == NULL by default, and settable with > >>>>>> -debug,id= > >>>>> > >>>>> Making dbg_print use DeviceState.debug_chardev (if present, stderr > >>>>> otherwise) looks sane to me. Buf I'd use standard properties then > >>>>> to set it instead of a separate -debug switch, i.e. > >>>>> > >>>>> -chardev file,id=messages,path=... \ > >>>>> -device foo,dbg=1,dbglog=messages > >>>>> > >>>> > >>>> Sounds good. I'll try to make it so. > >>> > >>> We have a tracing infrastructure for such use cases today. Better invest > >>> in adding missing features there. > >> > >> Start with docs/tracing.txt > > > > ok, I see that the file starts by saying it is meant to be used for debugging as > > well. Am I correct in the following? > > 1. to change tracing backends requires a recompile > > Right. I mostly work with stderr backend as it can be redirected > wherever you want it (console, file, ftrace trace_marker). > > > 2. can't have two tracing backends at the same time (i.e. either everything goes > > to stdout or everything is visible via systemtap etc.) > > 3. quoting: > > Do not pass in string arguments. > > how does that help debugging then? > > This needs to be fixed, the infrastructure should be able to record > fixed sized strings (not kilobytes, but 32 bytes or so). > > > 4. the equivalent of setting a loglevel is? > > Right now either the use of systemtap or ust which allow per-tracepoint > enabling/disabling or enable simpletrace and use the trace-event monitor > command. On top you need local grouping of the tracepoints. > > At least the latter needs fixing so that the developer of some module > can tag tracepoints in-tree. But I also think we should be tracepoint > control a generic feature, independent of the tracer backend as far as > possible (ust and systemtap likely provide interfaces to forward control > commands). > > > > > If I want to achieve my original intend, namely easily changing the target of specific > > qxl debug statements from stderr to a separate file, to view at runtime, I will need to: > > 1. turn all of them to trace calls > > 2. do one of: > > 2.a turn the stderr backend into a debug_chardev backend, defaulting to stderr > > (this sounds simplest to me) and add this option > > You should avoid using too much of qemu's infrastructure in the tracer, > specifically if holds some state. Or am I misunderstanding your plans? > My original patch allowed logging of qxl debug output to a chardev of my choice. I'm trying to achieve the same thing using the tracing framework. The way is I think to add a DeviceState.debug_chardev that is settable when the device is created (no state in the tracing framework) and use that from the stderr framework. This seems to be a little bit difficult with the current stderr backend implementation which never has any handle to a DeviceState. I'll have to fix that, and then it can be something like: static inline void trace_$name(DeviceState *dev, $args) { if (!dev || !dev->debug_chardev) { fprintf(stderr, "$name $fmt\n" $argnames); else { qemu_chr_print("$name $fmt\n" $argnames); } } > > 2.b augment the simpletrace backend in the same way. > > > > Any comments? > > > > Alon > > Jan > > -- > Siemens AG, Corporate Technology, CT T DE IT 1 > Corporate Competence Center Embedded Linux