From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41345) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WvXbY-0004i0-Tm for qemu-devel@nongnu.org; Fri, 13 Jun 2014 15:57:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WvXbT-0002qs-S6 for qemu-devel@nongnu.org; Fri, 13 Jun 2014 15:57:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56258) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WvXbT-0002qb-EZ for qemu-devel@nongnu.org; Fri, 13 Jun 2014 15:57:35 -0400 Message-ID: <539B57AB.9050000@redhat.com> Date: Fri, 13 Jun 2014 13:57:31 -0600 From: Eric Blake MIME-Version: 1.0 References: <1401970944-18735-1-git-send-email-wenchaoqemu@gmail.com> <1401970944-18735-9-git-send-email-wenchaoqemu@gmail.com> In-Reply-To: <1401970944-18735-9-git-send-email-wenchaoqemu@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="KPK2fG0oe9K4KXrtldUuOVEqpBRS9CddB" Subject: Re: [Qemu-devel] [PATCH V6 08/29] qapi event: convert SHUTDOWN 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) --KPK2fG0oe9K4KXrtldUuOVEqpBRS9CddB Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 06/05/2014 06:22 AM, Wenchao Xia wrote: > This patch also eliminates build time warning caused by > QAPI_EVENT_MAX =3D 0. I still don't know why I wasn't seeing a warning for that, but agree this cleans it up (or whichever event gets converted first, as there aren't really any dependency restrictions in the order in which you do conversions). If you do follow my suggestion of adding a workaround in 6/29 to avoid build warnings, then don't undo it here, but save it until 29/29; that way, no one individual conversion is doing more work than any other. >=20 > Signed-off-by: Wenchao Xia > --- > docs/qmp/qmp-events.txt | 15 --------------- > qapi-event.json | 12 ++++++++++++ > vl.c | 3 ++- > 3 files changed, 14 insertions(+), 16 deletions(-) Yay - I finally have enough to see what code gets generated in 3/29. The qapi-event.h header now has: void qapi_event_send_shutdown(Error **errp); extern const char *QAPIEvent_lookup[]; typedef enum QAPIEvent { QAPI_EVENT_SHUTDOWN =3D 0, QAPI_EVENT_MAX =3D 1, } QAPIEvent; and the .c file has: void qapi_event_send_shutdown(Error **errp) { QDict *qmp; Error *local_err =3D NULL; QMPEventFuncEmit emit; emit =3D qmp_event_get_func_emit(); if (!emit) { return; } qmp =3D qmp_event_build_dict("SHUTDOWN"); emit(QAPI_EVENT_SHUTDOWN, qmp, &local_err); error_propagate(errp, local_err); QDECREF(qmp); } Looks good, for a data-free event (I guess I'll have to wait till later in the series to see what gets generated for an event with data). Hmm, I wonder if patch 3/29 should have also modified docs/qapi-code-gen.txt to show a sample generated function. >=20 > - > -Emitted when the Virtual Machine is powered down. > +++ b/qapi-event.json > @@ -0,0 +1,12 @@ > +## > +# @SHUTDOWN > +# > +# Emitted when the virtual machine shutdown, qemu terminated the emula= tion and > +# is about to exit. Different wording than the text it replaces, and the grammar is a bit unusual. Maybe: Emitted when the virtual machine has shut down, indicating that qemu is about to exit. > if (qemu_shutdown_requested()) { > qemu_kill_report(); > - monitor_protocol_event(QEVENT_SHUTDOWN, NULL); > + qapi_event_send_shutdown(NULL); Note that the two NULLs serve different purposes. In the old code, NULL meant there was no additional data. In the new code, NULL indicates that we are ignoring the possibility for error. I wonder if it would be better to pass &error_abort instead of NULL? For that matter, I just re-read 6/29 - the one implementation we have of an emit function (monitor_qapi_event_queue) never sets errp. Is it better to just consider events as best-effort, and completely ditch the errp parameter from the emit callback typedef in 2/29, since it looks like you intend to pass NULL for all clients of the callback? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --KPK2fG0oe9K4KXrtldUuOVEqpBRS9CddB 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/ iQEcBAEBCAAGBQJTm1erAAoJEKeha0olJ0NqwH0H/0AXcsjpU8Svm6KEutwleuEF ZMmZdp2NtIh7lJhd3Dw7TJJZPYZs2Bkyer6d7lzpv5EO6lTat0u9QwWQURuWqqNt MO2elhqSEYfDRTsiGjGr/0rdJ/HcvWpyEPVu9n0A4moRb9CwJ9Q5Gf/6IlxQvR8P QkCrchAs7j5+Yfv5VlKWIFEacJ6sY5r9oAb8WmcGXPpzwMGg56hEw2lkx7v05kfG GUBA7AsJPfHQqZZQCwH3u2WiT9ynfolTNQg9NBcbwcNPYACHq+tziTNqF0863qVE ccUYCVaRVDcFzZmIl0dZHdy+lC1ZQl1Y+ldvp285lH6w0KhbXR2cfT7laGKjo2M= =FIN5 -----END PGP SIGNATURE----- --KPK2fG0oe9K4KXrtldUuOVEqpBRS9CddB--