From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38065) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZjYdf-0006ku-KM for qemu-devel@nongnu.org; Tue, 06 Oct 2015 16:16:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZjYcY-0000cj-Hn for qemu-devel@nongnu.org; Tue, 06 Oct 2015 16:15:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57837) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZjYcY-0000ca-5e for qemu-devel@nongnu.org; Tue, 06 Oct 2015 16:13:58 -0400 References: <1444161658-15038-1-git-send-email-pbonzini@redhat.com> <1444161658-15038-3-git-send-email-pbonzini@redhat.com> From: Eric Blake Message-ID: <56142B80.5060204@redhat.com> Date: Tue, 6 Oct 2015 14:13:52 -0600 MIME-Version: 1.0 In-Reply-To: <1444161658-15038-3-git-send-email-pbonzini@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="MltNWFcMxhUpQrtIfT5Pi5XXB78lot3w9" Subject: Re: [Qemu-devel] [PATCH 2/4] more replay fixes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: pavel.dovgaluk@ispras.ru This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --MltNWFcMxhUpQrtIfT5Pi5XXB78lot3w9 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/06/2015 02:00 PM, Paolo Bonzini wrote: > 1) Compile files once >=20 > 2) Move include file from replay/replay.h to include/sysemu/replay.h. >=20 > 3) Fix Error usage >=20 > 4) cleanup timerlistgroup_deadline_ns a bit and allow clock jump > notifiers to run >=20 > 5) move replay-user.c to stubs/ > --- > +++ b/include/qapi/qmp/qerror.h > @@ -107,6 +107,6 @@ > "this feature or command is not currently supported" > =20 > #define QERR_REPLAY_NOT_SUPPORTED \ > - ERROR_CLASS_GENERIC_ERROR, "Record/replay feature is not supported= for '%s'" > + "Record/replay feature is not supported for '%s'" We should not be adding new #defines to this file. Instead, inline the message into the callers that do error_setg() (I see hw/bt/hci.c as the first such caller). > +++ b/qapi/common.json > @@ -22,15 +22,11 @@ > # @KVMMissingCap: the requested operation can't be fulfilled because a= > # required KVM capability is missing > # > -# @ReplayNotSupported: the requested feature is not supported with > -# record/replay mode enabled > -# > # Since: 1.2 > ## > { 'enum': 'ErrorClass', > 'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted', > - 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap', > - 'ReplayNotSupported' ] } > + 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] } Thank you for this. We definitely do not want to be adding new error classes without a very strong reason, and even if such classes are added, they must properly be documented as 'Since 2.5'. > +++ b/vl.c > @@ -122,7 +122,7 @@ int main(int argc, char **argv) > #include "qapi-event.h" > #include "exec/semihost.h" > #include "crypto/init.h" > -#include "replay/replay.h" > +#include "sysemu/replay.h" > #include "qapi/qmp/qerror.h" > =20 > #define MAX_VIRTIO_CONSOLES 1 > @@ -851,7 +851,7 @@ static void configure_rtc(QemuOpts *opts) > } else if (!strcmp(value, "localtime")) { > Error *blocker =3D NULL; > rtc_utc =3D 0; > - error_set(&blocker, ERROR_CLASS_REPLAY_NOT_SUPPORTED, > + error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, > "-rtc base=3Dlocaltime"); > replay_add_blocker(blocker); > } else { > @@ -1258,7 +1258,7 @@ static void smp_parse(QemuOpts *opts) > =20 > if (smp_cpus > 1 || smp_cores > 1 || smp_threads > 1) { > Error *blocker =3D NULL; > - error_set(&blocker, ERROR_CLASS_REPLAY_NOT_SUPPORTED, "smp"); > + error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp"); > replay_add_blocker(blocker); Okay, I see that there is more than one location with the same failure, which is where using the #define sort of makes it nicer to guarantee a consistent message. But in general, use of error_setg() with a macro that passes a %s to printf at a distance is ugly, and should be avoided compared to just inlining the error message directly or writing a helper method that can properly set a consistent message. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --MltNWFcMxhUpQrtIfT5Pi5XXB78lot3w9 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWFCuAAAoJEKeha0olJ0NqtzAIAJ9BQdMLZWU1RDMl+20/skRT DVRHw3d4L6ULIpSmoHzQL/J4hoLzNf8AdJsFwPh/R8+TZ71BMg2FPqBut7mp5G4V ahHoukG3p+tLggYnQz6M1lejvLjpvy9MKuv9L3bbxIMnOY01Hz+3y/OZktgieoAs /wX5akqRN+6aRo+5RzNzrVpdJPeh6Oht1qTvErFdaDl8Ta8meNv1djRs/lgASR56 s+6wPyYHRFcm3Ru0xSpGvfLW7exybycpT6FanufnFX98NScD9ZmBqayluVeWCKkY B50AakSmDMI3A340g2CLrc45uJ+Qx9pRl8YB4I45ReA8jEn/jMDeMnj/iOdyra4= =qLh2 -----END PGP SIGNATURE----- --MltNWFcMxhUpQrtIfT5Pi5XXB78lot3w9--