* [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands
@ 2019-08-30 13:29 Michal Privoznik
2019-08-30 13:49 ` Eric Blake
2019-09-13 12:52 ` Markus Armbruster
0 siblings, 2 replies; 7+ messages in thread
From: Michal Privoznik @ 2019-08-30 13:29 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, mdroth, lcapitulino
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 <mprivozn@redhat.com>
---
This is a v2 of:
https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg06327.html
diff to v1:
- Don't introduce new error class (CommandDisabled)
- Use CommandNotFound error class
qapi/qmp-dispatch.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 3037d353a4..bc264b3c9b 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -104,8 +104,9 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request,
return NULL;
}
if (!cmd->enabled) {
- error_setg(errp, "The command %s has been disabled for this instance",
- command);
+ error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
+ "The command %s has been disabled for this instance",
+ command);
return NULL;
}
if (oob && !(cmd->options & QCO_ALLOW_OOB)) {
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands
2019-08-30 13:29 [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands Michal Privoznik
@ 2019-08-30 13:49 ` Eric Blake
2019-09-13 12:52 ` Markus Armbruster
1 sibling, 0 replies; 7+ messages in thread
From: Eric Blake @ 2019-08-30 13:49 UTC (permalink / raw)
To: Michal Privoznik, qemu-devel; +Cc: armbru, mdroth, lcapitulino
[-- Attachment #1.1: Type: text/plain, Size: 851 bytes --]
On 8/30/19 8:29 AM, Michal Privoznik wrote:
> 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 <mprivozn@redhat.com>
> ---
>
> This is a v2 of:
>
> https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg06327.html
>
> diff to v1:
> - Don't introduce new error class (CommandDisabled)
> - Use CommandNotFound error class
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands
2019-08-30 13:29 [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands Michal Privoznik
2019-08-30 13:49 ` Eric Blake
@ 2019-09-13 12:52 ` Markus Armbruster
2019-09-13 13:52 ` Michal Privoznik
1 sibling, 1 reply; 7+ messages in thread
From: Markus Armbruster @ 2019-09-13 12:52 UTC (permalink / raw)
To: Michal Privoznik; +Cc: mdroth, lcapitulino, qemu-devel, armbru
Michal Privoznik <mprivozn@redhat.com> 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 <mprivozn@redhat.com>
> ---
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?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands
2019-09-13 12:52 ` Markus Armbruster
@ 2019-09-13 13:52 ` Michal Privoznik
2019-09-13 18:41 ` Markus Armbruster
0 siblings, 1 reply; 7+ messages in thread
From: Michal Privoznik @ 2019-09-13 13:52 UTC (permalink / raw)
To: Markus Armbruster; +Cc: lcapitulino, qemu-devel, mdroth
On 9/13/19 2:52 PM, Markus Armbruster wrote:
> Michal Privoznik <mprivozn@redhat.com> 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 <mprivozn@redhat.com>
>> ---
>
> 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.
Michal
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands
2019-09-13 13:52 ` Michal Privoznik
@ 2019-09-13 18:41 ` Markus Armbruster
2019-09-24 12:33 ` Markus Armbruster
0 siblings, 1 reply; 7+ messages in thread
From: Markus Armbruster @ 2019-09-13 18:41 UTC (permalink / raw)
To: Michal Privoznik; +Cc: qemu-devel, mdroth, Markus Armbruster, lcapitulino
Michal Privoznik <mprivozn@redhat.com> writes:
> On 9/13/19 2:52 PM, Markus Armbruster wrote:
>> Michal Privoznik <mprivozn@redhat.com> 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 <mprivozn@redhat.com>
>>> ---
>>
>> 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);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands
2019-09-13 18:41 ` Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 14:45 ` Markus Armbruster
0 siblings, 1 reply; 7+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: Michal Privoznik; +Cc: mdroth, qemu-devel, lcapitulino
Markus Armbruster <armbru@redhat.com> writes:
> Michal Privoznik <mprivozn@redhat.com> writes:
>
>> On 9/13/19 2:52 PM, Markus Armbruster wrote:
>>> Michal Privoznik <mprivozn@redhat.com> 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 <mprivozn@redhat.com>
>>>> ---
>>>
>>> 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);
>
I tried to include the amended patch in today's pull request, but
observed "make check" hangs with it.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands
2019-09-24 12:33 ` Markus Armbruster
@ 2019-09-24 14:45 ` Markus Armbruster
0 siblings, 0 replies; 7+ messages in thread
From: Markus Armbruster @ 2019-09-24 14:45 UTC (permalink / raw)
To: Michal Privoznik; +Cc: mdroth, qemu-devel, lcapitulino
Markus Armbruster <armbru@redhat.com> writes:
> I tried to include the amended patch in today's pull request, but
> observed "make check" hangs with it.
False alarm: I just got a hang on master. I intend to include this
patch in my next pull request. Sorry for the delay.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-09-24 15:38 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-30 13:29 [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands Michal Privoznik
2019-08-30 13:49 ` Eric Blake
2019-09-13 12:52 ` Markus Armbruster
2019-09-13 13:52 ` Michal Privoznik
2019-09-13 18:41 ` Markus Armbruster
2019-09-24 12:33 ` Markus Armbruster
2019-09-24 14:45 ` Markus Armbruster
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).