From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36895) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QyjCf-0002qu-Sj for qemu-devel@nongnu.org; Wed, 31 Aug 2011 07:43:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QyjCe-00051c-DO for qemu-devel@nongnu.org; Wed, 31 Aug 2011 07:43:33 -0400 Received: from mail-gy0-f173.google.com ([209.85.160.173]:61372) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QyjCe-00051S-4x for qemu-devel@nongnu.org; Wed, 31 Aug 2011 07:43:32 -0400 Received: by gyd12 with SMTP id 12so506898gyd.4 for ; Wed, 31 Aug 2011 04:43:31 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20110825191804.1413.26801.stgit@ginnungagap.bsc.es> References: <20110825191731.1413.26838.stgit@ginnungagap.bsc.es> <20110825191804.1413.26801.stgit@ginnungagap.bsc.es> Date: Wed, 31 Aug 2011 12:43:31 +0100 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v7 05/13] trace: avoid conditional code compilation during option parsing List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-1?Q?Llu=EDs?= Cc: qemu-devel@nongnu.org, chouteau@adacore.com On Thu, Aug 25, 2011 at 8:18 PM, Llu=EDs wrote: > diff --git a/vl.c b/vl.c > index 145d738..ed2db9a 100644 > --- a/vl.c > +++ b/vl.c > @@ -156,7 +156,7 @@ int main(int argc, char **argv) > =A0#include "slirp/libslirp.h" > > =A0#include "trace.h" > -#include "trace/simple.h" > +#include "trace/control.h" > =A0#include "qemu-queue.h" > =A0#include "cpus.h" > =A0#include "arch_init.h" > @@ -2130,7 +2130,6 @@ int main(int argc, char **argv, char **envp) > =A0 =A0 int show_vnc_port =3D 0; > =A0#endif > =A0 =A0 int defconfig =3D 1; > - =A0 =A0const char *trace_file =3D NULL; > =A0 =A0 const char *log_mask =3D NULL; > =A0 =A0 const char *log_file =3D NULL; > =A0 =A0 GMemVTable mem_trace =3D { > @@ -2928,14 +2927,27 @@ int main(int argc, char **argv, char **envp) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 xen_mode =3D XEN_ATTACH; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break; > -#ifdef CONFIG_TRACE_SIMPLE > =A0 =A0 =A0 =A0 =A0 =A0 case QEMU_OPTION_trace: > + =A0 =A0 =A0 =A0 =A0 =A0{ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 opts =3D qemu_opts_parse(qemu_find_opts("= trace"), optarg, 0); > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (opts) { > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0trace_file =3D qemu_opt_get(opts= , "file"); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!opts) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0exit(1); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!trace_config_init()) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0fprintf(stderr, "qemu: error: op= tion \"-%s\" is not " > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"supported by th= e selected tracing backend\n", > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0popt->name); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0exit(1); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0const char *trace_file =3D qemu_opt_get(= opts, "file"); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (trace_file && !trace_config_init_fil= e(trace_file)) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0fprintf(stderr, "error: suboptio= n \"-%s file\" is not " > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"supported by th= e selected tracing backend\n", > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0popt->name); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0exit(1); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break; > -#endif > + =A0 =A0 =A0 =A0 =A0 =A0} > =A0 =A0 =A0 =A0 =A0 =A0 case QEMU_OPTION_readconfig: > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int ret =3D qemu_read_config_file= (optarg); > @@ -2993,10 +3005,6 @@ int main(int argc, char **argv, char **envp) > =A0 =A0 =A0 =A0 set_cpu_log(log_mask); > =A0 =A0 } > > - =A0 =A0if (!st_init(trace_file)) { > - =A0 =A0 =A0 =A0fprintf(stderr, "warning: unable to initialize simple tr= ace backend\n"); > - =A0 =A0} > - This changes the semantics of simpletrace. If you start without -trace ... then simpletrace will not be initialized and will not be functional (no write-out thread). That means you cannot start without an events file and interactively enable the events that you want. The stderr backend handles this case fine because it has no initialization code which this hunk replaces with trace_config_init() (only called when -trace ... is given). I suggest changing the trace backend interface to a single function: /** * Set up the trace backend with the -trace events=3D...,file=3D... argumen= ts * * @events file with events to be enabled on startup, may be NULL * @file name of trace output file, may be NULL * @return true on success, false on error */ bool trace_backend_init(const char *events, const char *file); In the vl.c:main() args parsing switch statement, just collect const char *events and *file. Then after the switch statement unconditionally call trace_backend_init(events, file) (even if -trace was not given). Default behavior for backends that do not support -trace becomes: bool trace_backend_init(const char *events, const char *file) { if (events || file) { fprintf(stderr, "The -trace option is not supported by this trace backend\n"); return false; } return true; } This guarantees that trace backends will receive the trace_backend_init() call even when -trace is not used. This is where they can create threads or do any other setup. Stefan