Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 3/11/21 5:11 PM, Philippe Mathieu-Daudé wrote:
[...]
>> diff --git a/accel/accel-qmp.c b/accel/accel-qmp.c
>> new file mode 100644
>> index 00000000000..f16e49b8956
>> --- /dev/null
>> +++ b/accel/accel-qmp.c
>> @@ -0,0 +1,47 @@
>> +/*
>> + * QEMU accelerators, QMP commands
>> + *
>> + * Copyright (c) 2021 Red Hat Inc.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qapi/qapi-commands-machine.h"
>> +
>> +static const Accelerator accel_list[] = {
>> +    ACCELERATOR_QTEST,
>> +#ifdef CONFIG_TCG
>> +    ACCELERATOR_TCG,
>> +#endif
>> +#ifdef CONFIG_KVM
>> +    ACCELERATOR_KVM,
>> +#endif
>
> ...would it be worth compiling the enum to only list enum values that
> were actually compiled in?  That would change it to:
>
> { 'enum': 'Accelerator',
>   'data': [ 'qtest',
>             { 'name': 'tcg', 'if': 'defined(CONFIG_TCG)' },
> ...

Makes introspection more useful.  Management applications can get the
information the list of compiled-in accelerators from query-qmp-schema.
They don't have to be taught to use query-accels.

In fact, query-accels becomes useless except as a tool to force
visibility of Accelerator in query-qmp-schema.  We wouldn't have to
force if we had CLI introspection that shows the type of -accel's
parameter @accel.  Adding a query command is a common work-around for
our anemic CLI introspection capabilities.

The query command could be made more useful than introspection if it
reflected run time state, i.e. it showed an accelerator only when the
host system actually supports it.  Can't say how practical that would
be.

>>
>> +AcceleratorInfoList *qmp_query_accels(Error **errp)
>> +{
>> +    AcceleratorInfoList *list = NULL, **tail = &list;
>> +
>> +    for (unsigned i = 0; i < ARRAY_SIZE(accel_list); i++) {
>> +        AcceleratorInfo *info = g_new0(AcceleratorInfo, 1);
>> +
>> +        info->name = accel_list[i];
>> +
>> +        QAPI_LIST_APPEND(tail, info);
>> +    }
>> +
>> +    return list;
>> +}

You could then use something like

        for (accel = 0; accel < ACCELERATOR__MAX; accel++) {
            AcceleratorInfo *info = g_new0(AcceleratorInfo, 1);

            info->name = Accelerator_str(accel);

            QAPI_LIST_APPEND(tail, info);
        }