From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60839) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WvZJm-0002oc-CL for qemu-devel@nongnu.org; Fri, 13 Jun 2014 17:47:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WvZJh-00077Z-DD for qemu-devel@nongnu.org; Fri, 13 Jun 2014 17:47:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32259) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WvZJh-00077P-4w for qemu-devel@nongnu.org; Fri, 13 Jun 2014 17:47:21 -0400 Message-ID: <539B7164.1030407@redhat.com> Date: Fri, 13 Jun 2014 15:47:16 -0600 From: Eric Blake MIME-Version: 1.0 References: <1401970944-18735-1-git-send-email-wenchaoqemu@gmail.com> <1401970944-18735-18-git-send-email-wenchaoqemu@gmail.com> In-Reply-To: <1401970944-18735-18-git-send-email-wenchaoqemu@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Bfm7b8uFTv9RoS87PkhdMHEto73IGMQcg" Subject: Re: [Qemu-devel] [PATCH V6 17/29] qapi event: convert WATCHDOG 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) --Bfm7b8uFTv9RoS87PkhdMHEto73IGMQcg Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 06/05/2014 06:22 AM, Wenchao Xia wrote: > Signed-off-by: Wenchao Xia > --- > docs/qmp/qmp-events.txt | 19 ------------------- > hw/watchdog/watchdog.c | 23 +++++++---------------- > monitor.c | 2 +- > qapi-event.json | 15 +++++++++++++++ > qapi-schema.json | 24 ++++++++++++++++++++++++ > 5 files changed, 47 insertions(+), 36 deletions(-) >=20 > @@ -117,31 +108,31 @@ void watchdog_perform_action(void) > { > switch(watchdog_action) { Worth fixing the missing space after switch while touching this area of code? > case WDT_RESET: /* same as 'system_reset' in monitor *= / > - watchdog_mon_event("reset"); > + qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_RESET, NUL= L); More instances of ignoring the errp argument, where eliminating it might be nicer. > +## > +# @WATCHDOG > +# > +# Emitted when the watchdog device's timer is expired > +# > +# @action: action that has been taken > +# > +# Note: If action is "reset", "shutdown", or "pause" the WATCHDOG even= t is > +# followed respectively by the RESET, SHUTDOWN, or STOP events > +# > +# Since: 2.1 0.14.0 > + 'data': { 'action': 'WatchdogExpirationAction' } } Hmm. You've managed to create error.json in such a manner that it is not self-sufficient. If some other file includes error.json, it must also define WatchdogExpirationAction or it will fail the generators. > +++ b/qapi-schema.json > +## > +# @WatchdogExpirationAction I think you will be better off to ensure that error.json is self-sufficient, perhaps by sticking any data type it references directly into common.json rather than qapi-schema.json, and having error.json include common.json. (This is the first instance of referencing an external type, but other events later in the series have the same issue). > +# Since: 2.1 > +## > +{ 'enum': 'WatchdogExpirationAction', > + 'data': [ 'reset', 'shutdown', 'poweroff', 'pause', 'debug', 'none' = ] } Nice conversion of open-coded string to an enum. While I've been asking for the earliest QMP version for Since fields on event objects proper, here, I think you're okay keeping the 'since 2.1' indication. Why? Because we already have other examples in the code base of converting open-coded strings to an enum, where the QMP wire format is the same, but where the version claimed on those enums was the qemu version that did the conversion rather than the age of the command being converted. (Maybe we could audit all of those conversions and retroactively update their Since field, or even come up with a notation for wire-stability release vs. QAPI introspection release - but that sounds like more pain than necessary with no obvious benefit) --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --Bfm7b8uFTv9RoS87PkhdMHEto73IGMQcg 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/ iQEcBAEBCAAGBQJTm3FkAAoJEKeha0olJ0Nq9l4IAKlrnGW6qe4TlLaKyihJte7y NIF6jOEw0lf+sTwD54kljN2QPR73G9VzvNGSBxjb9zDhfvSoBqiSI/OmlSGiwwgA Us12wx5GbmTq49AouZoj66Us7AlQsND1y48rH4Qtta4HNA1xQFi/EJu5E54uwxPm pUnSSuRk8JA6l0doQ+3VwBAXU7vEw681e2wGK572tmSbwINfgyDtNXJLh51axezM mWWLhWkAkn+CqlUd6IeayA7PJLeGf8uYq+bw0I/q9aSnlsleUkizkHAu8GVX4W9A vIc/vNmY78sFTLB8d1sPhbJ+gDuAJfdcfs/6HA6+iFx/uFFd5EMLWuo/K1X62lc= =Sc8v -----END PGP SIGNATURE----- --Bfm7b8uFTv9RoS87PkhdMHEto73IGMQcg--