[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block: Fix block-set-write-threshold not to use
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] block: Fix block-set-write-threshold not to use funky error class |
Date: |
Fri, 13 Mar 2015 21:17:34 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 03/13/2015 11:51 AM, Markus Armbruster wrote:
>> Error classes are a leftover from the days of "rich" error objects.
>> New code should always use ERROR_CLASS_GENERIC_ERROR.
>
> More precisely, new code should use ERROR_CLASS_GENERIC_ERROR unless
> there is a good reason where a caller knowing a different error class is
> likely to react differently as a result (for example, with
> 'drive-mirror', libvirt DOES rely on QERR_DEVICE_NOT_FOUND as a witness
> that the command supports an optional 'top' argument, vs.
> ERROR_CLASS_GENERIC_ERROR to state that 'top' was still mandatory).
In my experience, a caller / client wanting to figure out what exactly
failed is often a sign of a function / command trying to do too much at
once.
Using error classes to introspect is trickery, committed out of
desperation. We should offer proper introspection instead.
That said, "always" is rather strong language. No rule without
exceptions. If it bothers you, perhaps the word can be dropped on
commit.
> But I agree that this situation is one unlikely to be where libvirt
> cares what error class was used.
>
>> Commit e246211
>> added a use of ERROR_CLASS_DEVICE_NOT_FOUND. Replace it.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>> block/write-threshold.c | 2 +-
>> qapi/block-core.json | 4 ----
>> 2 files changed, 1 insertion(+), 5 deletions(-)
>
> Reviewed-by: Eric Blake <address@hidden>
Thanks!