"Daniel P. Berrange" <berra...@redhat.com> writes:
> On Wed, Jul 25, 2012 at 09:43:55PM -0500, Anthony Liguori wrote:
>> Luiz Capitulino <lcapitul...@redhat.com> 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.
>
> What we do in libvirt, is to define helper functions for
> reporting the specific error codes. So, as well as having
> the generic
>
> error_set(errp, QERR_CODE, "format string", args)
>
> you would have
>
> error_set_device_not_found(errp, args)
>
> which is just a #define
>
> #define error_set_device_not_found(errp, args) \
> error_set(errp, QERR_DEVICE_NOT_FOUND, "Device %s not found", args)
>
> for the most part this should avoid the duplication you're concerned
> about, while still letting use provided more detailed messsages.
Yup. I suggested the same on IRC, modulo macro vs. function.
