From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53363) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZaQaI-00075t-1N for qemu-devel@nongnu.org; Fri, 11 Sep 2015 11:49:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZaQaD-0002vf-VA for qemu-devel@nongnu.org; Fri, 11 Sep 2015 11:49:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35270) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZaQaD-0002uO-K4 for qemu-devel@nongnu.org; Fri, 11 Sep 2015 11:49:49 -0400 References: <19fff9b2e33050c340b1dc0fd5e0dbe4e11a31bf.1441667360.git.crosthwaite.peter@gmail.com> From: Eric Blake Message-ID: <55F2F817.3000601@redhat.com> Date: Fri, 11 Sep 2015 09:49:43 -0600 MIME-Version: 1.0 In-Reply-To: <19fff9b2e33050c340b1dc0fd5e0dbe4e11a31bf.1441667360.git.crosthwaite.peter@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="JWdGKi4rwXv2NUHVGQbB4XtCf2KwhOx0o" Subject: Re: [Qemu-devel] [RFC PATCH v1 04/25] error: Add support for multiple errors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, armbru@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --JWdGKi4rwXv2NUHVGQbB4XtCf2KwhOx0o Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 09/10/2015 11:33 PM, Peter Crosthwaite wrote: > Allow errors to stack. If an error is already set, don't assert, > instead, form a linked list. Recent errors are at the front of the > list, older ones at the back. >=20 > The assertion against the destination erro already being set is s/erro/error/ > removed. Do we want to do that blindly, or do we want a design where users must explicitly ask for nested errors? I'm wondering aloud here: (haven't actually thought too hard, but typing as I go) Since your goal was reducing boilerplate, is there some way we could have= : myfunc1(..., error_add(errp)); myfunc2(..., error_add(errp)); be some way to mark errp as allowing error concatenation? That is, error_add() would return errp; if *errp was NULL it does nothing further, but *errp is non-NULL it then sets a flag in errp that it is okay for further errors to be concatenated. Then when setting or propagating an error, we can use the flag within errp to determine if the caller is okay with us appending to the existing error, or whether there may be a programming error in that we are detecting a followup error to an errp that wasn't properly cleared earlier. Or maybe: error_start_chaining(errp); myfunc1(..., errp); myfunc2(..., errp); error_stop_chaining(errp); where we use a counter of how many requests (since myfunc1() may itself call nested start/stop, so chaining is okay if the counter is non-zero). >=20 > copy/free are all to call their functionality recursively. >=20 > Propagation is implemented as concatenation of two error lists. >=20 > Signed-off-by: Peter Crosthwaite > --- >=20 > qapi/common.json | 5 ++++- > util/error.c | 27 +++++++++++++++++++++------ > 2 files changed, 25 insertions(+), 7 deletions(-) >=20 > diff --git a/qapi/common.json b/qapi/common.json > index bad56bf..04175db 100644 > --- a/qapi/common.json > +++ b/qapi/common.json > @@ -22,11 +22,14 @@ > # @KVMMissingCap: the requested operation can't be fulfilled because a= > # required KVM capability is missing > # > +# @MultipleErrors: Multiple errors have occured s/occured/occurred/ Missing a (since 2.5) designation. Do we want to change the QMP wire format when multiple errors have been chained together, to return a linked list or array of those errors, for easier machine parsing of the individual errors? If so, it requires some documentation updates. If not, packing the chained error information into a single string is okay for humans, but not as nice for computers. > +# > # Since: 1.2 > ## > { 'enum': 'ErrorClass', > 'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted', > - 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] } > + 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap', > + 'MultipleErrors' ] } > =20 > ## > # @VersionTriple > diff --git a/util/error.c b/util/error.c > index b93e5c8..890ce58 100644 > --- a/util/error.c > +++ b/util/error.c > @@ -18,28 +18,25 @@ struct Error > { > char *msg; > ErrorClass err_class; > + struct Error *next; > }; Merge conflicts in this area; but doesn't hold up review of the concept. > =20 > Error *error_abort; > =20 > static void do_error_set(Error **errp, ErrorClass err_class, > void (*mod)(Error *, void *), void *mod_opaqu= e, > - const char *fmt, ...) > + const char *fmt, va_list ap) > { > Error *err; > - va_list ap; > int saved_errno =3D errno; > =20 > if (errp =3D=3D NULL) { > return; > } > - assert(*errp =3D=3D NULL); Here's where I'm wondering if we can have some sort of flag to say whether the caller was okay with error concatentation, in which case this would be something like: assert(!*errp || errp->chaining_okay); --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --JWdGKi4rwXv2NUHVGQbB4XtCf2KwhOx0o 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/ iQEcBAEBCAAGBQJV8vgXAAoJEKeha0olJ0NqDJwH/3oleEmKFf5DiBuGL2ZtzGiH Tw8IPzQqofuRAK0bpLz+TX5D4fBNbM7EbTPWfnjNaXWRxl4emT4Io7RcSdYFb/Ij jb5SAE/JrVVvOyrmqse3e1qXETpUDNAfHJF/JghxpWTOPmDE/FvCNoezS44hXLCN 1nKfXKLeMuSVNO35C0lFW7LXlfoYTeVJmFYaouGXtGKQdFkS6LU15IYv+idQO2xq EG2ATbJ0KonFx6CiGZdJkNYPD+LGZ6KMDtGOlA4B4+sGWJHu4L9GsB0/w59HBYlm kSQTniq331QCKuw/mdvwDI1P10MAuv+xpxPLPjlFu0n1HkUs6+uRPMv4/Mj6RUM= =GIFI -----END PGP SIGNATURE----- --JWdGKi4rwXv2NUHVGQbB4XtCf2KwhOx0o--