Re: [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands
Markus Armbruster 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.
Re: [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands
Markus Armbruster writes: > Michal Privoznik writes: > >> On 9/13/19 2:52 PM, Markus Armbruster wrote: >>> Michal Privoznik 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 --- >>> >>> 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.
Re: [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands
Michal Privoznik writes: > On 9/13/19 2:52 PM, Markus Armbruster wrote: >> Michal Privoznik 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 >>> --- >> >> 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);
Re: [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands
On 9/13/19 2:52 PM, Markus Armbruster wrote: Michal Privoznik 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 --- 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
Re: [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands
Michal Privoznik 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 > --- 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?
Re: [Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting disabled commands
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 > --- > > 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 -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH v2] qapi/qmp-dispatch: Fix error class for reporting 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. Use
CommandNotFound error class which is much closer to the actual
root cause.
Signed-off-by: Michal Privoznik
---
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
