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. > > 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). > > copy/free are all to call their functionality recursively. > > Propagation is implemented as concatenation of two error lists. > > Signed-off-by: Peter Crosthwaite <crosthwaite.pe...@gmail.com> > --- > > qapi/common.json | 5 ++++- > util/error.c | 27 +++++++++++++++++++++------ > 2 files changed, 25 insertions(+), 7 deletions(-) > > 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' ] } > > ## > # @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. > > Error *error_abort; > > static void do_error_set(Error **errp, ErrorClass err_class, > void (*mod)(Error *, void *), void *mod_opaque, > - const char *fmt, ...) > + const char *fmt, va_list ap) > { > Error *err; > - va_list ap; > int saved_errno = errno; > > if (errp == NULL) { > return; > } > - assert(*errp == 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); -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature