[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC 00/14]: add printf-like human msg to error_set()
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [RFC 00/14]: add printf-like human msg to error_set() |
Date: |
Thu, 26 Jul 2012 11:12:29 -0300 |
On Thu, 26 Jul 2012 07:41:07 -0500
Anthony Liguori <address@hidden> wrote:
> Kevin Wolf <address@hidden> writes:
>
> > Am 26.07.2012 04:43, schrieb Anthony Liguori:
> >> Luiz Capitulino <address@hidden> writes:
> >>
> >>> Basically, this series changes a call like:
> >>>
> >>> error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> >>>
> >>> to:
> >>>
> >>> error_set(errp, QERR_DEVICE_NOT_FOUND,
> >>> "Device 'device=%s' not found", device);
> >>>
> >>> In the first call, QERR_DEVICE_NOT_FOUND is a string containing a json
> >>> dict:
> >>>
> >>> "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
> >>
> >> This is the wrong direction. Looking through the patch, this makes the
> >> code much more redundant overall. You have dozens of calls that are
> >> duplicating the same error message. This is not progress.
> >
> > I believe this is mostly because it's a mechanical conversion. Once this
> > is done, we can change error messages to better fit the individual
> > cases.
Correct.
>
> We don't gain anything by touching every user of error and the code gets
> more verbose.
We do gain the possibility to have better and different human messages for the
same error class. That's impossible today. And creating a different error class
just to have a different error message is just crazy (which is what we do
today, btw).
Now, if the problem you see is that we shouldn't touch current users but
add a new function for new users to use (or change old users incrementally, when
it matters), then we can discuss that.
> If we want to modify an existing error for some good
> reason, we can do so my changing error types.
You mean, creating a new error class just to have a different human message?
We have 70+ classes today, how many will we have in another year?
>
> >> We should just stick with a simple QERR_GENERIC and call it a day.
> >> Let's not needlessly complicate existing code.
> >
> > Why even have error codes when everything should become QERR_GENERIC? Or
> > am I misunderstanding?
>
> If we want to add an error code, we can do:
>
> error_set(QERR_GENERIC, "domain", "My free form text")
>
> And then yes, we can change this to:
>
> error_setf(errp, "domain", "My free form text")
Would domain be the error classes we have today?
If error_setf() ends result is similar to what this series does with
error_set(), then that might be acceptable, although I fear we keep
adding new ways to report errors.
- [Qemu-devel] [PATCH 09/14] qerror: qerror_report(): take an index and a human error message, (continued)
- [Qemu-devel] [PATCH 09/14] qerror: qerror_report(): take an index and a human error message, Luiz Capitulino, 2012/07/25
- [Qemu-devel] [PATCH 11/14] qerror: drop qerror_table[] for good, Luiz Capitulino, 2012/07/25
- [Qemu-devel] [PATCH 12/14] error: turn QERR_ macros into an enumeration, Luiz Capitulino, 2012/07/25
- [Qemu-devel] [PATCH 13/14] qerror: change all qerror_report() calls to use the ErrClass enum, Luiz Capitulino, 2012/07/25
- [Qemu-devel] [PATCH 10/14] error: error_set(): take an index and a human error message, Luiz Capitulino, 2012/07/25
- [Qemu-devel] [PATCH 14/14] error: change all error_set() calls to use the ErrClass enum, Luiz Capitulino, 2012/07/25
- Re: [Qemu-devel] [RFC 00/14]: add printf-like human msg to error_set(), Anthony Liguori, 2012/07/25
- Re: [Qemu-devel] [RFC 00/14]: add printf-like human msg to error_set(), Kevin Wolf, 2012/07/26
- Re: [Qemu-devel] [RFC 00/14]: add printf-like human msg to error_set(), Anthony Liguori, 2012/07/26
Re: [Qemu-devel] [RFC 00/14]: add printf-like human msg to error_set(), Daniel P. Berrange, 2012/07/26
Re: [Qemu-devel] [RFC 00/14]: add printf-like human msg to error_set(), Markus Armbruster, 2012/07/26