[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for repor
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands |
Date: |
Fri, 13 Sep 2019 20:41:55 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) |
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);