Markus Armbruster <arm...@redhat.com> writes: > Michal Privoznik <mpriv...@redhat.com> writes: > >> On 9/13/19 2:52 PM, Markus Armbruster wrote: >>> Michal Privoznik <mpriv...@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 <mpriv...@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.
