From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54020) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLO7P-0007s1-4Y for qemu-devel@nongnu.org; Wed, 05 Mar 2014 21:33:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WLO7H-0003DO-Gi for qemu-devel@nongnu.org; Wed, 05 Mar 2014 21:33:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:24044) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLO7H-0003DH-73 for qemu-devel@nongnu.org; Wed, 05 Mar 2014 21:32:59 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s262Wwro014845 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 5 Mar 2014 21:32:58 -0500 Date: Thu, 6 Mar 2014 10:32:55 +0800 From: Amos Kong Message-ID: <20140306023255.GA7829@amosk.info> References: <1393912317-26221-1-git-send-email-akong@redhat.com> <1393912317-26221-3-git-send-email-akong@redhat.com> <53164D9C.4080500@redhat.com> <20140305064014.GD2669@amosk.info> <53174994.4010606@redhat.com> <531771EE.2000309@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Kj7319i9nmIyA2yE" Content-Disposition: inline In-Reply-To: <531771EE.2000309@redhat.com> Subject: Re: [Qemu-devel] [libvirt] [PATCH v3 2/2] query-command-line-options: query all the options in qemu-options.hx List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: libvir-list@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org --Kj7319i9nmIyA2yE Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Mar 05, 2014 at 11:50:22AM -0700, Eric Blake wrote: > On 03/05/2014 08:58 AM, Eric Blake wrote: > > On 03/04/2014 11:40 PM, Amos Kong wrote: > >=20 > >>> but the docs imply that I should now see: > >>> > >>> {"parameters": [], "option": "smbios", "argument": true} > >> > >> I really got : {"parameters": [], "option": "smbios", "argument": true} > >> > >> (I was testing with latest qemu upstream + my patches, attached the > >> output file) > >=20 > > Hmm, maybe I was testing a stale binary. Let me try re-running tests on > > my end - I recently changed my choice of configure arguments to speed up > > build time by building fewer binaries, so maybe I tested on a binary > > that my configure arguments no longer rebuild. >=20 > Aha, it WAS my configure options at fault. Apologies for the mis-test > on my side. I can now confirm that pre-patch, I see (rearranged a > subset of entries, and newlines added for legibility): >=20 > {"parameters": [], "option": "smbios"}, > {"parameters": [{"name": "file", "type": "string"}, > {"name": "events", "type": "string"}], "option": "trace"}, >=20 > and no fips, while post-patch, I see: >=20 > {"parameters": [], "option": "enable-fips", "argument": false}, > {"parameters": [], "option": "smbios", "argument": true}, > {"parameters": [{"name": "file", "type": "string"}, > {"name": "events", "type": "string"}], "option": "trace"}, >=20 > which matches the docs. However, I did notice that pre-patch, I see: >=20 > {"parameters": [], "option": "acpi"} >=20 > while post-patch, there is no hit for "acpi", but there is a new: >=20 > {"parameters": [], "option": "acpitable", "argument": true} >=20 > What's going on there? It is a potential regression if we fail to list > an option post-patch that was listed pre-patch. Then again, looking at > the actual -help text, I see my particular qemu binary mentions only > "-acpitable [sig=3Dstr]..." in the help text (pre- and post-patch), as > well as this test to prove there is no '-acpi': >=20 > $ ./x86_64-softmmu/qemu-system-x86_64 -acpi > qemu-system-x86_64: -acpi: invalid option > $ ./x86_64-softmmu/qemu-system-x86_64 -acpitable > qemu-system-x86_64: -acpitable: requires an argument >=20 > so it looks like your patch was silently fixing a bug. Indeed, reading > vl.c, I see: >=20 > case QEMU_OPTION_acpitable: > opts =3D qemu_opts_parse(qemu_find_opts("acpi"), optarg, = 1); > if (!opts) { > exit(1); > } > do_acpitable_option(opts); >=20 > so the option table named "acpi" is indeed for the command line argument > "acpitable". Can we update all the name in option tables to match with actual command-line spelling? (we can use another patch to fix it) =20 > It would be nice to mention bonus bug fixes like that in the commit > message (that is, you are not only adding support for flags like > -enable-fips, you are also fixing options to match their actual > command-line spelling rather than an alternate name associated with the > option table in use by the command). >=20 > So, at this point, we still need a v4 to avoid the duplicate static > tables, but I am now set up to give a Tested-by. Thanks for your confirm, I will post a V4. --=20 Amos. --Kj7319i9nmIyA2yE Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJTF95XAAoJELxSv6I5vP9jn+sP+wVvRb7JjDSCVBAlS/lSxnId ls23zidXAOar3+BApY9vWJ49yotZGNwffyZa0krsrj73g5Uw3eqfYR71+t4gSzbl io8b87KjxGaahIYCTxHzcs0f5TYZBw+5imgg0FzYftZ8h9AVOVhVraropg60kkUT XYFBl2+3NU0/i3LC/kNWf7fgwfR+bsZVNVye1txMGn+elYSj1TkOpkTzeHXOynhE VhHVh4TR0meNLoTEa9WRS++pEry8GDcRWfUJMr6zYaEj1xLdt2i62yu5nHo+DsP7 WSgEVPoc5Fp8HNpmd5/DfabBswlhq/rpAASAZqXta1nqLIrZELy87TtSdrmcflqn z1zNVXwgH2I9bzomIK6vJUeDb9imOruPrrSv+BPl6hQtrZh9tEBVApBCHydg7VKs uR00YHhG2ejXq301Wl41zN8E0dscNz+oEr2ABuJoiQfBAeT/1FRoR6ht2GPHp9sN q/Nji4juwTKxWxCGWH0aRj6og0OoEKnBNB9y/QUnokXcw7JH53WL4JOKnoztbZGb aKrXxlMbpj/wx3hdkONC/ttFLYLQgRoGYr8OW0XSkMdd2JIAc3FaoW+dYoUXYCK0 zk0IUAZ1VHGILdRv5XPLgj+5CEI1IfVcNnGCS+EdTe+va07UmVRvL811nY0f3Foi +7xgJ4OKdJkgxQbpEW6A =vGY4 -----END PGP SIGNATURE----- --Kj7319i9nmIyA2yE--