From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43102) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UT20B-0002Yn-3u for qemu-devel@nongnu.org; Thu, 18 Apr 2013 23:28:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UT20A-0002sN-2L for qemu-devel@nongnu.org; Thu, 18 Apr 2013 23:28:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37093) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UT209-0002sD-Qy for qemu-devel@nongnu.org; Thu, 18 Apr 2013 23:28:41 -0400 Message-ID: <5170B9E6.7070208@redhat.com> Date: Thu, 18 Apr 2013 21:28:38 -0600 From: Eric Blake MIME-Version: 1.0 References: <20130416134949.21588.80064.stgit@fimbulvetr.bsc.es> <20130416135126.21588.39829.stgit@fimbulvetr.bsc.es> In-Reply-To: <20130416135126.21588.39829.stgit@fimbulvetr.bsc.es> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2HISRBMNQJDUJORCTWLSI" Subject: Re: [Qemu-devel] [PATCH v2 17/23] instrument: Add commandline options to start with an instrumentation library List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?TGx1w61zIFZpbGFub3Zh?= Cc: qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2HISRBMNQJDUJORCTWLSI Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 04/16/2013 07:51 AM, Llu=C3=ADs Vilanova wrote: > Add commandline options to control initial loading of dynamic instrumen= tation > library. >=20 > Signed-off-by: Llu=C3=ADs Vilanova > --- > @@ -688,6 +690,15 @@ static void usage(void) > #endif > "-bsd type select emulated BSD type FreeBSD/NetBSD/= OpenBSD (default)\n" > "\n" > +#if defined(CONFIG_TRACE_INSTRUMENT) > + "Tracing options:\n" > +#if defined(CONFIG_TRACE_INSTRUMENT_DYNAMIC) > + "-instr path load a dynamic trace instrumentation lib= rary\n" > +#endif > + "-instr-arg string\n" > + " argument to dynamic trace instrumentatio= n library (can be given multiple times)\n" > + "\n" Why do you document -instr-arg unconditionally, but -instr only when dynamic trace support is enabled; especially since the text of -instr-arg mentions that it is tied to dynamic tracing? > @@ -852,6 +866,14 @@ int main(int argc, char **argv) > singlestep =3D 1; > } else if (!strcmp(r, "strace")) { > do_strace =3D 1; > +#if defined(CONFIG_TRACE_INSTRUMENT_DYNAMIC) > + } else if (!strcmp(r, "instr")) { > + instrument_path =3D argv[optind++]; > +#endif > +#if defined(CONFIG_TRACE_INSTRUMENT) > + } else if (!strcmp(r, "instr-arg")) { > + instr_parse_args(argv[optind++], &instrument_argc, &instru= ment_argv); > +#endif At least your parsing code matches your documentation, but it still feels weird. > @@ -3378,6 +3397,14 @@ static const struct qemu_argument arg_table[] =3D= { > {"R", "QEMU_RESERVED_VA", true, handle_arg_reserved_va, > "size", "reserve 'size' bytes for guest virtual address spa= ce"}, > #endif > +#if defined(CONFIG_TRACE_INSTRUMENT_DYNAMIC) > + {"instr", "QEMU_INSTR", true, handle_arg_instrument, > + "path", "load a dynamic trace instrumentation library"}, > +#endif > +#if defined(CONFIG_TRACE_INSTRUMENT) > + {"instr-arg", "QEMU_INSTR_ARGS", true, handle_arg_instrument_arg= , > + "string", "argument to dynamic trace instrumentation library = (can be given multiple times)"}, Another case of mentioning the term dynamic even when dynamic instrumentation is not enabled. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2HISRBMNQJDUJORCTWLSI Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJRcLnmAAoJEKeha0olJ0NqMo8H/jwCMsXEG6/FHPuD1O9uqjSE 6jG/uYrl1mfWV0xyuWXfH+3eOdxv+cDGoTR+6j3JWCtam4jwiVhgft/YP9PMSl6Z yfci2icODmDuUpJnFjcuwxsbxmr2arDHIXnnY5c1xwuUD6bp/hrYuJK0rdhs4Kmq F9QNe4a+Tu6nRgyHMxqZm/nS0XYakTiXfINAFA2tHXG5aKRHOGjmIBh+OocoMeSh 1Z/4d+fbrItyFPFWKfAAw6vI8lPCTS/pGelTI1MMN16aagFv0YcW9gDbMB2/hHe5 fNE+rx+wWnHXte/iZEnkZpe+6E2yAyQTjyDQuVaIwfY8p5BL3zwErfkNajvTJNc= =5ysl -----END PGP SIGNATURE----- ------enig2HISRBMNQJDUJORCTWLSI--