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

[Claudio Fontana <cfontana@suse.de>]

On 3/12/21 8:42 AM, Marc-André Lureau wrote:
> On Fri, Mar 12, 2021 at 3:14 AM Philippe Mathieu-Daudé <philmd@redhat.com>
> wrote:
> 
>> Introduce the 'query-accels' QMP command which returns a list
>> of built-in accelerators names.
>>
>> - Accelerator is an QAPI enum of all existing accelerators,
>>
>> - AcceleratorInfo is a QAPI structure providing accelerator
>>   specific information. Currently the common structure base
>>   provides the name of the accelerator, while the specific
>>   part is empty, but each accelerator can expand it.
>>
>> - 'query-accels' QMP command returns a list of @AcceleratorInfo
>>
>> For example on a KVM-only build we get:
>>
>>     { "execute": "query-accels" }
>>     {
>>         "return": [
>>             {
>>                 "type": "qtest"
>>             },
>>             {
>>                 "type": "kvm"
>>             }
>>
> 
> s/type/name (in this version)
> 
>         ]
>>     }
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  qapi/machine.json | 55 +++++++++++++++++++++++++++++++++++++++++++++++
>>  accel/accel-qmp.c | 47 ++++++++++++++++++++++++++++++++++++++++
>>  accel/meson.build |  2 +-
>>  3 files changed, 103 insertions(+), 1 deletion(-)
>>  create mode 100644 accel/accel-qmp.c
>>
>> diff --git a/qapi/machine.json b/qapi/machine.json
>> index 330189efe3d..ffbf28e5d50 100644
>> --- a/qapi/machine.json
>> +++ b/qapi/machine.json
>> @@ -1471,3 +1471,58 @@
>>  ##
>>  { 'event': 'MEM_UNPLUG_ERROR',
>>    'data': { 'device': 'str', 'msg': 'str' } }
>> +
>> +##
>> +# @Accelerator:
>> +#
>> +# An enumeration of accelerator names.
>> +#
>> +# Since: 6.0
>> +##
>> +{ 'enum': 'Accelerator',
>> +  'data': [ { 'name': 'qtest' },
>> +            { 'name': 'tcg' },
>> +            { 'name': 'kvm' },
>> +            { 'name': 'hax' },
>> +            { 'name': 'hvf' },
>> +            { 'name': 'whpx' },
>> +            { 'name': 'xen' } ] }
>> +
>>
> 
> Why not use a simple enum?
> 
> +##
>> +# @AcceleratorInfo:
>> +#
>> +# Accelerator information.
>> +#
>> +# @name: The accelerator name.
>> +#
>> +# Since: 6.0
>> +##
>> +{ 'union': 'AcceleratorInfo',
>> +  'base': {'name': 'Accelerator'},
>> +  'discriminator': 'name',
>> +  'data': { } }
>>
> +
>>
> 
> Making room for future details, why not.
> 
> +##
>> +# @query-accels:
>> +#
>> +# Get a list of AcceleratorInfo for all built-in accelerators.
>> +#
>> +# Returns: a list of @AcceleratorInfo describing each accelerator.
>> +#
>> +# Since: 6.0
>> +#
>> +# Example:
>> +#
>> +# -> { "execute": "query-accels" }
>> +# <- { "return": [
>> +#        {
>> +#            "type": "qtest"
>> +#        },
>> +#        {
>> +#            "type": "kvm"
>> +#        }
>> +#    ] }
>> +#
>> +##
>> +{ 'command': 'query-accels',
>> +  'returns': ['AcceleratorInfo'] }
>>
> 
> That's nice, but how do you know which accels are actually enabled?


Maybe for clarity this could be 'query-accels-available' (which is probably the goal of this series).
Possibly a separate one would be 'query-accel-enabled'?

Can we see these commands being used for libvirt too, to improve feature detection? Are these useful beyond the confines of just testing?
I would think so right?

Ciao,

Claudio


> 
> 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
>> +#ifdef CONFIG_HAX
>> +    ACCELERATOR_HAX,
>> +#endif
>> +#ifdef CONFIG_HVF
>> +    ACCELERATOR_HVF,
>> +#endif
>> +#ifdef CONFIG_WHPX
>> +    ACCELERATOR_WHPX,
>> +#endif
>> +#ifdef CONFIG_XEN_BACKEND
>> +    ACCELERATOR_XEN,
>> +#endif
>> +};
>> +
>> +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;
>> +}
>> diff --git a/accel/meson.build b/accel/meson.build
>> index b44ba30c864..7a48f6d568d 100644
>> --- a/accel/meson.build
>> +++ b/accel/meson.build
>> @@ -1,4 +1,4 @@
>> -specific_ss.add(files('accel-common.c'))
>> +specific_ss.add(files('accel-common.c', 'accel-qmp.c'))
>>  softmmu_ss.add(files('accel-softmmu.c'))
>>  user_ss.add(files('accel-user.c'))
>>
>> --
>> 2.26.2
>>
>>
>>
>