From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59993) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WvUdF-0001BJ-9a for qemu-devel@nongnu.org; Fri, 13 Jun 2014 12:47:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WvUdA-0004yV-Q3 for qemu-devel@nongnu.org; Fri, 13 Jun 2014 12:47:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52919) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WvUdA-0004y3-Gt for qemu-devel@nongnu.org; Fri, 13 Jun 2014 12:47:08 -0400 Message-ID: <539B2B07.50209@redhat.com> Date: Fri, 13 Jun 2014 10:47:03 -0600 From: Eric Blake MIME-Version: 1.0 References: <1401970944-18735-1-git-send-email-wenchaoqemu@gmail.com> <1401970944-18735-4-git-send-email-wenchaoqemu@gmail.com> In-Reply-To: <1401970944-18735-4-git-send-email-wenchaoqemu@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="He2FdIPpFMvPQgdJ3GJxl787uFiWjCMH1" Subject: Re: [Qemu-devel] [PATCH V6 03/29] qapi script: add event support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia , qemu-devel@nongnu.org Cc: mdroth@linux.vnet.ibm.com, armbru@redhat.com, lcapitulino@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --He2FdIPpFMvPQgdJ3GJxl787uFiWjCMH1 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 06/05/2014 06:21 AM, Wenchao Xia wrote: > qapi-event.py will parse the schema and generate qapi-event.c, then > the API in qapi-event.c can be used to handle event in qemu code. s/event in/events in/ > All API have prefix "qapi_event". >=20 > The script mainly includes two parts: generate API for each event > define, generate an enum type for all defined events. >=20 > Since in some cases the real emit behavior may change, for example, > qemu-img would not send a event, a callback layer is used to > control the behavior. As a result, the stubs at compile time > can be saved, the binding of block layer code and monitor code > will become looser. >=20 > Signed-off-by: Wenchao Xia > --- > Makefile | 11 +- > Makefile.objs | 2 +- > docs/qapi-code-gen.txt | 18 ++ > scripts/qapi-event.py | 369 ++++++++++++++++++++++= ++++++++ > scripts/qapi.py | 12 + > tests/Makefile | 2 +- > tests/qapi-schema/event-nest-struct.err | 1 + > tests/qapi-schema/event-nest-struct.exit | 1 + > tests/qapi-schema/event-nest-struct.json | 2 + > 9 files changed, 413 insertions(+), 5 deletions(-) > create mode 100644 scripts/qapi-event.py > create mode 100644 tests/qapi-schema/event-nest-struct.err > create mode 100644 tests/qapi-schema/event-nest-struct.exit > create mode 100644 tests/qapi-schema/event-nest-struct.json > create mode 100644 tests/qapi-schema/event-nest-struct.out My python is weak, but I did apply this patch (or rather, used Paolo's rebase of this patch) to test the generated files. At this point, I'm interested in seeing the series go in sooner rather than later, and my findings below are minor enough that I'm okay fixing them in a followup patch rather than waiting for a respin of the whole series. Your patch fails to update .gitignore for the new generated files: Untracked files: (use "git add ..." to include in what will be committed) qapi-event.c qapi-event.h > =20 > +=3D=3D=3D Events =3D=3D=3D > + > +Events are defined with key word 'event'. When 'data' is also specifi= ed, > +additional info will be carried on. Finally there will be C API gener= ated s/carried on/included in the event/ > +++ b/scripts/qapi-event.py > + > +def _generate_event_api_name(event_name, params): > + api_name =3D "void qapi_event_send_%s(" % c_fun(event_name).lower(= ); As of this commit, no events using qapi_event_send are generated yet. I'm trusting that this code works, but I reserve the right to revisit this patch later once I look at later patches and see what the generated code is doing there. But for now, the generated code has merely: const char *QAPIEvent_lookup[] =3D { NULL, }; which looks correct, even if a bit sparse :) > +++ b/scripts/qapi.py > @@ -248,6 +248,16 @@ def discriminator_find_enum_define(expr): > =20 > return find_enum(discriminator_type) > =20 > +def check_event(expr, expr_info): > + params =3D expr.get('data') > + if params: > + for argname, argentry, optional, structured in parse_args(para= ms): > + if structured: > + raise QAPIExprError(expr_info, > + "Nested structure define in event = is not " > + "supported now, event '%s', argnam= e '%s'" s/ now// - we don't EVER want to support nested structures in events (for that matter, I've been arguing in other threads that we want to completely ditch support for nested structures to simplify the generator code-base and make it possible to cleanly support argument defaults). > +++ b/tests/qapi-schema/event-nest-struct.err > @@ -0,0 +1 @@ > +tests/qapi-schema/event-nest-struct.json:1: Nested structure define in= event is not supported now, event 'EVENT_A', argname 'a' Of course, if you tweak the message above, you'll also have to tweak this test. But thanks for adding a test! Since I'm okay saving the cleanups mentioned above for a followup-patch, and assuming I don't revisit this file: Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --He2FdIPpFMvPQgdJ3GJxl787uFiWjCMH1 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 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTmysHAAoJEKeha0olJ0NqQZAH/02vwNlFpWesThwrewrdAQnC KgjbWH/IsH0C5hObxgOWgAMCEwCGNHcA9pNCzQa8gJTLdUlru0iUxl3JV3oZSW5Q z1tSF3pZgBtmfa/Q7AnTEBWIRT8JAI/BlpnApMRRcY+om+jxcA9OUWdxSLWGZ8LP puZqynzFZCQWX5riWdpg3nXyfNB/dx8l6ri5jDkhMRXgHIwo/xDsUG3YrwYRSTrr SQRfUZ8+xpk086Bh4XovMiQW9WtnU9/5SNgRGjUYk5Ufc3GqFXcCAvMZhlZbaXnW UVNy4ya4NQ2qHbTAuWTlay0Lna+2kpH3nftpJerryhNR0JCAbchXdmVuQnYS/5g= =6OJG -----END PGP SIGNATURE----- --He2FdIPpFMvPQgdJ3GJxl787uFiWjCMH1--