[Qemu-devel] [PATCH] block: Fix block-set-write-threshold not to use funky error class

[Qemu-devel] [PATCH] block: Fix block-set-write-threshold not to use funky error class

[Markus Armbruster]

Error classes are a leftover from the days of "rich" error objects.
New code should always use ERROR_CLASS_GENERIC_ERROR.  Commit e246211
added a use of ERROR_CLASS_DEVICE_NOT_FOUND.  Replace it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/write-threshold.c | 2 +-
 qapi/block-core.json    | 4 ----
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/block/write-threshold.c b/block/write-threshold.c
index c2cd517..a53c1f5 100644
--- a/block/write-threshold.c
+++ b/block/write-threshold.c
@@ -112,7 +112,7 @@ void qmp_block_set_write_threshold(const char *node_name,
 
     bs = bdrv_find_node(node_name);
     if (!bs) {
-        error_set(errp, QERR_DEVICE_NOT_FOUND, node_name);
+        error_setg(errp, "Device '%s' not found", node_name);
         return;
     }
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 90586a5..42c8850 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1961,10 +1961,6 @@
 # @write-threshold: configured threshold for the block device, bytes.
 #                   Use 0 to disable the threshold.
 #
-# Returns: Nothing on success
-#          If @node name is not found on the block device graph,
-#          DeviceNotFound
-#
 # Since: 2.3
 ##
 { 'command': 'block-set-write-threshold',
-- 
1.9.3

Re: [Qemu-devel] [PATCH] block: Fix block-set-write-threshold not to use funky error class

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]

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).

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 <armbru@redhat.com>
> ---
>  block/write-threshold.c | 2 +-
>  qapi/block-core.json    | 4 ----
>  2 files changed, 1 insertion(+), 5 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] block: Fix block-set-write-threshold not to use funky error class

[Markus Armbruster]

Eric Blake <eblake@redhat.com> 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 <armbru@redhat.com>
>> ---
>>  block/write-threshold.c | 2 +-
>>  qapi/block-core.json    | 4 ----
>>  2 files changed, 1 insertion(+), 5 deletions(-)
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!

Re: [Qemu-devel] [PATCH] block: Fix block-set-write-threshold not to use funky error class

[Kevin Wolf]

Am 13.03.2015 um 18:51 hat Markus Armbruster geschrieben:
> Error classes are a leftover from the days of "rich" error objects.
> New code should always use ERROR_CLASS_GENERIC_ERROR.  Commit e246211
> added a use of ERROR_CLASS_DEVICE_NOT_FOUND.  Replace it.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Thanks, applied to the block branch.

Kevin