Re: [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands

[Markus Armbruster]

Michal Privoznik <address@hidden> writes:

> On 9/13/19 2:52 PM, Markus Armbruster wrote:
>> Michal Privoznik <address@hidden> writes:
>>
>>> If a command is disabled an error is reported. But due to usage
>>> of error_setg() the class of the error is GenericError which does
>>> not help callers in distinguishing this case from a case where a
>>> qmp command fails regularly due to other reasons. Use
>>> CommandNotFound error class which is much closer to the actual
>>> root cause.
>>>
>>> Signed-off-by: Michal Privoznik <address@hidden>
>>> ---
>>
>> I'd like to tweak the commit message a bit:
>>
>>    qmp-dispatch: Use CommandNotFound error for disabled commands
>>
>>    If a command is disabled an error is reported.  But due to usage of
>>    error_setg() the class of the error is GenericError which does not
>>    help callers in distinguishing this case from a case where a qmp
>>    command fails regularly due to other reasons.
>>
>>    We used to use class CommandDisabled until the great error
>>    simplification (commit de253f1491 for QMP and commit 93b91c59db for
>>    qemu-ga, both v1.2.0).
>>
>>    Use CommandNotFound error class, which is close enough.
>>
>> Objections?
>>
>
> None, thanks for taking care of this.

Need to squash in:

diff --git a/tests/test-qga.c b/tests/test-qga.c
index 891aa3d322..1ca49bbced 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -668,7 +668,7 @@ static void test_qga_blacklist(gconstpointer data)
     error = qdict_get_qdict(ret, "error");
     class = qdict_get_try_str(error, "class");
     desc = qdict_get_try_str(error, "desc");
-    g_assert_cmpstr(class, ==, "GenericError");
+    g_assert_cmpstr(class, ==, "CommandNotFound");
     g_assert_nonnull(g_strstr_len(desc, -1, "has been disabled"));
     qobject_unref(ret);
 
@@ -677,7 +677,7 @@ static void test_qga_blacklist(gconstpointer data)
     error = qdict_get_qdict(ret, "error");
     class = qdict_get_try_str(error, "class");
     desc = qdict_get_try_str(error, "desc");
-    g_assert_cmpstr(class, ==, "GenericError");
+    g_assert_cmpstr(class, ==, "CommandNotFound");
     g_assert_nonnull(g_strstr_len(desc, -1, "has been disabled"));
     qobject_unref(ret);