On 08/10/2017 01:30 PM, Markus Armbruster wrote: > A command is a query if it has no side effect and yields a result. > Such commands are typically named query-FOO, but there are exceptions. > > The basic idea is to find candidates with query-qmp-schema, filter out > the ones that aren't queries with an explicit blacklist, and test the > remaining ones against a QEMU with no special arguments. > > The current blacklist is just add-fd.
I guess this is because it has no mandatory parameters. Hmm - I wonder
if introspection should flag WHICH commands require an fd over SCM
rights (I guess just add-fd) - as that is a USEFUL piece of information
to know (I can't call command XYZ unlss I also pass an fd) - and then
you could use that real bit of the introspection rather than your
blacklist as the mechanism for filtering this command (since anything
that requires an fd is obviously not a query).
>
> query-qmp-schema reports a few commands that aren't actually
> available. See qmp_unregister_commands_hack() for details. Work
> around this flaw by accepting CommandNotFound errors, but add a TODO
> to drop this when the flaw is fixed.
>
> The test can't do queries with arguments, because it knows nothing
> about the arguments. No coverage for query-cpu-model-baseline,
> query-cpu-model-comparison and query-cpu-model-expansion, because
s/because//
> query-rocker, query-rocker-ports, query-rocker-of-dpa-flows and
> query-rocker-of-dpa-groups.
>
> We get basic test coverage for the following commands:
Cool!
>
> qom-list-types
> query-acpi-ospm-status
> query-balloon (expected to fail)
> query-vm-generation-id (expected to fail)
> Most tested commands are expected to succeed. The test does not check
> the return value then. A few commands are expected to fail because
> they need special arguments to succeed, and this test is too dumb to
> supply them.
Sounds like it would just be a matter of adding additional command line
parameters to the qemu being invoked for testing those commands?
>
> +static int query_error_class(const char *cmd)
> +{
> + static struct {
> + const char *cmd;
> + int err_class;
> + } fails[] = {
> + { "query-balloon", ERROR_CLASS_DEVICE_NOT_ACTIVE },
> + { "query-vm-generation-id", ERROR_CLASS_GENERIC_ERROR },
But even THIS level of testing of those commands is pretty good!
> +static void test_query(const void *data)
> +{
> + const char *cmd = data;
> + int expected_error_class = query_error_class(cmd);
> + QDict *resp, *error;
> + const char *error_class;
> +
> + qtest_start("-nodefaults");
> +
> + resp = qmp("{ 'execute': %s }", cmd);
Oh fun - your patch and my libqtest cleanup series will collide :)
> +static void qmp_schema_init(QmpSchema *schema)
> +{
> + QDict *resp;
> + Visitor *qiv;
> + SchemaInfoList *tail;
> +
> + qtest_start("-nodefaults");
> + resp = qmp("{ 'execute': 'query-qmp-schema' }");
> +
> + qiv = qobject_input_visitor_new(qdict_get(resp, "return"));
> + visit_type_SchemaInfoList(qiv, NULL, &schema->list, &error_abort);
It's always fun to see this in action! Our efforts to add automated
introspection code generation have been WELL worth the cost in time.
Overall, I like the patch.
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
