[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: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [RFC 00/14]: add printf-like human msg to error_set() |
Date: |
Thu, 26 Jul 2012 10:20:53 -0500 |
User-agent: |
Notmuch/0.13.2+93~ged93d79 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) |
Kevin Wolf <address@hidden> writes:
> Am 26.07.2012 14:41, schrieb Anthony Liguori:
>> 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.
>>
>> We don't gain anything by touching every user of error and the code gets
>> more verbose. If we want to modify an existing error for some good
>> reason, we can do so my changing error types.
>
> We gain consistency instead of accumulating the relics of even more
> halfway completed direction changes.
Sorry, but taking the "Device 'device=%s' not found" string and
replicating a dozen times is not helpful at all. Having a single method
to generate device not found errors is a Good Thing. Could it be a
function around a string instead of JSON magic? Sure. But open coding
is not a step forward.
>>>> 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")
>>
>> Or pick your favorite short name.
>
> You mentioned this domain thing before, and when asked you never
> explained what you really mean with it. Can you do so now, please?
http://developer.gnome.org/glib/stable/glib-Error-Reporting.html
In terms of GError, domain is a unique string which defines the meaning
of the error codes. Most often, domain is either a library and/or
module name.
So we would probably have a "qcow2" domain and a "block" domain to
differientiate errors generated from qcow2 vs. the generic block layer.
> Assuming that it's just some error class string, I don't really see the
> difference between error_set(QERR_FOO, "Free form") as implemented by
> this series and error_set(QERR_GENERIC, "FOO", "Free form").
I don't care about using free strings vs. #defines. I care about open
coding strings that ought to be common and consistent.
Regards,
Anthony Liguori
>
> Kevin
- [Qemu-devel] [PATCH 13/14] qerror: change all qerror_report() calls to use the ErrClass enum, (continued)
- [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 <=
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